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

Re-add deprecated files to prevent major release #1884

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

paulomarg
Copy link
Contributor

What this PR does

In #1861, we (correctly) removed some files related to JWT that should no longer be used in shopify_app. However, by removing those files we'd be forced to make a major release, which is still a bit too close to the last one.

This PR adds back the code that was removed (but will be unused internally) so it doesn't break existing apps. It will log deprecations for them instead.

Log message will look like this

================================================
=> Upcoming deprecations in v23.0:
* 'CallbackController::perform_after_authenticate_job' and related methods 'install_webhooks', 'perform_after_authenticate_job'
* will be removed from CallbackController in the next major release. If you need to customize
* post authentication tasks, see https://github.com/Shopify/shopify_app/blob/main/docs/shopify_app/authentication.md#post-authenticate-tasks

* ShopifyApp::JWTMiddleware will be removed, use ShopifyApp::WithShopifyIdToken instead.
================================================

Reviewer's guide to testing

Use this branch in the Gemfile of an app using the ruby template, and set it up to use ShopifyApp::JWTMiddleware.

Things to focus on

  1. Is it ok to not have tests? The feature will be unchanged until the next major release.
  2. Can you see any fringe cases where it won't work?

@paulomarg paulomarg requested a review from a team as a code owner July 24, 2024 18:09
@paulomarg paulomarg force-pushed the readd_deprecated_jwt_files branch from 820f5a6 to f0c67e2 Compare July 24, 2024 18:10
@paulomarg paulomarg force-pushed the readd_deprecated_jwt_files branch from f0c67e2 to 0adba2a Compare July 24, 2024 18:49
Co-authored-by: Liz Kenyon <elizabeth.kenyon@shopify.com>
@lizkenyon lizkenyon merged commit 747917c into main Jul 24, 2024
7 checks passed
@lizkenyon lizkenyon deleted the readd_deprecated_jwt_files branch July 24, 2024 19:10
@joevandyk
Copy link

is there a way to silence these log messages? i'm installing shopify_app in a brand new app, and these messages don't seem terribly useful.

@paulomarg
Copy link
Contributor Author

paulomarg commented Aug 12, 2024

Fair question! I'll add that to our tracking to see if we can hide these warnings based on the package version.

For the sake of transparency, we might not get to this immediately though.

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