diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 78e559c031a..0b359742413 100644 --- a/app/serializers/post_serializer.rb +++ b/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? diff --git a/lib/topic_view.rb b/lib/topic_view.rb index ebfc6d04c7b..f6b9c19259e 100644 --- a/lib/topic_view.rb +++ b/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 diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 3feb0b52b4a..ad5310dd382 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/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 diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 3c00a04b904..5f9c62c03b9 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/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"