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

Add api_host to ShopifyAPI::Context #1241

Merged
merged 3 commits into from
Nov 7, 2023

Conversation

brendo
Copy link
Contributor

@brendo brendo commented Nov 7, 2023

RE: #1235

This PR adds new api_host "setting" to Context. The HttpClient will use api_host when set, otherwise default to calling the API as specified by the session.shop value.

Whilst this feels a little weird to add this value to Context, it seems to be way the ShopifyAPI gem is "configurable" by consumers.

How has this been tested?

Unit tests in this project and using this branch in a service.

Checklist:

  • My commit message follow the pattern described in here
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the project documentation.
  • I have added a changelog line.

@brendo brendo requested a review from a team as a code owner November 7, 2023 02:43
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Nicholas La Roux <larouxn@gmail.com>
Copy link
Contributor

@nelsonwittwer nelsonwittwer left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to do this!

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Nelson <nelsonwittwer@users.noreply.github.com>
@brendo brendo merged commit bf02f13 into main Nov 7, 2023
7 checks passed
@brendo brendo deleted the feat_issue-1235-configurable-base-url branch November 7, 2023 22:29
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