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 default value to Context::scope parameter to make it optional #1343

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

zzooeeyy
Copy link
Contributor

@zzooeeyy zzooeeyy commented Oct 4, 2024

Description

Fixes:

Background

The scope parameter was required as a part of the legacy OAuth flow (Authorization code grant). It's a required parameter during the ShopifyAPI::Context setup process. It can be set to [] or "".

We don't have to enforce scope in the setup with Shopify managed install since scopes are configured in the shopify.app.*.toml files already.

missing-keyword

Solution

Making this field to have a default value so it can be omitted during setup.

How has this been tested?

  • Linked a local version of shopify-api-ruby with my changes to my app
  • Removed scope setup in the ShopifyAPI::Context.setup
  • Start server an observe no errors

Configuration without scopes

configuration

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.

@zzooeeyy zzooeeyy requested a review from a team as a code owner October 4, 2024 15:33
@lizkenyon
Copy link
Contributor

What is the functionality when an app is not using managed install, and they omit scopes? Is OAuth successful, and you just get the default store properties scope granted?

@zzooeeyy
Copy link
Contributor Author

zzooeeyy commented Oct 4, 2024

What is the functionality when an app is not using managed install, and they omit scopes? Is OAuth successful, and you just get the default store properties scope granted?

Yup if they omit scopes, shopify-api-ruby will default their "configuration" to be an empty list and request the auth code grant flow for installation with an empty list of scopes, and this will prompt the shop for the default "store data"

Here's an example request:

https://admin.shopify.com/store/mystore/oauth/authorize?client_id=this-is-a-valid-client_id&scope=&redirect_uri=https%3A%2F%2Fsomething-something-something-missions.trycloudflare.com%2Fapi%2Fauth%2Fcallback&state=P5F5J74FDCYREpy&grant_options%5B%5D=

image


There's also gonna be another change in the shopify_app repo to also have a default value.

@zzooeeyy zzooeeyy force-pushed the zoey/make-scope-optional branch 3 times, most recently from 15863ed to 6441a5d Compare October 4, 2024 19:24
Copy link
Contributor

@lizkenyon lizkenyon left a comment

Choose a reason for hiding this comment

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

Thanks @zzooeeyy

@zzooeeyy zzooeeyy merged commit 39eda8b into main Oct 4, 2024
9 checks passed
@zzooeeyy zzooeeyy deleted the zoey/make-scope-optional branch October 4, 2024 20:08
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.

2 participants