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

Ecommerce Bridge API #43

Merged

Conversation

W1ldPo1nter
Copy link
Contributor

@W1ldPo1nter W1ldPo1nter commented Jun 28, 2019

This PR adds a client for a few (probably the most important) of the Ecommerce Bridge API endpoints.

It also contains additions/changes to a few other things:

  • It fixes a bug in the CLI that was introduced by renaming a variable in the black-application-commit after the CLI was merged. After this change, this variable only existed if the if block was entered and would therefore result in an error in any other case. This was reverted in this PR while making the type of this variable consistent for each case.
  • To be able to perform tests without actually calling the HubSpot API (see Running tests not against the live HubSpot demo account #40), a fixture for a mocked connection is introduced, which can be used in place of HTTP(S)Connection objects. It offers some utilities to quickly check if certain requests would have been made if it was a real connection. It is used in the unit tests for the Ecommerce Bridge endpoints.
  • The string representation of errors is fixed in Python 3.5, where the body of incoming responses is a byte string, which could not be JSON-decoded. Therefore the string representation raised an error in Python 3.5. At the same time, this removes some old Python 2 related code (there is no __unicode__ anymore in Python 3) as well as some unnecessary conversion attempts like trying to decode a value that already is a str.

The PR contains tests for all added Ecommerce Bridge endpoints. They can be run using tox -- hubspot3/test/test_ecommerce_bridge.py with a local tox setup (hence the introduction of {posargs} in tox.ini) or simply py.test hubspot3/test/test_ecommerce_bridge.py for a single Python version. The tests must be limited like this because the other tests are still problematic due to #40.

(black was already applied in between certain commits, but it certainly wouldn't hurt to run it again after merging.)

@W1ldPo1nter W1ldPo1nter mentioned this pull request Jun 28, 2019
@jpetrucciani
Copy link
Owner

Oops, thanks for catching that! Sorry that I unintentionally caused that bug with the style commit - I should've been more careful with that. I also need to get the new tests added into the Travis-CI config, as well as fixed up with a new API key!

I'm also in the process of getting our own test API key that we can hopefully use for full on integration tests instead of using the demo key! I hope to get this merged soon and some tests fixed up today with the new test key

@jpetrucciani jpetrucciani merged commit 8d40896 into jpetrucciani:master Jun 28, 2019
@jpetrucciani
Copy link
Owner

This is now included in the latest release 3.2.17, which is now live!

Thanks so much for your contributions! 😄

@W1ldPo1nter W1ldPo1nter deleted the feature/ecommerce_bridge branch July 1, 2019 07:20
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.

3 participants