Commit 44603ea9 authored by Sean McGivern's avatar Sean McGivern Committed by Rémy Coutable

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: default avatarRémy Coutable <remy@rymai.me>
parent 3c5901c4
...@@ -4,7 +4,8 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -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. - Honour issue and merge request visibility in their respective finders.
- Disable reference Markdown for unavailable features. - 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) ## 8.13.3 (2016-11-02)
......
...@@ -126,7 +126,7 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -126,7 +126,7 @@ class Projects::LabelsController < Projects::ApplicationController
alias_method :subscribable_resource, :label alias_method :subscribable_resource, :label
def find_labels 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 end
def authorize_admin_labels! def authorize_admin_labels!
......
...@@ -121,7 +121,7 @@ class IssuableFinder ...@@ -121,7 +121,7 @@ class IssuableFinder
@labels = @labels =
if labels? && !filter_by_no_label? 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 else
Label.none Label.none
end end
...@@ -268,7 +268,7 @@ class IssuableFinder ...@@ -268,7 +268,7 @@ class IssuableFinder
items = items.with_label(label_names, params[:sort]) items = items.with_label(label_names, params[:sort])
if projects 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 }) items = items.where(labels: { id: label_ids })
end end
end end
......
...@@ -5,6 +5,10 @@ class GroupLabel < Label ...@@ -5,6 +5,10 @@ class GroupLabel < Label
alias_attribute :subject, :group alias_attribute :subject, :group
def subject_foreign_key
'group_id'
end
def to_reference(source_project = nil, target_project = nil, format: :id) def to_reference(source_project = nil, target_project = nil, format: :id)
super(source_project, target_project, format: format) super(source_project, target_project, format: format)
end end
......
...@@ -92,16 +92,23 @@ class Label < ActiveRecord::Base ...@@ -92,16 +92,23 @@ class Label < ActiveRecord::Base
nil nil
end end
def open_issues_count(user = nil, project = nil) def open_issues_count(user = nil)
issues_count(user, project_id: project.try(:id) || project_id, state: 'opened') issues_count(user, state: 'opened')
end end
def closed_issues_count(user = nil, project = nil) def closed_issues_count(user = nil)
issues_count(user, project_id: project.try(:id) || project_id, state: 'closed') issues_count(user, state: 'closed')
end end
def open_merge_requests_count(user = nil, project = nil) def open_merge_requests_count(user = nil)
merge_requests_count(user, project_id: project.try(:id) || project_id, state: 'opened') params = {
subject_foreign_key => subject.id,
label_name: title,
scope: 'all',
state: 'opened'
}
MergeRequestsFinder.new(user, params.with_indifferent_access).execute.count
end end
def prioritize!(project, value) def prioritize!(project, value)
...@@ -167,15 +174,8 @@ class Label < ActiveRecord::Base ...@@ -167,15 +174,8 @@ class Label < ActiveRecord::Base
end end
def issues_count(user, params = {}) def issues_count(user, params = {})
IssuesFinder.new(user, params.reverse_merge(label_name: title, scope: 'all')) params.merge!(subject_foreign_key => subject.id, label_name: title, scope: 'all')
.execute IssuesFinder.new(user, params.with_indifferent_access).execute.count
.count
end
def merge_requests_count(user, params = {})
MergeRequestsFinder.new(user, params.reverse_merge(label_name: title, scope: 'all'))
.execute
.count
end end
def label_format_reference(format = :id) def label_format_reference(format = :id)
......
...@@ -420,7 +420,7 @@ class Project < ActiveRecord::Base ...@@ -420,7 +420,7 @@ class Project < ActiveRecord::Base
end end
def group_ids def group_ids
joins(:namespace).where(namespaces: { type: 'Group' }).pluck(:namespace_id) joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end end
end end
......
...@@ -12,6 +12,10 @@ class ProjectLabel < Label ...@@ -12,6 +12,10 @@ class ProjectLabel < Label
alias_attribute :subject, :project alias_attribute :subject, :project
def subject_foreign_key
'project_id'
end
def to_reference(target_project = nil, format: :id) def to_reference(target_project = nil, format: :id)
super(project, target_project, format: format) super(project, target_project, format: format)
end end
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
.other-labels .other-labels
- if @labels.present? - if @labels.present?
%ul.content-list.manage-labels-list.js-other-labels %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' = paginate @labels, theme: 'gitlab'
- else - else
.nothing-here-block .nothing-here-block
......
...@@ -22,14 +22,14 @@ ...@@ -22,14 +22,14 @@
%ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) } %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 %p.empty-message{ class: ('hidden' unless @prioritized_labels.empty?) } No prioritized labels yet
- if @prioritized_labels.present? - 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 .other-labels
- if can?(current_user, :admin_label, @project) - if can?(current_user, :admin_label, @project)
%h5{ class: ('hide' if hide) } Other Labels %h5{ class: ('hide' if hide) } Other Labels
%ul.content-list.manage-labels-list.js-other-labels %ul.content-list.manage-labels-list.js-other-labels
- if @labels.present? - 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' = paginate @labels, theme: 'gitlab'
- if @labels.blank? - if @labels.blank?
.nothing-here-block .nothing-here-block
......
- label_css_id = dom_id(label) - label_css_id = dom_id(label)
- open_issues_count = label.open_issues_count(current_user, @project) - open_issues_count = label.open_issues_count(current_user)
- open_merge_requests_count = label.open_merge_requests_count(current_user, @project) - open_merge_requests_count = label.open_merge_requests_count(current_user)
- subject = local_assigns[:subject]
%li{id: label_css_id, data: { id: label.id } } %li{id: label_css_id, data: { id: label.id } }
= render "shared/label_row", label: label = render "shared/label_row", label: label
...@@ -12,10 +13,10 @@ ...@@ -12,10 +13,10 @@
.dropdown-menu.dropdown-menu-align-right .dropdown-menu.dropdown-menu-align-right
%ul %ul
%li %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' = pluralize open_merge_requests_count, 'merge request'
%li %li
= link_to_label(label, subject: @project) do = link_to_label(label, subject: subject) do
= pluralize open_issues_count, 'open issue' = pluralize open_issues_count, 'open issue'
- if current_user - if current_user
%li.label-subscription{ data: toggle_subscription_data(label) } %li.label-subscription{ data: toggle_subscription_data(label) }
...@@ -28,9 +29,9 @@ ...@@ -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?'} = 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 .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' = 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' = pluralize open_issues_count, 'open issue'
- if current_user - if current_user
......
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