From 5597957cc6918032b2b2bb897a4f32af1625a55b Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 20 Nov 2015 15:32:08 -0500
Subject: [PATCH] FIX: don't send repeat notifications to moderators about the
 same pending approval users

---
 app/jobs/scheduled/pending_users_reminder.rb | 24 +++++++++++++++++++-
 spec/jobs/pending_users_reminder_spec.rb     |  8 +++----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/app/jobs/scheduled/pending_users_reminder.rb b/app/jobs/scheduled/pending_users_reminder.rb
index 95b02ea17..c72049d46 100644
--- a/app/jobs/scheduled/pending_users_reminder.rb
+++ b/app/jobs/scheduled/pending_users_reminder.rb
@@ -7,7 +7,13 @@ module Jobs
 
     def execute(args)
       if SiteSetting.must_approve_users
-        count = AdminUserIndexQuery.new({query: 'pending'}).find_users_query.count
+        query = AdminUserIndexQuery.new({query: 'pending'}).find_users_query # default order is: users.created_at DESC
+        newest_username = query.limit(1).pluck(:username).first
+
+        return true if newest_username == previous_newest_username # already notified
+
+        count = query.count
+
         if count > 0
           target_usernames = Group[:moderators].users.map do |u|
             u.id > 0 && u.notifications.joins(:topic)
@@ -26,9 +32,25 @@ module Jobs
               title: I18n.t("system_messages.pending_users_reminder.subject_template", {count: count}),
               raw: I18n.t("system_messages.pending_users_reminder.text_body_template", {count: count, base_url: Discourse.base_url})
             )
+
+            self.previous_newest_username = newest_username
           end
         end
       end
+
+      true
+    end
+
+    def previous_newest_username
+      $redis.get previous_newest_username_cache_key
+    end
+
+    def previous_newest_username=(username)
+      $redis.setex previous_newest_username_cache_key, 7.days, username
+    end
+
+    def previous_newest_username_cache_key
+      "pending-users-reminder:newest-username".freeze
     end
 
   end
diff --git a/spec/jobs/pending_users_reminder_spec.rb b/spec/jobs/pending_users_reminder_spec.rb
index a0cbbfb9c..301042c1e 100644
--- a/spec/jobs/pending_users_reminder_spec.rb
+++ b/spec/jobs/pending_users_reminder_spec.rb
@@ -4,19 +4,19 @@ describe Jobs::PendingUsersReminder do
 
   context 'must_approve_users is true' do
     before do
-      SiteSetting.stubs(:must_approve_users).returns(true)
+      SiteSetting.must_approve_users = true
+      Jobs::PendingUsersReminder.any_instance.stubs(:previous_newest_username).returns(nil)
     end
 
     it "doesn't send a message to anyone when there are no pending users" do
-      AdminUserIndexQuery.any_instance.stubs(:find_users_query).returns(stub_everything(count: 0))
       PostCreator.expects(:create).never
       Jobs::PendingUsersReminder.new.execute({})
     end
 
     it "sends a message when there are pending users" do
-      Fabricate(:moderator)
+      Fabricate(:moderator, approved: true, approved_by_id: -1, approved_at: 1.week.ago)
+      Fabricate(:user)
       Group.refresh_automatic_group!(:moderators)
-      AdminUserIndexQuery.any_instance.stubs(:find_users_query).returns(stub_everything(count: 1))
       PostCreator.expects(:create).once
       Jobs::PendingUsersReminder.new.execute({})
     end