diff --git a/lib/user_lookup.rb b/lib/user_lookup.rb index 590c6e27545..92ca3806bd2 100644 --- a/lib/user_lookup.rb +++ b/lib/user_lookup.rb @@ -21,7 +21,9 @@ class UserLookup def primary_groups @primary_groups ||= users.values.each_with_object({}) do |user, hash| if user.primary_group_id - hash[user.id] = groups[user.primary_group_id] + group = groups[user.primary_group_id] + set_user_group_preload(user, group, :primary_group) + hash[user.id] = group end end end @@ -29,13 +31,20 @@ class UserLookup def flair_groups @flair_groups ||= users.values.each_with_object({}) do |user, hash| if user.flair_group_id - hash[user.id] = groups[user.flair_group_id] + group = groups[user.flair_group_id] + set_user_group_preload(user, group, :flair_group) + hash[user.id] = group end end end private + def set_user_group_preload(user, group, group_association_name) + association = user.association(group_association_name) + association.target = group + end + def users @users ||= User .where(id: @user_ids) diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index a23955f681a..ee7ddbdfd24 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -99,6 +99,46 @@ RSpec.describe ListController do expect(first_item.css('[itemprop="position"]')[0]['content']).to eq('1') expect(first_item.css('[itemprop="url"]')[0]['href']).to eq(topic.url) end + + it 'does not N+1 queries when topic featured users have different primary groups' do + user.update!(primary_group: group) + + # warm up + get "/latest.json" + expect(response.status).to eq(200) + + initial_sql_queries_count = track_sql_queries do + get "/latest.json" + + expect(response.status).to eq(200) + + body = response.parsed_body + + expect(body["topic_list"]["topics"].map { |t| t["id"] }).to contain_exactly(topic.id) + expect(body["topic_list"]["topics"][0]["posters"].map { |p| p["user_id"] }).to contain_exactly(user.id) + end.count + + group2 = Fabricate(:group) + user2 = Fabricate(:user, primary_group: group2) + topic.update!(last_post_user_id: user2.id) + + group3 = Fabricate(:group) + user3 = Fabricate(:user, flair_group: group3) + topic.update!(featured_user3_id: user3.id) + + new_sql_queries_count = track_sql_queries do + get "/latest.json" + + expect(response.status).to eq(200) + + body = response.parsed_body + + expect(body["topic_list"]["topics"].map { |t| t["id"] }).to contain_exactly(topic.id) + expect(body["topic_list"]["topics"][0]["posters"].map { |p| p["user_id"] }).to contain_exactly(user.id, user2.id, user3.id) + end.count + + expect(new_sql_queries_count).to be <= initial_sql_queries_count + end end describe "categories and X" do