Skip to content

Commit

Permalink
FIX: Parse the digest_suppress_tags setting correctly (#23623)
Browse files Browse the repository at this point in the history
Meta topic: https://meta.discourse.org/t/suppress-these-tags-from-summary-emails-settings-is-not-working-in-preview-digest-email/279196?u=osama

Follow-up to 477a5dd

The `digest_suppress_tags` setting is designed to be a list of pipe-delimited tag names, but the tag-based topic suppression logic assumes (incorrectly) that the setting contains pipe-delimited tag IDs. This mismatch in expectations led to the setting not working as expected.

This PR adds a step that converts the list of tag names in the setting to their corresponding IDs, which is then used to suppress topics tagged with those specific tags.
  • Loading branch information
OsamaSayegh committed Sep 18, 2023
1 parent 2791e75 commit 83621cc
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
13 changes: 8 additions & 5 deletions app/models/topic.rb
Expand Up @@ -586,11 +586,14 @@ def self.for_digest(user, since, opts = nil)
)
end
if SiteSetting.digest_suppress_tags.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",
SiteSetting.digest_suppress_tags.split("|").map(&:to_i),
)
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?
Expand Down
3 changes: 2 additions & 1 deletion spec/models/topic_spec.rb
Expand Up @@ -2368,9 +2368,10 @@ def set_state!(group, user, state)
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)

SiteSetting.digest_suppress_tags = "#{tag.id}"
SiteSetting.digest_suppress_tags = "#{tag.name}|#{tag2.name}"
topics = Topic.for_digest(user, 1.year.ago, top_order: true)
expect(topics).to eq([topic2])

Expand Down

1 comment on commit 83621cc

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/suppress-these-tags-from-summary-emails-settings-is-not-working-in-preview-digest-email/279196/5

Please sign in to comment.