Commit 86bf928f authored by briankabiro's avatar briankabiro

Issue boards: Sort closed issues by `closed_at` time

On issue boards, issues are sorted based on their relative ordering
This changes the ordering of closed issues to use the time when
they were closed.
parent f5c7d8eb
...@@ -67,6 +67,7 @@ class Issue < ApplicationRecord ...@@ -67,6 +67,7 @@ class Issue < ApplicationRecord
scope :order_due_date_desc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) } 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_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_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 :preload_associated_models, -> { preload(:labels, project: :namespace) }
scope :with_api_entity_associations, -> { preload(:timelogs, :assignees, :author, :notes, :labels, project: [:route, { namespace: :route }] ) } scope :with_api_entity_associations, -> { preload(:timelogs, :assignees, :author, :notes, :labels, project: [:route, { namespace: :route }] ) }
......
...@@ -10,6 +10,8 @@ module Boards ...@@ -10,6 +10,8 @@ module Boards
end end
def execute 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?) fetch_issues.order_by_position_and_priority(with_cte: can_attempt_search_optimization?)
end end
......
---
title: Sort closed issues on issue boards using time of closing
merge_request: 23442
author: briankabiro
type: changed
...@@ -57,6 +57,18 @@ describe Boards::IssuesController do ...@@ -57,6 +57,18 @@ describe Boards::IssuesController do
expect(development.issues.map(&:relative_position)).not_to include(nil) expect(development.issues.map(&:relative_position)).not_to include(nil)
end 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 it 'avoids N+1 database queries' do
create(:labeled_issue, project: project, labels: [development]) create(:labeled_issue, project: project, labels: [development])
control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: board, list: list2) }.count control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: board, list: list2) }.count
......
...@@ -47,6 +47,31 @@ describe 'Issue Boards', :js do ...@@ -47,6 +47,31 @@ describe 'Issue Boards', :js do
end end
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 context 'ordering in list' do
before do before do
visit project_board_path(project, board) visit project_board_path(project, board)
......
...@@ -33,11 +33,11 @@ describe Boards::Issues::ListService do ...@@ -33,11 +33,11 @@ describe Boards::Issues::ListService do
let!(:list1_issue3) { create(:labeled_issue, project: project, milestone: m1, labels: [development, p1]) } 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!(:list2_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [testing]) }
let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) } 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]) } let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3], closed_at: 2.days.ago) }
let!(:closed_issue3) { create(:issue, :closed, project: project) } let!(:closed_issue3) { create(:issue, :closed, project: project, closed_at: 1.week.ago) }
let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) } 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]) } let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development], closed_at: 2.years.ago) }
let(:parent) { project } let(:parent) { project }
...@@ -94,11 +94,11 @@ describe Boards::Issues::ListService do ...@@ -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!(: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!(:list2_issue1) { create(:labeled_issue, project: project1, milestone: m1, labels: [testing]) }
let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) } 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]) } 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) } 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]) } 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]) } let!(:closed_issue5) { create(:labeled_issue, :closed, project: project1, labels: [development], closed_at: 2.years.ago) }
before do before do
group.add_developer(user) group.add_developer(user)
......
...@@ -36,20 +36,22 @@ RSpec.shared_examples 'issues list service' do ...@@ -36,20 +36,22 @@ RSpec.shared_examples 'issues list service' do
expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1]
end end
it 'returns closed issues when listing issues from Closed' do it 'returns opened issues that have label list applied when listing issues from a label list' do
params = { board_id: board.id, id: closed.id } params = { board_id: board.id, id: list1.id }
issues = described_class.new(parent, user, params).execute 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
end
it 'returns opened issues that have label list applied when listing issues from a label list' do context 'issues are ordered by date of closing' do
params = { board_id: board.id, id: list1.id } 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 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
end end
......
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