From 2525e55ec9bdd4b3a9e3284c71fde978cab6483a Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 26 Oct 2016 11:51:34 -0200 Subject: [PATCH 1/6] Remove unnecessary includes(:priorities) on Projects::LabelsController --- app/controllers/projects/labels_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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! From bc7895fff80a038ff14e7319ef303e58474d161c Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 28 Oct 2016 00:43:31 -0200 Subject: [PATCH 2/6] Use label subject to calculate number of issues/mrs within the group --- app/finders/labels_finder.rb | 2 +- app/models/group_label.rb | 4 +++ app/models/label.rb | 30 +++++++++++------------ app/models/project_label.rb | 4 +++ app/views/groups/labels/index.html.haml | 2 +- app/views/projects/labels/index.html.haml | 4 +-- app/views/shared/_label.html.haml | 12 ++++----- 7 files changed, 33 insertions(+), 25 deletions(-) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 865f093f04a..18d1396d78b 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -22,7 +22,7 @@ class LabelsFinder < UnionFinder label_ids << project.group.labels if project.group.present? label_ids << project.labels else - label_ids << Label.where(group_id: projects.group_ids) + label_ids << Label.where(group_id: projects.group_ids.uniq) label_ids << Label.where(project_id: projects.select(:id)) 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_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..e0e58a6b31e 100644 --- a/app/views/shared/_label.html.haml +++ b/app/views/shared/_label.html.haml @@ -1,6 +1,6 @@ - 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) %li{id: label_css_id, data: { id: label.id } } = render "shared/label_row", label: label @@ -12,10 +12,10 @@ .dropdown-menu.dropdown-menu-align-right %ul %li - = link_to_label(label, subject: @project, type: :merge_request) do + = link_to_label(label, subject: local_assigns[: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: local_assigns[:subject]) do = pluralize open_issues_count, 'open issue' - if current_user %li.label-subscription{ data: toggle_subscription_data(label) } @@ -28,9 +28,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: local_assigns[: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: local_assigns[:subject], css_class: 'btn btn-transparent btn-action') do = pluralize open_issues_count, 'open issue' - if current_user From adb8b8285818fe01294a56d1214c00fe70144a9a Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 28 Oct 2016 00:50:38 -0200 Subject: [PATCH 3/6] Skip authorization check when searching for labels on IssuableFinder --- app/finders/issuable_finder.rb | 4 ++-- app/finders/labels_finder.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index e27986ef95b..cc2073081b5 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -126,7 +126,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 @@ -273,7 +273,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/finders/labels_finder.rb b/app/finders/labels_finder.rb index 18d1396d78b..865f093f04a 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -22,7 +22,7 @@ class LabelsFinder < UnionFinder label_ids << project.group.labels if project.group.present? label_ids << project.labels else - label_ids << Label.where(group_id: projects.group_ids.uniq) + label_ids << Label.where(group_id: projects.group_ids) label_ids << Label.where(project_id: projects.select(:id)) end From 37cb2e7868b6551c009793d0c6e273efb5c10ed9 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 28 Oct 2016 16:20:46 -0200 Subject: [PATCH 4/6] Use select instead of pluck on Project.group_ids --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index ae57815c620..9f9f14d53db 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -390,7 +390,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 From 7e1ddfb81b229922c271dd07a278de4dd68b6d75 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 28 Oct 2016 16:21:45 -0200 Subject: [PATCH 5/6] Assign local_assigns[:subject] to a variable on the shared label partial --- app/views/shared/_label.html.haml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml index e0e58a6b31e..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) - 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: local_assigns[:subject], 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: local_assigns[:subject]) 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: local_assigns[:subject], 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: local_assigns[:subject], 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 From 7b2af24d9733d5ad6073aae271a718cea2c825b2 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 31 Oct 2016 23:29:13 -0200 Subject: [PATCH 6/6] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 372ddecc98b..14907e1546e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ Please view this file on the master branch, on stable branches it's out of date. - Shortened merge request modal to let clipboard button not overlap - In all filterable drop downs, put input field in focus only after load is complete (Ido @leibo) +## 8.13.3 + +- Reduce the overhead to calculate number of open/closed issues and merge requests within the group or project + ## 8.13.2 (2016-10-31) - Fix encoding issues on pipeline commits. !6832