FIX: Respect min_flags_staff_visibility for new flags too

FIX: Respect min_flags_staff_visibility for new flags too

There was a situation where if:

  • There were new flags to review that met the visibility threshold

AND

  • There were old flags that didn’t meet the threshold

THEN

a pending flags notification would be sent out. This fixes that case. Staff should not be notified of flags if they do not meet the threshold and are old.

diff --git a/app/jobs/scheduled/pending_flags_reminder.rb b/app/jobs/scheduled/pending_flags_reminder.rb
index a9b1201..4910b6a 100644
--- a/app/jobs/scheduled/pending_flags_reminder.rb
+++ b/app/jobs/scheduled/pending_flags_reminder.rb
@@ -12,7 +12,6 @@ module Jobs
         return unless flagged_posts_count > 0
 
         flag_ids = pending_flag_ids
-
         if flag_ids.size > 0 && last_notified_id.to_i < flag_ids.max
 
           usernames = active_moderator_usernames
@@ -33,9 +32,19 @@ module Jobs
     end
 
     def pending_flag_ids
+      by_post = {}
+
       FlagQuery.flagged_post_actions(filter: 'active')
         .where('post_actions.created_at < ?', SiteSetting.notify_about_flags_after.to_i.hours.ago)
-        .pluck(:id)
+        .pluck(:post_id, :id)
+        .each do |row|
+
+        by_post[row[0]] ||= []
+        by_post[row[0]] << row[1]
+      end
+
+      by_post.delete_if { |post_id, flags| flags.size < SiteSetting.min_flags_staff_visibility }
+      by_post.values.flatten.uniq
     end
 
     def last_notified_id
diff --git a/spec/jobs/pending_flags_reminder_spec.rb b/spec/jobs/pending_flags_reminder_spec.rb
index dee33a8..5a820e1 100644
--- a/spec/jobs/pending_flags_reminder_spec.rb
+++ b/spec/jobs/pending_flags_reminder_spec.rb
@@ -37,26 +37,37 @@ describe Jobs::PendingFlagsReminder do
       expect(job.last_notified_id).to eq(old_flag.id)
     end
 
-    it "doesn't send a message when min_flags_staff_visibility is not met" do
-      SiteSetting.min_flags_staff_visibility = 2
+    it "sends message when there is a flag older than 48 hours" do
       Fabricate(:flag, created_at: 49.hours.ago)
-      Fabricate(:flag, created_at: 51.hours.ago)
-      PostCreator.expects(:create).never
-      described_class.new.execute({})
-    end
-
-    it "sends a message when min_flags_staff_visibility is met" do
-      SiteSetting.min_flags_staff_visibility = 2
-      f = Fabricate(:flag, created_at: 49.hours.ago)
-      Fabricate(:flag, post: f.post, created_at: 51.hours.ago)
       PostCreator.expects(:create).once.returns(true)
       described_class.new.execute({})
     end
 
-    it "sends message when there is a flag older than 48 hours" do
-      Fabricate(:flag, created_at: 49.hours.ago)
-      PostCreator.expects(:create).once.returns(true)
-      described_class.new.execute({})
+    context "min_flags_staff_visibility" do
+      it "doesn't send a message when min_flags_staff_visibility is not met" do
+        SiteSetting.min_flags_staff_visibility = 2
+        Fabricate(:flag, created_at: 49.hours.ago)
+        Fabricate(:flag, created_at: 51.hours.ago)
+        PostCreator.expects(:create).never
+        described_class.new.execute({})
+      end
+
+      it "doesn't send a message when min_flags_staff_visibility is met on new flags but not old" do
+        SiteSetting.min_flags_staff_visibility = 2
+        flag = Fabricate(:flag, created_at: 24.hours.ago)
+        Fabricate(:flag, post: flag.post, created_at: 49.hours.ago)
+        Fabricate(:flag, created_at: 51.hours.ago)
+        PostCreator.expects(:create).never
+        described_class.new.execute({})
+      end
+
+      it "sends a message when min_flags_staff_visibility is met" do
+        SiteSetting.min_flags_staff_visibility = 2
+        f = Fabricate(:flag, created_at: 49.hours.ago)
+        Fabricate(:flag, post: f.post, created_at: 51.hours.ago)
+        PostCreator.expects(:create).once.returns(true)
+        described_class.new.execute({})
+      end
     end
 
   end

GitHub sha: 78ddc829

1 Like