Skip to content
New issue

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

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

Already on GitHub? # to your account

[feat/company-all-with-offset] improve Get all companies with other information #134

Merged
merged 5 commits into from
Nov 2, 2018

Conversation

CarolHsu
Copy link
Contributor

@CarolHsu CarolHsu commented Oct 30, 2018

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. For instance

response = Hubspot::Company.all_with_offset(count: 100)
while response['hasMore']
  # DO SOMETHING with response['results']
  response = Hubspot::Company.all_with_offset(count: 100, offset: response['offset'])
end

…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.

response = Hubspot::Connection.get_json(path, opts)
formatted_results = response['results'].map { |c| new(c) }
response = JSON.parse(response.to_json, object_class: OpenStruct)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the idea of using OpenStruct here to make a cleaner interface for the object attributes, but all of the other API calls return a hash. In the next version we can definitely take a look at updating all of the interfaces to do something like this but for this version, we should continue to match the existing conventions.

Check out Deal#all for an example of how this is currently being handled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good, modified 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw, if there's any certain plan for next version, feel free to notify me, let me see if I can help something :)
Thanks.

@cbisnett cbisnett merged commit f577e3a into HubspotCommunity:master Nov 2, 2018
@cbisnett
Copy link
Collaborator

cbisnett commented Nov 2, 2018

Thanks for making those updates. In the near future I'm going to start using the Projects feature to create some high level tickets to track things that I would like to see get fixed or clarified or whatever. Then we can start making some of the breaking changes for v1.0.0 that will standardize some of the things that are currently not.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants