FEATURE: remove duplicated messages about new advices (discourse#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

@@ -5,12 +5,22 @@ class DashboardStats < ::Jobs::Scheduled
     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

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 @@ def create
     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] || {})

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