[feat/company-all-with-offset] improve Get all companies with other information #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- This commit lets the user pass an "access_token" to Hubspot.configure. At that point, if an access token is passed, we set default headers on the Connection class, and this supports OAuth. - I added a check in the get_url method which will not set an API key if an access token is present. This is required. Otherwise, Hubspot will fail the request, thinking that the api key was the intended auth method, but that the key was blank.
- This class is responsbile for grabbing and refreshing OAuth access tokens. - It just returns the results to you for the caller to actually do something with them. - It inherits from the Connection class because it needs to re-use many of the methods for consistency, yet actually has fairly different needs. The headers are form-urlencoded, not json. So using the existing post_json methods would have required some re-jiggering to work correctly here. Also, these dont' require an API key in the same way as the others, so we would have to always pass in "hapikey": false, and this would have required more changes to the post_json method. Lastly, of course you aren't actually posting json here! So using that method would be extra weird.
…-ruby into hinthealth-feature/oauth
…ociate-notes-with-deals add the ability to associate an engagement note with a deal id
…into fonji-merge_contacts
…ot-ruby into fonji-engagement_associate
…ji/hubspot-ruby into fonji-company_add_contact_on_class
…nto Berardpi-deal-all
…fonji/hubspot-ruby into fonji-add_get_contact_vids_on_company
…nandooliveira/hubspot-ruby into nandooliveira-feature/find_deals_by_association
Also try to fix guard file
Should have "name" => "property_name"
bulk update might take a few milliseconds to propagate, causes spec failed.
…nformation Add new method #all_with_offset to return not only results as response but also hasMore and offset, so it would be possible to get all companies.
|
@paulld I have no permission for this repo. I would like to add Please do review and merge this PR if everything's fine. the previous PR has been accepted, so there's commits which already rebased to master of the original repo. you can simply check last 3 commits of mine. |
|
@CarolHsu I added you to the repo sorry |
paulld
approved these changes
Nov 2, 2018
|
Thanks @paulld, you can either merge this PR (which seems I have no access to do it), or wait for my PR being accepted officially (today or this weekend I guess). then let's no longer to keep this temporary repo ever. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add new method
#all_with_offsetto return not only results as response but alsohasMoreandoffset, so it would be possible to get all companies. For instance