FIX: Only show the review page to users that can see it. Do not publish the reviewable count update message to everyone. (#9556)

FIX: Only show the review page to users that can see it. Do not publish the reviewable count update message to everyone. (#9556)

diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb
index 3861998..c0312da 100644
--- a/app/controllers/reviewables_controller.rb
+++ b/app/controllers/reviewables_controller.rb
@@ -6,6 +6,7 @@ class ReviewablesController < ApplicationController
   PER_PAGE = 10
 
   before_action :version_required, only: [:update, :perform]
+  before_action :ensure_can_see
 
   def index
     offset = params[:offset].to_i
@@ -260,4 +261,7 @@ protected
     }
   end
 
+  def ensure_can_see
+    Guardian.new(current_user).ensure_can_see_review_queue!
+  end
 end
diff --git a/app/jobs/regular/notify_reviewable.rb b/app/jobs/regular/notify_reviewable.rb
index 589b095..95fef84 100644
--- a/app/jobs/regular/notify_reviewable.rb
+++ b/app/jobs/regular/notify_reviewable.rb
@@ -48,6 +48,8 @@ protected
   end
 
   def notify(count, user_ids)
+    return if user_ids.blank?
+
     data = { reviewable_count: count }
     MessageBus.publish("/reviewable_counts", data, user_ids: user_ids)
     @contacted += user_ids
diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb
index d8b65e1..4ee12fe 100644
--- a/lib/guardian/user_guardian.rb
+++ b/lib/guardian/user_guardian.rb
@@ -144,4 +144,14 @@ module UserGuardian
     return false if topic.read_restricted_category? || topic.private_message?
     true
   end
+
+  def can_see_review_queue?
+    is_staff? || (
+      SiteSetting.enable_category_group_review &&
+      Reviewable
+        .where(reviewable_by_group_id: @user.group_users.pluck(:group_id))
+        .where('category_id IS NULL or category_id IN (?)', allowed_category_ids)
+        .exists?
+    )
+  end
 end
diff --git a/spec/components/guardian/user_guardian_spec.rb b/spec/components/guardian/user_guardian_spec.rb
index e870843..b25f563 100644
--- a/spec/components/guardian/user_guardian_spec.rb
+++ b/spec/components/guardian/user_guardian_spec.rb
@@ -341,4 +341,50 @@ describe UserGuardian do
       include_examples "can_delete_user staff examples"
     end
   end
+
+  describe "#can_see_review_queue?" do
+    it 'returns true when the user is a staff member' do
+      guardian = Guardian.new(moderator)
+      expect(guardian.can_see_review_queue?).to eq(true)
+    end
+
+    it 'returns false for a regular user' do
+      guardian = Guardian.new(user)
+      expect(guardian.can_see_review_queue?).to eq(false)
+    end
+
+    it "returns true when the user's group can review an item in the queue" do
+      group = Fabricate(:group)
+      group.add(user)
+      guardian = Guardian.new(user)
+      SiteSetting.enable_category_group_review = true
+
+      Fabricate(:reviewable_flagged_post, reviewable_by_group: group, category: nil)
+
+      expect(guardian.can_see_review_queue?).to eq(true)
+    end
+
+    it 'returns false if category group review is disabled' do
+      group = Fabricate(:group)
+      group.add(user)
+      guardian = Guardian.new(user)
+      SiteSetting.enable_category_group_review = false
+
+      Fabricate(:reviewable_flagged_post, reviewable_by_group: group, category: nil)
+
+      expect(guardian.can_see_review_queue?).to eq(false)
+    end
+
+    it 'returns false if the reviewable is under a read restricted category' do
+      group = Fabricate(:group)
+      group.add(user)
+      guardian = Guardian.new(user)
+      SiteSetting.enable_category_group_review = true
+      category = Fabricate(:category, read_restricted: true)
+
+      Fabricate(:reviewable_flagged_post, reviewable_by_group: group, category: category)
+
+      expect(guardian.can_see_review_queue?).to eq(false)
+    end
+  end
 end
diff --git a/spec/jobs/notify_reviewable_spec.rb b/spec/jobs/notify_reviewable_spec.rb
index d4d9691..5d8702d 100644
--- a/spec/jobs/notify_reviewable_spec.rb
+++ b/spec/jobs/notify_reviewable_spec.rb
@@ -102,4 +102,15 @@ describe Jobs::NotifyReviewable do
       expect(group_msg.data[:reviewable_count]).to eq(0)
     end
   end
+
+  it 'skips sending notifications if user_ids is empty' do
+    reviewable = Fabricate(:reviewable, reviewable_by_moderator: true)
+    regular_user = Fabricate(:user)
+
+    messages = MessageBus.track_publish("/reviewable_counts") do
+      described_class.new.execute(reviewable_id: reviewable.id)
+    end
+
+    expect(messages.size).to eq(1)
+  end
 end
diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb
index 3763b72..b89af08 100644
--- a/spec/requests/reviewables_controller_spec.rb
+++ b/spec/requests/reviewables_controller_spec.rb
@@ -590,7 +590,7 @@ describe ReviewablesController do
     end
 
     context "#destroy" do
-      fab!(:user) { Fabricate(:user) }
+      fab!(:user) { Fabricate(:admin) }
 
       before do
         sign_in(user)

GitHub sha: 394babca

This commit appears in #9556 which was approved by eviltrout and eviltrout. It was merged by romanrizzi.