From 1743b1894bb250c4e88787bf659aaa2742aa4b84 Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Mon, 20 Oct 2025 16:30:22 +0200 Subject: [PATCH 1/7] Fix manual ordering on group work items list Manual ordering was not working for group work items list due to a deeper coupling between issues and projects. This commit fixes that by using the namespace on the manual ordering logic. Related to: https://gitlab.com/gitlab-org/gitlab/-/issues/555715 Changelog: fixed --- app/models/issue.rb | 4 ++-- app/services/work_items/reorder_service.rb | 8 -------- lib/gitlab/relative_positioning/mover.rb | 6 ++++++ 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 3fc57757d361ab..d907d6b21804f1 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -406,7 +406,7 @@ def next_object_by_relative_position(ignoring: nil, order: :asc) end def relative_positioning_parent_projects - project.group&.root_ancestor&.all_projects&.select(:id) || Project.id_in(project).select(:id) + namespace&.root_ancestor&.all_projects&.select(:id) || Project.id_in(project).select(:id) end def self.relative_positioning_query_base(issue) @@ -571,7 +571,7 @@ def check_repositioning_allowed! end def blocked_for_repositioning? - resource_parent.root_namespace&.issue_repositioning_disabled? + namespace.root_ancestor&.issue_repositioning_disabled? end # `from` argument can be a Namespace or Project. diff --git a/app/services/work_items/reorder_service.rb b/app/services/work_items/reorder_service.rb index 0a7d92247aadc8..d3eab94430701a 100644 --- a/app/services/work_items/reorder_service.rb +++ b/app/services/work_items/reorder_service.rb @@ -11,7 +11,6 @@ def initialize(current_user:, params:) def execute(work_item) return unauthorized_error(work_item) unless current_user.can?(:update_work_item, work_item) - return non_project_work_item_error(work_item) if work_item.project.blank? return missing_arguments_error(work_item) unless move_between_ids ::WorkItems::UpdateService.new( @@ -36,13 +35,6 @@ def success(work_item) }) end - def non_project_work_item_error(work_item) - error( - work_item, - message: 'Only project work items are supported at this moment' - ) - end - def unauthorized_error(work_item) error( work_item, diff --git a/lib/gitlab/relative_positioning/mover.rb b/lib/gitlab/relative_positioning/mover.rb index 9d891bfbe3b733..1eeba0d1900704 100644 --- a/lib/gitlab/relative_positioning/mover.rb +++ b/lib/gitlab/relative_positioning/mover.rb @@ -32,6 +32,12 @@ def move(object, first, last) focus = context(object) range = RelativePositioning.range(lhs, rhs) + Kassio.log( + object: object, + focus: focus, + range: range + ) + if range.cover?(focus) # Moving a object already within a range is a no-op elsif range.open_on_left? -- GitLab From 3405e44a92f45404ed958330f2d240102300f69b Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Tue, 21 Oct 2025 09:20:45 +0200 Subject: [PATCH 2/7] Remove local code --- lib/gitlab/relative_positioning/mover.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/gitlab/relative_positioning/mover.rb b/lib/gitlab/relative_positioning/mover.rb index 1eeba0d1900704..9d891bfbe3b733 100644 --- a/lib/gitlab/relative_positioning/mover.rb +++ b/lib/gitlab/relative_positioning/mover.rb @@ -32,12 +32,6 @@ def move(object, first, last) focus = context(object) range = RelativePositioning.range(lhs, rhs) - Kassio.log( - object: object, - focus: focus, - range: range - ) - if range.cover?(focus) # Moving a object already within a range is a no-op elsif range.open_on_left? -- GitLab From afa33ed2878f5610e0e24ed18ceec07b98e1d899 Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Tue, 21 Oct 2025 11:26:11 +0200 Subject: [PATCH 3/7] Fix test --- ee/spec/services/ee/work_items/reorder_service_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ee/spec/services/ee/work_items/reorder_service_spec.rb b/ee/spec/services/ee/work_items/reorder_service_spec.rb index 069b2b89ef224f..4b6146284cddc3 100644 --- a/ee/spec/services/ee/work_items/reorder_service_spec.rb +++ b/ee/spec/services/ee/work_items/reorder_service_spec.rb @@ -34,10 +34,8 @@ it 'does not update the position' do expect { service_result } - .not_to change { non_project_work_item.relative_position } - - expect(service_result[:errors]) - .to eq(["Only project work items are supported at this moment"]) + .to change { non_project_work_item.relative_position } + .to be_between(item1.relative_position, item2.relative_position) end end end -- GitLab From 646de2136ed0a30da9f9fbe21b449b20d1eaca21 Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Wed, 22 Oct 2025 10:37:50 +0200 Subject: [PATCH 4/7] Fix test description --- ee/spec/services/ee/work_items/reorder_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/ee/work_items/reorder_service_spec.rb b/ee/spec/services/ee/work_items/reorder_service_spec.rb index 4b6146284cddc3..6b11821bd4efa3 100644 --- a/ee/spec/services/ee/work_items/reorder_service_spec.rb +++ b/ee/spec/services/ee/work_items/reorder_service_spec.rb @@ -32,7 +32,7 @@ context 'with a non-project level work item' do let(:params) { { move_after_id: item1.id, move_before_id: item2.id } } - it 'does not update the position' do + it 'reorders correctly' do expect { service_result } .to change { non_project_work_item.relative_position } .to be_between(item1.relative_position, item2.relative_position) -- GitLab From 5e331f933cb29f37ebe5f3ee402e70214e08f5eb Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Wed, 22 Oct 2025 12:21:29 +0200 Subject: [PATCH 5/7] Ensure namespace is present before handling the move on creation Namespace is set on a before_valid callback, so we need to ensure we validate the issue before handling the move. --- app/services/issues/create_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index bc965cf1ec9d0b..3d0aee0518747e 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -29,7 +29,7 @@ def execute(skip_system_notes: false) # it can be set also from quick actions [:issue_type, :work_item_type, :work_item_type_id].each { |attribute| params.delete(attribute) } - handle_move_between_ids(@issue) + handle_move_between_ids(@issue) if @issue.valid? @add_related_issue ||= params.delete(:add_related_issue) filter_resolve_discussion_params -- GitLab From e5d430d8c60f937fbda329c4e3e96c89459f28e2 Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Thu, 23 Oct 2025 18:53:35 +0200 Subject: [PATCH 6/7] Try to fix tests --- app/models/issue.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index d907d6b21804f1..77e3f026c5a1c1 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -406,7 +406,11 @@ def next_object_by_relative_position(ignoring: nil, order: :asc) end def relative_positioning_parent_projects - namespace&.root_ancestor&.all_projects&.select(:id) || Project.id_in(project).select(:id) + if namespace.parent.user_namespace? + Project.id_in(namespace.project).select(:id) + else + namespace&.root_ancestor&.all_projects&.select(:id) + end end def self.relative_positioning_query_base(issue) -- GitLab From 9fc9207f14ce97174301df66f87afaced7dcb56f Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Thu, 23 Oct 2025 19:01:25 +0200 Subject: [PATCH 7/7] Fix some checks --- app/models/issue.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 77e3f026c5a1c1..e2ea54f8cfd83e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -406,10 +406,10 @@ def next_object_by_relative_position(ignoring: nil, order: :asc) end def relative_positioning_parent_projects - if namespace.parent.user_namespace? + if namespace.parent&.user_namespace? Project.id_in(namespace.project).select(:id) else - namespace&.root_ancestor&.all_projects&.select(:id) + namespace.root_ancestor&.all_projects&.select(:id) end end -- GitLab