From bcc75be863c53b6a5f0201d9918c99c64897cbfd Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Wed, 9 Nov 2022 14:18:01 -0500 Subject: [PATCH] Deprecation warning instead of raising error --- CHANGELOG.md | 2 +- .../concerns/shopify_app/require_known_shop.rb | 8 +++++--- .../controller_concerns/login_protection.rb | 7 ++++--- .../concerns/require_known_shop_test.rb | 14 ++++++++++++-- .../controller_concerns/login_protection_test.rb | 2 +- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13fcccad2..0d0ee9264 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Unreleased ---------- * Fixes a bug with `EnsureAuthenticatedLinks` causing deep links to not work [#1549](https://github.com/Shopify/shopify_app/pull/1549) * Ensure online token is properly used when using `current_shopify_session` [#1566](https://github.com/Shopify/shopify_app/pull/1566) -* Detect the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560). Depending on how your app is structured, this may result in an error on startup even though the app was working correctly. However, it is helping exposing an existing flaw that may have caused issues in future. +* Log a deprecation warning for the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560). Depending on how your app is structured, this may result in an error on startup even though the app was working correctly. However, it is helping exposing an existing flaw that may have caused issues in future. 21.2.0 (Oct 25, 2022) ---------- diff --git a/app/controllers/concerns/shopify_app/require_known_shop.rb b/app/controllers/concerns/shopify_app/require_known_shop.rb index 4999a9e3a..f0721fade 100644 --- a/app/controllers/concerns/shopify_app/require_known_shop.rb +++ b/app/controllers/concerns/shopify_app/require_known_shop.rb @@ -7,9 +7,11 @@ module RequireKnownShop included do if ancestors.include?(ShopifyApp::LoginProtection) - raise ConfigurationError, - "You are attempting to use both RequireKnownShop and LoginProtection in the same controller. "\ - "These are not compatible." + ActiveSupport::Deprecation.warn(<<~EOS) + We detected the use of incompatible concerns (RequireKnownShop and LoginProtection) in #{name}, + which may lead to unpredictable behavior. In a future release of this library this will raise an error. + EOS + 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 e7c473165..34685feef 100644 --- a/lib/shopify_app/controller_concerns/login_protection.rb +++ b/lib/shopify_app/controller_concerns/login_protection.rb @@ -10,9 +10,10 @@ module LoginProtection included do if ancestors.include?(ShopifyApp::RequireKnownShop) - raise ConfigurationError, - "You are attempting to use both RequireKnownShop and LoginProtection in the same controller. "\ - "These are not compatible." + ActiveSupport::Deprecation.warn(<<~EOS) + We detected the use of incompatible concerns (RequireKnownShop and LoginProtection) in #{name}, + which may lead to unpredictable behavior. In a future release of this library this will raise an error. + EOS end after_action :set_test_cookie diff --git a/test/controllers/concerns/require_known_shop_test.rb b/test/controllers/concerns/require_known_shop_test.rb index 73173d8bd..944d305e3 100644 --- a/test/controllers/concerns/require_known_shop_test.rb +++ b/test/controllers/concerns/require_known_shop_test.rb @@ -62,18 +62,28 @@ def index end test "detects incompatible controller concerns" do - assert_raises ShopifyApp::ConfigurationError do + assert_deprecated(/incompatible concerns/) do Class.new(ApplicationController) do include ShopifyApp::RequireKnownShop include ShopifyApp::LoginProtection end end - assert_raises ShopifyApp::ConfigurationError do + assert_deprecated(/incompatible concerns/) do Class.new(ApplicationController) do include ShopifyApp::RequireKnownShop include ShopifyApp::Authenticated # since this indirectly includes LoginProtection end end + + assert_deprecated(/incompatible concerns/) do + authenticated_controller = Class.new(ApplicationController) do + include ShopifyApp::Authenticated + end + + Class.new(authenticated_controller) do + include ShopifyApp::RequireKnownShop + end + end 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 bd024c08b..681ff5c17 100644 --- a/test/shopify_app/controller_concerns/login_protection_test.rb +++ b/test/shopify_app/controller_concerns/login_protection_test.rb @@ -467,7 +467,7 @@ class LoginProtectionControllerTest < ActionController::TestCase end test "detects incompatible controller concerns" do - assert_raises ShopifyApp::ConfigurationError do + assert_deprecated(/incompatible concerns/) do Class.new(ApplicationController) do include ShopifyApp::LoginProtection include ShopifyApp::RequireKnownShop