diff options
author | Stan Hu <stanhu@gmail.com> | 2019-08-19 21:40:39 +0000 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-08-19 21:40:39 +0000 |
commit | 708881c182f228cd8c1c5d3cef42fd21d232f897 (patch) | |
tree | 18f0860ea675feaa439f7b7a9facbe7a0d1e272e | |
parent | 6556e708ad5599c41ff236df340162ba6ff905af (diff) | |
parent | 2067f677df69200338d8d2b78f239759fc293fee (diff) | |
download | gitlab-ce-708881c182f228cd8c1c5d3cef42fd21d232f897.tar.gz |
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
-rw-r--r-- | app/controllers/projects/starrers_controller.rb | 4 | ||||
-rw-r--r-- | app/models/users_star_project.rb | 1 | ||||
-rw-r--r-- | spec/controllers/projects/starrers_controller_spec.rb | 14 |
3 files changed, 17 insertions, 2 deletions
diff --git a/app/controllers/projects/starrers_controller.rb b/app/controllers/projects/starrers_controller.rb index e4093bed0ef..4efe956e973 100644 --- a/app/controllers/projects/starrers_controller.rb +++ b/app/controllers/projects/starrers_controller.rb @@ -5,11 +5,11 @@ class Projects::StarrersController < Projects::ApplicationController def index @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 @total_count = @project.starrers.size @private_count = @total_count - @public_count - @sort = params[:sort].presence || sort_value_name - @starrers = @starrers.sort_by_attribute(@sort).page(params[:page]) end private diff --git a/app/models/users_star_project.rb b/app/models/users_star_project.rb index 3c7a805cc5c..c633e2d8b3d 100644 --- a/app/models/users_star_project.rb +++ b/app/models/users_star_project.rb @@ -17,6 +17,7 @@ class UsersStarProject < ApplicationRecord scope :by_project, -> (project) { where(project_id: project.id) } 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 :preload_users, -> { preload(:user) } class << self def sort_by_attribute(method) diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb index 7085cba08d5..5774ff7c576 100644 --- a/spec/controllers/projects/starrers_controller_spec.rb +++ b/spec/controllers/projects/starrers_controller_spec.rb @@ -32,6 +32,20 @@ describe Projects::StarrersController do 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 before do project.update_attribute(:visibility_level, Project::PUBLIC) |