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.

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

spec/jobs/pending_flags_reminder_spec.rb

@@ -37,27 +37,38 @@ 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
-      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({})
     end
 
+    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
 end