From f77350c785ae225fefecce7e983ab37b9ff58340 Mon Sep 17 00:00:00 2001
From: Marco Zille <marco.zille@gmail.com>
Date: Thu, 16 Jun 2022 19:41:53 +0000
Subject: [PATCH] Add backend changes to sort issues by closed at
Changelog: added
---
app/graphql/types/issue_sort_enum.rb | 2 ++
app/models/issue.rb | 5 ++++-
app/services/boards/issues/list_service.rb | 2 +-
...ndex_issues_on_project_id_and_closed_at.rb | 20 +++++++++++++++++++
db/schema_migrations/20220616092541 | 1 +
doc/api/graphql/reference/index.md | 2 ++
ee/app/models/ee/epic.rb | 3 ++-
ee/app/services/boards/epics/list_service.rb | 2 +-
ee/spec/models/epic_spec.rb | 20 +++++++++++++++++++
.../graphql/resolvers/issues_resolver_spec.rb | 16 +++++++++++++++
spec/models/issue_spec.rb | 20 +++++++++++++++++++
11 files changed, 89 insertions(+), 4 deletions(-)
create mode 100644 db/post_migrate/20220616092541_prepare_index_issues_on_project_id_and_closed_at.rb
create mode 100644 db/schema_migrations/20220616092541
diff --git a/app/graphql/types/issue_sort_enum.rb b/app/graphql/types/issue_sort_enum.rb
index 23d1d549d59b..7dced3c8e006 100644
--- a/app/graphql/types/issue_sort_enum.rb
+++ b/app/graphql/types/issue_sort_enum.rb
@@ -16,6 +16,8 @@ class IssueSortEnum < IssuableSortEnum
value 'POPULARITY_DESC', 'Number of upvotes (awarded "thumbs up" emoji) by descending order.', value: :popularity_desc
value 'ESCALATION_STATUS_ASC', 'Status from triggered to resolved.', value: :escalation_status_asc
value 'ESCALATION_STATUS_DESC', 'Status from resolved to triggered.', value: :escalation_status_desc
+ value 'CLOSED_AT_ASC', 'Closed time by ascending order.', value: :closed_at_asc
+ value 'CLOSED_AT_DESC', 'Closed time by descending order.', value: :closed_at_desc
end
end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 09607ca5e9bb..47aa2b24febb 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -122,12 +122,13 @@ def most_recent
scope :order_due_date_asc, -> { reorder(arel_table[:due_date].asc.nulls_last) }
scope :order_due_date_desc, -> { reorder(arel_table[:due_date].desc.nulls_last) }
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_closed_date_desc, -> { reorder(closed_at: :desc) }
scope :order_created_at_desc, -> { reorder(created_at: :desc) }
scope :order_severity_asc, -> { includes(:issuable_severity).order('issuable_severities.severity ASC NULLS FIRST') }
scope :order_severity_desc, -> { includes(:issuable_severity).order('issuable_severities.severity DESC NULLS LAST') }
scope :order_escalation_status_asc, -> { includes(:incident_management_issuable_escalation_status).order(IncidentManagement::IssuableEscalationStatus.arel_table[:status].asc.nulls_last).references(:incident_management_issuable_escalation_status) }
scope :order_escalation_status_desc, -> { includes(:incident_management_issuable_escalation_status).order(IncidentManagement::IssuableEscalationStatus.arel_table[:status].desc.nulls_last).references(:incident_management_issuable_escalation_status) }
+ scope :order_closed_at_asc, -> { reorder(arel_table[:closed_at].asc.nulls_last) }
+ scope :order_closed_at_desc, -> { reorder(arel_table[:closed_at].desc.nulls_last) }
scope :preload_associated_models, -> { preload(:assignees, :labels, project: :namespace) }
scope :with_web_entity_associations, -> { preload(:author, project: [:project_feature, :route, namespace: :route]) }
@@ -331,6 +332,8 @@ def self.sort_by_attribute(method, excluded_labels: [])
when 'severity_desc' then order_severity_desc.with_order_id_desc
when 'escalation_status_asc' then order_escalation_status_asc.with_order_id_desc
when 'escalation_status_desc' then order_escalation_status_desc.with_order_id_desc
+ when 'closed_at_asc' then order_closed_at_asc
+ when 'closed_at_desc' then order_closed_at_desc
else
super
end
diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb
index 6021d634f86f..465025ef2e94 100644
--- a/app/services/boards/issues/list_service.rb
+++ b/app/services/boards/issues/list_service.rb
@@ -20,7 +20,7 @@ def self.initialize_relative_positions(board, current_user, issues)
private
def order(items)
- return items.order_closed_date_desc if list&.closed?
+ return items.order_closed_at_desc if list&.closed?
items.order_by_relative_position
end
diff --git a/db/post_migrate/20220616092541_prepare_index_issues_on_project_id_and_closed_at.rb b/db/post_migrate/20220616092541_prepare_index_issues_on_project_id_and_closed_at.rb
new file mode 100644
index 000000000000..084cdb6166d3
--- /dev/null
+++ b/db/post_migrate/20220616092541_prepare_index_issues_on_project_id_and_closed_at.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class PrepareIndexIssuesOnProjectIdAndClosedAt < Gitlab::Database::Migration[2.0]
+ NEW_INDEX_NAME_1 = 'index_issues_on_project_id_closed_at_desc_state_id_and_id'
+ NEW_INDEX_NAME_2 = 'index_issues_on_project_id_closed_at_state_id_and_id'
+
+ def up
+ # Index to improve performance when sorting issues by closed_at desc
+ prepare_async_index :issues, 'project_id, closed_at DESC NULLS LAST, state_id, id', name: NEW_INDEX_NAME_1
+
+ # Index to improve performance when sorting issues by closed_at asc
+ # This replaces the old index which didn't account for state_id and id
+ prepare_async_index :issues, [:project_id, :closed_at, :state_id, :id], name: NEW_INDEX_NAME_2
+ end
+
+ def down
+ unprepare_async_index_by_name :issues, NEW_INDEX_NAME_1
+ unprepare_async_index_by_name :issues, NEW_INDEX_NAME_2
+ end
+end
diff --git a/db/schema_migrations/20220616092541 b/db/schema_migrations/20220616092541
new file mode 100644
index 000000000000..bccdcd90ed62
--- /dev/null
+++ b/db/schema_migrations/20220616092541
@@ -0,0 +1 @@
+2c177b0199019ebdbc06b43d21d47a35453e3b376ccbde21163128c77826478b
\ No newline at end of file
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index ca87a4b12981..750006950fde 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -18989,6 +18989,8 @@ Values for sorting issues.
| ----- | ----------- |
| <a id="issuesortblocking_issues_asc"></a>`BLOCKING_ISSUES_ASC` | Blocking issues count by ascending order. |
| <a id="issuesortblocking_issues_desc"></a>`BLOCKING_ISSUES_DESC` | Blocking issues count by descending order. |
+| <a id="issuesortclosed_at_asc"></a>`CLOSED_AT_ASC` | Closed time by ascending order. |
+| <a id="issuesortclosed_at_desc"></a>`CLOSED_AT_DESC` | Closed time by descending order. |
| <a id="issuesortcreated_asc"></a>`CREATED_ASC` | Created at ascending order. |
| <a id="issuesortcreated_desc"></a>`CREATED_DESC` | Created at descending order. |
| <a id="issuesortdue_date_asc"></a>`DUE_DATE_ASC` | Due date by ascending order. |
diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index afcf6e2d5815..9568d0db51de 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -121,7 +121,8 @@ def close
reorder(keyset_order)
end
- scope :order_closed_date_desc, -> { reorder(closed_at: :desc) }
+ scope :order_closed_at_asc, -> { reorder(arel_table[:closed_at].asc.nulls_last) }
+ scope :order_closed_at_desc, -> { reorder(arel_table[:closed_at].desc.nulls_last) }
scope :order_relative_position, -> do
reorder('relative_position ASC', 'id DESC')
diff --git a/ee/app/services/boards/epics/list_service.rb b/ee/app/services/boards/epics/list_service.rb
index 57fedbead33f..559ebd290344 100644
--- a/ee/app/services/boards/epics/list_service.rb
+++ b/ee/app/services/boards/epics/list_service.rb
@@ -53,7 +53,7 @@ def board
def order(items)
items = items.join_board_position(board.id) if needs_board_position?
- return items.order_closed_date_desc if list&.closed?
+ return items.order_closed_at_desc if list&.closed?
items.order_relative_position_on_board(board.id)
end
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index a1005fd4af42..759ad358f1e2 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -1104,4 +1104,24 @@ def as_item(item)
end
end
end
+
+ context 'order by closed_at' do
+ let!(:epic_a) { create(:epic, closed_at: 1.day.ago) }
+ let!(:epic_b) { create(:epic, closed_at: 5.days.ago) }
+ let!(:epic_c_nil) { create(:epic, closed_at: nil) }
+ let!(:epic_d) { create(:epic, closed_at: 3.days.ago) }
+ let!(:epic_e_nil) { create(:epic, closed_at: nil) }
+
+ describe '.order_closed_at_asc' do
+ it 'orders on closed at' do
+ expect(described_class.order_closed_at_asc.to_a).to eq([epic_b, epic_d, epic_a, epic_c_nil, epic_e_nil])
+ end
+ end
+
+ describe '.order_closed_at_desc' do
+ it 'orders on closed at' do
+ expect(described_class.order_closed_at_desc.to_a).to eq([epic_a, epic_d, epic_b, epic_c_nil, epic_e_nil])
+ end
+ end
+ end
end
diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb
index 1f2e2aec0239..707bd7251e5b 100644
--- a/spec/graphql/resolvers/issues_resolver_spec.rb
+++ b/spec/graphql/resolvers/issues_resolver_spec.rb
@@ -428,6 +428,22 @@
end
end
+ context 'when sorting by closed at' do
+ let_it_be(:project) { create(:project, :public) }
+ let_it_be(:closed_issue1) { create(:issue, project: project, closed_at: 3.days.from_now) }
+ let_it_be(:closed_issue2) { create(:issue, project: project, closed_at: nil) }
+ let_it_be(:closed_issue3) { create(:issue, project: project, closed_at: 2.days.ago) }
+ let_it_be(:closed_issue4) { create(:issue, project: project, closed_at: nil) }
+
+ it 'sorts issues ascending' do
+ expect(resolve_issues(sort: :closed_at_asc).to_a).to eq [closed_issue3, closed_issue1, closed_issue4, closed_issue2]
+ end
+
+ it 'sorts issues descending' do
+ expect(resolve_issues(sort: :closed_at_desc).to_a).to eq [closed_issue1, closed_issue3, closed_issue4, closed_issue2]
+ end
+ end
+
context 'when sorting by due date' do
let_it_be(:project) { create(:project, :public) }
let_it_be(:due_issue1) { create(:issue, project: project, due_date: 3.days.from_now) }
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 97c76c585608..d45a23a7ef80 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -1607,4 +1607,24 @@
end
end
end
+
+ context 'order by closed_at' do
+ let!(:issue_a) { create(:issue, closed_at: 1.day.ago) }
+ let!(:issue_b) { create(:issue, closed_at: 5.days.ago) }
+ let!(:issue_c_nil) { create(:issue, closed_at: nil) }
+ let!(:issue_d) { create(:issue, closed_at: 3.days.ago) }
+ let!(:issue_e_nil) { create(:issue, closed_at: nil) }
+
+ describe '.order_closed_at_asc' do
+ it 'orders on closed at' do
+ expect(described_class.order_closed_at_asc.to_a).to eq([issue_b, issue_d, issue_a, issue_c_nil, issue_e_nil])
+ end
+ end
+
+ describe '.order_closed_at_desc' do
+ it 'orders on closed at' do
+ expect(described_class.order_closed_at_desc.to_a).to eq([issue_a, issue_d, issue_b, issue_c_nil, issue_e_nil])
+ end
+ end
+ end
end
--
GitLab