From 38592dc48e6c67878c4ed64850a51e0453498b9a Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 11 Sep 2024 10:39:14 +0300 Subject: [PATCH] PERF: Preload user options when status is enabled (#28827) The user option 'hide_profile_and_presence' is necessary to figure out if the user status has to be displayed or not. In order to avoid N+1s generated by `include_status?` method, both `user_status` and `user_option` relations have to be included. --- lib/topic_view.rb | 5 +++-- spec/requests/topics_controller_spec.rb | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index f2df2c26b3d..23f21afa66c 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -752,8 +752,9 @@ class TopicView usernames.flatten! usernames.uniq! - users = - User.where(username_lower: usernames).includes(:user_status).index_by(&:username_lower) + users = User.where(username_lower: usernames) + users = users.includes(:user_option, :user_status) if SiteSetting.enable_user_status + users = users.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/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 782fafb702f..06d6bb3f19c 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2438,6 +2438,25 @@ RSpec.describe TopicsController do expect(second_request_queries.count).to eq(first_request_queries.count) end + it "does not result in N+1 queries loading mentioned users" do + SiteSetting.enable_user_status = true + + post = + Fabricate( + :post, + raw: + "post with many mentions: @#{user.username}, @#{user_2.username}, @#{admin.username}, @#{moderator.username}", + ) + + queries = track_sql_queries { get "/t/#{post.topic_id}.json" } + + user_statuses_queries = queries.filter { |q| q =~ /FROM "?user_statuses"?/ } + expect(user_statuses_queries.size).to eq(2) # for current user and for all mentioned users + + user_options_queries = queries.filter { |q| q =~ /FROM "?user_options"?/ } + expect(user_options_queries.size).to eq(1) # for all mentioned users + end + context "with registered redirect_to_correct_topic_additional_query_parameters" do let(:modifier_block) { Proc.new { |allowed_params| allowed_params << :silly_param } }