Skip to content
This repository was archived by the owner on Jan 26, 2022. It is now read-only.

Remove organization from invalid properties on post lead#38

Open
marczahn wants to merge 1 commit into
developfrom
feature/remove_organization_from_invalid_properties
Open

Remove organization from invalid properties on post lead#38
marczahn wants to merge 1 commit into
developfrom
feature/remove_organization_from_invalid_properties

Conversation

@marczahn

@marczahn marczahn commented Apr 4, 2018

Copy link
Copy Markdown
Contributor

@mickadoo Is there a reason for excluding exactly this field? And would it make sense to remove validateLeadForPost completely and to rely on errors sent from closeio?

@mickadoo

mickadoo commented Apr 4, 2018

Copy link
Copy Markdown
Collaborator

@marczahn I'm not sure why I added this, it was a while ago. If close.io is happy with it then I'd say it's fine to remove. I think if we can catch errors before sending the request to close.io it could be more efficient than waiting for the response, but I guess either is fine.

@marczahn

marczahn commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

It is even stranger since organization is not used at all :-)

@mickadoo

mickadoo commented Apr 4, 2018

Copy link
Copy Markdown
Collaborator

I'll leave it with you @marczahn. As long as the tests pass and you've tested it with real close.io requests then it should be fine. It would be very nice if we had a test close.io instance we could run real API requests against, just to ensure our wrapper is compatible with their changes.

I'll approve this anyway and if anything goes wrong then we all know who broke it 😛

@mickadoo mickadoo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me

@marczahn

marczahn commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

@mickadoo I just remembered why this was: The lead requests and responses are slightly different and some fields may not be sent.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants