FIX: Make sure reset-new for tracked is not limited by per_page count (#13395)

FIX: Make sure reset-new for tracked is not limited by per_page count (#13395)

When dismissing new topics for the Tracked filter, the dismiss was limited to 30 topics which is the default per page count for TopicQuery. This happened even if you specified which topic IDs you were selectively dismissing. This PR fixes that bug, and also moves the per_page_count into a DEFAULT_PER_PAGE_COUNT for the TopicQuery so it can be stubbed in tests.

Also moves the unused stub_const method into the spec helpers for cases like this; it is much better to handle this in one place with an ensure. In a follow up PR I will clean up other specs that do the same thing and make them use stub_const.

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 010c51b..7f5e36a 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -968,7 +968,7 @@ class TopicsController < ApplicationController
         Topic.joins(:tags).where(tags: { name: params[:tag_id] })
       else
         if params[:tracked].to_s == "true"
-          TopicQuery.tracked_filter(TopicQuery.new(current_user).new_results, current_user.id)
+          TopicQuery.tracked_filter(TopicQuery.new(current_user).new_results(limit: false), current_user.id)
         else
           current_user.user_stat.update_column(:new_since, Time.zone.now)
           Topic
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index dd402eb..d09701f 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -7,6 +7,7 @@
 
 class TopicQuery
   PG_MAX_INT ||= 2147483647
+  DEFAULT_PER_PAGE_COUNT ||= 30
 
   def self.validators
     @validators ||= begin
@@ -578,7 +579,7 @@ class TopicQuery
   protected
 
   def per_page_setting
-    30
+    DEFAULT_PER_PAGE_COUNT
   end
 
   def private_messages_for(user, type)
@@ -702,7 +703,7 @@ class TopicQuery
   # Create results based on a bunch of default options
   def default_results(options = {})
     options.reverse_merge!(@options)
-    options.reverse_merge!(per_page: per_page_setting)
+    options.reverse_merge!(per_page: per_page_setting) unless options[:limit] == false
 
     # Whether to return visible topics
     options[:visible] = true if @user.nil? || @user.regular?
diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb
index 6d8f12d..0d929a2 100644
--- a/spec/models/theme_spec.rb
+++ b/spec/models/theme_spec.rb
@@ -749,16 +749,6 @@ HTML
 
   describe "automatic recompile" do
     it 'must recompile after bumping theme_field version' do
-      def stub_const(target, const, value)
-        old = target.const_get(const)
-        target.send(:remove_const, const)
-        target.const_set(const, value)
-        yield
-      ensure
-        target.send(:remove_const, const)
-        target.const_set(const, old)
-      end
-
       child.set_field(target: :common, name: "header", value: "World")
       child.set_field(target: :extra_js, name: "test.js.es6", value: "const hello = 'world';")
       child.save!
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index f72e7d8..444fc31 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -2974,7 +2974,7 @@ RSpec.describe TopicsController do
         expect(user.user_stat.new_since.to_date).to eq(old_date.to_date)
       end
 
-      it "creates topic user records for each unread topic" do
+      it "creates dismissed topic user records for each new topic" do
         sign_in(user)
         user.user_stat.update_column(:new_since, 2.years.ago)
 
@@ -2982,11 +2982,57 @@ RSpec.describe TopicsController do
         CategoryUser.set_notification_level_for_category(user,
                                                      NotificationLevels.all[:tracking],
                                                      tracked_category.id)
-        tracked_topic = create_post.topic
-        tracked_topic.update!(category_id: tracked_category.id)
+        tracked_topic = create_post(category: tracked_category).topic
 
         create_post # This is a new post, but is not tracked so a record will not be created for it
-        expect { put "/topics/reset-new.json?tracked=true" }.to change { DismissedTopicUser.where(user_id: user.id).count }.by(1)
+        expect do
+          put "/topics/reset-new.json?tracked=true"
+        end.to change {
+          DismissedTopicUser.where(user_id: user.id, topic_id: tracked_topic.id).count
+        }.by(1)
+      end
+
+      it "creates dismissed topic user records if there are > 30 (default pagination) 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
+
+        expect do
+          stub_const(TopicQuery, "DEFAULT_PER_PAGE_COUNT", 2) do
+            put "/topics/reset-new.json?tracked=true"
+          end
+        end.to change {
+          DismissedTopicUser.where(user_id: user.id, topic_id: topic_ids).count
+        }.by(5)
+      end
+
+      it "creates dismissed topic user records if there are > 30 (default pagination) topics and topic_ids are provided" 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
+        dismissing_topic_ids = topic_ids.sample(4)
+
+        expect do
+          stub_const(TopicQuery, "DEFAULT_PER_PAGE_COUNT", 2) do
+            put "/topics/reset-new.json?tracked=true", params: { topic_ids: dismissing_topic_ids }
+          end
+        end.to change {
+          DismissedTopicUser.where(user_id: user.id, topic_id: topic_ids).count
+        }.by(4)
       end
     end
 
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index aabf9f9..417dcdc 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -181,4 +181,14 @@ module Helpers
     `cd #{repo_dir} && git commit -am 'first commit'`
     repo_dir
   end
+
+  def stub_const(target, const, value)
+    old = target.const_get(const)
+    target.send(:remove_const, const)
+    target.const_set(const, value)
+    yield
+  ensure
+    target.send(:remove_const, const)
+    target.const_set(const, old)
+  end
 end

GitHub sha: 6fe78cd542194576b0a02ff52cb9dbaa79bba82d

This commit appears in #13395 which was approved by eviltrout. It was merged by martin.