From 83621ccbe797223b48b624b00ef04f24672e1f03 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Mon, 18 Sep 2023 10:45:43 +0300 Subject: [PATCH] FIX: Parse the digest_suppress_tags setting correctly (#23623) 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 https://github.com/discourse/discourse/commit/477a5dd371e9605a0c0058d0805c767c77fa7c93 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. --- app/models/topic.rb | 13 ++++++++----- spec/models/topic_spec.rb | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/models/topic.rb b/app/models/topic.rb index 7e4219e15dc0b7..e2c6434c2ba4c2 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -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? diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index e902ea49cc60e8..4cdefea362522b 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -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])