From 2ff5a858e80743437b50d59871fd7ddf287a41d1 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Thu, 31 Jul 2025 16:37:51 +1000 Subject: [PATCH 01/14] Sort work items by status Allow sorting work items by status in ascending and descending order. Changelog: added EE: true --- .../non_stable_cursor_sort_options.rb | 3 +- .../models/concerns/work_items/has_status.rb | 60 ++++++++ ee/app/models/ee/issue.rb | 2 + ee/spec/models/work_item_spec.rb | 144 ++++++++++++++++++ 4 files changed, 208 insertions(+), 1 deletion(-) 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 d5234e4679f135..b4075b6c6f372e 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 0e78ba3370d34c..6d89a7d611dd1c 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) } + # rubocop: disable Layout/LineLength -- temp + scope :with_status_ordering, -> { + categories = WorkItems::Statuses::SharedConstants::CATEGORIES + category_cases = categories.map { |_category, order| "WHEN #{order} THEN #{order}" }.join(' ') + + system_status_cases = WorkItems::Statuses::SystemDefined::Status::ITEMS.map do |item| + category_order = categories[item[:category]] + # Combine category (primary) with ID (secondary) to ensure won't do goes before duplicated status + combined_order = (category_order * 100) + item[:id] + "WHEN #{item[:id]} THEN #{combined_order}" + end.join(' ') + + lifecycle = WorkItems::Statuses::SystemDefined::Lifecycle.all.first + default_open_status = WorkItems::Statuses::SystemDefined::Status.find(lifecycle.default_open_status_id) + default_duplicate_status = WorkItems::Statuses::SystemDefined::Status.find(lifecycle.default_duplicate_status_id) + default_closed_status = WorkItems::Statuses::SystemDefined::Status.find(lifecycle.default_closed_status_id) + + default_open_category = (categories[default_open_status.category] * 100) + default_open_status.id + default_duplicate_category = (categories[default_duplicate_status.category] * 100) + default_duplicate_status.id + default_closed_category = (categories[default_closed_status.category] * 100) + default_closed_status.id + + opened_state_value = Issue.available_states[:opened] + closed_state_value = Issue.available_states[:closed] + + status_order_sql = <<-SQL.squish + CASE + WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN + CASE work_item_custom_statuses.category #{category_cases} END + WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN + COALESCE( + (SELECT CASE converted_cs.category #{category_cases} END + FROM work_item_custom_statuses converted_cs + WHERE converted_cs.converted_from_system_defined_status_identifier = work_item_current_statuses.system_defined_status_id + AND converted_cs.namespace_id = #{table_name}.namespace_id + LIMIT 1), + CASE work_item_current_statuses.system_defined_status_id #{system_status_cases} END + ) + ELSE + CASE + WHEN #{table_name}.state_id = #{opened_state_value} THEN #{default_open_category} + WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NOT NULL THEN #{default_duplicate_category} + WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NULL THEN #{default_closed_category} + ELSE #{default_open_category} + END + END as status_order + SQL + + left_joins(current_status: :custom_status) + .select("#{table_name}.*", status_order_sql) + } + # rubocop: enable Layout/LineLength + + scope :order_status_asc, -> { + with_status_ordering.reorder('status_order ASC NULLS LAST, id DESC') + } + + scope :order_status_desc, -> { + with_status_ordering.reorder('status_order DESC NULLS FIRST, id DESC') + } + 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 34b915b8b0df93..99edfda639a048 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/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index c0e12187122a49..8077dd705fd363 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -521,6 +521,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) } @@ -723,6 +724,149 @@ 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 + # 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 + + context 'with explicitly set statuses' do + let_it_be(:work_items) do + [ + wi_system_defined_todo, 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_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_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_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_custom_done, wi_custom_duplicated + ] + end + + it 'sorts work items by status asc' do + expect(result).to eq([ + wi_custom, + wi_custom_todo, + wi_custom_done, + wi_custom_duplicated + ]) + end + 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 + # 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 + + context 'with explicitly set statuses' do + let_it_be(:work_items) do + [ + wi_system_defined_todo, 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_duplicated, + wi_system_defined_wont_do, + wi_system_defined_done, + wi_system_defined_todo + ]) + end + end + + context 'with explicitly and implicitly set statuses' do + let_it_be(:work_items) do + [ + wi_default_open, wi_system_defined_todo, 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_duplicated, + wi_default_duplicated, + wi_system_defined_wont_do, + wi_system_defined_done, + wi_default_closed, + wi_system_defined_todo, + wi_default_open + ]) + 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_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_custom, + wi_custom_todo + ]) + end + end + + # context 'with explicitly and implicitly set statuses' do + # let_it_be(:work_items) do + # [ + # wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_default_open, + # wi_default_closed, wi_default_duplicated + # ] + # end + # end + end + end end describe 'versioned descriptions' do -- GitLab From 8d0e28109e7bd595539f8abfbbdd6292969dfce4 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Tue, 12 Aug 2025 10:57:00 +1000 Subject: [PATCH 02/14] Optimise status sorting with LEFT JOIN Replace subquery with LEFT JOIN for custom status conversion lookup to improve query performance in status ordering. --- ee/app/models/concerns/work_items/has_status.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index 6d89a7d611dd1c..61252d3764c1d4 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -80,9 +80,9 @@ module HasStatus end.join(' ') lifecycle = WorkItems::Statuses::SystemDefined::Lifecycle.all.first - default_open_status = WorkItems::Statuses::SystemDefined::Status.find(lifecycle.default_open_status_id) - default_duplicate_status = WorkItems::Statuses::SystemDefined::Status.find(lifecycle.default_duplicate_status_id) - default_closed_status = WorkItems::Statuses::SystemDefined::Status.find(lifecycle.default_closed_status_id) + default_open_status = lifecycle.default_open_status + default_duplicate_status = lifecycle.default_duplicate_status + default_closed_status = lifecycle.default_closed_status default_open_category = (categories[default_open_status.category] * 100) + default_open_status.id default_duplicate_category = (categories[default_duplicate_status.category] * 100) + default_duplicate_status.id @@ -97,11 +97,7 @@ module HasStatus CASE work_item_custom_statuses.category #{category_cases} END WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN COALESCE( - (SELECT CASE converted_cs.category #{category_cases} END - FROM work_item_custom_statuses converted_cs - WHERE converted_cs.converted_from_system_defined_status_identifier = work_item_current_statuses.system_defined_status_id - AND converted_cs.namespace_id = #{table_name}.namespace_id - LIMIT 1), + CASE converted_statuses.category #{category_cases} END, CASE work_item_current_statuses.system_defined_status_id #{system_status_cases} END ) ELSE @@ -115,6 +111,9 @@ module HasStatus SQL left_joins(current_status: :custom_status) + .joins("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 = #{table_name}.namespace_id") .select("#{table_name}.*", status_order_sql) } # rubocop: enable Layout/LineLength -- GitLab From 957a2a661e426cabe7e0e06ac8d2f5701ea4f465 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Tue, 12 Aug 2025 14:38:29 +1000 Subject: [PATCH 03/14] Order by status with priority-based sorting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add priority tiers (custom, system-defined, default) and increase base multiplier to 1000 for proper category separation. Ensures correct ASC/DESC ordering: Custom → System-defined → Default within each category, with support for status conversion scenarios. --- .../models/concerns/work_items/has_status.rb | 42 +++++---- ee/spec/models/work_item_spec.rb | 87 ++++++++++++++----- 2 files changed, 92 insertions(+), 37 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index 61252d3764c1d4..a9b63372c83748 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -67,15 +67,18 @@ module HasStatus merge(items_to_exclude.invert_where) } - # rubocop: disable Layout/LineLength -- temp scope :with_status_ordering, -> { + base_multiplier = 1000 + custom_priority = 100 + system_priority = 200 + default_priority = 300 + categories = WorkItems::Statuses::SharedConstants::CATEGORIES category_cases = categories.map { |_category, order| "WHEN #{order} THEN #{order}" }.join(' ') system_status_cases = WorkItems::Statuses::SystemDefined::Status::ITEMS.map do |item| category_order = categories[item[:category]] - # Combine category (primary) with ID (secondary) to ensure won't do goes before duplicated status - combined_order = (category_order * 100) + item[:id] + combined_order = (category_order * base_multiplier) + item[:id] + system_priority "WHEN #{item[:id]} THEN #{combined_order}" end.join(' ') @@ -84,9 +87,12 @@ module HasStatus default_duplicate_status = lifecycle.default_duplicate_status default_closed_status = lifecycle.default_closed_status - default_open_category = (categories[default_open_status.category] * 100) + default_open_status.id - default_duplicate_category = (categories[default_duplicate_status.category] * 100) + default_duplicate_status.id - default_closed_category = (categories[default_closed_status.category] * 100) + default_closed_status.id + default_open_category = (categories[default_open_status.category] * base_multiplier) + + default_open_status.id + default_priority + default_duplicate_category = (categories[default_duplicate_status.category] * base_multiplier) + + default_duplicate_status.id + default_priority + default_closed_category = (categories[default_closed_status.category] * base_multiplier) + + default_closed_status.id + default_priority opened_state_value = Issue.available_states[:opened] closed_state_value = Issue.available_states[:closed] @@ -94,29 +100,35 @@ module HasStatus status_order_sql = <<-SQL.squish CASE WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN - CASE work_item_custom_statuses.category #{category_cases} END + (CASE work_item_custom_statuses.category #{category_cases} END) * #{base_multiplier} + #{custom_priority} WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN COALESCE( - CASE converted_statuses.category #{category_cases} END, - CASE work_item_current_statuses.system_defined_status_id #{system_status_cases} END + (CASE converted_statuses.category #{category_cases} END) * #{base_multiplier} + #{system_priority}, + (CASE work_item_current_statuses.system_defined_status_id #{system_status_cases} END) ) ELSE CASE WHEN #{table_name}.state_id = #{opened_state_value} THEN #{default_open_category} - WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NOT NULL THEN #{default_duplicate_category} - WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NULL THEN #{default_closed_category} + WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NOT NULL + THEN #{default_duplicate_category} + WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NULL + THEN #{default_closed_category} ELSE #{default_open_category} END END as status_order SQL + converted_statuses_join = <<-SQL.squish + 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 = #{table_name}.namespace_id + SQL + left_joins(current_status: :custom_status) - .joins("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 = #{table_name}.namespace_id") + .joins(converted_statuses_join) .select("#{table_name}.*", status_order_sql) } - # rubocop: enable Layout/LineLength scope :order_status_asc, -> { with_status_ordering.reorder('status_order ASC NULLS LAST, id DESC') diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index 8077dd705fd363..523a78a876409c 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -521,7 +521,6 @@ 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) } @@ -530,6 +529,11 @@ 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: build(:work_item_system_defined_status, :in_progress).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 @@ -729,20 +733,22 @@ let(:result) { described_class.where(id: work_items.map(&:id)).order_status_asc } context 'with system-defined statuses' do - # 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 + before do + lifecycle.destroy! + end context 'with explicitly set statuses' do let_it_be(:work_items) do [ - wi_system_defined_todo, wi_system_defined_done, wi_system_defined_wont_do, wi_system_defined_duplicated + 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 @@ -753,8 +759,8 @@ context 'with explicitly and implicitly set statuses' do let_it_be(:work_items) do [ - wi_default_open, wi_system_defined_todo, wi_default_closed, wi_system_defined_done, - wi_system_defined_wont_do, wi_default_duplicated, wi_system_defined_duplicated + 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 @@ -762,6 +768,7 @@ 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, @@ -789,6 +796,27 @@ ]) end end + + context 'with explicitly and implicitly set statuses' do + let_it_be(:work_items) do + [ + wi_custom, wi_custom_todo, 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_custom_done, + wi_default_closed, + wi_custom_duplicated, + wi_default_duplicated + ]) + end + end end end @@ -796,14 +824,15 @@ let(:result) { described_class.where(id: work_items.map(&:id)).order_status_desc } context 'with system-defined statuses' do - # 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 + before do + lifecycle.destroy! + end context 'with explicitly set statuses' do let_it_be(:work_items) do [ - wi_system_defined_todo, wi_system_defined_done, wi_system_defined_wont_do, wi_system_defined_duplicated + wi_system_defined_todo, wi_system_defined_in_progress, wi_system_defined_done, + wi_system_defined_wont_do, wi_system_defined_duplicated ] end @@ -812,6 +841,7 @@ wi_system_defined_duplicated, wi_system_defined_wont_do, wi_system_defined_done, + wi_system_defined_in_progress, wi_system_defined_todo ]) end @@ -820,18 +850,19 @@ context 'with explicitly and implicitly set statuses' do let_it_be(:work_items) do [ - wi_default_open, wi_system_defined_todo, wi_default_closed, wi_system_defined_done, - wi_system_defined_wont_do, wi_default_duplicated, wi_system_defined_duplicated + 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 + it 'sorts work items by status desc' do expect(result).to eq([ wi_system_defined_duplicated, wi_default_duplicated, wi_system_defined_wont_do, wi_system_defined_done, wi_default_closed, + wi_system_defined_in_progress, wi_system_defined_todo, wi_default_open ]) @@ -857,14 +888,26 @@ end end - # context 'with explicitly and implicitly set statuses' do - # let_it_be(:work_items) do - # [ - # wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_default_open, - # wi_default_closed, wi_default_duplicated - # ] - # end - # end + context 'with explicitly and implicitly set statuses' do + let_it_be(:work_items) do + [ + wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_default_open, + wi_default_closed, wi_default_duplicated + ] + 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_custom_todo, + wi_custom, + wi_default_open + ]) + end + end end end end -- GitLab From 5030b301808e6ea73e90a00f7d1e24998f77df39 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Tue, 9 Sep 2025 12:34:58 +1000 Subject: [PATCH 04/14] Remove priority offsets from status sorting Simplify the status sorting implementation by removing the priority offsets for custom, system and default statuses. Rely solely on category-based sorting for all status types, and fall back to ID-based secondary sorting within each category. --- .../models/concerns/work_items/has_status.rb | 22 ++++++------------- .../statuses/system_defined/status.rb | 8 +++++++ ee/spec/models/work_item_spec.rb | 18 +++++++-------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index a9b63372c83748..abcdc49ea504d9 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -69,17 +69,12 @@ module HasStatus scope :with_status_ordering, -> { base_multiplier = 1000 - custom_priority = 100 - system_priority = 200 - default_priority = 300 categories = WorkItems::Statuses::SharedConstants::CATEGORIES category_cases = categories.map { |_category, order| "WHEN #{order} THEN #{order}" }.join(' ') - system_status_cases = WorkItems::Statuses::SystemDefined::Status::ITEMS.map do |item| - category_order = categories[item[:category]] - combined_order = (category_order * base_multiplier) + item[:id] + system_priority - "WHEN #{item[:id]} THEN #{combined_order}" + system_status_cases = WorkItems::Statuses::SystemDefined::Status::SORT_ORDER.map do |id, sort_order| + "WHEN #{id} THEN #{sort_order}" end.join(' ') lifecycle = WorkItems::Statuses::SystemDefined::Lifecycle.all.first @@ -87,12 +82,9 @@ module HasStatus default_duplicate_status = lifecycle.default_duplicate_status default_closed_status = lifecycle.default_closed_status - default_open_category = (categories[default_open_status.category] * base_multiplier) + - default_open_status.id + default_priority - default_duplicate_category = (categories[default_duplicate_status.category] * base_multiplier) + - default_duplicate_status.id + default_priority - default_closed_category = (categories[default_closed_status.category] * base_multiplier) + - default_closed_status.id + default_priority + default_open_category = (categories[default_open_status.category] * base_multiplier) + default_duplicate_category = (categories[default_duplicate_status.category] * base_multiplier) + default_closed_category = (categories[default_closed_status.category] * base_multiplier) opened_state_value = Issue.available_states[:opened] closed_state_value = Issue.available_states[:closed] @@ -100,10 +92,10 @@ module HasStatus status_order_sql = <<-SQL.squish CASE WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN - (CASE work_item_custom_statuses.category #{category_cases} END) * #{base_multiplier} + #{custom_priority} + (CASE work_item_custom_statuses.category #{category_cases} END) * #{base_multiplier} WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN COALESCE( - (CASE converted_statuses.category #{category_cases} END) * #{base_multiplier} + #{system_priority}, + (CASE converted_statuses.category #{category_cases} END) * #{base_multiplier}, (CASE work_item_current_statuses.system_defined_status_id #{system_status_cases} END) ) ELSE 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 66069ae3143b33..71c32c693e4e49 100644 --- a/ee/app/models/work_items/statuses/system_defined/status.rb +++ b/ee/app/models/work_items/statuses/system_defined/status.rb @@ -43,6 +43,14 @@ class Status } ].freeze + SORT_ORDER = { + 1 => 1010, + 2 => 2010, + 3 => 3010, + 4 => 4010, + 5 => 4020 + }.freeze + attribute :name, :string attribute :color, :string attribute :category diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index 523a78a876409c..5478aa995124c7 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -685,8 +685,8 @@ 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 ) end end @@ -701,7 +701,7 @@ 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 ) end end @@ -714,7 +714,7 @@ it 'excludes items with custom status and mapped system-defined items' do is_expected.to contain_exactly( - wi_system_defined_wont_do, wi_custom, wi_no_status + wi_system_defined_wont_do, wi_custom, wi_no_status, wi_system_defined_in_progress ) end end @@ -857,14 +857,14 @@ it 'sorts work items by status desc' do expect(result).to eq([ - wi_system_defined_duplicated, wi_default_duplicated, + wi_system_defined_duplicated, wi_system_defined_wont_do, - wi_system_defined_done, wi_default_closed, + wi_system_defined_done, wi_system_defined_in_progress, - wi_system_defined_todo, - wi_default_open + wi_default_open, + wi_system_defined_todo ]) end end @@ -902,8 +902,8 @@ wi_default_duplicated, wi_custom_done, wi_default_closed, - wi_custom_todo, wi_custom, + wi_custom_todo, wi_default_open ]) end -- GitLab From 17aa0c673f5be7bf51b9b4d2123f435c11650648 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Tue, 9 Sep 2025 13:25:13 +1000 Subject: [PATCH 05/14] Test status sorting across multiple namespaces Add test coverage to verify that work items from different namespaces are properly sorted by status category. --- ee/spec/models/work_item_spec.rb | 76 +++++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index 5478aa995124c7..d823e5cce2f3bc 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) } @@ -552,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 @@ -580,6 +595,14 @@ let_it_be(:wi_custom) { create(:work_item, project: project, custom_status_id: custom_status.id) } + 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) } @@ -782,9 +805,7 @@ context 'with custom statuses' do context 'with explicitly set statuses' do let_it_be(:work_items) do - [ - wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated - ] + [wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated] end it 'sorts work items by status asc' do @@ -798,7 +819,7 @@ end context 'with explicitly and implicitly set statuses' do - let_it_be(:work_items) 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 @@ -817,6 +838,22 @@ ]) 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 end @@ -829,7 +866,7 @@ end context 'with explicitly set statuses' do - let_it_be(:work_items) 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 @@ -848,7 +885,7 @@ end context 'with explicitly and implicitly set statuses' do - let_it_be(:work_items) 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 @@ -872,10 +909,8 @@ context 'with custom statuses' do context 'with explicitly set statuses' do - let_it_be(:work_items) do - [ - wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated - ] + let(:work_items) do + [wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated] end it 'sorts work items by status desc' do @@ -889,7 +924,7 @@ end context 'with explicitly and implicitly set statuses' do - let_it_be(:work_items) 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 @@ -908,9 +943,26 @@ ]) 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_duplicated, + wi_custom_done_2, + wi_custom_done, + wi_custom_todo_2, + wi_custom_todo + ]) + end + end end end end + # rubocop:enable RSpec/MultipleMemoizedHelpers describe 'versioned descriptions' do it_behaves_like 'versioned description' -- GitLab From cdf48719240161478782116b3ea97a0169a2eaf4 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Tue, 9 Sep 2025 14:29:50 +1000 Subject: [PATCH 06/14] Add request specs for sorting work items by status Improve test coverage by adding request specs for testing sorting work items by status in ascending and descending order. --- ee/spec/models/work_item_spec.rb | 2 +- .../api/graphql/project/work_items_spec.rb | 30 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index d823e5cce2f3bc..1145fc640dac9d 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -949,7 +949,7 @@ [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 + it 'sorts work items by status desc' do expect(result).to eq([ wi_custom_duplicated, wi_custom_done_2, diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index dc17b0ba21ebc5..3980ccac29fc55 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -9,11 +9,14 @@ let_it_be(:label2) { create(:label, project: project) } let_it_be(:milestone1) { create(:milestone, project: project, due_date: 5.days.ago) } let_it_be(:milestone2) { create(:milestone, project: project, due_date: 3.days.from_now) } + let_it_be(:status1) { build(:work_item_system_defined_status, :to_do) } + let_it_be(:status2) { build(:work_item_system_defined_status, :in_progress) } let_it_be_with_reload(:item1) do create( :work_item, project: project, discussion_locked: true, - title: 'item1', milestone: milestone2, labels: [label1] + title: 'item1', milestone: milestone2, labels: [label1], + system_defined_status_id: status1.id ) end @@ -25,11 +28,16 @@ last_edited_by: current_user, last_edited_at: 1.day.ago, labels: [label2], - milestone: milestone1 + milestone: milestone1, + system_defined_status_id: status2.id ) end - let_it_be_with_reload(:confidential_item) { create(:work_item, confidential: true, project: project, title: 'item3') } + let_it_be_with_reload(:confidential_item) do + create(:work_item, confidential: true, project: project, + title: 'item3', system_defined_status_id: status1.id) + end + let_it_be(:other_item) { create(:work_item) } let(:items_data) { graphql_data['project']['workItems']['nodes'] } @@ -391,6 +399,22 @@ def pagination_query(params) let(:all_records) { [item2, item1, confidential_item].map { |item| item.to_global_id.to_s } } end 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) { [confidential_item, item1, item2].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) { [item2, confidential_item, item1].map { |item| item.to_global_id.to_s } } + end + end end context 'when fetching work item notifications widget' do -- GitLab From e17737b13b88f60f00b0c63a04fbc4a5e920b4e5 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Mon, 15 Sep 2025 15:02:57 +1000 Subject: [PATCH 07/14] Simplyfy sorting work items by status category Simplify status sorting by using category values (1-5) directly as sort keys instead of the previous 1000x multiplier approach. Work items with the same status category now sort by ID as a tiebreaker. --- .../models/concerns/work_items/has_status.rb | 25 +++++++++---------- .../statuses/system_defined/status.rb | 10 ++++---- ee/spec/models/work_item_spec.rb | 12 ++++----- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index abcdc49ea504d9..b6d6faf3c7012b 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -68,12 +68,11 @@ module HasStatus } scope :with_status_ordering, -> { - base_multiplier = 1000 - categories = WorkItems::Statuses::SharedConstants::CATEGORIES category_cases = categories.map { |_category, order| "WHEN #{order} THEN #{order}" }.join(' ') - system_status_cases = WorkItems::Statuses::SystemDefined::Status::SORT_ORDER.map do |id, sort_order| + system_defined_sort_orders = WorkItems::Statuses::SystemDefined::Status::SORT_ORDER + system_defined_status_cases = system_defined_sort_orders.map do |id, sort_order| "WHEN #{id} THEN #{sort_order}" end.join(' ') @@ -82,9 +81,9 @@ module HasStatus default_duplicate_status = lifecycle.default_duplicate_status default_closed_status = lifecycle.default_closed_status - default_open_category = (categories[default_open_status.category] * base_multiplier) - default_duplicate_category = (categories[default_duplicate_status.category] * base_multiplier) - default_closed_category = (categories[default_closed_status.category] * base_multiplier) + 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] @@ -92,20 +91,20 @@ module HasStatus status_order_sql = <<-SQL.squish CASE WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN - (CASE work_item_custom_statuses.category #{category_cases} END) * #{base_multiplier} + CASE work_item_custom_statuses.category #{category_cases} END WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN COALESCE( - (CASE converted_statuses.category #{category_cases} END) * #{base_multiplier}, - (CASE work_item_current_statuses.system_defined_status_id #{system_status_cases} END) + CASE converted_statuses.category #{category_cases} END, + 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_category} + 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_category} + THEN #{default_duplicate_sort_order} WHEN #{table_name}.state_id = #{closed_state_value} AND #{table_name}.duplicated_to_id IS NULL - THEN #{default_closed_category} - ELSE #{default_open_category} + THEN #{default_closed_sort_order} + ELSE #{default_open_sort_order} END END as status_order SQL 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 71c32c693e4e49..d28629195386e3 100644 --- a/ee/app/models/work_items/statuses/system_defined/status.rb +++ b/ee/app/models/work_items/statuses/system_defined/status.rb @@ -44,11 +44,11 @@ class Status ].freeze SORT_ORDER = { - 1 => 1010, - 2 => 2010, - 3 => 3010, - 4 => 4010, - 5 => 4020 + 1 => 2, + 2 => 3, + 3 => 4, + 4 => 5, + 5 => 5 }.freeze attribute :name, :string diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index 1145fc640dac9d..75ff82bbec71ba 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -875,8 +875,8 @@ it 'sorts work items by status desc' do expect(result).to eq([ - wi_system_defined_duplicated, wi_system_defined_wont_do, + wi_system_defined_duplicated, wi_system_defined_done, wi_system_defined_in_progress, wi_system_defined_todo @@ -894,14 +894,14 @@ it 'sorts work items by status desc' do expect(result).to eq([ - wi_default_duplicated, - wi_system_defined_duplicated, wi_system_defined_wont_do, - wi_default_closed, + wi_system_defined_duplicated, + wi_default_duplicated, wi_system_defined_done, + wi_default_closed, wi_system_defined_in_progress, - wi_default_open, - wi_system_defined_todo + wi_system_defined_todo, + wi_default_open ]) end end -- GitLab From 65560b863514ca6876aa71ed272deb569cec7f48 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Mon, 15 Sep 2025 15:23:09 +1000 Subject: [PATCH 08/14] Use category values directly for status sorting Remove redundant CASE statements and use the category column values (1-5) directly in SQL queries. This simplifies the code by eliminating the unnecessary mapping of category values to themselves. --- ee/app/models/concerns/work_items/has_status.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index b6d6faf3c7012b..e1ea4becc6f6ec 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -68,9 +68,6 @@ module HasStatus } scope :with_status_ordering, -> { - categories = WorkItems::Statuses::SharedConstants::CATEGORIES - category_cases = categories.map { |_category, order| "WHEN #{order} THEN #{order}" }.join(' ') - system_defined_sort_orders = WorkItems::Statuses::SystemDefined::Status::SORT_ORDER system_defined_status_cases = system_defined_sort_orders.map do |id, sort_order| "WHEN #{id} THEN #{sort_order}" @@ -91,10 +88,10 @@ module HasStatus status_order_sql = <<-SQL.squish CASE WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN - CASE work_item_custom_statuses.category #{category_cases} END + work_item_custom_statuses.category WHEN work_item_current_statuses.system_defined_status_id IS NOT NULL THEN COALESCE( - CASE converted_statuses.category #{category_cases} END, + converted_statuses.category, CASE work_item_current_statuses.system_defined_status_id #{system_defined_status_cases} END ) ELSE -- GitLab From b1a5b4b866a5efc0c17270e6d754986c93d5f33e Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Mon, 15 Sep 2025 15:57:35 +1000 Subject: [PATCH 09/14] Build sort orders directly from category values Build the system_defined_sort_orders hash dynamically based on each status's category value. Eliminate the dependency on the predefined SORT_ORDER constant. Ensure consistent sorting between custom and system-defined statuses. --- ee/app/models/concerns/work_items/has_status.rb | 6 +++--- .../work_items/statuses/system_defined/status.rb | 14 ++++++-------- .../statuses/system_defined/status_spec.rb | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index e1ea4becc6f6ec..8dfa26c44c28b4 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -68,9 +68,9 @@ module HasStatus } scope :with_status_ordering, -> { - system_defined_sort_orders = WorkItems::Statuses::SystemDefined::Status::SORT_ORDER - system_defined_status_cases = system_defined_sort_orders.map do |id, sort_order| - "WHEN #{id} THEN #{sort_order}" + 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 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 d28629195386e3..cc222137900578 100644 --- a/ee/app/models/work_items/statuses/system_defined/status.rb +++ b/ee/app/models/work_items/statuses/system_defined/status.rb @@ -43,14 +43,6 @@ class Status } ].freeze - SORT_ORDER = { - 1 => 2, - 2 => 3, - 3 => 4, - 4 => 5, - 5 => 5 - }.freeze - attribute :name, :string attribute :color, :string attribute :category @@ -74,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_items/statuses/system_defined/status_spec.rb b/ee/spec/models/work_items/statuses/system_defined/status_spec.rb index fb0dd6d959cb68..56c10da6e43c12 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,21 @@ 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) } -- GitLab From b71348663a0486a984221d5d9b5c8d595fc22248 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Mon, 15 Sep 2025 16:29:34 +1000 Subject: [PATCH 10/14] Improve test coverage for sorting by status Test sorting work items by status with a namespace that uses system-defined statuses and a namespace that uses custom statuses. --- ee/spec/models/work_item_spec.rb | 54 +++++++++++++++++++ .../statuses/system_defined/status_spec.rb | 1 - 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index 75ff82bbec71ba..abaf9ce8434043 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -595,6 +595,14 @@ let_it_be(:wi_custom) { create(:work_item, project: project, custom_status_id: custom_status.id) } + 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 @@ -855,6 +863,21 @@ 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_todo_2, + wi_custom_todo, + wi_system_defined_done_2, + wi_custom_done + ]) + end + end end describe '.order_status_desc' do @@ -905,6 +928,22 @@ ]) 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 @@ -960,6 +999,21 @@ 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 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 56c10da6e43c12..5c46ce955de3c8 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 @@ -83,7 +83,6 @@ 3 => 4, 4 => 5, 5 => 5 - } expect(described_class.sort_order_by_id).to eq(expected_mapping) -- GitLab From 71c14b8df136dcec0cf53714ca033c1ee0e12ff5 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Tue, 16 Sep 2025 14:42:17 +1000 Subject: [PATCH 11/14] Move specs for sorting by status to EE Move the request specs for sorting work items by status to the EE module, since the status f eature is EE-only. --- .../api/graphql/project/work_items_spec.rb | 33 +++++++++++++++++++ .../api/graphql/project/work_items_spec.rb | 30 ++--------------- 2 files changed, 36 insertions(+), 27 deletions(-) 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 35ad10ea6d866d..f1c6d8459220e2 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) } diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index 3980ccac29fc55..dc17b0ba21ebc5 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -9,14 +9,11 @@ let_it_be(:label2) { create(:label, project: project) } let_it_be(:milestone1) { create(:milestone, project: project, due_date: 5.days.ago) } let_it_be(:milestone2) { create(:milestone, project: project, due_date: 3.days.from_now) } - let_it_be(:status1) { build(:work_item_system_defined_status, :to_do) } - let_it_be(:status2) { build(:work_item_system_defined_status, :in_progress) } let_it_be_with_reload(:item1) do create( :work_item, project: project, discussion_locked: true, - title: 'item1', milestone: milestone2, labels: [label1], - system_defined_status_id: status1.id + title: 'item1', milestone: milestone2, labels: [label1] ) end @@ -28,16 +25,11 @@ last_edited_by: current_user, last_edited_at: 1.day.ago, labels: [label2], - milestone: milestone1, - system_defined_status_id: status2.id + milestone: milestone1 ) end - let_it_be_with_reload(:confidential_item) do - create(:work_item, confidential: true, project: project, - title: 'item3', system_defined_status_id: status1.id) - end - + let_it_be_with_reload(:confidential_item) { create(:work_item, confidential: true, project: project, title: 'item3') } let_it_be(:other_item) { create(:work_item) } let(:items_data) { graphql_data['project']['workItems']['nodes'] } @@ -399,22 +391,6 @@ def pagination_query(params) let(:all_records) { [item2, item1, confidential_item].map { |item| item.to_global_id.to_s } } end 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) { [confidential_item, item1, item2].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) { [item2, confidential_item, item1].map { |item| item.to_global_id.to_s } } - end - end end context 'when fetching work item notifications widget' do -- GitLab From 89b874a071c9cdecff9ce826de9e2776e7f08f7c Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Thu, 18 Sep 2025 13:41:50 +1000 Subject: [PATCH 12/14] Fix root namespace lookup for status sorting Use namespaces.traversal_ids[1] to correctly identify the root namespace ID when joining with converted custom statuses. This ensures that work items with system-defined statuses properly match with their corresponding converted custom statuses at the root namespace level. The traversal_ids array stores the hierarchy of namespace IDs from root to current namespace, with the root namespace ID at index 1. --- .../models/concerns/work_items/has_status.rb | 3 +- ee/spec/models/work_item_spec.rb | 52 ++++++++++++++----- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index 8dfa26c44c28b4..bfbc7bbdd39939 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -107,10 +107,11 @@ module HasStatus SQL 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 = #{table_name}.namespace_id + AND converted_statuses.namespace_id = namespaces.traversal_ids[1] SQL left_joins(current_status: :custom_status) diff --git a/ee/spec/models/work_item_spec.rb b/ee/spec/models/work_item_spec.rb index abaf9ce8434043..80923af938eece 100644 --- a/ee/spec/models/work_item_spec.rb +++ b/ee/spec/models/work_item_spec.rb @@ -526,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) } @@ -535,8 +536,7 @@ end let_it_be(:wi_system_defined_in_progress) do - create(:work_item, project: project, - system_defined_status_id: build(:work_item_system_defined_status, :in_progress).id) + create(:work_item, project: project, system_defined_status_id: system_defined_in_progress_status.id) end let_it_be(:wi_system_defined_done) do @@ -581,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 @@ -595,6 +599,17 @@ 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 @@ -717,7 +732,8 @@ is_expected.to contain_exactly( wi_default_open, wi_default_closed, wi_default_duplicated, 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_system_defined_wont_do, wi_custom_todo, wi_custom_done, wi_custom_duplicated, wi_custom, wi_no_status, + wi_converted_in_progress ) end end @@ -732,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_system_defined_in_progress + 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 @@ -740,12 +757,15 @@ 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 is_expected.to contain_exactly( - wi_system_defined_wont_do, wi_custom, wi_no_status, wi_system_defined_in_progress + wi_system_defined_wont_do, wi_custom, wi_no_status ) end end @@ -813,13 +833,14 @@ context 'with custom statuses' do context 'with explicitly set statuses' do let_it_be(:work_items) do - [wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated] + [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 ]) @@ -829,8 +850,8 @@ 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_custom, wi_custom_todo, wi_converted_in_progress, wi_custom_done, wi_custom_duplicated, + wi_default_open, wi_default_closed, wi_default_duplicated ] end @@ -839,6 +860,7 @@ wi_custom, wi_custom_todo, wi_default_open, + wi_converted_in_progress, wi_custom_done, wi_default_closed, wi_custom_duplicated, @@ -866,13 +888,17 @@ 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] + [ + 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 ]) @@ -949,13 +975,14 @@ context 'with custom statuses' do context 'with explicitly set statuses' do let(:work_items) do - [wi_custom, wi_custom_todo, wi_custom_done, wi_custom_duplicated] + [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 ]) @@ -966,7 +993,7 @@ 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_default_closed, wi_default_duplicated, wi_converted_in_progress ] end @@ -976,6 +1003,7 @@ wi_default_duplicated, wi_custom_done, wi_default_closed, + wi_converted_in_progress, wi_custom, wi_custom_todo, wi_default_open -- GitLab From f43e8b22aa44492e69d7786aa1ffd90c2d85b6d8 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Thu, 18 Sep 2025 14:42:38 +1000 Subject: [PATCH 13/14] Optimize status ordering with CASE in ORDER BY Avoid adding a calculated status_order column in the SELECT clause when ordering work items by status. Instead, use a CASE expression directly in the ORDER BY clause to reduce query complexity and result set size. --- .../models/concerns/work_items/has_status.rb | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/ee/app/models/concerns/work_items/has_status.rb b/ee/app/models/concerns/work_items/has_status.rb index bfbc7bbdd39939..091f9d094cefa4 100644 --- a/ee/app/models/concerns/work_items/has_status.rb +++ b/ee/app/models/concerns/work_items/has_status.rb @@ -67,7 +67,27 @@ module HasStatus merge(items_to_exclude.invert_where) } - scope :with_status_ordering, -> { + 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}" @@ -85,7 +105,7 @@ module HasStatus opened_state_value = Issue.available_states[:opened] closed_state_value = Issue.available_states[:closed] - status_order_sql = <<-SQL.squish + <<-SQL.squish CASE WHEN work_item_current_statuses.custom_status_id IS NOT NULL THEN work_item_custom_statuses.category @@ -103,29 +123,9 @@ module HasStatus THEN #{default_closed_sort_order} ELSE #{default_open_sort_order} END - END as status_order + END SQL - - 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) - .select("#{table_name}.*", status_order_sql) - } - - scope :order_status_asc, -> { - with_status_ordering.reorder('status_order ASC NULLS LAST, id DESC') - } - - scope :order_status_desc, -> { - with_status_ordering.reorder('status_order DESC NULLS FIRST, id DESC') - } + end def status_with_fallback current_status_with_fallback&.status -- GitLab From 202da3872d955fbe1d5102556fc4dc2f07fdc774 Mon Sep 17 00:00:00 2001 From: Agnes Slota Date: Thu, 18 Sep 2025 14:54:19 +1000 Subject: [PATCH 14/14] Document category ordering dependency Add a comment to explaining that the numeric values in the CATEGORIES hash are used for work item status sorting. --- ee/app/models/work_items/statuses/shared_constants.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ee/app/models/work_items/statuses/shared_constants.rb b/ee/app/models/work_items/statuses/shared_constants.rb index d901bdf27537dd..26b1b1e1758b27 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, -- GitLab