FIX: do not notify admins on suppressed categories (#20238)

* FIX: do not notify admins on suppressed categories

Avoid notifying admins on categories where they are not explicitly members
in cases where SiteSetting.suppress_secured_categories_from_admin is
enabled.

This helps keep notification stream clean and avoids admins mistakenly
being invited to discussions that should be suppressed

app/services/post_alerter.rb

@@ -466,8 +466,7 @@ class PostAlerter
       return
     end
 
-    # Make sure the user can see the post
+    return if !Guardian.new(user).can_receive_post_notifications?(post)
-    return unless Guardian.new(user).can_see?(post)
 
     return if user.staged? && topic.category&.mailinglist_mirror?
 

lib/guardian/post_guardian.rb

@@ -249,6 +249,18 @@ module PostGuardian
       !post_action.post&.topic&.archived?
   end
 
+  def can_receive_post_notifications?(post)
+    return false if !authenticated?
+
+    if is_admin? && SiteSetting.suppress_secured_categories_from_admin
+      topic = post.topic
+      if !topic.private_message? && topic.category.read_restricted
+        return secure_category_ids.include?(topic.category_id)
+      end
+    end
+    can_see_post?(post)
+  end
+
   def can_see_post?(post)
     return false if post.blank?
     return true if is_admin?

spec/lib/guardian_spec.rb

@@ -820,6 +820,42 @@ RSpec.describe Guardian do
     end
   end
 
+  describe "can_receive_post_notifications?" do
+    it "returns false with a nil object" do
+      expect(Guardian.new.can_receive_post_notifications?(nil)).to be_falsey
+      expect(Guardian.new(user).can_receive_post_notifications?(nil)).to be_falsey
+    end
+
+    it "does not allow anonymous to be notified" do
+      expect(Guardian.new.can_receive_post_notifications?(post)).to be_falsey
+    end
+
+    it "allows public categories" do
+      expect(Guardian.new(trust_level_0).can_receive_post_notifications?(post)).to be_truthy
+      expect(Guardian.new(admin).can_receive_post_notifications?(post)).to be_truthy
+    end
+
+    it "diallows secure categories with no access" do
+      secure_category = Fabricate(:category, read_restricted: true)
+      post.topic.update!(category_id: secure_category.id)
+
+      expect(Guardian.new(trust_level_0).can_receive_post_notifications?(post)).to be_falsey
+      expect(Guardian.new(admin).can_receive_post_notifications?(post)).to be_truthy
+
+      SiteSetting.suppress_secured_categories_from_admin = true
+
+      expect(Guardian.new(admin).can_receive_post_notifications?(post)).to be_falsey
+
+      secure_category.set_permissions(group => :write)
+      secure_category.save!
+
+      group.add(admin)
+      group.save!
+
+      expect(Guardian.new(admin).can_receive_post_notifications?(post)).to be_truthy
+    end
+  end
+
   describe "can_see?" do
     it "returns false with a nil object" do
       expect(Guardian.new.can_see?(nil)).to be_falsey

spec/services/post_action_notifier_spec.rb

@@ -202,25 +202,24 @@ RSpec.describe PostActionNotifier do
   end
 
   context "with private message" do
+    fab!(:private_message) do
+      Fabricate(:topic, archetype: Archetype.private_message, category_id: nil)
+    end
     fab!(:user) { Fabricate(:user) }
-    fab!(:mention_post) { Fabricate(:post, user: user, raw: "Hello @eviltrout") }
+    fab!(:mention_post) do
-    let(:topic) do
+      Fabricate(:post, topic: private_message, user: user, raw: "Hello @eviltrout")
-      topic = mention_post.topic
-      topic.update_columns archetype: Archetype.private_message, category_id: nil
-      topic
     end
 
     it "won't notify someone who can't see the post" do
-      expect {
+      expect { PostAlerter.post_created(mention_post) }.not_to change(
-        Guardian.any_instance.expects(:can_see?).with(instance_of(Post)).returns(false)
+        evil_trout.notifications,
-        mention_post
+        :count,
-        PostAlerter.post_created(mention_post)
+      )
-      }.not_to change(evil_trout.notifications, :count)
     end
 
     it "creates like notifications" do
       other_user = Fabricate(:user)
-      topic.allowed_users << user << other_user
+      private_message.allowed_users << user << other_user
       expect { PostActionCreator.like(other_user, mention_post) }.to change(
         user.notifications,
         :count,

spec/services/post_alerter_spec.rb

@@ -45,6 +45,15 @@ RSpec.describe PostAlerter do
   fab!(:user) { Fabricate(:user) }
   fab!(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2]) }
 
+  fab!(:private_category) do
+    Fabricate(
+      :private_category,
+      group: group,
+      email_in: "test@test.com",
+      email_in_allow_strangers: true,
+    )
+  end
+
   def create_post_with_alerts(args = {})
     post = Fabricate(:post, args)
     PostAlerter.post_created(post)
@@ -609,22 +618,13 @@ RSpec.describe PostAlerter do
       group_member = Fabricate(:user)
       group.add(group_member)
 
-      private_category =
-        Fabricate(
-          :private_category,
-          group: group,
-          email_in: "test@test.com",
-          email_in_allow_strangers: true,
-        )
-
       staged_user_post = create_post(user: staged_user, category: private_category)
 
-      linking =
+      create_post(
-        create_post(
+        user: group_member,
-          user: group_member,
+        category: private_category,
-          category: private_category,
+        raw: "my magic topic\n##{Discourse.base_url}#{staged_user_post.url}",
-          raw: "my magic topic\n##{Discourse.base_url}#{staged_user_post.url}",
+      )
-        )
 
       staged_user.reload
       expect(
@@ -1392,6 +1392,17 @@ RSpec.describe PostAlerter do
       )
     end
 
+    it "does not notify admins when suppress_secured_categories_from_admin is enabled" do
+      SiteSetting.suppress_secured_categories_from_admin = true
+
+      topic = Fabricate(:topic, category: private_category)
+      post = Fabricate(:post, raw: "hello @#{admin.username} how are you today?", topic: topic)
+
+      PostAlerter.post_created(post)
+
+      expect(admin.notifications.count).to eq(0)
+    end
+
     it "doesn't notify regular user about whispered reply" do
       _post = Fabricate(:post, user: user, topic: topic)
 
@@ -1639,14 +1650,6 @@ RSpec.describe PostAlerter do
         group.add(group_member)
         group.add(staged_member)
 
-        private_category =
-          Fabricate(
-            :private_category,
-            group: group,
-            email_in: "test@test.com",
-            email_in_allow_strangers: false,
-          )
-
         level = CategoryUser.notification_levels[:watching]
         CategoryUser.set_notification_level_for_category(group_member, level, private_category.id)
         CategoryUser.set_notification_level_for_category(staged_member, level, private_category.id)