Raise error when incompatible controller concerns are detected (#1809)

* Raise error when incompatible controller concerns are detected

* changelog

* update tests on reverse side as well

* lint fix

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)

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

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

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

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