From 3602f83cf443d9736e94747f25b31a8e9a318e93 Mon Sep 17 00:00:00 2001
From: Roman Rizzi <rizziromanalejandro@gmail.com>
Date: Fri, 10 Dec 2021 10:17:39 -0300
Subject: [PATCH] FEATURE: Delete previous reviewable reminders. (#15250)

We send the reminder using the GroupMessage class, which supports removing previous messages. We can't match them by raw because they could mention different moderators. Also, I had to change the subject to remove dynamically generated values, which is necessary for finding them.
---
 .../scheduled/pending_reviewables_reminder.rb | 21 ++++++++++++-------
 app/services/group_message.rb                 |  7 +++++--
 config/locales/server.en.yml                  | 14 ++++++-------
 .../jobs/pending_reviewables_reminder_spec.rb | 12 +++++++++++
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/app/jobs/scheduled/pending_reviewables_reminder.rb b/app/jobs/scheduled/pending_reviewables_reminder.rb
index 3cc4b124a0a..6d96530f19c 100644
--- a/app/jobs/scheduled/pending_reviewables_reminder.rb
+++ b/app/jobs/scheduled/pending_reviewables_reminder.rb
@@ -22,14 +22,19 @@ module Jobs
           usernames = active_moderator_usernames
           mentions = usernames.size > 0 ? "@#{usernames.join(', @')} " : ""
 
-          @sent_reminder = PostCreator.create(
-            Discourse.system_user,
-            target_group_names: Group[:moderators].name,
-            archetype: Archetype.private_message,
-            subtype: TopicSubtype.system_message,
-            title: I18n.t('reviewables_reminder.subject_template', count: reviewable_ids.size),
-            raw: mentions + I18n.t('reviewables_reminder.submitted', count: SiteSetting.notify_about_flags_after, base_path: Discourse.base_path)
-          ).present?
+          message = GroupMessage.new(
+            Group[:moderators].name,
+            'reviewables_reminder',
+            {
+              limit_once_per: false,
+              message_params: { mentions: mentions, count: SiteSetting.notify_about_flags_after }
+            }
+          )
+
+          Topic.transaction do
+            message.delete_previous!(match_raw: false)
+            @sent_reminder = message.create.present?
+          end
 
           self.class.last_notified_id = reviewable_ids[0]
         end
diff --git a/app/services/group_message.rb b/app/services/group_message.rb
index fba523a15b2..4ffac25e128 100644
--- a/app/services/group_message.rb
+++ b/app/services/group_message.rb
@@ -43,7 +43,7 @@ class GroupMessage
     end
   end
 
-  def delete_previous!
+  def delete_previous!(match_raw: true)
     posts = Post
       .joins(topic: { topic_allowed_groups: :group })
       .where(topic: {
@@ -57,7 +57,10 @@ class GroupMessage
         }
       })
       .where("posts.created_at > ?", RECENT_MESSAGE_PERIOD.ago)
-      .where(raw: I18n.t("system_messages.#{@message_type}.text_body_template", message_params).rstrip)
+
+    if match_raw
+      posts = posts.where(raw: I18n.t("system_messages.#{@message_type}.text_body_template", message_params).rstrip)
+    end
 
     posts.find_each do |post|
       PostDestroyer.new(Discourse.system_user, post).destroy
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index a85fe23ffbc..4aff5357822 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2630,14 +2630,6 @@ en:
       one: "User %{username} has %{count} post in a public topic or personal message, so they can't be deleted."
       other: "User %{username} has %{count} posts in public topics or personal messages, so they can't be deleted."
 
-  reviewables_reminder:
-    submitted:
-      one: "Items were submitted over %{count} hour ago. [Please review them](%{base_path}/review)."
-      other: "Items were submitted over %{count} hours ago. [Please review them](%{base_path}/review)."
-    subject_template:
-      one: "%{count} item needs to be reviewed"
-      other: "%{count} items need to be reviewed"
-
   unsubscribe_mailer:
     title: "Unsubscribe Mailer"
     subject_template: "Confirm you no longer want to receive email updates from %{site_title}"
@@ -2829,6 +2821,12 @@ en:
     other: "This topic is temporarily closed for at least %{count} hours due to a large number of community flags."
 
   system_messages:
+    reviewables_reminder:
+      subject_template: "There're items in the review queue that need reviewing"
+      text_body_template:
+        one: "%{mentions} Items were submitted over %{count} hour ago. [Please review them](%{base_path}/review)."
+        other: "%{mentions} Items were submitted over %{count} hours ago. [Please review them](%{base_path}/review)."
+
     private_topic_title: "Topic #%{id}"
     contents_hidden: "Please visit the post to see its contents."
 
diff --git a/spec/jobs/pending_reviewables_reminder_spec.rb b/spec/jobs/pending_reviewables_reminder_spec.rb
index ff122d855a8..bab0d56847c 100644
--- a/spec/jobs/pending_reviewables_reminder_spec.rb
+++ b/spec/jobs/pending_reviewables_reminder_spec.rb
@@ -74,5 +74,17 @@ describe Jobs::PendingReviewablesReminder do
         expect(execute.sent_reminder).to eq(true)
       end
     end
+
+    it 'deletes previous messages' do
+      GroupMessage.create(
+        Group[:moderators].name, 'reviewables_reminder',
+        { limit_once_per: false, message_params: { mentions: [], count: 1 } }
+      )
+
+      create_flag(49.hours.ago)
+      execute
+
+      expect(Topic.where(title: I18n.t("system_messages.reviewables_reminder.subject_template")).count).to eq(1)
+    end
   end
 end