short-circuit if there is no policy, and add :read_project check

3 files changed, 16 insertions, 12 deletions

diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb
index 28ac22da6e2..837b62ec0cb 100644
--- a/app/models/notification_recipient.rb
+++ b/app/models/notification_recipient.rb
@@ -76,10 +76,13 @@ class NotificationRecipient
   end
 
   def has_access?
-    return false unless user.can?(:receive_notifications)
-    return true unless @read_ability
-
     DeclarativePolicy.subject_scope do
+      return false unless user.can?(:receive_notifications)
+      return false if @project && !user.can?(:read_project, @project)
+
+      return true unless @read_ability
+      return true unless DeclarativePolicy.has_policy?(@target)
+
       user.can?(@read_ability, @target)
     end
   end
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 1ce92c8cbdc..540e568fed2 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -308,11 +308,7 @@ module NotificationRecipientService
       end
 
       def read_ability
-        @read_ability ||=
-          case target
-          when Commit then nil
-          else :"read_#{target.class.model_name.name.underscore}"
-          end
+        @read_ability ||= :"read_#{target.class.model_name.name.underscore}"
       end
 
       def subject
diff --git a/lib/declarative_policy.rb b/lib/declarative_policy.rb
index b1eb1a6cef1..4936669a73a 100644
--- a/lib/declarative_policy.rb
+++ b/lib/declarative_policy.rb
@@ -28,7 +28,12 @@ module DeclarativePolicy
 
       subject = find_delegate(subject)
 
-      class_for_class(subject.class)
+      class_for_class(subject.class) \
+        or raise "no policy for #{subject.class.name}"
+    end
+
+    def has_policy?(subject)
+      !class_for_class(subject.class).nil?
     end
 
     private
@@ -51,9 +56,7 @@ module DeclarativePolicy
         end
       end
 
-      policy_class = subject_class.instance_variable_get(CLASS_CACHE_IVAR)
-      raise "no policy for #{subject.class.name}" if policy_class.nil?
-      policy_class
+      subject_class.instance_variable_get(CLASS_CACHE_IVAR)
     end
 
     def compute_class_for_class(subject_class)
@@ -71,6 +74,8 @@ module DeclarativePolicy
           nil
         end
       end
+
+      nil
     end
 
     def find_delegate(subject)