diff --git a/app/graphql/resolvers/concerns/work_items/non_stable_cursor_sort_options.rb b/app/graphql/resolvers/concerns/work_items/non_stable_cursor_sort_options.rb index d5234e4679f135dbb774e0c315b4497f346be339..b4075b6c6f372ed727254f9cfaa8de848c853d87 100644 --- a/app/graphql/resolvers/concerns/work_items/non_stable_cursor_sort_options.rb +++ b/app/graphql/resolvers/concerns/work_items/non_stable_cursor_sort_options.rb @@ -8,7 +8,8 @@ module NonStableCursorSortOptions popularity_asc popularity_desc label_priority_asc label_priority_desc milestone_due_asc milestone_due_desc - escalation_status_asc escalation_status_desc].freeze + escalation_status_asc escalation_status_desc + status_asc status_desc].freeze private diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index 0e78ba3370d34cf510cd09957c061c2579daedc4..091f9d094cefa40a6fee3389c48de3e90338b4d9 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -67,6 +67,66 @@ module HasStatus merge(items_to_exclude.invert_where) } + scope :with_status_joins, -> { + converted_statuses_join = <<-SQL.squish + LEFT JOIN namespaces ON namespaces.id = #{table_name}.namespace_id + LEFT JOIN work_item_custom_statuses converted_statuses ON + converted_statuses.converted_from_system_defined_status_identifier = + work_item_current_statuses.system_defined_status_id + AND converted_statuses.namespace_id = namespaces.traversal_ids[1] + SQL + + left_joins(current_status: :custom_status).joins(converted_statuses_join) + } + + scope :order_status_asc, -> { + with_status_joins.order(Arel.sql("#{generate_status_order_sql} ASC NULLS LAST, id DESC")) + } + + scope :order_status_desc, -> { + with_status_joins.order(Arel.sql("#{generate_status_order_sql} DESC NULLS FIRST, id DESC")) + } + + def self.generate_status_order_sql + system_defined_sort_orders = WorkItems::Statuses::SystemDefined::Status.sort_order_by_id + system_defined_status_cases = system_defined_sort_orders.map do |id, category| + "WHEN #{id} THEN #{category}" + end.join(' ') + + lifecycle = WorkItems::Statuses::SystemDefined::Lifecycle.all.first + default_open_status = lifecycle.default_open_status + default_duplicate_status = lifecycle.default_duplicate_status + default_closed_status = lifecycle.default_closed_status + + default_open_sort_order = system_defined_sort_orders[default_open_status.id] + default_duplicate_sort_order = system_defined_sort_orders[default_duplicate_status.id] + default_closed_sort_order = system_defined_sort_orders[default_closed_status.id] + + opened_state_value = Issue.available_states[:opened] + closed_state_value = Issue.available_states[:closed] + + <<-SQL.squish + CASE + WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN + work_item_custom_statuses.category + WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN + COALESCE( + converted_statuses.category, + CASE work_item_current_statuses.system_defined_status_id #{system_defined_status_cases} END + ) + ELSE + CASE + WHEN #{table_name}.state_id = #{opened_state_value} THEN #{default_open_sort_order} + WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NOT NULL + THEN #{default_duplicate_sort_order} + WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NULL + THEN #{default_closed_sort_order} + ELSE #{default_open_sort_order} + END + END + SQL + end + def status_with_fallback current_status_with_fallback&.status end diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb index 34b915b8b0df93d4d5250eeda9f98c3f92919bda..99edfda639a048b1219efb08854a4b8c56099318 100644 --- a/ee/app/models/ee/issue.rb +++ b/ee/app/models/ee/issue.rb @@ -307,6 +307,8 @@ def sort_by_attribute(method, excluded_labels: []) when 'sla_due_at_desc' then with_feature(:sla).order_sla_due_at_desc when 'health_status_asc' then order_health_status_asc when 'health_status_desc' then order_health_status_desc + when 'status_asc' then order_status_asc + when 'status_desc' then order_status_desc else super end diff --git a/ee/app/models/work_items/statuses/shared_constants.rb b/ee/app/models/work_items/statuses/shared_constants.rb index d901bdf27537dd354b95e00bea69d6ba36ae082a..26b1b1e1758b276707feac7e019eb5585ccf9430 100644 --- a/ee/app/models/work_items/statuses/shared_constants.rb +++ b/ee/app/models/work_items/statuses/shared_constants.rb @@ -3,6 +3,8 @@ module WorkItems module Statuses module SharedConstants + # Note: We rely on categories for ordering work items by status. + # If new categories are added, their placement will affect sort order. CATEGORIES = { triage: 1, to_do: 2, diff --git a/ee/app/models/work_items/statuses/system_defined/status.rb b/ee/app/models/work_items/statuses/system_defined/status.rb index 66069ae3143b33d71f63df8b25ed3f01d5b41e21..cc222137900578877a1faf31f92c7a923c0f019d 100644 --- a/ee/app/models/work_items/statuses/system_defined/status.rb +++ b/ee/app/models/work_items/statuses/system_defined/status.rb @@ -66,6 +66,12 @@ def find_by_name(status_name, partial_match: false) all.find { |status| status.matches_name?(status_name) } end end + + def sort_order_by_id + all.each_with_object({}) do |status, hash| + hash[status.id] = WorkItems::Statuses::SharedConstants::CATEGORIES[status.category] + end + end end def allowed_for_work_item?(work_item) diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index c0e12187122a497675c798fe1617b8710d156a5a..80923af938eece6b250e52f0a80d2cb9f1485fcf 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -510,8 +510,13 @@ it { is_expected.to contain_exactly(work_item_without_progress, work_item_with_stale_reminder) } end + # rubocop:disable RSpec/MultipleMemoizedHelpers -- Extra helpers are required for checking all scenarios describe 'status scopes' do + let_it_be(:reusable_group_2) { create(:group) } let_it_be(:project) { create(:project, group: reusable_group) } + let_it_be(:project_2) { create(:project, group: reusable_group_2) } + + let_it_be(:issue_work_item_type) { create(:work_item_type, :issue) } let_it_be(:wi_no_status) { create(:work_item, :incident, project: project) } let_it_be(:wi_default_open) { create(:work_item, project: project) } @@ -521,6 +526,7 @@ end let_it_be(:system_defined_todo_status) { build(:work_item_system_defined_status, :to_do) } + let_it_be(:system_defined_in_progress_status) { build(:work_item_system_defined_status, :in_progress) } let_it_be(:system_defined_done_status) { build(:work_item_system_defined_status, :done) } let_it_be(:system_defined_duplicate_status) { build(:work_item_system_defined_status, :duplicate) } let_it_be(:system_defined_wont_do_status) { build(:work_item_system_defined_status, :wont_do) } @@ -529,6 +535,10 @@ create(:work_item, project: project, system_defined_status_id: system_defined_todo_status.id) end + let_it_be(:wi_system_defined_in_progress) do + create(:work_item, project: project, system_defined_status_id: system_defined_in_progress_status.id) + end + let_it_be(:wi_system_defined_done) do create(:work_item, :closed, project: project, system_defined_status_id: system_defined_done_status.id) end @@ -547,7 +557,17 @@ # We can't stub licensed features for let_it_be blocks. build(:work_item_type_custom_lifecycle, namespace: reusable_group, - work_item_type: create(:work_item_type, :issue), + work_item_type: issue_work_item_type, + lifecycle: lifecycle + ).save!(validate: false) + end + end + + let(:lifecycle_2) do + create(:work_item_custom_lifecycle, namespace: reusable_group_2).tap do |lifecycle| + build(:work_item_type_custom_lifecycle, + namespace: reusable_group_2, + work_item_type: issue_work_item_type, lifecycle: lifecycle ).save!(validate: false) end @@ -561,6 +581,10 @@ ) end + let_it_be(:converted_in_progress_status) do + create(:work_item_custom_status, :in_progress, namespace: reusable_group, lifecycles: [lifecycle]) + end + let_it_be(:wi_custom_todo) do create(:work_item, project: project, custom_status_id: lifecycle.default_open_status_id) end @@ -575,6 +599,33 @@ let_it_be(:wi_custom) { create(:work_item, project: project, custom_status_id: custom_status.id) } + let_it_be(:wi_converted_in_progress) { create(:work_item, project: project) } + + let_it_be(:wi_converted_in_progress_current_status) do + # Skip validations since we are simulating an old record + # when the namespace still used the system defined lifecycle + build(:work_item_current_status, + work_item: wi_converted_in_progress, + system_defined_status_id: system_defined_in_progress_status.id + ).save!(validate: false) + end + + let(:wi_system_defined_todo_2) do + create(:work_item, project: project_2, system_defined_status_id: system_defined_todo_status.id) + end + + let(:wi_system_defined_done_2) do + create(:work_item, :closed, project: project_2, system_defined_status_id: system_defined_done_status.id) + end + + let(:wi_custom_todo_2) do + create(:work_item, project: project_2, custom_status_id: lifecycle_2.default_open_status_id) + end + + let(:wi_custom_done_2) do + create(:work_item, project: project_2, custom_status_id: lifecycle_2.default_closed_status_id) + end + describe '.with_status' do subject { described_class.with_status(status) } @@ -680,8 +731,9 @@ it 'returns all work items' do is_expected.to contain_exactly( wi_default_open, wi_default_closed, wi_default_duplicated, - wi_system_defined_todo, wi_system_defined_done, wi_system_defined_duplicated, wi_system_defined_wont_do, - wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom, wi_no_status + wi_system_defined_todo, wi_system_defined_in_progress, wi_system_defined_done, wi_system_defined_duplicated, + wi_system_defined_wont_do, wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom, wi_no_status, + wi_converted_in_progress ) end end @@ -696,7 +748,8 @@ it 'excludes items with matching current_status or its equivalent fallback status' do is_expected.to contain_exactly( - wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom, wi_no_status + wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom, wi_no_status, + wi_system_defined_in_progress, wi_converted_in_progress ) end end @@ -704,7 +757,10 @@ context 'with custom statuses' do context 'with statuses that has system-defined mapping' do let(:statuses) do - [lifecycle.default_open_status, lifecycle.default_closed_status, lifecycle.default_duplicate_status] + [ + lifecycle.default_open_status, lifecycle.default_closed_status, + lifecycle.default_duplicate_status, converted_in_progress_status + ] end it 'excludes items with custom status and mapped system-defined items' do @@ -723,7 +779,272 @@ end end end + + describe '.order_status_asc' do + let(:result) { described_class.where(id: work_items.map(&:id)).order_status_asc } + + context 'with system-defined statuses' do + before do + lifecycle.destroy! + end + + context 'with explicitly set statuses' do + let_it_be(:work_items) do + [ + wi_system_defined_todo, wi_system_defined_in_progress, wi_system_defined_done, + wi_system_defined_wont_do, wi_system_defined_duplicated + ] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_system_defined_todo, + wi_system_defined_in_progress, + wi_system_defined_done, + wi_system_defined_wont_do, + wi_system_defined_duplicated + ]) + end + end + + context 'with explicitly and implicitly set statuses' do + let_it_be(:work_items) do + [ + wi_default_open, wi_system_defined_todo, wi_system_defined_in_progress, wi_default_closed, + wi_system_defined_done, wi_system_defined_wont_do, wi_default_duplicated, wi_system_defined_duplicated + ] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_system_defined_todo, + wi_default_open, + wi_system_defined_in_progress, + wi_system_defined_done, + wi_default_closed, + wi_system_defined_wont_do, + wi_system_defined_duplicated, + wi_default_duplicated + ]) + end + end + end + + context 'with custom statuses' do + context 'with explicitly set statuses' do + let_it_be(:work_items) do + [wi_custom, wi_custom_todo, wi_converted_in_progress, wi_custom_done, wi_custom_duplicated] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_custom, + wi_custom_todo, + wi_converted_in_progress, + wi_custom_done, + wi_custom_duplicated + ]) + end + end + + context 'with explicitly and implicitly set statuses' do + let(:work_items) do + [ + wi_custom, wi_custom_todo, wi_converted_in_progress, wi_custom_done, wi_custom_duplicated, + wi_default_open, wi_default_closed, wi_default_duplicated + ] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_custom, + wi_custom_todo, + wi_default_open, + wi_converted_in_progress, + wi_custom_done, + wi_default_closed, + wi_custom_duplicated, + wi_default_duplicated + ]) + end + end + + context 'with work items from multiple namespaces' do + let(:work_items) do + [wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom_todo_2, wi_custom_done_2] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_custom_todo_2, + wi_custom_todo, + wi_custom_done_2, + wi_custom_done, + wi_custom_duplicated + ]) + end + end + end + + context 'with system-defined and custom statuses from multiple namespaces' do + let(:work_items) do + [ + wi_custom_todo, wi_converted_in_progress, wi_custom_done, + wi_system_defined_todo_2, wi_system_defined_done_2 + ] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_system_defined_todo_2, + wi_custom_todo, + wi_converted_in_progress, + wi_system_defined_done_2, + wi_custom_done + ]) + end + end + end + + describe '.order_status_desc' do + let(:result) { described_class.where(id: work_items.map(&:id)).order_status_desc } + + context 'with system-defined statuses' do + before do + lifecycle.destroy! + end + + context 'with explicitly set statuses' do + let(:work_items) do + [ + wi_system_defined_todo, wi_system_defined_in_progress, wi_system_defined_done, + wi_system_defined_wont_do, wi_system_defined_duplicated + ] + end + + it 'sorts work items by status desc' do + expect(result).to eq([ + wi_system_defined_wont_do, + wi_system_defined_duplicated, + wi_system_defined_done, + wi_system_defined_in_progress, + wi_system_defined_todo + ]) + end + end + + context 'with explicitly and implicitly set statuses' do + let(:work_items) do + [ + wi_default_open, wi_system_defined_todo, wi_system_defined_in_progress, wi_default_closed, + wi_system_defined_done, wi_system_defined_wont_do, wi_default_duplicated, wi_system_defined_duplicated + ] + end + + it 'sorts work items by status desc' do + expect(result).to eq([ + wi_system_defined_wont_do, + wi_system_defined_duplicated, + wi_default_duplicated, + wi_system_defined_done, + wi_default_closed, + wi_system_defined_in_progress, + wi_system_defined_todo, + wi_default_open + ]) + end + end + + context 'with work items from multiple namespaces' do + let(:work_items) do + [wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom_todo_2, wi_custom_done_2] + end + + it 'sorts work items by status desc' do + expect(result).to eq([ + wi_custom_duplicated, + wi_custom_done_2, + wi_custom_done, + wi_custom_todo_2, + wi_custom_todo + ]) + end + end + end + + context 'with custom statuses' do + context 'with explicitly set statuses' do + let(:work_items) do + [wi_custom, wi_custom_todo, wi_converted_in_progress, wi_custom_done, wi_custom_duplicated] + end + + it 'sorts work items by status desc' do + expect(result).to eq([ + wi_custom_duplicated, + wi_custom_done, + wi_converted_in_progress, + wi_custom, + wi_custom_todo + ]) + end + end + + context 'with explicitly and implicitly set statuses' do + let(:work_items) do + [ + wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_default_open, + wi_default_closed, wi_default_duplicated, wi_converted_in_progress + ] + end + + it 'sorts work items by status desc' do + expect(result).to eq([ + wi_custom_duplicated, + wi_default_duplicated, + wi_custom_done, + wi_default_closed, + wi_converted_in_progress, + wi_custom, + wi_custom_todo, + wi_default_open + ]) + end + end + + context 'with work items from multiple namespaces' do + let(:work_items) do + [wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom_todo_2, wi_custom_done_2] + end + + it 'sorts work items by status desc' do + expect(result).to eq([ + wi_custom_duplicated, + wi_custom_done_2, + wi_custom_done, + wi_custom_todo_2, + wi_custom_todo + ]) + end + end + end + + context 'with system-defined and custom statuses from multiple namespaces' do + let(:work_items) do + [wi_custom_todo, wi_custom_done, wi_system_defined_todo_2, wi_system_defined_done_2] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_system_defined_done_2, + wi_custom_done, + wi_system_defined_todo_2, + wi_custom_todo + ]) + end + end + end end + # rubocop:enable RSpec/MultipleMemoizedHelpers describe 'versioned descriptions' do it_behaves_like 'versioned description' diff --git a/ee/spec/models/work_items/statuses/system_defined/status_spec.rb b/ee/spec/models/work_items/statuses/system_defined/status_spec.rb index fb0dd6d959cb68ebe56c641eda7d1a4b3c2a7a17..5c46ce955de3c8d5d21c49d21945253ac0eec189 100644 --- a/ee/spec/models/work_items/statuses/system_defined/status_spec.rb +++ b/ee/spec/models/work_items/statuses/system_defined/status_spec.rb @@ -75,6 +75,20 @@ end end + describe '.sort_order_by_id' do + it 'returns a hash mapping status IDs to their category values' do + expected_mapping = { + 1 => 2, + 2 => 3, + 3 => 4, + 4 => 5, + 5 => 5 + } + + expect(described_class.sort_order_by_id).to eq(expected_mapping) + end + end + describe '#allowed_for_work_item?' do let(:work_item) { build_stubbed(:work_item, :task) } diff --git a/ee/spec/requests/api/graphql/project/work_items_spec.rb b/ee/spec/requests/api/graphql/project/work_items_spec.rb index 35ad10ea6d866d3292cf2748e06763b3404adc53..f1c6d8459220e261e915f2d63033bc5a92ccdbd7 100644 --- a/ee/spec/requests/api/graphql/project/work_items_spec.rb +++ b/ee/spec/requests/api/graphql/project/work_items_spec.rb @@ -606,6 +606,39 @@ end end + describe 'sorting' do + let_it_be(:status_1) { build(:work_item_system_defined_status, :to_do) } + let_it_be(:status_2) { build(:work_item_system_defined_status, :in_progress) } + let_it_be(:work_item_1) { create(:work_item, project: project, system_defined_status_id: status_1.id) } + let_it_be(:work_item_2) { create(:work_item, :closed, project: project, system_defined_status_id: status_2.id) } + + let(:data_path) { [:project, :work_items] } + + def pagination_query(params) + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('workItems', params, "#{page_info} nodes { id }") + ) + end + + context 'when sorting by STATUS_ASC' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :STATUS_ASC } + let(:first_param) { 2 } + let(:all_records) { [work_item_1, work_item_2].map { |item| item.to_global_id.to_s } } + end + end + + context 'when sorting by STATUS_DESC' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :STATUS_DESC } + let(:first_param) { 2 } + let(:all_records) { [work_item_2, work_item_1].map { |item| item.to_global_id.to_s } } + end + end + end + context 'when work item epics are present' do let_it_be(:epic1) { create(:work_item, :epic, project: project) } let_it_be(:epic2) { create(:work_item, :epic, project: project) }