Merge branch '19880-sort-closed-issues-by-recently-closed' into 'master'

Issue Boards: Sort closed issues in order of when they were closed

Closes #19880

See merge request gitlab-org/gitlab!23442

app/models/issue.rb

@@ -67,6 +67,7 @@ class Issue < ApplicationRecord
   scope :order_due_date_desc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) }
   scope :order_closest_future_date, -> { reorder(Arel.sql('CASE WHEN issues.due_date >= CURRENT_DATE THEN 0 ELSE 1 END ASC, ABS(CURRENT_DATE - issues.due_date) ASC')) }
   scope :order_relative_position_asc, -> { reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) }
+  scope :order_closed_date_desc, -> { reorder(closed_at: :desc) }
 
   scope :preload_associated_models, -> { preload(:labels, project: :namespace) }
   scope :with_api_entity_associations, -> { preload(:timelogs, :assignees, :author, :notes, :labels, project: [:route, { namespace: :route }] ) }

app/services/boards/issues/list_service.rb

@@ -10,6 +10,8 @@ module Boards
       end
 
       def execute
+        return fetch_issues.order_closed_date_desc if list&.closed?
+
         fetch_issues.order_by_position_and_priority(with_cte: can_attempt_search_optimization?)
       end
 

changelogs/unreleased/19880-sort-closed-issues-by-recently-closed.yml

+---
+title: Sort closed issues on issue boards using time of closing
+merge_request: 23442
+author: briankabiro
+type: changed

spec/controllers/boards/issues_controller_spec.rb

@@ -57,6 +57,18 @@ describe Boards::IssuesController do
           expect(development.issues.map(&:relative_position)).not_to include(nil)
         end
 
+        it 'returns issues by closed_at in descending order in closed list' do
+          create(:closed_issue, project: project, title: 'New Issue 1', closed_at: 1.day.ago)
+          create(:closed_issue, project: project, title: 'New Issue 2', closed_at: 1.week.ago)
+
+          list_issues user: user, board: board, list: board.lists.last.id
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['issues'].length).to eq(2)
+          expect(json_response['issues'][0]['title']).to eq('New Issue 1')
+          expect(json_response['issues'][1]['title']).to eq('New Issue 2')
+        end
+
         it 'avoids N+1 database queries' do
           create(:labeled_issue, project: project, labels: [development])
           control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: board, list: list2) }.count

spec/features/boards/issue_ordering_spec.rb

@@ -47,6 +47,31 @@ describe 'Issue Boards', :js do
     end
   end
 
+  context 'closed issues' do
+    let!(:issue7) { create(:closed_issue, project: project, title: 'Closed issue 1', closed_at: 1.day.ago) }
+    let!(:issue8) { create(:closed_issue, project: project, title: 'Closed issue 2', closed_at: 1.week.ago) }
+    let!(:issue9) { create(:closed_issue, project: project, title: 'Closed issue 3', closed_at: 2.weeks.ago) }
+
+    before do
+      visit project_board_path(project, board)
+      wait_for_requests
+
+      expect(page).to have_selector('.board', count: 3)
+    end
+
+    it 'orders issues by closed_at' do
+      wait_for_requests
+
+      page.within(find('.board:nth-child(3)')) do
+        first, second, third = all('.board-card').to_a
+
+        expect(first).to have_content(issue7.title)
+        expect(second).to have_content(issue8.title)
+        expect(third).to have_content(issue9.title)
+      end
+    end
+  end
+
   context 'ordering in list' do
     before do
       visit project_board_path(project, board)

spec/services/boards/issues/list_service_spec.rb

@@ -33,11 +33,11 @@ describe Boards::Issues::ListService do
       let!(:list1_issue3) { create(:labeled_issue, project: project, milestone: m1, labels: [development, p1]) }
       let!(:list2_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [testing]) }
 
-      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) }
-      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3]) }
-      let!(:closed_issue3) { create(:issue, :closed, project: project) }
-      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) }
-      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development]) }
+      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug], closed_at: 1.day.ago) }
+      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3], closed_at: 2.days.ago) }
+      let!(:closed_issue3) { create(:issue, :closed, project: project, closed_at: 1.week.ago) }
+      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1], closed_at: 1.year.ago) }
+      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development], closed_at: 2.years.ago) }
 
       let(:parent) { project }
 
@@ -94,11 +94,11 @@ describe Boards::Issues::ListService do
       let!(:list1_issue3) { create(:labeled_issue, project: project1, milestone: m1, labels: [development, p1, p1_project1]) }
       let!(:list2_issue1) { create(:labeled_issue, project: project1, milestone: m1, labels: [testing]) }
 
-      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) }
-      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3, p3_project]) }
-      let!(:closed_issue3) { create(:issue, :closed, project: project1) }
-      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project1, labels: [p1, p1_project1]) }
-      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project1, labels: [development]) }
+      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug], closed_at: 1.day.ago) }
+      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3, p3_project], closed_at: 2.days.ago) }
+      let!(:closed_issue3) { create(:issue, :closed, project: project1, closed_at: 1.week.ago) }
+      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project1, labels: [p1, p1_project1], closed_at: 1.year.ago) }
+      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project1, labels: [development], closed_at: 2.years.ago) }
 
       before do
         group.add_developer(user)

spec/support/shared_examples/services/boards/issues_list_service_shared_examples.rb

@@ -36,20 +36,22 @@ RSpec.shared_examples 'issues list service' do
       expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1]
     end
 
-    it 'returns closed issues when listing issues from Closed' do
-      params = { board_id: board.id, id: closed.id }
+    it 'returns opened issues that have label list applied when listing issues from a label list' do
+      params = { board_id: board.id, id: list1.id }
 
       issues = described_class.new(parent, user, params).execute
 
-      expect(issues).to eq [closed_issue4, closed_issue2, closed_issue5, closed_issue3, closed_issue1]
+      expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2]
     end
+  end
 
-    it 'returns opened issues that have label list applied when listing issues from a label list' do
-      params = { board_id: board.id, id: list1.id }
+  context 'issues are ordered by date of closing' do
+    it 'returns closed issues when listing issues from Closed' do
+      params = { board_id: board.id, id: closed.id }
 
       issues = described_class.new(parent, user, params).execute
 
-      expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2]
+      expect(issues).to eq [closed_issue1, closed_issue2, closed_issue3, closed_issue4, closed_issue5]
     end
   end