Merge branch 'issue_39453_2' into 'master'

Add milestone issues display limit

See merge request gitlab-org/gitlab!27361

app/helpers/milestones_helper.rb

@@ -26,7 +26,7 @@ module MilestonesHelper
     end
   end
 
-  def milestones_label_path(opts = {})
+  def milestones_issues_path(opts = {})
     if @project
       project_issues_path(@project, opts)
     elsif @group
@@ -283,6 +283,27 @@ module MilestonesHelper
       can?(current_user, :admin_milestone, @project.group)
     end
   end
+
+  def display_issues_count_warning?(milestone)
+    milestone_visible_issues_count(milestone) > Milestone::DISPLAY_ISSUES_LIMIT
+  end
+
+  def milestone_issues_count_message(milestone)
+    total_count = milestone_visible_issues_count(milestone)
+    limit = Milestone::DISPLAY_ISSUES_LIMIT
+    link_options = { milestone_title: @milestone.title }
+
+    message = _('Showing %{limit} of %{total_count} issues. ') % { limit: limit, total_count: total_count }
+    message += link_to(_('View all issues'), milestones_issues_path(link_options))
+
+    message.html_safe
+  end
+
+  private
+
+  def milestone_visible_issues_count(milestone)
+    @milestone_visible_issues_count ||= milestone.issues_visible_to_user(current_user).size
+  end
 end
 
 MilestonesHelper.prepend_if_ee('EE::MilestonesHelper')

app/models/concerns/milestoneish.rb

 # frozen_string_literal: true
 
 module Milestoneish
+  DISPLAY_ISSUES_LIMIT = 3000
+
   def total_issues_count
     @total_issues_count ||= Milestones::IssuesCountService.new(self).count
   end
@@ -55,7 +57,15 @@ module Milestoneish
   end
 
   def sorted_issues(user)
-    issues_visible_to_user(user).preload_associated_models.sort_by_attribute('label_priority')
+    # This method is used on milestone view to filter opened assigned, opened unassigned and closed issues columns.
+    # We want a limit of DISPLAY_ISSUES_LIMIT for total issues present on all columns.
+    limited_ids =
+      issues_visible_to_user(user).sort_by_attribute('label_priority').limit(DISPLAY_ISSUES_LIMIT)
+
+    Issue
+      .where(id: Issue.select(:id).from(limited_ids))
+      .preload_associated_models
+      .sort_by_attribute('label_priority')
   end
 
   def sorted_merge_requests(user)

app/models/issue.rb

@@ -70,7 +70,7 @@ class Issue < ApplicationRecord
   scope :order_closed_date_desc, -> { reorder(closed_at: :desc) }
   scope :order_created_at_desc, -> { reorder(created_at: :desc) }
 
-  scope :preload_associated_models, -> { preload(:labels, project: :namespace) }
+  scope :preload_associated_models, -> { preload(:assignees, :labels, project: :namespace) }
   scope :with_api_entity_associations, -> { preload(:timelogs, :assignees, :author, :notes, :labels, project: [:route, { namespace: :route }] ) }
 
   scope :public_only, -> { where(confidential: false) }

app/views/shared/milestones/_issues_tab.html.haml

 - args = { show_project_name: local_assigns.fetch(:show_project_name, false),
            show_full_project_name: local_assigns.fetch(:show_full_project_name, false) }
 
+- if display_issues_count_warning?(@milestone)
+  .flash-container
+    .flash-warning#milestone-issue-count-warning
+      = milestone_issues_count_message(@milestone)
+
 .row.prepend-top-default
   .col-md-4
     = render 'shared/milestones/issuables', args.merge(title: 'Unstarted Issues (open and unassigned)', issuables: issues.opened.unassigned, id: 'unassigned', show_counter: true)

app/views/shared/milestones/_labels_tab.html.haml

@@ -3,12 +3,12 @@
     - options = { milestone_title: @milestone.title, label_name: label.title }
 
     %li.no-border
-      = render_label(label, tooltip: false, link: milestones_label_path(options))
+      = render_label(label, tooltip: false, link: milestones_issues_path(options))
       %span.prepend-description-left
         = markdown_field(label, :description)
 
       .float-right.d-none.d-lg-block.d-xl-block
-        = link_to milestones_label_path(options.merge(state: 'opened')), class: 'btn btn-transparent btn-action' do
+        = link_to milestones_issues_path(options.merge(state: 'opened')), class: 'btn btn-transparent btn-action' do
           - pluralize milestone_issues_by_label_count(@milestone, label, state: :opened), 'open issue'
-        = link_to milestones_label_path(options.merge(state: 'closed')), class: 'btn btn-transparent btn-action' do
+        = link_to milestones_issues_path(options.merge(state: 'closed')), class: 'btn btn-transparent btn-action' do
           - pluralize milestone_issues_by_label_count(@milestone, label, state: :closed), 'closed issue'

changelogs/unreleased/issue_39453.yml

+---
+title: Limits issues displayed on milestones
+merge_request: 23102
+author:
+type: performance

locale/gitlab.pot

@@ -18230,6 +18230,9 @@ msgid_plural "Showing %d events"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "Showing %{limit} of %{total_count} issues. "
+msgstr ""
+
 msgid "Showing %{pageSize} of %{total} issues"
 msgstr ""
 
@@ -22239,6 +22242,9 @@ msgstr ""
 msgid "View Documentation"
 msgstr ""
 
+msgid "View all issues"
+msgstr ""
+
 msgid "View blame prior to this change"
 msgstr ""
 

spec/features/milestones/user_views_milestone_spec.rb

@@ -25,6 +25,37 @@ describe "User views milestone" do
     expect { visit_milestone }.not_to exceed_query_limit(control)
   end
 
+  context 'limiting milestone issues' do
+    before_all do
+      2.times do
+        create(:issue, milestone: milestone, project: project)
+        create(:issue, milestone: milestone, project: project, assignees: [user])
+        create(:issue, milestone: milestone, project: project, state: :closed)
+      end
+    end
+
+    context 'when issues on milestone are over DISPLAY_ISSUES_LIMIT' do
+      it "limits issues to display and shows warning" do
+        stub_const('Milestoneish::DISPLAY_ISSUES_LIMIT', 3)
+
+        visit(project_milestone_path(project, milestone))
+
+        expect(page).to have_selector('.issuable-row', count: 3)
+        expect(page).to have_selector('#milestone-issue-count-warning', text: 'Showing 3 of 6 issues. View all issues')
+        expect(page).to have_link('View all issues', href: project_issues_path(project, { milestone_title: milestone.title }))
+      end
+    end
+
+    context 'when issues on milestone are below DISPLAY_ISSUES_LIMIT' do
+      it 'does not display warning' do
+        visit(project_milestone_path(project, milestone))
+
+        expect(page).not_to have_selector('#milestone-issue-count-warning', text: 'Showing 3 of 6 issues. View all issues')
+        expect(page).to have_selector('.issuable-row', count: 6)
+      end
+    end
+  end
+
   private
 
   def visit_milestone

spec/models/concerns/milestoneish_spec.rb

@@ -33,17 +33,34 @@ describe Milestone, 'Milestoneish' do
   end
 
   describe '#sorted_issues' do
-    it 'sorts issues by label priority' do
+    before do
       issue.labels << label_1
       security_issue_1.labels << label_2
       closed_issue_1.labels << label_3
+    end
 
+    it 'sorts issues by label priority' do
       issues = milestone.sorted_issues(member)
 
       expect(issues.first).to eq(issue)
       expect(issues.second).to eq(security_issue_1)
       expect(issues.third).not_to eq(closed_issue_1)
     end
+
+    it 'limits issue count and keeps the ordering' do
+      stub_const('Milestoneish::DISPLAY_ISSUES_LIMIT', 4)
+
+      issues = milestone.sorted_issues(member)
+      # Cannot use issues.count here because it is sorting
+      # by a virtual column 'highest_priority' and it will break
+      # the query.
+      total_issues_count = issues.opened.unassigned.length + issues.opened.assigned.length + issues.closed.length
+      expect(issues.length).to eq(4)
+      expect(total_issues_count).to eq(4)
+      expect(issues.first).to eq(issue)
+      expect(issues.second).to eq(security_issue_1)
+      expect(issues.third).not_to eq(closed_issue_1)
+    end
   end
 
   context 'attributes visibility' do