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

Improve upgrade docs #1525

Merged
merged 1 commit into from
Oct 31, 2022
Merged

Improve upgrade docs #1525

merged 1 commit into from
Oct 31, 2022

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Oct 5, 2022

What this PR does

Reviewer's guide to testing

Things to focus on

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@andyw8 andyw8 changed the title Andyw8/improve upgrade docs WIP: Improve upgrade docs Oct 5, 2022
generate a block in the `shopify_app` initializer. To do so manually, ensure the following is
part of the `after_initialize` block in `shopify_app.rb`.

```ruby
Copy link
Contributor Author

@andyw8 andyw8 Oct 5, 2022

Choose a reason for hiding this comment

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

(For easier long-term maintenance of this document, I think it's better to refer to the template rather than duplicating its contents here.)

@andyw8
Copy link
Contributor Author

andyw8 commented Oct 5, 2022

FYI @teddy. (I have more to add).

I'm also thinking it maybe useful to have separate internal page for aspects that apply only to first-party apps.


Previously, we set the entire app user object in the `session` object.
As of v19, since we no longer save the app user to the session (but only the shopify user id), we now store it as `session[:shopify_user_id]`. Please make sure to update any references to that object.

#### Webhook Jobs

It is assumed that you have an ActiveJob implementation configured for `perform_later`, e.g. Sidekiq.
Copy link
Contributor Author

@andyw8 andyw8 Oct 5, 2022

Choose a reason for hiding this comment

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

(some apps may have been using a plain Ruby class with the perform method).

@andyw8 andyw8 force-pushed the andyw8/improve-upgrade-docs branch 2 times, most recently from 28199b4 to 67dfce6 Compare October 14, 2022 14:21
# Common Errors

* `redirect_uri is not whitelisted` – you likely need to update the App Setup on https://partners.shopify.com/.
* `This app can’t load due to an issue with browser cookies` - this can be caused by an infinite redirect due to coding error.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we proceed with #1530 then it should hopefully reduce the need for this.

@andyw8 andyw8 force-pushed the andyw8/improve-upgrade-docs branch 3 times, most recently from f6c0c9d to 381293c Compare October 14, 2022 14:41
@andyw8 andyw8 changed the title WIP: Improve upgrade docs Improve upgrade docs Oct 14, 2022
@andyw8 andyw8 marked this pull request as ready for review October 14, 2022 14:48
@andyw8 andyw8 requested a review from teddyhwang October 14, 2022 14:48
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.

Great additions here! 🎉 One added suggestion on whitelist issues, but looking great!

docs/Upgrading.md Outdated Show resolved Hide resolved
@andyw8 andyw8 force-pushed the andyw8/improve-upgrade-docs branch 3 times, most recently from d188e55 to d261a19 Compare October 14, 2022 16:55
Copy link
Collaborator

@teddyhwang teddyhwang left a comment

Choose a reason for hiding this comment

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

Is it possible to get into more specific examples that we know of from the many reported issues?

Comment on lines 50 to 51
This can be caused by an infinite redirect due to a coding error.
To investigate the cause, you can add a breakpoint or logging to the `rescue` clause of `ShopifyApp::CallbackController`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add in specific examples that we are aware of that would cause this redirect loop?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we might want to mention that this could be cause by using the Authenticated concern in a Fetch request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 909b6f8.

Copy link
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

The changes make sense to me.

@andyw8 andyw8 force-pushed the andyw8/improve-upgrade-docs branch 3 times, most recently from 04ff80b to 25d19f0 Compare October 28, 2022 14:01
@andyw8
Copy link
Contributor Author

andyw8 commented Oct 28, 2022

/shipit

@andyw8 andyw8 force-pushed the andyw8/improve-upgrade-docs branch from 25d19f0 to 86f8fc1 Compare October 31, 2022 14:31
@andyw8 andyw8 merged commit 1476a9d into main Oct 31, 2022
@andyw8 andyw8 deleted the andyw8/improve-upgrade-docs branch October 31, 2022 14:33
fabriazza pushed a commit to fabriazza/shopify_app that referenced this pull request Feb 1, 2023
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.

4 participants