diff --git a/CHANGELOG.md b/CHANGELOG.md index c49720b77..9b630cf59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Unreleased * ⚠️ [Breaking] Bumps minimum supported Ruby version to 3.0. Bumps `shopify_api` to 14.0 [1801](https://github.com/Shopify/shopify_app/pull/1801) * ⚠️ [Breaking] Removes deprecated controller concerns that were renamed in `v21.10.0`. [1805](https://github.com/Shopify/shopify_app/pull/1805) * ⚠️ [Breaking] Removes deprecated `ScripttagManager`. We realize there was communication error in our logging where we logged future deprecation instead of our inteded removal. Since we have been logging that for 2 years we felt we'd move forward with the removal instead pushing this off until the next major release. [1806](https://github.com/Shopify/shopify_app/pull/1806) +* ⚠️ [Breaking] Thows an error if a controller includes incompatible concerns (LoginProtection/EnsureInstalled) [1809](https://github.com/Shopify/shopify_app/pull/1809) * ⚠️ [Breaking] No longer rescues non-shopify API errors during OAuth callback [1807](https://github.com/Shopify/shopify_app/pull/1807) * Make type param for webhooks route optional. This will fix a bug with CLI initiated webhooks.[1786](https://github.com/Shopify/shopify_app/pull/1786) diff --git a/app/controllers/concerns/shopify_app/ensure_installed.rb b/app/controllers/concerns/shopify_app/ensure_installed.rb index fb32e3a9d..21b624a37 100644 --- a/app/controllers/concerns/shopify_app/ensure_installed.rb +++ b/app/controllers/concerns/shopify_app/ensure_installed.rb @@ -9,10 +9,10 @@ module EnsureInstalled if defined?(ShopifyApp::LoginProtection) && ancestors.include?(ShopifyApp::LoginProtection) message = <<~EOS We detected the use of incompatible concerns (EnsureInstalled and LoginProtection) in #{name}, - which may lead to unpredictable behavior. In a future release of this library this will raise an error. + which leads to unpredictable behavior. You cannot include both concerns in the same controller. EOS - ShopifyApp::Logger.deprecated(message, "22.0.0") + raise message end before_action :check_shop_domain diff --git a/lib/shopify_app/controller_concerns/login_protection.rb b/lib/shopify_app/controller_concerns/login_protection.rb index c60a51750..df0214637 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -8,14 +8,13 @@ module LoginProtection include ShopifyApp::SanitizedParams included do - if defined?(ShopifyApp::RequireKnownShop) && - defined?(ShopifyApp::EnsureInstalled) && - ancestors.include?(ShopifyApp::RequireKnownShop || ShopifyApp::EnsureInstalled) + if defined?(ShopifyApp::EnsureInstalled) && + ancestors.include?(ShopifyApp::EnsureInstalled) message = <<~EOS - We detected the use of incompatible concerns (RequireKnownShop/EnsureInstalled and LoginProtection) in #{name}, - which may lead to unpredictable behavior. In a future release of this library this will raise an error. + We detected the use of incompatible concerns (EnsureInstalled and LoginProtection) in #{name}, + which leads to unpredictable behavior. You cannot include both concerns in the same controller. EOS - ShopifyApp::Logger.deprecated(message, "22.0.0") + raise message end rescue_from ShopifyAPI::Errors::HttpResponseError, with: :handle_http_error diff --git a/test/controllers/concerns/ensure_installed_test.rb b/test/controllers/concerns/ensure_installed_test.rb index aaddd6f6a..e2fadfaad 100644 --- a/test/controllers/concerns/ensure_installed_test.rb +++ b/test/controllers/concerns/ensure_installed_test.rb @@ -139,30 +139,23 @@ def index get :index, params: { shop: "shop1.myshopify.com" } end - test "detects incompatible controller concerns" do - version = "22.0.0" - ShopifyApp::Logger.expects(:deprecated).with(regexp_matches(/incompatible concerns/), version) - ShopifyApp::Logger.stubs(:deprecated).with("Itp will be removed in an upcoming version", "22.0.0") - - Class.new(ApplicationController) do - include ShopifyApp::LoginProtection - include ShopifyApp::EnsureInstalled + test "detects incompatible controller concerns and raises an error" do + assert_raise do + Class.new(ApplicationController) do + include ShopifyApp::LoginProtection + include ShopifyApp::EnsureInstalled + end end - ShopifyApp::Logger.expects(:deprecated).with(regexp_matches(/incompatible concerns/), version) - Class.new(ApplicationController) do - include ShopifyApp::EnsureHasSession # since this indirectly includes LoginProtection - include ShopifyApp::EnsureInstalled + assert_raise do + Class.new(ApplicationController) do + include ShopifyApp::EnsureHasSession # since this indirectly includes LoginProtection + include ShopifyApp::EnsureInstalled + end end - ShopifyApp::Logger.expects(:deprecated).with(regexp_matches(/incompatible concerns/), version) - authenticated_controller = Class.new(ApplicationController) do - include ShopifyApp::EnsureHasSession - end - Class.new(authenticated_controller) do + Class.new(ApplicationController) do include ShopifyApp::EnsureInstalled end - - assert_within_deprecation_schedule(version) end end diff --git a/test/shopify_app/controller_concerns/login_protection_test.rb b/test/shopify_app/controller_concerns/login_protection_test.rb index 36e25716f..0ef8c5b3b 100644 --- a/test/shopify_app/controller_concerns/login_protection_test.rb +++ b/test/shopify_app/controller_concerns/login_protection_test.rb @@ -555,18 +555,17 @@ class LoginProtectionControllerTest < ActionController::TestCase end end - test "detects incompatible controller concerns" do - version = "22.0.0" - - ShopifyApp::Logger.expects(:deprecated).with(regexp_matches(/incompatible concerns/), version) - ShopifyApp::Logger.stubs(:deprecated).with("Itp will be removed in an upcoming version", version) + test "detects incompatible controller concerns and raises an error" do + assert_raise do + Class.new(ApplicationController) do + include ShopifyApp::EnsureInstalled + include ShopifyApp::LoginProtection + end + end Class.new(ApplicationController) do include ShopifyApp::LoginProtection - include ShopifyApp::EnsureInstalled end - - assert_within_deprecation_schedule(version) end private