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 config option to disable the REST client #1348

Merged
merged 5 commits into from
Dec 2, 2024

Conversation

lizkenyon
Copy link
Contributor

@lizkenyon lizkenyon commented Nov 27, 2024

Description

Fixes Disable REST client for new apps

  • We want new apps created with the Ruby template to use the GraphQL API, and not the REST API
  • After this is released we will enable this config option in the template
  • By default this is false, apps currently using the REST API will not be affected
  • If the config option is true, an error will be raised if REST client is used.

How has this been tested?

  • Tested in a local development app.
disable-rest.mp4

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.

@lizkenyon lizkenyon requested a review from a team as a code owner November 27, 2024 15:21
New ruby apps will have this config option turned on

As of April 1st 2025 all new apps must use only GraphQL
@lizkenyon lizkenyon changed the title [WIP] Add config option to disable the REST client Add config option to disable the REST client Nov 27, 2024
Copy link
Contributor

@andy-liuu andy-liuu left a comment

Choose a reason for hiding this comment

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

Code looks good! a quick question:

  • should we be more forceful in disabling rest? (either now or in the future?)
    • i.e. remove rest entirely in a pr and call it a major release to the shopify_api gem. I think existing apps are unaffected this way too

@lizkenyon lizkenyon merged commit c6f95f0 into main Dec 2, 2024
9 checks passed
@lizkenyon lizkenyon deleted the liz/config-to-disable-rest branch December 2, 2024 14:54
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