PERF: Ensure suggested topics is only loaded on last page of topic view (#28507)

This commit improves `TopicsController#show` to not load suggested and
related topics unless it is the last page of the topic's view.
Previously, we avoided loading suggested and related topics by the use
of conditionals in the `TopicViewSerializer` to avoid calling
`TopicView#suggested_topics` and `TopicView#related_topics`. However,
this pattern is not reliable as the methods can still be called from
other spots in the code base. Instead, we ensure that
`TopicView#include_suggested` and `TopicView#include_related` is set
correctly on the instance of `TopicView` which ensures that for the
given instance, `TopicView#suggested_topics` and
`TopicView#related_topics` will be a noop.

app/controllers/topics_controller.rb

@@ -1336,15 +1336,18 @@ class TopicsController < ApplicationController
       return
     end
 
+    if params[:replies_to_post_number] || params[:filter_upwards_post_id] ||
+         params[:filter_top_level_replies] || @topic_view.next_page.present?
+      @topic_view.include_suggested = false
+      @topic_view.include_related = false
+    end
+
     topic_view_serializer =
       TopicViewSerializer.new(
         @topic_view,
         scope: guardian,
         root: false,
         include_raw: !!params[:include_raw],
-        exclude_suggested_and_related:
-          !!params[:replies_to_post_number] || !!params[:filter_upwards_post_id] ||
-            !!params[:filter_top_level_replies],
       )
 
     respond_to do |format|

app/serializers/suggested_topics_mixin.rb

@@ -8,13 +8,11 @@ module SuggestedTopicsMixin
   end
 
   def include_related_messages?
-    return false if @options[:exclude_suggested_and_related]
+    object.related_messages&.topics
-    object.next_page.nil? && object.related_messages&.topics
   end
 
   def include_suggested_topics?
-    return false if @options[:exclude_suggested_and_related]
+    object.suggested_topics&.topics
-    object.next_page.nil? && object.suggested_topics&.topics
   end
 
   def include_suggested_group_name?

lib/topic_view.rb

@@ -41,6 +41,8 @@ class TopicView
     :user_custom_fields,
     :post_custom_fields,
     :post_number,
+    :include_suggested,
+    :include_related,
   )
 
   delegate :category, to: :topic, allow_nil: true, private: true
@@ -50,8 +52,10 @@ class TopicView
     1000
   end
 
+  CHUNK_SIZE = 20
+
   def self.chunk_size
-    20
+    CHUNK_SIZE
   end
 
   def self.default_post_custom_fields
@@ -644,6 +648,7 @@ class TopicView
               { include_random: true, pm_params: pm_params },
               self,
             )
+
           TopicQuery.new(@user).list_suggested_for(topic, **kwargs)
         end
     else

spec/requests/topics_controller_spec.rb

@@ -2236,9 +2236,6 @@ RSpec.describe TopicsController do
     fab!(:private_topic) { pm }
     fab!(:topic) { Fabricate(:post, user: post_author1).topic }
 
-    fab!(:p1) { Fabricate(:post, user: topic.user) }
-    fab!(:p2) { Fabricate(:post, user: topic.user) }
-
     describe "when topic is not allowed" do
       it "should return the right response" do
         SiteSetting.detailed_404 = true
@@ -3277,18 +3274,50 @@ RSpec.describe TopicsController do
       end
     end
 
-    it "returns a list of categories" do
+    it "returns suggested topics only when loading the last chunk of posts in a topic" do
+      topic_post_2 = Fabricate(:post, topic: topic)
+      topic_post_3 = Fabricate(:post, topic: topic)
+      topic_post_4 = Fabricate(:post, topic: topic)
+
+      stub_const(TopicView, "CHUNK_SIZE", 2) do
+        get "/t/#{topic.slug}/#{topic.id}.json"
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body.has_key?("suggested_topics")).to eq(false)
+
+        get "/t/#{topic.slug}/#{topic.id}/4.json"
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body.has_key?("suggested_topics")).to eq(true)
+      end
+    end
+
+    it "returns a list of categories when `lazy_load_categories_group` site setting is enabled for the current user" do
       SiteSetting.lazy_load_categories_groups = "#{Group::AUTO_GROUPS[:everyone]}"
-      topic.update!(category: Fabricate(:category))
+
+      topic_post_2 = Fabricate(:post, topic: topic)
+      topic_post_3 = Fabricate(:post, topic: topic)
+      topic_post_4 = Fabricate(:post, topic: topic)
       dest_topic.update!(category: Fabricate(:category))
 
-      get "/t/#{topic.slug}/#{topic.id}.json"
+      stub_const(TopicView, "CHUNK_SIZE", 2) do
+        get "/t/#{topic.slug}/#{topic.id}.json"
 
-      expect(response.parsed_body["categories"].map { |c| c["id"] }).to contain_exactly(
+        expect(response.status).to eq(200)
-        SiteSetting.uncategorized_category_id,
+        expect(response.parsed_body.has_key?("suggested_topics")).to eq(false)
-        topic.category_id,
+        expect(response.parsed_body["categories"].map { _1["id"] }).to contain_exactly(
-        dest_topic.category_id,
+          topic.category_id,
-      )
+        )
+
+        get "/t/#{topic.slug}/#{topic.id}/4.json"
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body.has_key?("suggested_topics")).to eq(true)
+        expect(response.parsed_body["categories"].map { _1["id"] }).to contain_exactly(
+          topic.category_id,
+          dest_topic.category_id,
+        )
+      end
     end
   end
 

spec/serializers/topic_view_serializer_spec.rb

@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
 RSpec.describe TopicViewSerializer do
-  def serialize_topic(topic, user_arg)
+  def serialize_topic(topic, user_arg, opts = {})
-    topic_view = TopicView.new(topic.id, user_arg)
+    topic_view = TopicView.new(topic.id, user_arg, opts)
+
     serializer =
       TopicViewSerializer.new(topic_view, scope: Guardian.new(user_arg), root: false).as_json
+
     JSON.parse(MultiJson.dump(serializer)).deep_symbolize_keys!
   end
 
@@ -99,27 +101,19 @@ RSpec.describe TopicViewSerializer do
 
     before { TopicUser.update_last_read(user, topic2.id, 0, 0, 0) }
 
-    describe "when loading last chunk" do
+    it "should include suggested topics if `TopicView#include_suggested` is set to `true`" do
-      it "should include suggested topics" do
+      json = serialize_topic(topic, user, include_suggested: true)
-        json = serialize_topic(topic, user)
 
-        expect(json[:suggested_topics].first[:id]).to eq(topic2.id)
+      expect(json[:suggested_topics].first[:id]).to eq(topic2.id)
-      end
     end
 
-    describe "when not loading last chunk" do
+    it "should not include suggested topics if `TopicView#include_suggested` is set to `false`" do
-      fab!(:post) { Fabricate(:post, topic: topic) }
+      post = Fabricate(:post, topic:)
-      fab!(:post2) { Fabricate(:post, topic: topic) }
+      post2 = Fabricate(:post, topic:)
 
-      it "should not include suggested topics" do
+      json = serialize_topic(topic, user, include_suggested: false)
-        post
-        post2
-        topic_view = TopicView.new(topic.id, user, post_ids: [post.id])
-        topic_view.next_page
-        json = described_class.new(topic_view, scope: Guardian.new(user), root: false).as_json
 
-        expect(json[:suggested_topics]).to eq(nil)
+      expect(json[:suggested_topics]).to eq(nil)
-      end
     end
 
     describe "with private messages" do