diff --git a/app/graphql/types/issue_sort_enum.rb b/app/graphql/types/issue_sort_enum.rb index 23d1d549d59b7fce7eafb984c388e8ab78304944..7dced3c8e0068d62b1c3ef8229c6887afb75f165 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 09607ca5e9bbc8bff820610bc43173c62e8edb36..47aa2b24febb4405576d4902ad209fb2f8d7cb5d 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 6021d634f86fc2926919545550d8d5544dda60bf..465025ef2e941314d9e9c4a28e02fd934d84e250 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 0000000000000000000000000000000000000000..084cdb6166d3d9ec911d9ce00898c92041bca9d2 --- /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 0000000000000000000000000000000000000000..bccdcd90ed62ee6c8f03d1196b90d74b6b7b8576 --- /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 ca87a4b129814d54c739f1dc3ac6a88a0c400eac..750006950fdee612c3e995cce5041df91626fa86 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 afcf6e2d581581fa956ff9fb907228bf2149e576..9568d0db51de0b1a5e19763c4214dca8d6f04923 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 57fedbead33fd4674ef6b7462dbab87fb8ecb7ba..559ebd290344f65f471e979534da4ad65d6a9a72 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 a1005fd4af42614c0841803f220fc6bebe2a1697..759ad358f1e291dfcfd4620255593b6ff220f7c4 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 1f2e2aec0239d07c57b1923858b71b6a89a073c4..707bd7251e5be287e627d795de1f5a2c73dd913d 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 97c76c585608d587eddc445e6f1bf2afffdf6b7d..d45a23a7ef8082107f0fb63ebbc75cfa5547790e 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