-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add hubspot properties #206
Conversation
8004868
to
798bb1a
Compare
21e8b82
to
dab40ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! what we still miss:
- show how to fetch history in the demo script
- enable history on the
test_all_reources
(which calls real backend)
d6f0464
to
125546b
Compare
@rudolfix added it to tests and demo. I don't think I have the right test account locally so I'm not checking the data very thoroughly for now. |
) | ||
# Get history separately, as requesting both all properties and history together | ||
# is likely to hit hubspot's URL length limit | ||
for history_entries in fetch_property_history( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this too. With some endpoints the URL was too long when listing all properties twice. 😨
@steinitzu I sent you the credentials to test account. let's make sure that some proper test cases are there before merging. thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, LTM!
Tell us what you do here
Relevant issue
Implements #195
More PR info