diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2016-11-01 09:05:31 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-11-04 18:23:22 +0100 |
commit | 44603ea9d90284d0a6b4046314a79b70997cc33a (patch) | |
tree | 6d72d84d0514307147536803d4e4eae6de6d3102 | |
parent | 3c5901c4113fac52c31f293b268a7179301cffb7 (diff) | |
download | gitlab-ce-44603ea9d90284d0a6b4046314a79b70997cc33a.tar.gz |
Merge branch 'optimize/labels-finder' into 'master'
Optimize group labels page
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23684
Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/1148
See merge request !7123
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | CHANGELOG.md | 3 | ||||
-rw-r--r-- | app/controllers/projects/labels_controller.rb | 2 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 4 | ||||
-rw-r--r-- | app/models/group_label.rb | 4 | ||||
-rw-r--r-- | app/models/label.rb | 30 | ||||
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/models/project_label.rb | 4 | ||||
-rw-r--r-- | app/views/groups/labels/index.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/labels/index.html.haml | 4 | ||||
-rw-r--r-- | app/views/shared/_label.html.haml | 13 |
10 files changed, 39 insertions, 29 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 0baf1d29ae0..2972716197e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ Please view this file on the master branch, on stable branches it's out of date. - Honour issue and merge request visibility in their respective finders. - Disable reference Markdown for unavailable features. -- Allow owners to fetch source code in CI builds. +- Allow owners to fetch source code in CI builds. !6943 +- Reduce the overhead to calculate number of open/closed issues and merge requests within the group or project. !7123 ## 8.13.3 (2016-11-02) diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 4f855134368..42fd09e9b7e 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -126,7 +126,7 @@ class Projects::LabelsController < Projects::ApplicationController alias_method :subscribable_resource, :label def find_labels - @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute.includes(:priorities) + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute end def authorize_admin_labels! diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 7f4b5c3cefd..6297b2db369 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -121,7 +121,7 @@ class IssuableFinder @labels = if labels? && !filter_by_no_label? - LabelsFinder.new(current_user, project_ids: projects, title: label_names).execute + LabelsFinder.new(current_user, project_ids: projects, title: label_names).execute(skip_authorization: true) else Label.none end @@ -268,7 +268,7 @@ class IssuableFinder items = items.with_label(label_names, params[:sort]) if projects - label_ids = LabelsFinder.new(current_user, project_ids: projects).execute.select(:id) + label_ids = LabelsFinder.new(current_user, project_ids: projects).execute(skip_authorization: true).select(:id) items = items.where(labels: { id: label_ids }) end end diff --git a/app/models/group_label.rb b/app/models/group_label.rb index a698b532d19..68841ace2e6 100644 --- a/app/models/group_label.rb +++ b/app/models/group_label.rb @@ -5,6 +5,10 @@ class GroupLabel < Label alias_attribute :subject, :group + def subject_foreign_key + 'group_id' + end + def to_reference(source_project = nil, target_project = nil, format: :id) super(source_project, target_project, format: format) end diff --git a/app/models/label.rb b/app/models/label.rb index 149fd98ecb3..d9287f2dc29 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -92,16 +92,23 @@ class Label < ActiveRecord::Base nil end - def open_issues_count(user = nil, project = nil) - issues_count(user, project_id: project.try(:id) || project_id, state: 'opened') + def open_issues_count(user = nil) + issues_count(user, state: 'opened') end - def closed_issues_count(user = nil, project = nil) - issues_count(user, project_id: project.try(:id) || project_id, state: 'closed') + def closed_issues_count(user = nil) + issues_count(user, state: 'closed') end - def open_merge_requests_count(user = nil, project = nil) - merge_requests_count(user, project_id: project.try(:id) || project_id, state: 'opened') + def open_merge_requests_count(user = nil) + params = { + subject_foreign_key => subject.id, + label_name: title, + scope: 'all', + state: 'opened' + } + + MergeRequestsFinder.new(user, params.with_indifferent_access).execute.count end def prioritize!(project, value) @@ -167,15 +174,8 @@ class Label < ActiveRecord::Base end def issues_count(user, params = {}) - IssuesFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) - .execute - .count - end - - def merge_requests_count(user, params = {}) - MergeRequestsFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) - .execute - .count + params.merge!(subject_foreign_key => subject.id, label_name: title, scope: 'all') + IssuesFinder.new(user, params.with_indifferent_access).execute.count end def label_format_reference(format = :id) diff --git a/app/models/project.rb b/app/models/project.rb index 54bc70f81d0..158cbbacfae 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -420,7 +420,7 @@ class Project < ActiveRecord::Base end def group_ids - joins(:namespace).where(namespaces: { type: 'Group' }).pluck(:namespace_id) + joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) end end diff --git a/app/models/project_label.rb b/app/models/project_label.rb index 33c2b617715..82f47f0e8fd 100644 --- a/app/models/project_label.rb +++ b/app/models/project_label.rb @@ -12,6 +12,10 @@ class ProjectLabel < Label alias_attribute :subject, :project + def subject_foreign_key + 'project_id' + end + def to_reference(target_project = nil, format: :id) super(project, target_project, format: format) end diff --git a/app/views/groups/labels/index.html.haml b/app/views/groups/labels/index.html.haml index 70783a63409..45325d6bc4b 100644 --- a/app/views/groups/labels/index.html.haml +++ b/app/views/groups/labels/index.html.haml @@ -13,7 +13,7 @@ .other-labels - if @labels.present? %ul.content-list.manage-labels-list.js-other-labels - = render partial: 'shared/label', collection: @labels, as: :label + = render partial: 'shared/label', subject: @group, collection: @labels, as: :label = paginate @labels, theme: 'gitlab' - else .nothing-here-block diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index f135bf6f6b4..05a8475dcd6 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -22,14 +22,14 @@ %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) } %p.empty-message{ class: ('hidden' unless @prioritized_labels.empty?) } No prioritized labels yet - if @prioritized_labels.present? - = render partial: 'shared/label', collection: @prioritized_labels, as: :label + = render partial: 'shared/label', subject: @project, collection: @prioritized_labels, as: :label .other-labels - if can?(current_user, :admin_label, @project) %h5{ class: ('hide' if hide) } Other Labels %ul.content-list.manage-labels-list.js-other-labels - if @labels.present? - = render partial: 'shared/label', collection: @labels, as: :label + = render partial: 'shared/label', subject: @project, collection: @labels, as: :label = paginate @labels, theme: 'gitlab' - if @labels.blank? .nothing-here-block diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml index 40c8d2af226..6ccdef0df46 100644 --- a/app/views/shared/_label.html.haml +++ b/app/views/shared/_label.html.haml @@ -1,6 +1,7 @@ - label_css_id = dom_id(label) -- open_issues_count = label.open_issues_count(current_user, @project) -- open_merge_requests_count = label.open_merge_requests_count(current_user, @project) +- open_issues_count = label.open_issues_count(current_user) +- open_merge_requests_count = label.open_merge_requests_count(current_user) +- subject = local_assigns[:subject] %li{id: label_css_id, data: { id: label.id } } = render "shared/label_row", label: label @@ -12,10 +13,10 @@ .dropdown-menu.dropdown-menu-align-right %ul %li - = link_to_label(label, subject: @project, type: :merge_request) do + = link_to_label(label, subject: subject, type: :merge_request) do = pluralize open_merge_requests_count, 'merge request' %li - = link_to_label(label, subject: @project) do + = link_to_label(label, subject: subject) do = pluralize open_issues_count, 'open issue' - if current_user %li.label-subscription{ data: toggle_subscription_data(label) } @@ -28,9 +29,9 @@ = link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, remote: true, data: {confirm: 'Remove this label? Are you sure?'} .pull-right.hidden-xs.hidden-sm.hidden-md - = link_to_label(label, subject: @project, type: :merge_request, css_class: 'btn btn-transparent btn-action') do + = link_to_label(label, subject: subject, type: :merge_request, css_class: 'btn btn-transparent btn-action') do = pluralize open_merge_requests_count, 'merge request' - = link_to_label(label, subject: @project, css_class: 'btn btn-transparent btn-action') do + = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action') do = pluralize open_issues_count, 'open issue' - if current_user |