Merge branch '231332-epics-not-inheriting-dates' into 'master'

Update epic inherited dates and create notes when reordering in epic tree

See merge request gitlab-org/gitlab!39742

ee/app/services/epics/tree_reorder_service.rb

@@ -26,19 +26,26 @@ module Epics
     def set_new_parent
       return unless new_parent && new_parent_different?
 
-      moving_object.parent = new_parent
-      validate_new_parent
+      service = create_issuable_links(new_parent)
+      return unless service[:status] == :error
+
+      service[:message]
     end
 
     def new_parent_different?
       params[:new_parent_id] != GitlabSchema.id_from_object(moving_object.parent)
     end
 
-    def validate_new_parent
-      return unless moving_object.respond_to?(:valid_parent?)
-      return if moving_object.valid_parent?
+    def create_issuable_links(parent)
+      service, issuable = if moving_object.is_a?(Epic)
+                            [EpicLinks::CreateService, moving_object]
+                          elsif moving_object.is_a?(EpicIssue)
+                            [EpicIssues::CreateService, moving_object.issue]
+                          end
+
+      return unless service.present?
 
-      moving_object.errors[:parent]&.first
+      service.new(parent, current_user, { target_issuable: issuable }).execute
     end
 
     def move!
@@ -110,6 +117,10 @@ module Epics
       if new_parent
         return false unless can?(current_user, :admin_epic, new_parent.group)
         return false unless moving_object.parent && can?(current_user, :admin_epic, moving_object.parent.group)
+
+        if moving_object.is_a?(Epic)
+          return false unless can?(current_user, :admin_epic_link, moving_object.parent.group)
+        end
       end
 
       true

ee/changelogs/unreleased/231332-epics-not-inheriting-dates.yml

+---
+title: Update epic inherited dates and create notes when reordering in epic tree
+merge_request: 39742
+author:
+type: fixed

ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb

@@ -49,9 +49,9 @@ RSpec.describe 'Updating an epic tree' do
     end
   end
 
-  context 'when epic feature is enabled' do
+  context 'when epics and subepics features are enabled' do
     before do
-      stub_licensed_features(epics: true)
+      stub_licensed_features(epics: true, subepics: true)
     end
 
     context 'when the user does not have permission' do
@@ -141,6 +141,24 @@ RSpec.describe 'Updating an epic tree' do
             expect(mutation_response['errors']).to eq(["The sibling object's parent must match the current parent epic."])
           end
         end
+
+        context 'when the new parent is another epic and subepics feature is disabled' do
+          let(:new_parent_id) { GitlabSchema.id_from_object(base_epic).to_s }
+
+          before do
+            stub_licensed_features(epics: true, subepics: false)
+            other_epic = create(:epic, group: group)
+            epic2.update(parent: other_epic)
+          end
+
+          it_behaves_like 'a mutation that does not update the tree'
+
+          it 'returns the error message' do
+            post_graphql_mutation(mutation, current_user: current_user)
+
+            expect(mutation_response['errors']).to eq(['You don\'t have permissions to move the objects.'])
+          end
+        end
       end
 
       context 'when moving an issue' do

ee/spec/services/epics/tree_reorder_service_spec.rb

@@ -66,7 +66,7 @@ RSpec.describe Epics::TreeReorderService do
 
       context 'when user does have permission to admin the base epic' do
         before do
-          group.add_developer(user)
+          group.add_reporter(user)
         end
 
         context 'when relative_position is not valid' do
@@ -102,6 +102,10 @@ RSpec.describe Epics::TreeReorderService do
             it 'updates the parent' do
               expect { subject }.to change { tree_object_2.reload.epic }.from(epic1).to(epic)
             end
+
+            it 'creates system notes' do
+              expect { subject }.to change { Note.system.count }.by(3)
+            end
           end
 
           context 'when object being moved is from another epic' do
@@ -207,6 +211,10 @@ RSpec.describe Epics::TreeReorderService do
 
                 expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
               end
+
+              it 'creates system notes' do
+                expect { subject }.to change { Note.system.count }.by(3)
+              end
             end
           end
         end
@@ -215,86 +223,118 @@ RSpec.describe Epics::TreeReorderService do
           let!(:tree_object_1) { epic1 }
           let!(:tree_object_2) { epic2 }
 
-          context 'when user does not have permissions to admin the previous parent' do
-            let(:other_group) { create(:group) }
-            let(:other_epic) { create(:epic, group: other_group) }
+          context 'when subepics feature is disabled' do
             let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
             before do
-              epic2.update(parent: other_epic)
+              stub_licensed_features(epics: true, subepics: false)
             end
 
             it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
           end
 
-          context 'when there is some other error with the new parent' do
-            let(:other_group) { create(:group) }
-            let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
-
+          context 'when subepics feature is enabled' do
             before do
-              other_group.add_developer(user)
-              epic.update(group: other_group)
-              epic2.update(parent: epic1)
+              stub_licensed_features(epics: true, subepics: true)
             end
 
-            it_behaves_like 'error for the tree update', "This epic cannot be added. An epic must belong to the same group or subgroup as its parent epic."
-          end
+            context 'when user does not have permissions to admin the previous parent' do
+              let(:other_group) { create(:group) }
+              let(:other_epic) { create(:epic, group: other_group) }
+              let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
-          context 'when user does not have permissions to admin the new parent' do
-            let(:other_group) { create(:group) }
-            let(:other_epic) { create(:epic, group: other_group) }
-            let(:new_parent_id) { GitlabSchema.id_from_object(other_epic) }
+              before do
+                epic2.update(parent: other_epic)
+              end
 
-            it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
-          end
+              it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
+            end
 
-          context 'when the reordered epics are not subepics of the base epic' do
-            let(:another_group) { create(:group) }
-            let(:another_epic) { create(:epic, group: another_group) }
+            context 'when user does not have permissions to admin the previous parent links' do
+              let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
-            before do
-              epic1.update(group: another_group, parent: another_epic)
-              epic2.update(group: another_group, parent: another_epic)
+              before do
+                group.add_guest(user)
+              end
+
+              it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
             end
 
-            it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
-          end
+            context 'when there is some other error with the new parent' do
+              let(:other_group) { create(:group) }
+              let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
-          context 'when moving is successful' do
-            it 'updates the links relative positions' do
-              subject
+              before do
+                other_group.add_developer(user)
+                epic.update(group: other_group)
+                epic2.update(parent: epic1)
+              end
 
-              expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
+              it_behaves_like 'error for the tree update', "This epic cannot be added. An epic must belong to the same group or subgroup as its parent epic."
             end
 
-            context 'when new parent is current epic' do
-              let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
+            context 'when user does not have permissions to admin the new parent' do
+              let(:other_group) { create(:group) }
+              let(:other_epic) { create(:epic, group: other_group) }
+              let(:new_parent_id) { GitlabSchema.id_from_object(other_epic) }
 
-              it 'updates the relative positions' do
-                subject
+              it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
+            end
 
-                expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
-              end
+            context 'when user '
 
-              it 'does not update the parent_id' do
-                expect { subject }.not_to change { tree_object_2.reload.parent }
+            context 'when the reordered epics are not subepics of the base epic' do
+              let(:another_group) { create(:group) }
+              let(:another_epic) { create(:epic, group: another_group) }
+
+              before do
+                epic1.update(group: another_group, parent: another_epic)
+                epic2.update(group: another_group, parent: another_epic)
               end
-            end
 
-            context 'when object being moved is from another epic and new_parent_id matches parent of adjacent object' do
-              let(:other_epic) { create(:epic, group: group) }
-              let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
-              let(:epic3) { create(:epic, parent: other_epic, group: group) }
-              let(:tree_object_2) { epic3 }
+              it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
+            end
 
-              it 'updates the relative positions and parent_id' do
+            context 'when moving is successful' do
+              it 'updates the links relative positions' do
                 subject
 
                 expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
               end
 
-              it 'updates the parent' do
-                expect { subject }.to change { tree_object_2.reload.parent }.from(other_epic).to(epic)
+              context 'when new parent is current epic' do
+                let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
+
+                it 'updates the relative positions' do
+                  subject
+
+                  expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
+                end
+
+                it 'does not update the parent_id' do
+                  expect { subject }.not_to change { tree_object_2.reload.parent }
+                end
+              end
+
+              context 'when object being moved is from another epic and new_parent_id matches parent of adjacent object' do
+                let(:other_epic) { create(:epic, group: group) }
+                let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
+                let(:epic3) { create(:epic, parent: other_epic, group: group) }
+                let(:tree_object_2) { epic3 }
+
+                it 'updates the relative positions' do
+                  subject
+
+                  expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
+                end
+
+                it 'updates the parent' do
+                  expect { subject }.to change { tree_object_2.reload.parent }.from(other_epic).to(epic)
+                end
+
+                it 'creates system notes' do
+                  expect { subject }.to change { Note.system.count }.by(2)
+                end
               end
             end
           end