PERF: Don't parse posts for mentions when user status is disabled (#19915)

Prior to this change, we were parsing `Post#cooked` every time we
serialize a post to extract the usernames of mentioned users in the
post. However, the only reason we have to do this is to support
displaying a user's status beside each mention in a post on the client side when
the `enable_user_status` site setting is enabled. When
`enable_user_status` is disabled, we should avoid having to parse
`Post#cooked` since there is no point in doing so.

app/serializers/post_serializer.rb

@@ -561,15 +561,20 @@ class PostSerializer < BasicPostSerializer
   end
 
   def mentioned_users
-    if @topic_view && (mentions = @topic_view.mentions[object.id])
-      users = mentions.map { |username| @topic_view.mentioned_users[username] }.compact
-    else
-      users = User.where(username: object.mentions)
-    end
+    users =
+      if @topic_view && (mentioned_users = @topic_view.mentioned_users[object.id])
+        mentioned_users
+      else
+        User.where(username: object.mentions)
+      end
 
     users.map { |user| BasicUserWithStatusSerializer.new(user, root: false) }
   end
 
+  def include_mentioned_users?
+    SiteSetting.enable_user_status
+  end
+
   private
 
   def can_review_topic?

lib/topic_view.rb

@@ -31,8 +31,6 @@ class TopicView
     :personal_message,
     :can_review_topic,
     :page,
-    :mentioned_users,
-    :mentions,
   )
   alias queued_posts_enabled? queued_posts_enabled
 
@@ -144,9 +142,6 @@ class TopicView
       end
     end
 
-    parse_mentions
-    load_mentioned_users
-
     TopicView.preload(self)
 
     @draft_key = @topic.draft_key
@@ -700,17 +695,21 @@ class TopicView
     @topic.published_page
   end
 
-  def parse_mentions
-    @mentions = @posts.to_h { |p| [p.id, p.mentions] }.reject { |_, v| v.empty? }
-  end
+  def mentioned_users
+    @mentioned_users ||=
+      begin
+        mentions = @posts.to_h { |p| [p.id, p.mentions] }.reject { |_, v| v.empty? }
+        usernames = mentions.values
+        usernames.flatten!
+        usernames.uniq!
 
-  def load_mentioned_users
-    usernames = @mentions.values.flatten.uniq
-    mentioned_users = User.where(username: usernames)
+        users = User.where(username: usernames).includes(:user_status).index_by(&:username)
 
-    mentioned_users = mentioned_users.includes(:user_status) if SiteSetting.enable_user_status
-
-    @mentioned_users = mentioned_users.to_h { |u| [u.username, u] }
+        mentions.reduce({}) do |hash, (post_id, post_mentioned_usernames)|
+          hash[post_id] = post_mentioned_usernames.map { |username| users[username] }.compact
+          hash
+        end
+      end
   end
 
   def tags

spec/requests/posts_controller_spec.rb

@@ -1578,10 +1578,30 @@ RSpec.describe PostsController do
         end
       end
 
-      context "with mentions" do
+      context "when `enable_user_status` site setting is enabled" do
         fab!(:user_to_mention) { Fabricate(:user) }
 
+        before { SiteSetting.enable_user_status = true }
+
+        it "does not return mentioned users when `enable_user_status` site setting is disabled" do
+          SiteSetting.enable_user_status = false
+
+          post "/posts.json",
+               params: {
+                 raw: "I am mentioning @#{user_to_mention.username}",
+                 topic_id: topic.id,
+               }
+
+          expect(response.status).to eq(200)
+
+          json = response.parsed_body
+
+          expect(json["mentioned_users"]).to eq(nil)
+        end
+
         it "returns mentioned users" do
+          user_to_mention.set_status!("off to dentist", "tooth")
+
           post "/posts.json",
                params: {
                  raw: "I am mentioning @#{user_to_mention.username}",
@@ -1596,6 +1616,11 @@ RSpec.describe PostsController do
           expect(mentioned_user["id"]).to be(user_to_mention.id)
           expect(mentioned_user["name"]).to eq(user_to_mention.name)
           expect(mentioned_user["username"]).to eq(user_to_mention.username)
+
+          status = mentioned_user["status"]
+          expect(status).to be_present
+          expect(status["emoji"]).to eq(user_to_mention.user_status.emoji)
+          expect(status["description"]).to eq(user_to_mention.user_status.description)
         end
 
         it "returns an empty list of mentioned users if nobody was mentioned" do
@@ -1611,43 +1636,6 @@ RSpec.describe PostsController do
           expect(response.status).to eq(200)
           expect(response.parsed_body["mentioned_users"].length).to be(0)
         end
-
-        it "doesn't return user status on mentions by default" do
-          user_to_mention.set_status!("off to dentist", "tooth")
-
-          post "/posts.json",
-               params: {
-                 raw: "I am mentioning @#{user_to_mention.username}",
-                 topic_id: topic.id,
-               }
-
-          expect(response.status).to eq(200)
-          json = response.parsed_body
-          expect(json["mentioned_users"].length).to be(1)
-
-          status = json["mentioned_users"][0]["status"]
-          expect(status).to be_nil
-        end
-
-        it "returns user status on mentions if status is enabled in site settings" do
-          SiteSetting.enable_user_status = true
-          user_to_mention.set_status!("off to dentist", "tooth")
-
-          post "/posts.json",
-               params: {
-                 raw: "I am mentioning @#{user_to_mention.username}",
-                 topic_id: topic.id,
-               }
-
-          expect(response.status).to eq(200)
-          json = response.parsed_body
-          expect(json["mentioned_users"].length).to be(1)
-
-          status = json["mentioned_users"][0]["status"]
-          expect(status).to be_present
-          expect(status["emoji"]).to eq(user_to_mention.user_status.emoji)
-          expect(status["description"]).to eq(user_to_mention.user_status.description)
-        end
       end
     end
 

spec/requests/topics_controller_spec.rb

@@ -2804,7 +2804,7 @@ RSpec.describe TopicsController do
       expect(response.status).to eq(200)
     end
 
-    context "with mentions" do
+    context "when `enable_user_status` site setting is enabled" do
       fab!(:post) { Fabricate(:post, user: post_author1) }
       fab!(:topic) { post.topic }
       fab!(:post2) do
@@ -2816,7 +2816,23 @@ RSpec.describe TopicsController do
         )
       end
 
-      it "returns mentions" do
+      before { SiteSetting.enable_user_status = true }
+
+      it "does not return mentions when `enable_user_status` site setting is disabled" do
+        SiteSetting.enable_user_status = false
+
+        get "/t/#{topic.slug}/#{topic.id}.json"
+
+        expect(response.status).to eq(200)
+
+        json = response.parsed_body
+
+        expect(json["post_stream"]["posts"][1]["mentioned_users"]).to eq(nil)
+      end
+
+      it "returns mentions with status" do
+        post_author1.set_status!("off to dentist", "tooth")
+
         get "/t/#{topic.slug}/#{topic.id}.json"
 
         expect(response.status).to eq(200)
@@ -2828,39 +2844,14 @@ RSpec.describe TopicsController do
         expect(mentioned_user["id"]).to be(post_author1.id)
         expect(mentioned_user["name"]).to eq(post_author1.name)
         expect(mentioned_user["username"]).to eq(post_author1.username)
-      end
 
-      it "doesn't return status on mentions by default" do
-        post_author1.set_status!("off to dentist", "tooth")
-
-        get "/t/#{topic.slug}/#{topic.id}.json"
-
-        expect(response.status).to eq(200)
-
-        json = response.parsed_body
-        expect(json["post_stream"]["posts"][1]["mentioned_users"].length).to be(1)
-        status = json["post_stream"]["posts"][1]["mentioned_users"][0]["status"]
-        expect(status).to be_nil
-      end
-
-      it "returns mentions with status if user status is enabled" do
-        SiteSetting.enable_user_status = true
-        post_author1.set_status!("off to dentist", "tooth")
-
-        get "/t/#{topic.slug}/#{topic.id}.json"
-
-        expect(response.status).to eq(200)
-
-        json = response.parsed_body
-        expect(json["post_stream"]["posts"][1]["mentioned_users"].length).to be(1)
-
-        status = json["post_stream"]["posts"][1]["mentioned_users"][0]["status"]
+        status = mentioned_user["status"]
         expect(status).to be_present
         expect(status["emoji"]).to eq(post_author1.user_status.emoji)
         expect(status["description"]).to eq(post_author1.user_status.description)
       end
 
-      it "returns an empty list of mentioned users if there is no mentions in a post" do
+      it "returns an empty list of mentioned users if there are no mentions in a post" do
         Fabricate(:post, user: post_author2, topic: topic, raw: "Post without mentions.")
 
         get "/t/#{topic.slug}/#{topic.id}.json"