FIX: remove muted topics/tags/categories from top and hot topics lists (#30892)

So it matches the behavior of latest and new.
This commit is contained in:
Régis Hanol
2025-01-29 11:51:10 +01:00
committed by GitHub
parent a480c40f81
commit cfa281a697
2 changed files with 33 additions and 48 deletions

View File

@ -359,9 +359,7 @@ class TopicQuery
def list_hot def list_hot
create_list(:hot, unordered: true, prioritize_pinned: true) do |topics| create_list(:hot, unordered: true, prioritize_pinned: true) do |topics|
topics = remove_muted_topics(topics, user) topics = remove_muted(topics, user, options)
topics = remove_muted_categories(topics, user, exclude: options[:category])
TopicQuery.remove_muted_tags(topics, user, options)
topics.joins("JOIN topic_hot_scores on topics.id = topic_hot_scores.topic_id").order( topics.joins("JOIN topic_hot_scores on topics.id = topic_hot_scores.topic_id").order(
"topic_hot_scores.score DESC", "topic_hot_scores.score DESC",
) )
@ -371,9 +369,9 @@ class TopicQuery
def list_top_for(period) def list_top_for(period)
score_column = TopTopic.score_column_for_period(period) score_column = TopTopic.score_column_for_period(period)
create_list(:top, unordered: true) do |topics| create_list(:top, unordered: true) do |topics|
topics = remove_muted_categories(topics, @user) topics = remove_muted(topics, user, options)
topics = topics.joins(:top_topic).where("top_topics.#{score_column} > 0") topics = topics.joins(:top_topic).where("top_topics.#{score_column} > 0")
if period == :yearly && @user.try(:trust_level) == TrustLevel[0] if period == :yearly && user&.new_user?
topics.order(<<~SQL) topics.order(<<~SQL)
CASE WHEN ( CASE WHEN (
COALESCE(topics.pinned_at, '1900-01-01') > COALESCE(tu.cleared_pinned_at, '1900-01-01') COALESCE(topics.pinned_at, '1900-01-01') > COALESCE(tu.cleared_pinned_at, '1900-01-01')
@ -1215,8 +1213,7 @@ class TopicQuery
result = result =
result.where("topics.id NOT IN (?)", excluded_topic_ids) unless excluded_topic_ids.empty? result.where("topics.id NOT IN (?)", excluded_topic_ids) unless excluded_topic_ids.empty?
result = remove_muted_categories(result, @user) result = remove_muted(result, @user, @options)
result = remove_muted_topics(result, @user)
# If we are in a category, prefer it for the random results # If we are in a category, prefer it for the random results
if topic.category_id if topic.category_id

View File

@ -700,18 +700,19 @@ RSpec.describe TopicQuery do
end end
describe "muted categories" do describe "muted categories" do
it "is removed from top, new and latest lists" do it "is removed from latest, new, top, and hot lists" do
category = Fabricate(:category_with_definition) category = Fabricate(:category_with_definition)
topic = Fabricate(:topic, category: category) topic = Fabricate(:topic, category:)
CategoryUser.create!(
user_id: user.id, notification_level = CategoryUser.notification_levels[:muted]
category_id: category.id, CategoryUser.create!(user:, category:, notification_level:)
notification_level: CategoryUser.notification_levels[:muted], TopTopic.create!(topic: topic, all_score: 1)
) TopicHotScore.create!(topic: topic, score: 1.0)
expect(topic_query.list_new.topics.map(&:id)).not_to include(topic.id) expect(topic_query.list_new.topics.map(&:id)).not_to include(topic.id)
expect(topic_query.list_latest.topics.map(&:id)).not_to include(topic.id) expect(topic_query.list_latest.topics.map(&:id)).not_to include(topic.id)
TopTopic.create!(topic: topic, all_score: 1)
expect(topic_query.list_top_for(:all).topics.map(&:id)).not_to include(topic.id) expect(topic_query.list_top_for(:all).topics.map(&:id)).not_to include(topic.id)
expect(topic_query.list_hot.topics.map(&:id)).not_to include(topic.id)
end end
end end
@ -784,7 +785,7 @@ RSpec.describe TopicQuery do
end end
describe "muted tags" do describe "muted tags" do
it "is removed from new and latest lists" do it "is removed from latest, new, top, and hot lists" do
SiteSetting.tagging_enabled = true SiteSetting.tagging_enabled = true
SiteSetting.remove_muted_tags_from_latest = "always" SiteSetting.remove_muted_tags_from_latest = "always"
@ -801,45 +802,32 @@ RSpec.describe TopicQuery do
notification_level: CategoryUser.notification_levels[:muted], notification_level: CategoryUser.notification_levels[:muted],
) )
topic_ids = topic_query.list_latest.topics.map(&:id) [muted_topic, tagged_topic, muted_tagged_topic, untagged_topic].each do |topic|
expect(topic_ids).to contain_exactly(tagged_topic.id, untagged_topic.id) TopTopic.create(topic:, all_score: 1)
TopicHotScore.create!(topic:, score: 1.0)
end
topic_ids = topic_query.list_new.topics.map(&:id) ids = [tagged_topic, untagged_topic].map &:id
expect(topic_ids).to contain_exactly(tagged_topic.id, untagged_topic.id) expect(topic_query.list_latest.topics.map(&:id)).to contain_exactly(*ids)
expect(topic_query.list_new.topics.map(&:id)).to contain_exactly(*ids)
expect(topic_query.list_top_for(:all).topics.map(&:id)).to contain_exactly(*ids)
expect(topic_query.list_hot.topics.map(&:id)).to contain_exactly(*ids)
SiteSetting.remove_muted_tags_from_latest = "only_muted" SiteSetting.remove_muted_tags_from_latest = "only_muted"
topic_ids = topic_query.list_latest.topics.map(&:id) ids = [tagged_topic, muted_tagged_topic, untagged_topic].map &:id
expect(topic_ids).to contain_exactly( expect(topic_query.list_latest.topics.map(&:id)).to contain_exactly(*ids)
tagged_topic.id, expect(topic_query.list_new.topics.map(&:id)).to contain_exactly(*ids)
muted_tagged_topic.id, expect(topic_query.list_top_for(:all).topics.map(&:id)).to contain_exactly(*ids)
untagged_topic.id, expect(topic_query.list_hot.topics.map(&:id)).to contain_exactly(*ids)
)
topic_ids = topic_query.list_new.topics.map(&:id)
expect(topic_ids).to contain_exactly(
tagged_topic.id,
muted_tagged_topic.id,
untagged_topic.id,
)
SiteSetting.remove_muted_tags_from_latest = "never" SiteSetting.remove_muted_tags_from_latest = "never"
topic_ids = topic_query.list_latest.topics.map(&:id) ids = [muted_topic, tagged_topic, muted_tagged_topic, untagged_topic].map &:id
expect(topic_ids).to contain_exactly( expect(topic_query.list_latest.topics.map(&:id)).to contain_exactly(*ids)
muted_topic.id, expect(topic_query.list_new.topics.map(&:id)).to contain_exactly(*ids)
tagged_topic.id, expect(topic_query.list_top_for(:all).topics.map(&:id)).to contain_exactly(*ids)
muted_tagged_topic.id, expect(topic_query.list_hot.topics.map(&:id)).to contain_exactly(*ids)
untagged_topic.id,
)
topic_ids = topic_query.list_new.topics.map(&:id)
expect(topic_ids).to contain_exactly(
muted_topic.id,
tagged_topic.id,
muted_tagged_topic.id,
untagged_topic.id,
)
end end
it "is not removed from the tag page itself" do it "is not removed from the tag page itself" do