From 1646856974be1e96b5d631d4dc89d7d875164f93 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Thu, 26 Aug 2021 11:25:20 +1000
Subject: [PATCH] FIX: Topic reset_new unscoped causing huge queries (#14158)

Since ad3ec5809faf2cb9553b0c530967bbd1eb5c58ed 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.
---
 app/controllers/topics_controller.rb    |  5 +--
 spec/requests/topics_controller_spec.rb | 47 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 41861c3dd5f..f0a2a562d7d 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -985,11 +985,12 @@ class TopicsController < ApplicationController
       elsif params[:tag_id].present?
         Topic.joins(:tags).where(tags: { name: params[:tag_id] })
       else
+        new_results = TopicQuery.new(current_user).new_results(limit: false)
         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
           current_user.user_stat.update_column(:new_since, Time.zone.now)
-          Topic
+          new_results
         end
       end
 
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index 0bbafc88900..76e3711f750 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -3252,6 +3252,53 @@ RSpec.describe TopicsController do
           DismissedTopicUser.where(user_id: user.id, topic_id: topic_ids).count
         }.by(4)
       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
 
     context 'category' do