diff --git a/ee/app/services/ee/work_items/parent_links/reorder_service.rb b/ee/app/services/ee/work_items/parent_links/reorder_service.rb
index c1a4aef8e187f7e596673e8c2915f50d73bba61b..4d51171bd3efc7fec46c3cff9a1c8f37d8ce5e6c 100644
--- a/ee/app/services/ee/work_items/parent_links/reorder_service.rb
+++ b/ee/app/services/ee/work_items/parent_links/reorder_service.rb
@@ -17,12 +17,14 @@ def execute
override :move_link
def move_link(link, adjacent_work_item, relative_position)
+ parent_changed = link.changes.include?(:work_item_parent_id)
create_missing_synced_link!(adjacent_work_item)
- return unless adjacent_work_item.parent_link
-
- return super unless sync_to_epic?(link, adjacent_work_item)
+ return unless adjacent_work_item.parent_link || parent_changed
+ return super unless sync_to_epic?(link)
ApplicationRecord.transaction do
+ # set this attribute to skip the validation ParentLink#validate_legacy_hierarchy
+ link.work_item_syncing = true if parent_changed
move_synced_object!(link, adjacent_work_item, relative_position) if super
end
end
@@ -42,10 +44,10 @@ def create_missing_synced_link!(adjacent_work_item)
def move_synced_object!(link, adjacent_work_item, relative_position)
synced_moving_object = synced_object_for(link.work_item)
- synced_adjacent_object = synced_object_for(adjacent_work_item)
+ return unless synced_moving_object
- synced_moving_object.move_before(synced_adjacent_object) if relative_position == 'BEFORE'
- synced_moving_object.move_after(synced_adjacent_object) if relative_position == 'AFTER'
+ sync_parent_change!(link, synced_moving_object)
+ reorder_synced_object(synced_moving_object, adjacent_work_item, relative_position)
synced_moving_object.save!(touch: false)
rescue StandardError => error
@@ -71,11 +73,35 @@ def synced_object_for(work_item)
end
end
- def sync_to_epic?(link, adjacent_work_item)
+ def sync_to_epic?(link)
return false if synced_work_item
return false if link.work_item_parent.synced_epic.nil?
- synced_object_for(link.work_item) && synced_object_for(adjacent_work_item)
+ true
+ end
+
+ def reorder_synced_object(synced_moving_object, adjacent_work_item, relative_position)
+ synced_adjacent_object = synced_object_for(adjacent_work_item)
+ return unless synced_adjacent_object
+
+ synced_moving_object.move_before(synced_adjacent_object) if relative_position == 'BEFORE'
+ synced_moving_object.move_after(synced_adjacent_object) if relative_position == 'AFTER'
+ end
+
+ def sync_parent_change!(link, synced_moving_object)
+ parent_attributes =
+ case synced_moving_object
+ when ::EpicIssue
+ # set work_item_syncing to skip the validation EpicIssue#check_existing_parent_link
+ { epic: link.work_item_parent.synced_epic, work_item_syncing: true }
+ when ::Epic
+ { parent: link.work_item_parent.synced_epic }
+ end
+
+ return if link.work_item_parent == synced_moving_object.try(parent_attributes.keys[0])
+
+ synced_moving_object.assign_attributes(parent_attributes)
+ synced_moving_object.save!
end
override :can_admin_link?
diff --git a/ee/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/ee/spec/requests/api/graphql/mutations/work_items/update_spec.rb
index db32204b48fc056999d744fbffd6912f3003ea79..4906a3904fa3f75ef202ad367cec7b1e7cadfe49 100644
--- a/ee/spec/requests/api/graphql/mutations/work_items/update_spec.rb
+++ b/ee/spec/requests/api/graphql/mutations/work_items/update_spec.rb
@@ -1039,4 +1039,216 @@ def work_item_status
end
end
end
+
+ context 'with hierarchy widget input' do
+ let(:widgets_response) { mutation_response['workItem']['widgets'] }
+ let(:fields) do
+ <<~FIELDS
+ workItem {
+ description
+ widgets {
+ type
+ ... on WorkItemWidgetHierarchy {
+ parent {
+ id
+ }
+ children {
+ edges {
+ node {
+ id
+ }
+ }
+ }
+ }
+ }
+ }
+ errors
+ FIELDS
+ end
+
+ before do
+ stub_licensed_features(epics: true, subepics: true)
+ end
+
+ context 'when updating parent' do
+ let_it_be(:work_item_epic, reload: true) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+ let_it_be(:work_item_issue, reload: true) { create(:work_item, :issue, project: project) }
+
+ let_it_be(:new_parent) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+
+ let(:input) { { 'hierarchyWidget' => { 'parentId' => new_parent.to_global_id.to_s } } }
+
+ it 'updates work item parent and synced epic parent when moving child is epic' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_epic), current_user: reporter)
+ end.to change { work_item_epic.reload.work_item_parent }.from(nil).to(new_parent)
+ .and change { work_item_epic.synced_epic.reload.parent }.from(nil).to(new_parent.synced_epic)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(widgets_response).to include({ 'type' => 'HIERARCHY', 'children' => { 'edges' => [] },
+ 'parent' => { 'id' => new_parent.to_global_id.to_s } })
+ end
+
+ it 'updates work item parent when moving child is issue' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_issue), current_user: reporter)
+ end.to change { work_item_issue.reload.work_item_parent }.from(nil).to(new_parent)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(widgets_response).to include({ 'type' => 'HIERARCHY', 'children' => { 'edges' => [] },
+ 'parent' => { 'id' => new_parent.to_global_id.to_s } })
+ end
+
+ context 'when a parent is already present' do
+ let_it_be(:existing_parent) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+
+ let_it_be(:work_item_epic_link) do
+ create(:parent_link, work_item: work_item_epic, work_item_parent: existing_parent, relative_position: 10)
+ end
+
+ let_it_be(:work_item_issue_link) do
+ create(:parent_link, work_item: work_item_issue, work_item_parent: existing_parent, relative_position: 20)
+ end
+
+ before do
+ work_item_epic.synced_epic.update!(parent: existing_parent.synced_epic)
+ create(:epic_issue, epic: existing_parent.synced_epic, issue: work_item_issue)
+ end
+
+ it 'syncs with legacy epic if child is epic' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_epic), current_user: reporter)
+ end.to change { work_item_epic.reload.work_item_parent }.from(existing_parent).to(new_parent)
+ .and change { work_item_epic.synced_epic.reload.parent }.from(existing_parent.synced_epic)
+ .to(new_parent.synced_epic)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(widgets_response).to include({ 'type' => 'HIERARCHY', 'children' => { 'edges' => [] },
+ 'parent' => { 'id' => new_parent.to_global_id.to_s } })
+ end
+
+ it 'syncs with epic_issue if child is issue' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_issue), current_user: reporter)
+ end.to change { work_item_issue.reload.work_item_parent }.from(existing_parent).to(new_parent)
+ .and change { work_item_issue.epic_issue.reload.epic }.from(existing_parent.synced_epic)
+ .to(new_parent.synced_epic)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(widgets_response).to include({ 'type' => 'HIERARCHY', 'children' => { 'edges' => [] },
+ 'parent' => { 'id' => new_parent.to_global_id.to_s } })
+ end
+
+ context 'and new parent has existing children' do
+ let_it_be(:child_in_new_parent) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+
+ let(:input) do
+ { 'hierarchyWidget' => {
+ 'parentId' => new_parent.to_global_id.to_s,
+ adjacentWorkItemId: child_in_new_parent.to_global_id.to_s,
+ relativePosition: "AFTER"
+ } }
+ end
+
+ before do
+ create(:parent_link, work_item: child_in_new_parent, work_item_parent: new_parent, relative_position: 10)
+ child_in_new_parent.synced_epic.update!(parent: new_parent.synced_epic)
+ end
+
+ context 'when moving child is an epic' do
+ it 'syncs with legacy epic' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_epic), current_user: reporter)
+ end.to change { work_item_epic.reload.work_item_parent }.from(existing_parent).to(new_parent)
+ .and change { work_item_epic.synced_epic.reload.parent }.from(existing_parent.synced_epic)
+ .to(new_parent.synced_epic)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(new_parent.work_item_children_by_relative_position.pluck(:id))
+ .to match_array([child_in_new_parent.id, work_item_epic.id])
+ end
+
+ it 'does not move child if syncing parent fails' do
+ allow_next_found_instance_of(::Epic) do |instance|
+ allow(instance).to receive(:save!).and_raise(ActiveRecord::RecordInvalid.new)
+ end
+
+ expect do
+ post_graphql_mutation(mutation_for(work_item_epic), current_user: reporter)
+ end.to not_change { work_item_epic.reload.work_item_parent }
+ .and not_change { work_item_epic.synced_epic.reload.parent }
+ .and not_change { work_item_epic_link.reload }
+
+ expect(mutation_response["errors"]).to include("Couldn't re-order due to an internal error.")
+ end
+ end
+
+ context 'when moving child is an issue' do
+ it 'syncs with epic_issue' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_issue), current_user: reporter)
+ end.to change { work_item_issue.reload.work_item_parent }.from(existing_parent).to(new_parent)
+ .and change { work_item_issue.epic_issue.reload.epic }.from(existing_parent.synced_epic)
+ .to(new_parent.synced_epic)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(new_parent.work_item_children_by_relative_position.pluck(:id))
+ .to match_array([child_in_new_parent.id, work_item_issue.id])
+ end
+
+ it 'does not move child if syncing parent fails' do
+ allow_next_found_instance_of(::EpicIssue) do |instance|
+ allow(instance).to receive(:save!).and_raise(ActiveRecord::RecordInvalid.new)
+ end
+
+ expect do
+ post_graphql_mutation(mutation_for(work_item_issue), current_user: reporter)
+ end.to not_change { work_item_issue.reload.work_item_parent }
+ .and not_change { work_item_issue.epic_issue.reload.epic }
+ .and not_change { work_item_issue_link.reload }
+
+ expect(mutation_response["errors"]).to include("Couldn't re-order due to an internal error.")
+ end
+ end
+
+ context 'when changing parent fails' do
+ before do
+ allow_next_found_instance_of(::WorkItems::ParentLink) do |instance|
+ allow(instance).to receive(:save).and_return(false)
+
+ errors = ActiveModel::Errors.new(instance).tap { |e| e.add(:work_item, 'error message') }
+ allow(instance).to receive(:errors).and_return(errors)
+ end
+ end
+
+ it 'does not sync change to legacy epic parent when moving an epic' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_epic), current_user: reporter)
+ end.to not_change { work_item_epic.reload.work_item_parent }
+ .and not_change { work_item_epic.synced_epic.reload.parent }
+ .and not_change { work_item_epic_link.reload }
+
+ expect(mutation_response["errors"])
+ .to include("#{work_item_epic.to_reference} cannot be added: error message")
+ end
+
+ it 'does not sync change to epic_issue when moving an issue' do
+ expect do
+ post_graphql_mutation(mutation_for(work_item_issue), current_user: reporter)
+ end.to not_change { work_item_issue.reload.work_item_parent }
+ .and not_change { work_item_issue.epic_issue.reload.epic }
+ .and not_change { work_item_issue_link.reload }
+
+ expect(mutation_response["errors"])
+ .to include("#{work_item_issue.to_reference} cannot be added: error message")
+ end
+ end
+ end
+ end
+ end
+ end
+
+ def mutation_for(item)
+ graphql_mutation(:workItemUpdate, input.merge('id' => item.to_global_id.to_s), fields)
+ end
end
diff --git a/ee/spec/services/ee/work_items/parent_links/reorder_service_spec.rb b/ee/spec/services/ee/work_items/parent_links/reorder_service_spec.rb
index 5c5b3b4ce3c6246d2e13a86deab0f3131341ffb8..f01fe3c03d2ef1326311777abef46aafc18c086f 100644
--- a/ee/spec/services/ee/work_items/parent_links/reorder_service_spec.rb
+++ b/ee/spec/services/ee/work_items/parent_links/reorder_service_spec.rb
@@ -96,6 +96,34 @@
end
end
+ shared_context 'with new parent that has children' do
+ let_it_be_with_reload(:new_parent) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+ let_it_be_with_reload(:new_sibling1) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+ let_it_be_with_reload(:new_sibling2) { create(:work_item, :epic_with_legacy_epic, namespace: group) }
+
+ let_it_be_with_reload(:new_parent_link) do
+ create(:parent_link, work_item: new_parent, work_item_parent: parent, relative_position: 30)
+ end
+
+ let_it_be_with_reload(:new_sibling1_link1) do
+ create(:parent_link, work_item: new_sibling1, work_item_parent: new_parent, relative_position: 10)
+ end
+
+ let_it_be_with_reload(:new_sibling1_link2) do
+ create(:parent_link, work_item: new_sibling2, work_item_parent: new_parent, relative_position: 20)
+ end
+
+ let(:params) { base_params.merge(adjacent_work_item: new_sibling2, relative_position: "BEFORE") }
+
+ before do
+ new_parent.synced_epic.update!(parent: parent.synced_epic, relative_position: 50)
+ new_sibling1.synced_epic.update!(parent: new_parent.synced_epic, relative_position: 10)
+ new_sibling2.synced_epic.update!(parent: new_parent.synced_epic, relative_position: 20)
+ end
+
+ subject(:move_child) { described_class.new(new_parent, user, params).execute }
+ end
+
before do
stub_licensed_features(subepics: true)
stub_feature_flags(work_item_epics: true)
@@ -149,6 +177,18 @@
it_behaves_like 'reorders the hierarchy'
+ context 'when changing parent and reordering' do
+ include_context 'with new parent that has children'
+
+ it 'updates work item parent and legacy epic parent' do
+ expect { move_child }.to change { work_item.reload.work_item_parent }.from(parent).to(new_parent)
+ .and change { work_item.synced_epic.reload.parent }.from(parent.synced_epic)
+ .to(new_parent.synced_epic)
+
+ expect(new_parent.work_item_children_by_relative_position).to eq([new_sibling1, work_item, new_sibling2])
+ end
+ end
+
context 'when synced_work_item param is set' do
let(:synced_moving_object) { nil }
let(:params) { base_params.merge(adjacent_work_item: adjacent_work_item, synced_work_item: true) }
@@ -205,6 +245,18 @@
it_behaves_like 'reorders the hierarchy'
+ context 'when changing parent and reordering' do
+ include_context 'with new parent that has children'
+
+ it 'updates work item parent and epic_issue epic' do
+ expect { move_child }.to change { work_item.reload.work_item_parent }.from(parent).to(new_parent)
+ .and change { work_item.epic_issue.reload.epic }.from(parent.synced_epic)
+ .to(new_parent.synced_epic)
+
+ expect(new_parent.work_item_children_by_relative_position).to eq([new_sibling1, work_item, new_sibling2])
+ end
+ end
+
context 'when synced EpicIssue for the moving work item do not exist' do
let(:synced_moving_object) { nil }
let_it_be_with_reload(:epic_issue) { nil }