diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 34ff6107eab..a268413e84f 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -225,6 +225,26 @@ module ProjectsHelper end end + # Returns true if any projects are present. + # + # If the relation has a LIMIT applied we'll cast the relation to an Array + # since repeated any? checks would otherwise result in multiple COUNT queries + # being executed. + # + # If no limit is applied we'll just issue a COUNT since the result set could + # be too large to load into memory. + def any_projects?(projects) + if projects.limit_value + projects.to_a.any? + else + projects.except(:offset).any? + end + end + + def has_projects_or_name?(projects, params) + !!(params[:name] || any_projects?(projects)) + end + private def repo_children_classes(field) diff --git a/app/views/dashboard/projects/index.html.haml b/app/views/dashboard/projects/index.html.haml index ec6cb1a9624..c546252455a 100644 --- a/app/views/dashboard/projects/index.html.haml +++ b/app/views/dashboard/projects/index.html.haml @@ -13,10 +13,8 @@ - if show_callout?('user_callout_dismissed') = render 'shared/user_callout' - - if @projects.any? || params[:name] + - if has_projects_or_name?(@projects, params) = render 'dashboard/projects_head' - - - if @projects.any? || params[:name] = render 'projects' - else = render "zero_authorized_projects" diff --git a/app/views/dashboard/projects/starred.html.haml b/app/views/dashboard/projects/starred.html.haml index ae1d733a516..14f9f8cd70a 100644 --- a/app/views/dashboard/projects/starred.html.haml +++ b/app/views/dashboard/projects/starred.html.haml @@ -9,7 +9,7 @@ %div{ class: container_class } = render 'dashboard/projects_head' - - if @projects.any? || params[:filter_projects] + - if params[:filter_projects] || any_projects?(@projects) = render 'projects' - else %h3 You don't have starred projects yet diff --git a/app/views/shared/_new_project_item_select.html.haml b/app/views/shared/_new_project_item_select.html.haml index 5f3cdaefd54..b417e83cdb6 100644 --- a/app/views/shared/_new_project_item_select.html.haml +++ b/app/views/shared/_new_project_item_select.html.haml @@ -1,4 +1,4 @@ -- if @projects.any? +- if any_projects?(@projects) .project-item-select-holder = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at', relative_path: local_assigns[:path] }, with_feature_enabled: local_assigns[:with_feature_enabled] %a.btn.btn-new.new-project-item-select-button diff --git a/app/views/shared/projects/_list.html.haml b/app/views/shared/projects/_list.html.haml index 7ed6c622558..66783fac6b2 100644 --- a/app/views/shared/projects/_list.html.haml +++ b/app/views/shared/projects/_list.html.haml @@ -10,7 +10,7 @@ - load_pipeline_status(projects) .js-projects-list-holder - - if projects.any? + - if any_projects?(projects) %ul.projects-list - projects.each_with_index do |project, i| - css_class = (i >= projects_limit) || project.pending_delete? ? 'hide' : nil diff --git a/changelogs/unreleased/dont-use-limit-offset-when-counting-projects.yml b/changelogs/unreleased/dont-use-limit-offset-when-counting-projects.yml new file mode 100644 index 00000000000..8ecea635ce5 --- /dev/null +++ b/changelogs/unreleased/dont-use-limit-offset-when-counting-projects.yml @@ -0,0 +1,4 @@ +--- +title: "Improve performance of checking for projects on the projects dashboard" +merge_request: +author: diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 236a7c29634..37a5e6b474e 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -411,4 +411,48 @@ describe ProjectsHelper do end end end + + describe '#has_projects_or_name?' do + let(:projects) do + create(:project) + Project.all + end + + it 'returns true when there are projects' do + expect(helper.has_projects_or_name?(projects, {})).to eq(true) + end + + it 'returns true when there are no projects but a name is given' do + expect(helper.has_projects_or_name?(Project.none, name: 'foo')).to eq(true) + end + + it 'returns false when there are no projects and there is no name' do + expect(helper.has_projects_or_name?(Project.none, {})).to eq(false) + end + end + + describe '#any_projects?' do + before do + create(:project) + end + + it 'returns true when projects will be returned' do + expect(helper.any_projects?(Project.all)).to eq(true) + end + + it 'returns false when no projects will be returned' do + expect(helper.any_projects?(Project.none)).to eq(false) + end + + it 'only executes a single query when a LIMIT is applied' do + relation = Project.limit(1) + recorder = ActiveRecord::QueryRecorder.new do + 2.times do + helper.any_projects?(relation) + end + end + + expect(recorder.count).to eq(1) + end + end end