Redirect to shopify managed install path during login #1818
Conversation
| "http://my-shop.myshopify.com", | ||
| "https://admin.shopify.com/store/my-shop", | ||
| ].each do |good_url| | ||
| test "#create redirects to Shopify managed install path instead if use_new_embedded_auth_strategy is enabled - #{good_url}" do |
There was a problem hiding this comment.
Do we / should we have a test for bad URLs?
There was a problem hiding this comment.
There are already tests for bad URLs, and the logic to redirect to invalid shop error happens regardless of "use new embedded auth strategy".. So I think the existing tests should be sufficient? If not it'll be pretty easy to add the extra cases like:
[
true,
false,
].each do |use_new_embedded_auth_strategy|
test "existing test cases for invalid urls"
#
#
endLet me know if you want me to add it!
There was a problem hiding this comment.
Yeah if it's simple to duplicate it might help protect us against regressions just in case we ever change that code.
| "https://admin.shopify.com/store/my-shop", | ||
| ].each do |good_url| | ||
| test "#create redirects to Shopify managed install path instead if use_new_embedded_auth_strategy is enabled - #{good_url}" do | ||
| ShopifyApp.configuration.wip_new_embedded_auth_strategy = true |
There was a problem hiding this comment.
might be a bit too late for this question, why do we have wip in the name? Will it be changed to something more official in the future?
There was a problem hiding this comment.
Yess it's wip right now since we're still building and merging feature incrementally. Once it's ready to be "shipped" officially, we'll rename this to be the same as Remix's future flag- unstable_new_embedded_auth_strategy
zzooeeyy
force-pushed
the
login-to-install
branch
from
4ed367b to
9a7dd5c
Compare
For issue:
Depends on issue: (not directly, but the redirects won't work properly unless we have token exchange)
📹 VIDEO DEMO
What this PR does
Redirect to Shopify managed install path during login instead of starting auth code OAuth flow.
Checklist
Before submitting the PR, please consider if any of the following are needed:
CHANGELOG.mdif the changes would impact usersREADME.md, if appropriate./docs, if necessary