Shopify / shopify_app

A Rails Engine for building Shopify Apps
MIT License
1.76k stars 683 forks source link

Set CurrentTenant to nil when gem is present #1898

Closed rachel-carvalho closed 1 month ago

rachel-carvalho commented 1 month ago

What this PR does

This makes sure the CurrentTenant is set to nil if gem is present whenever ShopifyApp ends the request in a before action, which happens:

Reviewer's guide to testing

Steps to reproduce the error:

  1. create a new app with the CLI
  2. make sure it has write_products scope and shopify app deploy
  3. make sure new_embedded_auth_strategy is false
  4. add the line to set the current tenant to HomeController#index, ProductsController#count and ProductsController#create
  5. add the security gem
  6. bundle and start the server
  7. install the app
  8. delete the row created in the shops table
  9. click on Populate 5 products
  10. πŸ‘€ see the current tenant exception in the server logs πŸ’₯

Seeing the fix

  1. point shopify_app in the Gemfile to this branch with gem "shopify_app", github: "Shopify/shopify_app", ref: "fix_current_tenant_error"
  2. bundle and restart the server
  3. open the app again
  4. delete the row created by install in the shops table
  5. click on Populate 5 products
  6. πŸ‘€ see how it correctly redirects to get a new access token ✨

Things to focus on

I couldn't find a nicer way to test the path of defined?(ClassName), I'd appreciate if anyone has better suggestions.

Anything else I'm missing?

Checklist

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