Fix pundit namespace detection (activeadmin#7144)

lib/active_admin/pundit_adapter.rb

@@ -31,13 +31,11 @@ def scope_collection(collection, action = Auth::READ)
     end
 
     def retrieve_policy(subject)
-      case subject
-      when nil then Pundit.policy!(user, namespace(resource))
-      when Class then Pundit.policy!(user, namespace(subject.new))
-      else Pundit.policy!(user, namespace(subject))
-      end
+      Pundit.policy!(user, namespace(policy_target(subject)))
     rescue Pundit::NotDefinedError => e
-      if default_policy_class
+      if (policy = compat_policy(subject))
+        policy
+      elsif default_policy_class
         default_policy(user, subject)
       else
         raise e
@@ -57,8 +55,41 @@ def format_action(action, subject)
 
     private
 
+    def policy_target(subject)
+      case subject
+      when nil then resource
+      when Class then subject.new
+      else subject
+      end
+    end
+
+    # This method is needed to fallback to our previous policy searching logic.
+    # I.e.: when class name contains `default_policy_namespace` (eg: ShopAdmin)
+    # we should try to search it without namespace. This is because that's
+    # the only thing that worked in this case before we fixed our buggy namespace
+    # detection, so people are probably relying on it.
+    # This fallback might be removed in future versions of ActiveAdmin, so
+    # pundit_adapter search will work consistently with provided namespaces
+    def compat_policy(subject)
+      target = policy_target(subject)
+
+      return unless default_policy_namespace &&
+        target.class.to_s.include?(default_policy_module) &&
+        (policy = Pundit.policy(user, target))
+
+      policy_name = policy.class.to_s
+
+      Deprecation.warn "You have `pundit_policy_namespace` configured as `#{default_policy_namespace}`, " \
+        "but ActiveAdmin was unable to find policy #{default_policy_module}::#{policy_name}. " \
+        "#{policy_name} will be used instead. " \
+        "This behavior will be removed in future versions of ActiveAdmin. " \
+        "To fix this warning, move your #{policy_name} policy to the #{default_policy_module} namespace"
+
+      policy
+    end
+
     def namespace(object)
-      if default_policy_namespace && !object.class.to_s.include?(default_policy_namespace.to_s.camelize)
+      if default_policy_namespace && !object.class.to_s.match?(/^#{default_policy_module}::/)
         [default_policy_namespace.to_sym, object]
       else
         object
@@ -77,6 +108,10 @@ def default_policy_namespace
       ActiveAdmin.application.pundit_policy_namespace
     end
 
+    def default_policy_module
+      default_policy_namespace.to_s.camelize
+    end
+
   end
 
 end

spec/spec_helper.rb

@@ -2,6 +2,7 @@
 require "simplecov" if ENV["COVERAGE"] == "true"
 
 require_relative "support/matchers/perform_database_query_matcher"
+require_relative "support/shared_contexts/capture_stderr"
 
 RSpec.configure do |config|
   config.disable_monkey_patching!

spec/support/shared_contexts/capture_stderr.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+RSpec.shared_context "capture stderr" do
+  around do |example|
+    original_stderr = $stderr
+    $stderr = StringIO.new
+    example.run
+    $stderr = original_stderr
+  end
+end

spec/unit/action_builder_spec.rb

@@ -123,12 +123,7 @@
   end
 
   context "when method with given name is already defined" do
-    around do |example|
-      original_stderr = $stderr
-      $stderr = StringIO.new
-      example.run
-      $stderr = original_stderr
-    end
+    include_context "capture stderr"
 
     describe "defining member action" do
       let :action! do

spec/unit/pundit_adapter_spec.rb

@@ -91,6 +91,34 @@ def resolve
       auth.authorized?(:index)
     end
 
+    context "when model name contains policy namespace name" do
+      include_context "capture stderr"
+
+      before do
+        allow(ActiveAdmin.application).to receive(:pundit_policy_namespace).and_return :pub
+        namespace.register(Publisher)
+        ActiveSupport::Deprecation.behavior = :stderr
+      end
+
+      after do
+        ActiveSupport::Deprecation.behavior = :raise
+      end
+
+      it "looks for a namespaced policy" do
+        expect(Pundit).to receive(:policy!).with(anything, [:pub, Publisher]).and_return(DefaultPolicy.new(double, double))
+        auth.authorized?(:read, Publisher)
+      end
+
+      it "fallbacks to the policy without namespace" do
+        expect(Pundit).to receive(:policy!).with(anything, [:pub, Publisher]).and_raise(Pundit::NotDefinedError)
+        expect(Pundit).to receive(:policy).with(anything, Publisher).and_return(DefaultPolicy.new(double, double))
+
+        auth.authorized?(:read, Publisher)
+
+        expect($stderr.string).to include("ActiveAdmin was unable to find policy Pub::DefaultPolicy. DefaultPolicy will be used instead.")
+      end
+    end
+
     context "when Pundit is unable to find policy scope" do
       let(:collection) { double("collection", to_sym: :collection) }
       subject(:scope) { auth.scope_collection(collection, :read) }