mirror of
https://github.com/discourse/discourse.git
synced 2026-08-03 01:49:44 -05:00
FIX: Exclude unlisted topics from hot scores (#37312)
Unlisted topics (visible=false) should not appear in hot topic rankings. This change: - Filters out unlisted topics when updating hot scores - Excludes unlisted topics from hottest_topic_ids cache - Applies 50% score reduction to closed topics - Removes unlisted topics from index
This commit is contained in:
@@ -11,7 +11,11 @@ class TopicHotScore < ActiveRecord::Base
|
||||
end
|
||||
|
||||
def self.recreate_hottest_topic_ids_cache
|
||||
hot_topics = Topic.where(archetype: Archetype.default).where.not(id: Category.topic_ids)
|
||||
hot_topics =
|
||||
Topic
|
||||
.where(archetype: Archetype.default)
|
||||
.where(visible: true)
|
||||
.where.not(id: Category.topic_ids)
|
||||
|
||||
# 10% of the topics with activity since the hot topics cutoff.
|
||||
limit =
|
||||
@@ -77,6 +81,7 @@ class TopicHotScore < ActiveRecord::Base
|
||||
AND topics.deleted_at IS NULL
|
||||
AND topics.archetype <> :private_message
|
||||
AND topics.created_at <= :now
|
||||
AND topics.visible = true
|
||||
ORDER BY
|
||||
CASE WHEN topics.pinned_at IS NOT NULL THEN 0 ELSE 1 END ASC,
|
||||
topics.bumped_at desc
|
||||
@@ -118,6 +123,7 @@ class TopicHotScore < ActiveRecord::Base
|
||||
p.created_at >= :recent_cutoff
|
||||
AND t.archetype <> 'private_message'
|
||||
AND t.deleted_at IS NULL
|
||||
AND t.visible = true
|
||||
AND p.deleted_at IS NULL
|
||||
AND p.user_deleted = false
|
||||
AND t.created_at <= :now
|
||||
@@ -154,17 +160,19 @@ class TopicHotScore < ActiveRecord::Base
|
||||
) X
|
||||
)
|
||||
UPDATE topic_hot_scores ths
|
||||
SET score = (
|
||||
CASE WHEN topics.created_at > :recent_cutoff
|
||||
THEN ths.recent_likes ELSE topics.like_count END
|
||||
) /
|
||||
(EXTRACT(EPOCH FROM (:now - topics.created_at)) / 3600 + 2) ^ :gravity
|
||||
+
|
||||
CASE WHEN ths.recent_first_bumped_at IS NULL THEN 0 ELSE
|
||||
(ths.recent_likes + ths.recent_posters - 1) /
|
||||
(EXTRACT(EPOCH FROM (:now - recent_first_bumped_at)) / 3600 + 2) ^ :gravity
|
||||
END
|
||||
,
|
||||
SET score = CASE WHEN topics.visible = false THEN 0 ELSE
|
||||
((
|
||||
CASE WHEN topics.created_at > :recent_cutoff
|
||||
THEN ths.recent_likes ELSE topics.like_count END
|
||||
) /
|
||||
(EXTRACT(EPOCH FROM (:now - topics.created_at)) / 3600 + 2) ^ :gravity
|
||||
+
|
||||
CASE WHEN ths.recent_first_bumped_at IS NULL THEN 0 ELSE
|
||||
(ths.recent_likes + ths.recent_posters - 1) /
|
||||
(EXTRACT(EPOCH FROM (:now - recent_first_bumped_at)) / 3600 + 2) ^ :gravity
|
||||
END
|
||||
) * CASE WHEN topics.closed THEN 0.5 ELSE 1 END
|
||||
END,
|
||||
updated_at = :now
|
||||
|
||||
FROM topics
|
||||
|
||||
@@ -85,6 +85,8 @@ TopicStatusUpdater =
|
||||
CategoryFeaturedTopic.where(topic_id: topic.id).delete_all
|
||||
end
|
||||
|
||||
TopicHotScore.where(topic_id: topic.id).delete_all if status.visible? && status.disabled?
|
||||
|
||||
result
|
||||
end
|
||||
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
# frozen_string_literal: true
|
||||
class CleanupUnlistedTopicHotScores < ActiveRecord::Migration[8.0]
|
||||
def up
|
||||
execute <<~SQL
|
||||
DELETE FROM topic_hot_scores
|
||||
WHERE topic_id IN (
|
||||
SELECT id FROM topics WHERE visible = false
|
||||
)
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
||||
@@ -193,5 +193,62 @@ RSpec.describe TopicHotScore do
|
||||
|
||||
expect(hottest_ids).not_to include(category_topic.id)
|
||||
end
|
||||
|
||||
it "applies faster decay to closed topics" do
|
||||
freeze_time
|
||||
|
||||
open_topic = Fabricate(:topic, like_count: 10, created_at: 2.weeks.ago)
|
||||
closed_topic = Fabricate(:topic, like_count: 10, created_at: 2.weeks.ago, closed: true)
|
||||
|
||||
TopicHotScore.update_scores
|
||||
|
||||
open_score = TopicHotScore.find_by(topic_id: open_topic.id).score
|
||||
closed_score = TopicHotScore.find_by(topic_id: closed_topic.id).score
|
||||
|
||||
expect(closed_score).to be < open_score
|
||||
expect(closed_score).to be_within(0.0001).of(open_score * 0.5)
|
||||
end
|
||||
|
||||
it "does not create hot scores for unlisted topics" do
|
||||
freeze_time
|
||||
|
||||
unlisted_topic = Fabricate(:topic, visible: false, like_count: 10, created_at: 1.day.ago)
|
||||
|
||||
TopicHotScore.update_scores
|
||||
|
||||
expect(TopicHotScore.find_by(topic_id: unlisted_topic.id)).to be_nil
|
||||
end
|
||||
|
||||
it "excludes unlisted topics from hottest_topic_ids cache" do
|
||||
freeze_time
|
||||
|
||||
unlisted_topic =
|
||||
Fabricate(
|
||||
:topic,
|
||||
visible: false,
|
||||
like_count: 100,
|
||||
created_at: 1.day.ago,
|
||||
last_posted_at: 10.minutes.ago,
|
||||
)
|
||||
TopicHotScore.create!(topic_id: unlisted_topic.id, score: 999.0)
|
||||
|
||||
TopicHotScore.recreate_hottest_topic_ids_cache
|
||||
|
||||
expect(TopicHotScore.hottest_topic_ids).not_to include(unlisted_topic.id)
|
||||
end
|
||||
|
||||
it "sets score to 0 for unlisted topics during update" do
|
||||
freeze_time
|
||||
|
||||
topic = Fabricate(:topic, like_count: 10, created_at: 2.weeks.ago)
|
||||
TopicHotScore.update_scores
|
||||
|
||||
expect(TopicHotScore.find_by(topic_id: topic.id).score).to be > 0
|
||||
|
||||
topic.update!(visible: false)
|
||||
TopicHotScore.update_scores
|
||||
|
||||
expect(TopicHotScore.find_by(topic_id: topic.id).score).to eq(0)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -269,5 +269,14 @@ RSpec.describe TopicStatusUpdater do
|
||||
expect(topic.visible).to eq(true)
|
||||
expect(topic.visibility_reason_id).to eq(Topic.visibility_reasons[:manually_relisted])
|
||||
end
|
||||
|
||||
it "deletes hot score when topic is unlisted" do
|
||||
topic = Fabricate(:topic)
|
||||
TopicHotScore.create!(topic_id: topic.id, score: 1.0)
|
||||
|
||||
TopicStatusUpdater.new(topic, admin).update!("visible", false)
|
||||
|
||||
expect(TopicHotScore.find_by(topic_id: topic.id)).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user