From 5b17e85fe1dc21459da8883e675250606947fd05 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 19 Aug 2024 20:57:45 +0200 Subject: [PATCH] FIX: broken mentioned users with capitalized usernames (#28421) This commit fixes two codepaths which where incorrectly working with capitalized usernames as we were doing a mix of username_lower and non lower username. Also adds two specs for these cases. --- app/serializers/post_serializer.rb | 2 +- lib/topic_view.rb | 3 +- spec/lib/topic_view_spec.rb | 11 ++++++ spec/serializers/post_serializer_spec.rb | 47 +++++++++++++++--------- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index dd444c0b704..ebb9f45bb7c 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -591,7 +591,7 @@ class PostSerializer < BasicPostSerializer else query = User.includes(:user_option) query = query.includes(:user_status) if SiteSetting.enable_user_status - query = query.where(username: object.mentions) + query = query.where(username_lower: object.mentions) end users.map { |user| BasicUserSerializer.new(user, root: false, include_status: true).as_json } diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 9b7f0fdd53e..9dbf50264c4 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -744,7 +744,8 @@ class TopicView usernames.flatten! usernames.uniq! - users = User.where(username: usernames).includes(:user_status).index_by(&:username) + users = + User.where(username_lower: usernames).includes(:user_status).index_by(&:username_lower) mentions.reduce({}) do |hash, (post_id, post_mentioned_usernames)| hash[post_id] = post_mentioned_usernames.map { |username| users[username] }.compact diff --git a/spec/lib/topic_view_spec.rb b/spec/lib/topic_view_spec.rb index 15660e0e08d..b4789fb92d8 100644 --- a/spec/lib/topic_view_spec.rb +++ b/spec/lib/topic_view_spec.rb @@ -950,6 +950,17 @@ RSpec.describe TopicView do end end + describe "#mentioned_users" do + it "works with capitalized usernames" do + user = Fabricate(:user, username: "JoJo") + post_1 = Fabricate(:post, topic: topic, raw: "Hey @#{user.username}") + + view = TopicView.new(topic.id, user).mentioned_users + + expect(view[post_1.id]).to eq([user]) + end + end + describe "#image_url" do fab!(:op_upload) { Fabricate(:image_upload) } fab!(:post3_upload) { Fabricate(:image_upload) } diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index f172bc7cab6..68de390caf9 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -365,29 +365,42 @@ RSpec.describe PostSerializer do context "with mentions" do fab!(:user_status) fab!(:user) - fab!(:user1) { Fabricate(:user, user_status: user_status) } - fab!(:post) { Fabricate(:post, user: user, raw: "Hey @#{user1.username}") } + + let(:username) { "joffrey" } + let(:user1) { Fabricate(:user, user_status: user_status, username: username) } + let(:post) { Fabricate(:post, user: user, raw: "Hey @#{user1.username}") } let(:serializer) { described_class.new(post, scope: Guardian.new(user), root: false) } - it "returns mentioned users with user status when user status is enabled" do - SiteSetting.enable_user_status = true + context "when user status is enabled" do + before { SiteSetting.enable_user_status = true } - json = serializer.as_json + it "returns mentioned users with user status" do + json = serializer.as_json + expect(json[:mentioned_users]).to be_present + expect(json[:mentioned_users].length).to be(1) + expect(json[:mentioned_users][0]).to_not be_nil + expect(json[:mentioned_users][0][:id]).to eq(user1.id) + expect(json[:mentioned_users][0][:username]).to eq(user1.username) + expect(json[:mentioned_users][0][:name]).to eq(user1.name) + expect(json[:mentioned_users][0][:status][:description]).to eq(user_status.description) + expect(json[:mentioned_users][0][:status][:emoji]).to eq(user_status.emoji) + end - expect(json[:mentioned_users]).to be_present - expect(json[:mentioned_users].length).to be(1) - expect(json[:mentioned_users][0]).to_not be_nil - expect(json[:mentioned_users][0][:id]).to eq(user1.id) - expect(json[:mentioned_users][0][:username]).to eq(user1.username) - expect(json[:mentioned_users][0][:name]).to eq(user1.name) - expect(json[:mentioned_users][0][:status][:description]).to eq(user_status.description) - expect(json[:mentioned_users][0][:status][:emoji]).to eq(user_status.emoji) + context "when username has a capital letter" do + let(:username) { "JoJo" } + + it "returns mentioned users with user status" do + expect(serializer.as_json[:mentioned_users][0][:username]).to eq(user1.username) + end + end end - it "doesn't return mentioned users when user status is disabled" do - SiteSetting.enable_user_status = false - json = serializer.as_json - expect(json[:mentioned_users]).to be_nil + context "when user status is disabled" do + before { SiteSetting.enable_user_status = false } + + it "doesn't return mentioned users" do + expect(serializer.as_json[:mentioned_users]).to be_nil + end end end