Merge branch '230440-graphql-epic-unassign' into 'master'

Add epic_id param to issue update graphQL mutation

See merge request gitlab-org/gitlab!38678

doc/api/graphql/reference/gitlab_schema.graphql

@@ -16560,6 +16560,11 @@ input UpdateIssueInput {
   """
   dueDate: Time
 
+  """
+  The ID of the parent epic. NULL when removing the association
+  """
+  epicId: ID
+
   """
   The desired health status
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -48797,6 +48797,16 @@
               },
               "defaultValue": null
             },
+            {
+              "name": "epicId",
+              "description": "The ID of the parent epic. NULL when removing the association",
+              "type": {
+                "kind": "SCALAR",
+                "name": "ID",
+                "ofType": null
+              },
+              "defaultValue": null
+            },
             {
               "name": "clientMutationId",
               "description": "A unique identifier for the client performing the mutation.",

ee/app/graphql/ee/mutations/issues/update.rb

@@ -11,6 +11,10 @@ module EE
                    ::Types::HealthStatusEnum,
                    required: false,
                    description: 'The desired health status'
+          argument :epic_id,
+                   GraphQL::ID_TYPE,
+                   required: false,
+                   description: 'The ID of the parent epic. NULL when removing the association'
         end
       end
     end

ee/app/services/ee/issues/base_service.rb

@@ -5,17 +5,38 @@ module EE
     module BaseService
       extend ::Gitlab::Utils::Override
 
-      override :filter_params
       def filter_params(issue)
         set_epic_param(issue)
         super
       end
 
-      private
+      def handle_epic(issue)
+        set_epic_param(issue)
+
+        return unless params.key?(:epic)
+
+        if epic_param
+          EpicIssues::CreateService.new(epic_param, current_user, { target_issuable: issue }).execute
+        else
+          link = EpicIssue.find_by_issue_id(issue.id)
+
+          return unless link
+
+          EpicIssues::DestroyService.new(link, current_user).execute
+        end
+
+        params.delete(:epic)
+      end
 
       def set_epic_param(issue)
-        epic = find_epic(issue)
-        return unless epic
+        return unless epic_param_present?
+
+        epic = epic_param || find_epic(issue)
+
+        unless epic
+          params[:epic] = nil
+          return
+        end
 
         unless can?(current_user, :admin_epic, epic)
           raise ::Gitlab::Access::AccessDeniedError
@@ -25,14 +46,22 @@ module EE
       end
 
       def find_epic(issue)
-        id = params.delete(:epic_id)
-        return if id.to_i == 0
+        epic_id = params.delete(:epic_id)
+        return if epic_id.to_i == 0
 
         group = issue.project.group
         return unless group.present?
 
         EpicsFinder.new(current_user, group_id: group.id,
-                        include_ancestor_groups: true).find(id)
+                        include_ancestor_groups: true).find(epic_id)
+      end
+
+      def epic_param
+        params[:epic]
+      end
+
+      def epic_param_present?
+        params.key?(:epic) || params.key?(:epic_id)
       end
     end
   end

ee/app/services/ee/issues/create_service.rb

@@ -7,31 +7,15 @@ module EE
 
       override :before_create
       def before_create(issue)
-        handle_issue_epic_link(issue)
+        handle_epic(issue)
 
         super
       end
 
-      def handle_issue_epic_link(issue)
-        return unless params.key?(:epic)
+      def handle_epic(issue)
+        issue.confidential = true if epic_param&.confidential
 
-        epic = params.delete(:epic)
-
-        if epic
-          issue.confidential = true if epic.confidential?
-
-          EpicIssues::CreateService.new(epic, current_user, { target_issuable: issue }).execute
-        else
-          destroy_epic_link(issue)
-        end
-      end
-
-      def destroy_epic_link(issue)
-        link = EpicIssue.find_by_issue_id(issue.id)
-
-        return unless link
-
-        EpicIssues::DestroyService.new(link, current_user).execute
+        super
       end
     end
   end

ee/app/services/ee/issues/update_service.rb

@@ -4,6 +4,7 @@ module EE
   module Issues
     module UpdateService
       extend ::Gitlab::Utils::Override
+      include ::Gitlab::Utils::StrongMemoize
 
       override :execute
       def execute(issue)
@@ -49,22 +50,6 @@ module EE
 
         Epics::IssuePromoteService.new(issue.project, current_user).execute(issue)
       end
-
-      def handle_epic(issue)
-        return unless params.key?(:epic)
-
-        epic_param = params.delete(:epic)
-
-        if epic_param
-          EpicIssues::CreateService.new(epic_param, current_user, { target_issuable: issue }).execute
-        else
-          link = EpicIssue.find_by(issue_id: issue.id) # rubocop: disable CodeReuse/ActiveRecord
-
-          return unless link
-
-          EpicIssues::DestroyService.new(link, current_user).execute
-        end
-      end
     end
   end
 end

ee/changelogs/unreleased/230440-graphql-epic-unassign.yml

+---
+title: Add epic_id param to issue update graphQL mutation
+merge_request: 38678
+author:
+type: added

ee/spec/graphql/mutations/issues/update_spec.rb

@@ -3,8 +3,63 @@
 require 'spec_helper'
 
 RSpec.describe Mutations::Issues::Update do
+  let(:user) { create(:user) }
+
   it_behaves_like 'updating health status' do
     let(:resource) { create(:issue) }
-    let(:user) { create(:user) }
+  end
+
+  context 'updating parent epic' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
+    let_it_be(:issue) { create(:issue, project: project) }
+    let_it_be(:epic) { create(:epic, group: group) }
+
+    let(:epic_id) { epic.id }
+    let(:params) { { project_path: project.full_path, iid: issue.iid, epic_id: epic_id } }
+    let(:mutated_issue) { subject[:issue] }
+    let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
+
+    subject { mutation.resolve(params) }
+
+    context 'when epics feature is disabled' do
+      it 'raises an error' do
+        group.add_developer(user)
+
+        expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context 'when epics feature is enabled' do
+      before do
+        stub_licensed_features(epics: true)
+      end
+
+      context 'for user without permissions' do
+        it 'raises an error' do
+          expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
+        end
+      end
+
+      context 'for user with correct permissions' do
+        before do
+          group.add_developer(user)
+        end
+
+        context 'when a valid epic is given' do
+          it 'updates the epic' do
+            expect(mutated_issue.epic).to eq(epic)
+          end
+        end
+
+        context 'when nil epic is given' do
+          let(:epic_id) { nil }
+
+          it 'set the epic to nil' do
+            expect(mutated_issue.epic).to be_nil
+          end
+        end
+      end
+    end
   end
 end

ee/spec/services/ee/issues/update_service_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 RSpec.describe Issues::UpdateService do
-  let(:group) { create(:group) }
+  let_it_be(:group) { create(:group) }
   let(:project) { create(:project, group: group) }
   let(:issue) { create(:issue, project: project) }
   let(:user) { issue.author }
@@ -13,7 +13,7 @@ RSpec.describe Issues::UpdateService do
     end
 
     context 'refresh epic dates' do
-      let(:epic) { create(:epic) }
+      let_it_be(:epic) { create(:epic) }
       let(:issue) { create(:issue, epic: epic, project: project) }
 
       context 'updating milestone' do
@@ -187,48 +187,57 @@ RSpec.describe Issues::UpdateService do
     context 'assigning epic' do
       before do
         stub_licensed_features(epics: true)
-        group.add_maintainer(user)
       end
 
       let(:epic) { create(:epic, group: group) }
 
-      context 'when issue does not belong to an epic yet' do
-        it 'assigns an issue to the provided epic' do
-          expect { update_issue(epic: epic) }.to change { issue.reload.epic }.from(nil).to(epic)
-        end
-
-        it 'creates system notes for the epic and the issue' do
-          expect { update_issue(epic: epic) }.to change { Note.count }.from(0).to(2)
+      subject { update_issue(epic: epic) }
 
-          epic_note = Note.find_by(noteable_id: epic.id, noteable_type: 'Epic')
-          issue_note = Note.find_by(noteable_id: issue.id, noteable_type: 'Issue')
-
-          expect(epic_note.system_note_metadata.action).to eq('epic_issue_added')
-          expect(issue_note.system_note_metadata.action).to eq('issue_added_to_epic')
+      context 'when a user does not have permissions to assign an epic' do
+        it 'raises an exception' do
+          expect { subject }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
 
-      context 'when issue does belongs to another epic' do
-        let(:epic2) { create(:epic, group: group) }
-
+      context 'when a user has permissions to assign an epic' do
         before do
-          issue.update!(epic: epic2)
+          group.add_maintainer(user)
         end
 
-        it 'assigns the issue passed to the provided epic' do
-          expect { update_issue(epic: epic) }.to change { issue.reload.epic }.from(epic2).to(epic)
+        context 'when issue does not belong to an epic yet' do
+          it 'assigns an issue to the provided epic' do
+            expect { update_issue(epic: epic) }.to change { issue.reload.epic }.from(nil).to(epic)
+          end
+
+          it 'calls EpicIssues::CreateService' do
+            link_sevice = double
+            expect(EpicIssues::CreateService).to receive(:new).with(epic, user, { target_issuable: issue })
+              .and_return(link_sevice)
+            expect(link_sevice).to receive(:execute)
+
+            subject
+          end
         end
 
-        it 'creates system notes for the epic and the issue' do
-          expect { update_issue(epic: epic) }.to change { Note.count }.from(0).to(3)
+        context 'when issue belongs to another epic' do
+          let(:epic2) { create(:epic, group: group) }
 
-          epic_note = Note.find_by(noteable_id: epic.id, noteable_type: 'Epic')
-          epic2_note = Note.find_by(noteable_id: epic2.id, noteable_type: 'Epic')
-          issue_note = Note.find_by(noteable_id: issue.id, noteable_type: 'Issue')
+          before do
+            issue.update!(epic: epic2)
+          end
+
+          it 'assigns the issue passed to the provided epic' do
+            expect { subject }.to change { issue.reload.epic }.from(epic2).to(epic)
+          end
+
+          it 'calls EpicIssues::CreateService' do
+            link_sevice = double
+            expect(EpicIssues::CreateService).to receive(:new).with(epic, user, { target_issuable: issue })
+              .and_return(link_sevice)
+            expect(link_sevice).to receive(:execute)
 
-          expect(epic_note.system_note_metadata.action).to eq('epic_issue_moved')
-          expect(epic2_note.system_note_metadata.action).to eq('epic_issue_moved')
-          expect(issue_note.system_note_metadata.action).to eq('issue_changed_epic')
+            subject
+          end
         end
       end
     end
@@ -236,34 +245,38 @@ RSpec.describe Issues::UpdateService do
     context 'removing epic' do
       before do
         stub_licensed_features(epics: true)
-        group.add_maintainer(user)
       end
 
       let(:epic) { create(:epic, group: group) }
 
-      context 'when issue does not belong to an epic yet' do
-        it 'does not do anything' do
-          expect { update_issue(epic: nil) }.not_to change { issue.reload.epic }
-        end
-      end
+      subject { update_issue(epic: nil) }
 
-      context 'when issue belongs to an epic' do
+      context 'when a user has permissions to assign an epic' do
         before do
-          issue.update!(epic: epic)
+          group.add_maintainer(user)
         end
 
-        it 'assigns a new issue to the provided epic' do
-          expect { update_issue(epic: nil) }.to change { issue.reload.epic }.from(epic).to(nil)
+        context 'when issue does not belong to an epic yet' do
+          it 'does not do anything' do
+            expect { subject }.not_to change { issue.reload.epic }
+          end
         end
 
-        it 'creates system notes for the epic and the issue' do
-          expect { update_issue(epic: nil) }.to change { Note.count }.from(0).to(2)
+        context 'when issue belongs to an epic' do
+          let!(:epic_issue) { create(:epic_issue, issue: issue, epic: epic)}
+
+          it 'unassigns the epic' do
+            expect { subject }.to change { issue.reload.epic }.from(epic).to(nil)
+          end
 
-          epic_note = Note.find_by(noteable_id: epic.id, noteable_type: 'Epic')
-          issue_note = Note.find_by(noteable_id: issue.id, noteable_type: 'Issue')
+          it 'calls EpicIssues::DestroyService' do
+            link_sevice = double
+            expect(EpicIssues::DestroyService).to receive(:new).with(EpicIssue.last, user)
+              .and_return(link_sevice)
+            expect(link_sevice).to receive(:execute)
 
-          expect(epic_note.system_note_metadata.action).to eq('epic_issue_removed')
-          expect(issue_note.system_note_metadata.action).to eq('issue_removed_from_epic')
+            subject
+          end
         end
       end
     end
@@ -278,6 +291,32 @@ RSpec.describe Issues::UpdateService do
       let(:epic) { create(:epic, group: group) }
     end
 
+    context 'when epic_id is nil' do
+      before do
+        stub_licensed_features(epics: true)
+        group.add_maintainer(user)
+      end
+
+      let(:epic) { create(:epic, group: group) }
+      let!(:epic_issue) { create(:epic_issue, epic: epic, issue: issue) }
+      let(:params) { { epic_id: nil } }
+
+      subject { update_issue(params) }
+
+      it 'removes epic issue link' do
+        expect { subject }.to change { issue.reload.epic }.from(epic).to(nil)
+      end
+
+      it 'calls EpicIssues::DestroyService' do
+        link_sevice = double
+        expect(EpicIssues::DestroyService).to receive(:new).with(epic_issue, user)
+          .and_return(link_sevice)
+        expect(link_sevice).to receive(:execute)
+
+        subject
+      end
+    end
+
     context 'promoting to epic' do
       before do
         stub_licensed_features(epics: true)

ee/spec/support/shared_examples/services/issue_epic_shared_examples.rb

@@ -16,10 +16,10 @@ RSpec.shared_examples 'issue with epic_id parameter' do
   context 'when epic_id is 0' do
     let(:params) { { title: 'issue1', epic_id: 0 } }
 
-    it 'ignores epic_id' do
+    it 'does not assign any epic' do
       issue = execute
 
-      expect(issue).to be_persisted
+      expect(issue.reload).to be_persisted
       expect(issue.epic).to be_nil
     end
   end
@@ -48,9 +48,17 @@ RSpec.shared_examples 'issue with epic_id parameter' do
       it 'creates epic issue link' do
         issue = execute
 
-        expect(issue).to be_persisted
+        expect(issue.reload).to be_persisted
         expect(issue.epic).to eq(epic)
       end
+
+      it 'calls EpicIssues::CreateService' do
+        link_sevice = double
+        expect(EpicIssues::CreateService).to receive(:new).and_return(link_sevice)
+        expect(link_sevice).to receive(:execute)
+
+        execute
+      end
     end
 
     context 'when a project is from a subgroup of the epic group' do
@@ -63,7 +71,7 @@ RSpec.shared_examples 'issue with epic_id parameter' do
       it 'creates epic issue link' do
         issue = execute
 
-        expect(issue).to be_persisted
+        expect(issue.reload).to be_persisted
         expect(issue.epic).to eq(epic)
       end
     end