Detect controller concern clash (#1560)

* Detect controller concern clash

* Deprecation warning instead of raising error

Co-authored-by: Andy Waite <andyw8@users.noreply.github.com>

CHANGELOG.md

@@ -2,6 +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)
+* Log a deprecation warning for the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560)
 
 21.2.0 (Oct 25, 2022)
 ----------

app/controllers/concerns/shopify_app/require_known_shop.rb

@@ -6,6 +6,14 @@ module RequireKnownShop
     include ShopifyApp::RedirectForEmbedded
 
     included do
+      if ancestors.include?(ShopifyApp::LoginProtection)
+        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
       before_action :check_shop_known
     end

lib/shopify_app/controller_concerns/login_protection.rb

@@ -9,6 +9,13 @@ module LoginProtection
     include ShopifyApp::SanitizedParams
 
     included do
+      if ancestors.include?(ShopifyApp::RequireKnownShop)
+        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
       rescue_from ShopifyAPI::Errors::HttpResponseError, with: :handle_http_error
     end

test/controllers/concerns/require_known_shop_test.rb

@@ -60,4 +60,30 @@ def index
 
     assert_response :ok
   end
+
+  test "detects incompatible controller concerns" do
+    assert_deprecated(/incompatible concerns/) do
+      Class.new(ApplicationController) do
+        include ShopifyApp::RequireKnownShop
+        include ShopifyApp::LoginProtection
+      end
+    end
+
+    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

test/dummy/app/controllers/application_controller.rb

@@ -1,5 +1,4 @@
 # frozen_string_literal: true
 
 class ApplicationController < ActionController::Base
-  include ShopifyApp::LoginProtection
 end

test/dummy/app/controllers/home_controller.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class HomeController < ApplicationController
+  include ShopifyApp::EmbeddedApp
+  include ShopifyApp::RequireKnownShop
+
   def index
     "index"
   end

test/shopify_app/controller_concerns/login_protection_test.rb

@@ -466,6 +466,15 @@ class LoginProtectionControllerTest < ActionController::TestCase
     end
   end
 
+  test "detects incompatible controller concerns" do
+    assert_deprecated(/incompatible concerns/) do
+      Class.new(ApplicationController) do
+        include ShopifyApp::LoginProtection
+        include ShopifyApp::RequireKnownShop
+      end
+    end
+  end
+
   private
 
   def assert_fullpage_redirected(shop_domain, _response)