FIX: Topic reset_new unscoped causing huge queries (#14158)

Since ad3ec5809f when a user chooses
the Dismiss New... option in the New topic list, we send a request
to topics/reset-new.json with ?tracked=false as the only parameter.

This then uses Topic as the scope for topics to dismiss, with no
other limitations. When we do topic_scope.pluck(:id), it gets the
ID of every single topic in the database (that is not deleted) to
pass to TopicsBulkAction, causing a huge query with severe performance
issues.

This commit changes the default scope to use
`TopicQuery.new(current_user).new_results(limit: false)`
which should only use the topics in the user's New list, which
will be a much smaller list, depending on the user's "new_topic_duration_minutes"
setting.
This commit is contained in:
Martin Brennan 2021-08-26 11:25:20 +10:00 committed by GitHub
parent 75b0d6df93
commit 1646856974
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 2 deletions

View File

@ -985,11 +985,12 @@ class TopicsController < ApplicationController
elsif params[:tag_id].present? elsif params[:tag_id].present?
Topic.joins(:tags).where(tags: { name: params[:tag_id] }) Topic.joins(:tags).where(tags: { name: params[:tag_id] })
else else
new_results = TopicQuery.new(current_user).new_results(limit: false)
if params[:tracked].to_s == "true" if params[:tracked].to_s == "true"
TopicQuery.tracked_filter(TopicQuery.new(current_user).new_results(limit: false), current_user.id) TopicQuery.tracked_filter(new_results, current_user.id)
else else
current_user.user_stat.update_column(:new_since, Time.zone.now) current_user.user_stat.update_column(:new_since, Time.zone.now)
Topic new_results
end end
end end

View File

@ -3252,6 +3252,53 @@ RSpec.describe TopicsController do
DismissedTopicUser.where(user_id: user.id, topic_id: topic_ids).count DismissedTopicUser.where(user_id: user.id, topic_id: topic_ids).count
}.by(4) }.by(4)
end end
context "when tracked=false" do
it "updates the user_stat new_since column and dismisses all the new topics" do
sign_in(user)
tracked_category = Fabricate(:category)
CategoryUser.set_notification_level_for_category(user,
NotificationLevels.all[:tracking],
tracked_category.id)
topic_ids = []
5.times do
topic_ids << create_post(category: tracked_category).topic.id
end
topic_ids << Fabricate(:topic).id
topic_ids << Fabricate(:topic).id
old_new_since = user.user_stat.new_since
put "/topics/reset-new.json?tracked=false"
expect(DismissedTopicUser.where(user_id: user.id, topic_id: topic_ids).count).to eq(7)
expect(user.reload.user_stat.new_since > old_new_since).to eq(true)
end
it "does not pass topic ids that are not new for the user to the bulk action, limit the scope to new topics" do
sign_in(user)
tracked_category = Fabricate(:category)
CategoryUser.set_notification_level_for_category(user,
NotificationLevels.all[:tracking],
tracked_category.id)
topic_ids = []
5.times do
topic_ids << create_post(category: tracked_category).topic.id
end
topic_ids << Fabricate(:topic).id
topic_ids << Fabricate(:topic).id
dismiss_ids = topic_ids[0..1]
other_ids = topic_ids[2..-1].sort.reverse
DismissedTopicUser.create(user_id: user.id, topic_id: dismiss_ids.first)
DismissedTopicUser.create(user_id: user.id, topic_id: dismiss_ids.second)
expect { put "/topics/reset-new.json?tracked=false" }.to change {
DismissedTopicUser.where(user_id: user.id).count
}.by(5)
end
end
end end
context 'category' do context 'category' do