Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deal.associate! can associate contacts and companies in a single call #120

Conversation

fonji
Copy link
Contributor

@fonji fonji commented Jul 6, 2018

Currently, if someone calls Deal.associate(deal_id, [company_id], [contact_vid]), it only associates the company to the deal.
This fix allows to associate both of them, using two API calls.

Also I didn't find any spec for Deal.associate! so here's a bonus 😄

@cbisnett
Copy link
Collaborator

The existing interface for this API is a bit weird (mostly due to the API limitations) and I wouldn't have expected the behavior given it takes two arguments and ignores one of them. I see the reasoning behind your change and I think it makes sense for what the expected behavior would be. Unfortunately the PR as it is may break backwards compatibility because it changes the API calls.

Since the HubSpot API only allows a single object type to be associated for each request, the library should also mimic this behavior. With that said, we can provide a wrapper function that does what you're doing here in calling the API twice, once for each object type. Instead of adding the other functions, can you make a single wrapper function that calls associate! twice, passing the correct arguments to associate the company IDs and contact IDs? You could call it update_associations! or something like that to make it more obvious that it's updating multiple associations with a single call. This way we can maintain backwards compatibility.

If you can do that, I'll get this merged and released. Thanks.

@fonji
Copy link
Contributor Author

fonji commented Nov 21, 2019

Replaced by #210

@fonji fonji closed this Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants