Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Don't delete previous messages when we're inside the `sent_recently` window. (#18239) 

`delete_previous!` deletes existing topics even when we cannot send a new one due to the `limit_once_per` option. The dashboard problems PM gets deleted the next time the job runs (30 minutes), so the inbox could be empty when
admins click on the summary notification.
This commit is contained in:
Roman Rizzi 2022-09-13 12:43:24 -03:00 committed by GitHub
parent 5865868c76
commit 08cb9ecca4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
app/services/group_message.rb
View File

@ -27,23 +27,23 @@ class GroupMessage
end end
def create def create
unless sent_recently? return false if sent_recently?
post = PostCreator.create(
Discourse.system_user, post = PostCreator.create(
target_group_names: [@group_name], Discourse.system_user,
archetype: Archetype.private_message, target_group_names: [@group_name],
subtype: TopicSubtype.system_message, archetype: Archetype.private_message,
title: I18n.t("system_messages.#{@message_type}.subject_template", message_params), subtype: TopicSubtype.system_message,
raw: I18n.t("system_messages.#{@message_type}.text_body_template", message_params) title: I18n.t("system_messages.#{@message_type}.subject_template", message_params),
) raw: I18n.t("system_messages.#{@message_type}.text_body_template", message_params)
remember_message_sent )
post remember_message_sent
else post
false
end
end end
def delete_previous!(match_raw: true) def delete_previous!(respect_sent_recently: true, match_raw: true)
return false if respect_sent_recently && sent_recently?
posts = Post posts = Post
.joins(topic: { topic_allowed_groups: :group }) .joins(topic: { topic_allowed_groups: :group })
.where(topic: { .where(topic: {

8
spec/jobs/dashboard_stats_spec.rb
View File

@ -4,6 +4,7 @@ RSpec.describe ::Jobs::DashboardStats do
let(:group_message) { GroupMessage.new(Group[:admins].name, :dashboard_problems, limit_once_per: 7.days.to_i) } let(:group_message) { GroupMessage.new(Group[:admins].name, :dashboard_problems, limit_once_per: 7.days.to_i) }
def clear_recently_sent! def clear_recently_sent!
# We won't immediately create new PMs due to the limit_once_per option, reset the value for testing purposes.
Discourse.redis.del(group_message.sent_recently_key) Discourse.redis.del(group_message.sent_recently_key)
end end
@ -31,6 +32,13 @@ RSpec.describe ::Jobs::DashboardStats do
expect(new_topic.title).to eq(old_topic.title) expect(new_topic.title).to eq(old_topic.title)
end end
it 'respects the sent_recently? check when deleting previous 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)
expect { described_class.new.execute({}) }.not_to change { Topic.count }
end
it 'duplicates message if previous one has replies' do it 'duplicates message if previous one has replies' do
Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago) 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) expect { described_class.new.execute({}) }.to change { Topic.count }.by(1)