Commit 708881c1 authored by Stan Hu's avatar Stan Hu

Merge branch 'da-fix-n-plus-1-query-on-starrers-list' into 'master'

Fix N+1s queries while loading users on the project starrers list

See merge request gitlab-org/gitlab-ce!31984
parents 6556e708 2067f677
...@@ -5,11 +5,11 @@ class Projects::StarrersController < Projects::ApplicationController ...@@ -5,11 +5,11 @@ class Projects::StarrersController < Projects::ApplicationController
def index def index
@starrers = UsersStarProjectsFinder.new(@project, params, current_user: @current_user).execute @starrers = UsersStarProjectsFinder.new(@project, params, current_user: @current_user).execute
@sort = params[:sort].presence || sort_value_name
@starrers = @starrers.preload_users.sort_by_attribute(@sort).page(params[:page])
@public_count = @project.starrers.with_public_profile.size @public_count = @project.starrers.with_public_profile.size
@total_count = @project.starrers.size @total_count = @project.starrers.size
@private_count = @total_count - @public_count @private_count = @total_count - @public_count
@sort = params[:sort].presence || sort_value_name
@starrers = @starrers.sort_by_attribute(@sort).page(params[:page])
end end
private private
......
...@@ -17,6 +17,7 @@ class UsersStarProject < ApplicationRecord ...@@ -17,6 +17,7 @@ class UsersStarProject < ApplicationRecord
scope :by_project, -> (project) { where(project_id: project.id) } scope :by_project, -> (project) { where(project_id: project.id) }
scope :with_visible_profile, -> (user) { joins(:user).merge(User.with_visible_profile(user)) } scope :with_visible_profile, -> (user) { joins(:user).merge(User.with_visible_profile(user)) }
scope :with_public_profile, -> { joins(:user).merge(User.with_public_profile) } scope :with_public_profile, -> { joins(:user).merge(User.with_public_profile) }
scope :preload_users, -> { preload(:user) }
class << self class << self
def sort_by_attribute(method) def sort_by_attribute(method)
......
...@@ -32,6 +32,20 @@ describe Projects::StarrersController do ...@@ -32,6 +32,20 @@ describe Projects::StarrersController do
end end
end end
context 'N+1 queries' do
render_views
it 'avoids N+1s loading users', :request_store do
get_starrers
control_count = ActiveRecord::QueryRecorder.new { get_starrers }.count
create_list(:user, 5).each { |user| user.toggle_star(project) }
expect { get_starrers }.not_to exceed_query_limit(control_count)
end
end
context 'when project is public' do context 'when project is public' do
before do before do
project.update_attribute(:visibility_level, Project::PUBLIC) project.update_attribute(:visibility_level, Project::PUBLIC)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment