IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 08:57:10 -06:00
Code Issues Packages Projects Releases Wiki Activity

FIX: duplicate topics in digests / summaries (#29517)

Browse Source 
When using the `digest_suppress_tags` site setting to list some tags that should be removed from the digests, if there was a topic that had one suppressed tag and other regular tag(s), it would be duplicated in the summaries.

https://meta.discourse.org/t/multiple-repeated-summary-mail-entries/296539

Thanks to @scossar for [figuring it out](https://meta.discourse.org/t/multiple-repeated-summary-mail-entries/296539/30).
This commit is contained in:
Régis Hanol 2024-10-31 18:22:41 +01:00 committed by GitHub
parent 3fb3e3560e
commit 927054b01e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 41 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
app/models/topic.rb
View File

@ -546,7 +546,8 @@ class Topic < ActiveRecord::Base
# Returns hot topics since a date for display in email digest.
def self.for_digest(user, since, opts = nil)
opts = opts || {}
opts ||= {}
period = ListController.best_period_for(since)
topics =
@ -589,30 +590,29 @@ class Topic < ActiveRecord::Base
# Remove category topics
topics = topics.where.not(id: Category.select(:topic_id).where.not(topic_id: nil))
# Remove suppressed categories
if SiteSetting.digest_suppress_categories.present?
topics =
topics.where.not(category_id: SiteSetting.digest_suppress_categories.split("|").map(&:to_i))
end
# Remove suppressed tags
if SiteSetting.digest_suppress_tags.present?
tag_ids = Tag.where_name(SiteSetting.digest_suppress_tags.split("|")).pluck(:id)
topics =
topics.where.not(id: TopicTag.where(tag_id: tag_ids).select(:topic_id)) if tag_ids.present?
end
# Remove muted and shared draft categories
remove_category_ids =
CategoryUser.where(
user_id: user.id,
notification_level: CategoryUser.notification_levels[:muted],
).pluck(:category_id)
if SiteSetting.digest_suppress_categories.present?
topics =
topics.where(
"topics.category_id NOT IN (?)",
SiteSetting.digest_suppress_categories.split("|").map(&:to_i),
)
end
if SiteSetting.digest_suppress_tags.present?
tag_ids = Tag.where_name(SiteSetting.digest_suppress_tags.split("|")).pluck(:id)
if tag_ids.present?
topics =
topics.joins("LEFT JOIN topic_tags tg ON topics.id = tg.topic_id").where(
"tg.tag_id NOT IN (?) OR tg.tag_id IS NULL",
tag_ids,
)
end
end
remove_category_ids << SiteSetting.shared_drafts_category if SiteSetting.shared_drafts_enabled?
if remove_category_ids.present?
remove_category_ids.uniq!
topics =

34
spec/models/topic_spec.rb
View File

@ -2341,26 +2341,38 @@ RSpec.describe Topic do
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to be_blank
end
it "doesn't return topics from suppressed tags" do
category = Fabricate(:category_with_definition, created_at: 2.minutes.ago)
topic = Fabricate(:topic, category: category, created_at: 1.minute.ago)
topic2 = Fabricate(:topic, category: category, created_at: 1.minute.ago)
tag = Fabricate(:tag)
tag2 = Fabricate(:tag)
Fabricate(:topic_tag, topic: topic, tag: tag)
it "doesn't return topics with a suppressed tag" do
topic_with_tags = Fabricate(:topic, created_at: 1.minute.ago)
topic_without_tags = Fabricate(:topic, created_at: 1.minute.ago)
topic_with_other_tags = Fabricate(:topic, created_at: 1.minute.ago)
tag_1 = Fabricate(:tag)
tag_2 = Fabricate(:tag)
tag_3 = Fabricate(:tag)
Fabricate(:topic_tag, topic: topic_with_tags, tag: tag_1)
Fabricate(:topic_tag, topic: topic_with_tags, tag: tag_2)
Fabricate(:topic_tag, topic: topic_with_other_tags, tag: tag_2)
Fabricate(:topic_tag, topic: topic_with_other_tags, tag: tag_3)
SiteSetting.digest_suppress_tags = "#{tag_1.name}"
SiteSetting.digest_suppress_tags = "#{tag.name}|#{tag2.name}"
topics = Topic.for_digest(user, 1.year.ago, top_order: true)
expect(topics).to eq([topic2])
expect(topics).to contain_exactly(topic_without_tags, topic_with_other_tags)
Fabricate(
:topic_user,
user: user,
topic: topic,
topic: topic_with_tags,
notification_level: TopicUser.notification_levels[:regular],
)
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to eq([topic2])
expect(Topic.for_digest(user, 1.year.ago, top_order: true)).to contain_exactly(
topic_without_tags,
topic_with_other_tags,
)
end
it "doesn't return topics from TL0 users" do