diff --git a/app/models/topic.rb b/app/models/topic.rb index 2f68222495e..d796b3c895f 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -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 = diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 2b17397b60c..0df2e451a56 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -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