Skip to content

Commit

Permalink
Deprecation warning instead of raising error
Browse files Browse the repository at this point in the history
  • Loading branch information
andyw8 committed Nov 9, 2022
1 parent 9d7e60a commit bcc75be
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
----------
Expand Down
8 changes: 5 additions & 3 deletions app/controllers/concerns/shopify_app/require_known_shop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions lib/shopify_app/controller_concerns/login_protection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions test/controllers/concerns/require_known_shop_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bcc75be

Please sign in to comment.