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

Remove organization from invalid properties on post lead #38

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

marczahn
Copy link
Contributor

@marczahn marczahn commented Apr 4, 2018

@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
Copy link
Collaborator

mickadoo commented Apr 4, 2018

@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
Copy link
Contributor Author

marczahn commented Apr 4, 2018

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

@mickadoo
Copy link
Collaborator

mickadoo commented Apr 4, 2018

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 😛

Copy link
Collaborator

@mickadoo mickadoo left a comment

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
Copy link
Contributor Author

marczahn commented Apr 4, 2018

@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