Merge branch 'issue_39453' into 'master'

Add milestone issues display limit

See merge request gitlab-org/gitlab!23102

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
@@ -281,6 +281,26 @@ module MilestonesHelper
       can?(current_user, :admin_milestone, @project.group)
     end
   end
+
+  def display_issues_count_warning?
+    milestone_visible_issues_count > Milestone::DISPLAY_ISSUES_LIMIT
+  end
+
+  def milestone_issues_count_message
+    total_count = milestone_visible_issues_count
+    limit = Milestone::DISPLAY_ISSUES_LIMIT
+
+    message = _('Showing %{limit} of %{total_count} issues. ') % { limit: limit, total_count: total_count }
+    message += link_to(_('View all issues'), milestones_issues_path)
+
+    message.html_safe
+  end
+
+  private
+
+  def milestone_visible_issues_count
+    @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 = 20
+
   def total_issues_count(user)
     count_issues_by_state(user).values.sum
   end
@@ -53,7 +55,11 @@ 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).limit(DISPLAY_ISSUES_LIMIT).select(:id)
+
+    Issue.where(id: limited_ids).preload_associated_models.sort_by_attribute('label_priority')
   end
 
   def sorted_merge_requests(user)

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?
+  .flash-container
+    .flash-warning#milestone-issue-count-warning
+      = milestone_issues_count_message
+
 .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

@@ -5,12 +5,12 @@
     %li.no-border
       %span.label-row
         %span.label-name
-          = 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

@@ -17051,6 +17051,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 ""
 
@@ -20671,6 +20674,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))
+      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,32 @@ 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' 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)
+    end
   end
 
   context 'attributes visibility' do