From d99735e24de739e48720e2eed05ae3f8a3f5983f Mon Sep 17 00:00:00 2001
From: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Date: Wed, 15 Sep 2021 08:59:25 +1000
Subject: [PATCH] FEATURE: remove duplicated messages about new advices
 (#14319)

Discourse is sending regularly message to admins when potential problems are persisted. Most of the time they have exactly the same content. In that case, when there are no replies, the old one should be trashed before a new one is created.
---
 app/jobs/scheduled/dashboard_stats.rb | 16 ++++++--
 app/services/group_message.rb         | 23 ++++++++++++
 spec/jobs/dashboard_stats_spec.rb     | 54 +++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 spec/jobs/dashboard_stats_spec.rb

diff --git a/app/jobs/scheduled/dashboard_stats.rb b/app/jobs/scheduled/dashboard_stats.rb
index c256466642d..2c1bd74fa32 100644
--- a/app/jobs/scheduled/dashboard_stats.rb
+++ b/app/jobs/scheduled/dashboard_stats.rb
@@ -5,12 +5,22 @@ module Jobs
     every 30.minutes
 
     def execute(args)
-      problems_started_at = AdminDashboardData.problems_started_at
-      if problems_started_at && problems_started_at < 2.days.ago
+      if persistent_problems?
         # If there have been problems reported on the dashboard for a while,
         # send a message to admins no more often than once per week.
-        GroupMessage.create(Group[:admins].name, :dashboard_problems, limit_once_per: 7.days.to_i)
+        group_message = GroupMessage.new(Group[:admins].name, :dashboard_problems, limit_once_per: 7.days.to_i)
+        Topic.transaction do
+          group_message.delete_previous!
+          group_message.create
+        end
       end
     end
+
+    private
+
+    def persistent_problems?
+      problems_started_at = AdminDashboardData.problems_started_at
+      problems_started_at && problems_started_at < 2.days.ago
+    end
   end
 end
diff --git a/app/services/group_message.rb b/app/services/group_message.rb
index d944f22e6da..fba523a15b2 100644
--- a/app/services/group_message.rb
+++ b/app/services/group_message.rb
@@ -14,6 +14,8 @@ class GroupMessage
 
   include Rails.application.routes.url_helpers
 
+  RECENT_MESSAGE_PERIOD = 3.months
+
   def self.create(group_name, message_type, opts = {})
     GroupMessage.new(group_name, message_type, opts).create
   end
@@ -41,6 +43,27 @@ class GroupMessage
     end
   end
 
+  def delete_previous!
+    posts = Post
+      .joins(topic: { topic_allowed_groups: :group })
+      .where(topic: {
+        posts_count: 1,
+        user_id: Discourse.system_user,
+        archetype: Archetype.private_message,
+        subtype: TopicSubtype.system_message,
+        title: I18n.t("system_messages.#{@message_type}.subject_template", message_params),
+        topic_allowed_groups: {
+          groups: { name: @group_name }
+        }
+      })
+      .where("posts.created_at > ?", RECENT_MESSAGE_PERIOD.ago)
+      .where(raw: I18n.t("system_messages.#{@message_type}.text_body_template", message_params).rstrip)
+
+    posts.find_each do |post|
+      PostDestroyer.new(Discourse.system_user, post).destroy
+    end
+  end
+
   def message_params
     @message_params ||= begin
       h = { base_url: Discourse.base_url }.merge(@opts[:message_params] || {})
diff --git a/spec/jobs/dashboard_stats_spec.rb b/spec/jobs/dashboard_stats_spec.rb
new file mode 100644
index 00000000000..e4b2fdafc38
--- /dev/null
+++ b/spec/jobs/dashboard_stats_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe ::Jobs::DashboardStats do
+  let(:group_message) { GroupMessage.new(Group[:admins].name, :dashboard_problems, limit_once_per: 7.days.to_i) }
+
+  def clear_recently_sent!
+    Discourse.redis.del(group_message.sent_recently_key)
+  end
+
+  after do
+    clear_recently_sent!
+  end
+
+  it 'creates group message when problems are persistent for 2 days' do
+    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, Time.zone.now.to_s)
+    expect { described_class.new.execute({}) }.not_to change { Topic.count }
+
+    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago)
+    expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)
+  end
+
+  it 'replaces old message' do
+    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago)
+    expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)
+    old_topic = Topic.last
+    clear_recently_sent!
+
+    new_topic = described_class.new.execute({}).topic
+    expect(old_topic.reload.deleted_at.present?).to eq(true)
+    expect(new_topic.reload.deleted_at).to be_nil
+    expect(new_topic.title).to eq(old_topic.title)
+  end
+
+  it 'duplicates message if previous one has replies' do
+    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago)
+    expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)
+    clear_recently_sent!
+
+    _reply_1 = Fabricate(:post, topic: Topic.last)
+    expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)
+  end
+
+  it 'duplicates message if previous was 3 months ago' do
+    freeze_time 3.months.ago do
+      Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago)
+      expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)
+      clear_recently_sent!
+    end
+
+    expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)
+  end
+end