Add specific type to epic_id for updateIssue

This required changes to support nullable global-IDs, which up to this
point have explicitly rejected nil values.

The Epic ID handling is changed to:

- use the declarative loading mechanism
- check for the correct epic permissions before trying to apply the
  change (which eliminates some uncaught exceptions)

app/graphql/mutations/base_mutation.rb

@@ -34,5 +34,9 @@ module Mutations
         true
       end
     end
+
+    def can?(ability, subject)
+      Ability.allowed?(current_user, ability, subject)
+    end
   end
 end

app/graphql/types/global_id_type.rb

@@ -30,6 +30,8 @@ module Types
     # @param value [String]
     # @return [GID]
     def self.coerce_input(value, _ctx)
+      return if value.nil?
+
       gid = GlobalID.parse(value)
       raise GraphQL::CoercionError, "#{value.inspect} is not a valid Global ID" if gid.nil?
       raise GraphQL::CoercionError, "#{value.inspect} is not a Gitlab Global ID" unless gid.app == GlobalID.app

doc/api/graphql/reference/gitlab_schema.graphql

@@ -21417,7 +21417,7 @@ input UpdateIssueInput {
   """
   The ID of the parent epic. NULL when removing the association
   """
-  epicId: ID
+  epicId: EpicID
 
   """
   The desired health status

doc/api/graphql/reference/gitlab_schema.json

@@ -62243,7 +62243,7 @@
               "description": "The ID of the parent epic. NULL when removing the association",
               "type": {
                 "kind": "SCALAR",
-                "name": "ID",
+                "name": "EpicID",
                 "ofType": null
               },
               "defaultValue": null

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

@@ -9,15 +9,33 @@ module EE
         prepended do
           include ::Mutations::Issues::CommonEEMutationArguments
 
-          argument :epic_id, GraphQL::ID_TYPE,
+          argument :epic_id, ::Types::GlobalIDType[::Epic],
                    required: false,
+                   loads: ::Types::EpicType,
                    description: 'The ID of the parent epic. NULL when removing the association'
         end
 
-        def resolve(project_path:, iid:, **args)
-          args[:epic_id] = ::GitlabSchema.parse_gid(args[:epic_id], expected_type: ::Epic).model_id if args[:epic_id]
+        def resolve(**args)
+          if args.key?(:epic)
+            args[:epic_id] = epic_id(args.delete(:epic))
+          end
 
-          super
+          super(**args)
+        end
+
+        private
+
+        def epic_id(epic)
+          return unless epic
+
+          authorize_epic!(epic)
+          epic.id
+        end
+
+        def authorize_epic!(epic)
+          return if can?(:admin_epic, epic)
+
+          raise_resource_not_available_error!
         end
       end
     end

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

@@ -15,26 +15,34 @@ RSpec.describe Mutations::Issues::Update do
     let_it_be(:issue) { create(:issue, project: project) }
     let_it_be(:epic) { create(:epic, group: group) }
 
-    let(:epic_id) { epic.to_global_id.to_s }
     let(:params) do
       {
         project_path: project.full_path,
         iid: issue.iid,
-        epic_id: epic_id,
         weight: 10
-      }
+      }.merge(epic_params)
+    end
+
+    let(:epic_params) do
+      { epic: epic }
     end
 
     let(:mutated_issue) { subject[:issue] }
-    let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
+    let(:current_user) { user }
+    let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) }
 
     subject { mutation.resolve(params) }
 
+    before do
+      group.clear_memoization(:feature_available)
+      group.add_developer(user)
+    end
+
     context 'when epics feature is disabled' do
       it 'raises an error' do
         group.add_developer(user)
 
-        expect { subject }.to raise_error(ActiveRecord::RecordNotFound)
+        expect { subject }.to raise_error(::Gitlab::Graphql::Errors::ResourceNotAvailable)
       end
     end
 
@@ -44,16 +52,14 @@ RSpec.describe Mutations::Issues::Update do
       end
 
       context 'for user without permissions' do
+        let(:current_user) { create(:user) }
+
         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 { subject }.to change { issue.reload.epic }.from(nil).to(epic)
@@ -70,7 +76,7 @@ RSpec.describe Mutations::Issues::Update do
             issue.update!(epic: epic)
           end
 
-          let(:epic_id) { nil }
+          let(:epic_params) { { epic: nil } }
 
           it 'set the epic to nil' do
             expect { subject }.to change { issue.reload.epic }.from(epic).to(nil)

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

@@ -5,8 +5,9 @@ require 'spec_helper'
 RSpec.describe 'Update of an existing issue' do
   include GraphqlHelpers
 
+  let_it_be(:group) { create(:group) }
   let_it_be(:current_user) { create(:user) }
-  let_it_be(:project) { create(:project, :public) }
+  let_it_be(:project) { create(:project, :public, group: group) }
   let_it_be(:issue) { create(:issue, project: project) }
   let(:input) do
     {
@@ -17,7 +18,7 @@ RSpec.describe 'Update of an existing issue' do
   end
 
   let(:mutation) { graphql_mutation(:update_issue, input.merge(project_path: project.full_path)) }
-  let(:mutation_response) { graphql_mutation_response(:update_issue) }
+  let(:mutated_issue) { graphql_mutation_response(:update_issue)['issue'] }
 
   before do
     stub_licensed_features(issuable_health_status: true)
@@ -28,6 +29,76 @@ RSpec.describe 'Update of an existing issue' do
     post_graphql_mutation(mutation, current_user: current_user)
 
     expect(response).to have_gitlab_http_status(:success)
-    expect(mutation_response['issue']).to include(input)
+    expect(mutated_issue).to include(input)
+  end
+
+  context 'setting epic' do
+    let(:epic) { create(:epic, group: group) }
+
+    let(:input) do
+      { iid: issue.iid.to_s, epic_id: global_id_of(epic) }
+    end
+
+    before do
+      stub_licensed_features(epics: true)
+      group.add_developer(current_user)
+    end
+
+    it 'sets the epic' do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(response).to have_gitlab_http_status(:success)
+      expect(graphql_errors).to be_blank
+      expect(mutated_issue).to include(
+        'epic' => include('id' => global_id_of(epic))
+      )
+    end
+
+    context 'the epic is not readable to the current user' do
+      let(:epic) { create(:epic) }
+
+      it 'does not set the epic' do
+        post_graphql_mutation(mutation, current_user: current_user)
+
+        expect(response).to have_gitlab_http_status(:success)
+        expect(graphql_errors).not_to be_blank
+      end
+    end
+  end
+
+  context 'removing epic' do
+    let(:epic) { create(:epic, group: group) }
+
+    let(:input) do
+      { iid: issue.iid.to_s, epic_id: nil }
+    end
+
+    before do
+      stub_licensed_features(epics: true)
+      group.add_developer(current_user)
+      issue.update!(epic: epic)
+    end
+
+    it 'removes the epic' do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(response).to have_gitlab_http_status(:success)
+      expect(graphql_errors).to be_blank
+      expect(mutated_issue).to include('epic' => be_nil)
+    end
+
+    context 'the epic argument is not provided' do
+      let(:input) do
+        { iid: issue.iid.to_s, weight: 1 }
+      end
+
+      it 'does not remove the epic' do
+        post_graphql_mutation(mutation, current_user: current_user)
+
+        expect(response).to have_gitlab_http_status(:success)
+        expect(graphql_errors).to be_blank
+        expect(mutated_issue).to include('epic' => be_present)
+      end
+    end
   end
 end

spec/graphql/types/global_id_type_spec.rb

@@ -45,8 +45,7 @@ RSpec.describe Types::GlobalIDType do
     end
 
     it 'rejects nil' do
-      expect { described_class.coerce_isolated_input(nil) }
-        .to raise_error(GraphQL::CoercionError)
+      expect(described_class.coerce_isolated_input(nil)).to be_nil
     end
 
     it 'rejects gids from different apps' do