Add mutation to accept merge requests

This is based mainly on the merge requests controller, since that
supports a wider range of merge strategies.

This verifies that we can merge:

- immediately
- MWPS
- using merge trains
- adding to merge trains when pipeline succeeds

Includes:

Move note mutation descriptions to classes

This is a rubocop appeasement change that moves the descriptions to the
mutation definitions, keeping the mutation mount points cleaner.

app/graphql/mutations/merge_requests/accept.rb

+# frozen_string_literal: true
+
+module Mutations
+  module MergeRequests
+    class Accept < Base
+      NOT_MERGEABLE = 'This branch cannot be merged'
+      HOOKS_VALIDATION_ERROR = 'Pre-merge hooks failed'
+      SHA_MISMATCH = 'The merge-head is not at the anticipated SHA'
+      MERGE_FAILED = 'The merge failed'
+      ALREADY_SCHEDULED = 'The merge request is already scheduled to be merged'
+
+      graphql_name 'MergeRequestAccept'
+      authorize :accept_merge_request
+      description <<~DESC
+        Accepts a merge request.
+        When accepted, the source branch will be merged into the target branch, either
+        immediately if possible, or using one of the automatic merge strategies.
+      DESC
+
+      argument :strategy,
+               ::Types::MergeStrategyEnum,
+               required: false,
+               as: :auto_merge_strategy,
+               description: 'How to merge this merge request.'
+
+      argument :commit_message, ::GraphQL::STRING_TYPE,
+               required: false,
+               description: 'Custom merge commit message.'
+      argument :squash_commit_message, ::GraphQL::STRING_TYPE,
+               required: false,
+               description: 'Custom squash commit message (if squash is true).'
+      argument :sha, ::GraphQL::STRING_TYPE,
+               required: true,
+               description: 'The HEAD SHA at the time when this merge was requested.'
+
+      argument :should_remove_source_branch, ::GraphQL::BOOLEAN_TYPE,
+               required: false,
+               description: 'Should the source branch be removed.'
+      argument :squash, ::GraphQL::BOOLEAN_TYPE,
+               required: false,
+               default_value: false,
+               description: 'Squash commits on the source branch before merge.'
+
+      def resolve(project_path:, iid:, **args)
+        Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42317')
+        merge_request = authorized_find!(project_path: project_path, iid: iid)
+        project = merge_request.target_project
+        merge_params = args.compact.with_indifferent_access
+        merge_service = ::MergeRequests::MergeService.new(project, current_user, merge_params)
+
+        if error = validate(merge_request, merge_service, merge_params)
+          return { merge_request: merge_request, errors: [error] }
+        end
+
+        merge_request.update(merge_error: nil, squash: merge_params[:squash])
+
+        result = if merge_params.key?(:auto_merge_strategy)
+                   service = AutoMergeService.new(project, current_user, merge_params)
+                   service.execute(merge_request, merge_params[:auto_merge_strategy])
+                 else
+                   merge_service.execute(merge_request)
+                 end
+
+        {
+          merge_request: merge_request,
+          errors: result == :failed ? [MERGE_FAILED] : []
+        }
+      rescue ::MergeRequests::MergeBaseService::MergeError => e
+        {
+          merge_request: merge_request,
+          errors: [e.message]
+        }
+      end
+
+      def validate(merge_request, merge_service, merge_params)
+        if merge_request.auto_merge_enabled?
+          ALREADY_SCHEDULED
+        elsif !merge_request.mergeable?(skip_ci_check: merge_params.key?(:auto_merge_strategy))
+          NOT_MERGEABLE
+        elsif !merge_service.hooks_validation_pass?(merge_request)
+          HOOKS_VALIDATION_ERROR
+        elsif merge_params[:sha] != merge_request.diff_head_sha
+          SHA_MISMATCH
+        end
+      end
+    end
+  end
+end

app/graphql/mutations/notes/update/base.rb

@@ -6,12 +6,18 @@ module Mutations
       # This is a Base class for the Note update mutations and is not
       # mounted as a GraphQL mutation itself.
       class Base < Mutations::Notes::Base
+        QUICK_ACTION_ONLY_WARNING = <<~NB
+          If the body of the Note contains only quick actions,
+          the Note will be destroyed during the update, and no Note will be
+          returned.
+        NB
+
         authorize :admin_note
 
         argument :id,
-                  ::Types::GlobalIDType[::Note],
-                  required: true,
-                  description: 'The global ID of the note to update.'
+                 ::Types::GlobalIDType[::Note],
+                 required: true,
+                 description: 'The global ID of the note to update.'
 
         def resolve(args)
           note = authorized_find!(id: args[:id])

app/graphql/mutations/notes/update/image_diff_note.rb

@@ -5,16 +5,20 @@ module Mutations
     module Update
       class ImageDiffNote < Mutations::Notes::Update::Base
         graphql_name 'UpdateImageDiffNote'
+        description <<~DESC
+          Updates a DiffNote on an image (a `Note` where the `position.positionType` is `"image"`).
+          #{QUICK_ACTION_ONLY_WARNING}
+        DESC
 
         argument :body,
-                  GraphQL::STRING_TYPE,
-                  required: false,
-                  description: copy_field_description(Types::Notes::NoteType, :body)
+                 GraphQL::STRING_TYPE,
+                 required: false,
+                 description: copy_field_description(Types::Notes::NoteType, :body)
 
         argument :position,
-                  Types::Notes::UpdateDiffImagePositionInputType,
-                  required: false,
-                  description: copy_field_description(Types::Notes::NoteType, :position)
+                 Types::Notes::UpdateDiffImagePositionInputType,
+                 required: false,
+                 description: copy_field_description(Types::Notes::NoteType, :position)
 
         def ready?(**args)
           # As both arguments are optional, validate here that one of the
@@ -34,10 +38,9 @@ module Mutations
         private
 
         def pre_update_checks!(note, _args)
-          unless note.is_a?(DiffNote) && note.position.on_image?
-            raise Gitlab::Graphql::Errors::ResourceNotAvailable,
-                  'Resource is not an ImageDiffNote'
-          end
+          return if note.is_a?(DiffNote) && note.position.on_image?
+
+          raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Resource is not an ImageDiffNote'
         end
 
         def note_params(note, args)

app/graphql/mutations/notes/update/note.rb

@@ -5,16 +5,17 @@ module Mutations
     module Update
       class Note < Mutations::Notes::Update::Base
         graphql_name 'UpdateNote'
+        description "Updates a Note.\n#{QUICK_ACTION_ONLY_WARNING}"
 
         argument :body,
-                  GraphQL::STRING_TYPE,
-                  required: false,
-                  description: copy_field_description(Types::Notes::NoteType, :body)
+                 GraphQL::STRING_TYPE,
+                 required: false,
+                 description: copy_field_description(Types::Notes::NoteType, :body)
 
         argument :confidential,
-                  GraphQL::BOOLEAN_TYPE,
-                  required: false,
-                  description: 'The confidentiality flag of a note. Default is false.'
+                 GraphQL::BOOLEAN_TYPE,
+                 required: false,
+                 description: 'The confidentiality flag of a note. Default is false.'
 
         private
 

app/graphql/types/merge_strategy_enum.rb

+# frozen_string_literal: true
+
+module Types
+  class MergeStrategyEnum < BaseEnum
+    AutoMergeService.all_strategies_ordered_by_preference.each do |strat|
+      value strat.upcase, value: strat, description: "Use the #{strat} merge strategy."
+    end
+  end
+end

app/graphql/types/mutation_type.rb

@@ -44,6 +44,7 @@ module Types
     mount_mutation Mutations::Issues::Update
     mount_mutation Mutations::Issues::Move
     mount_mutation Mutations::Labels::Create
+    mount_mutation Mutations::MergeRequests::Accept
     mount_mutation Mutations::MergeRequests::Create
     mount_mutation Mutations::MergeRequests::Update
     mount_mutation Mutations::MergeRequests::SetLabels
@@ -58,14 +59,8 @@ module Types
     mount_mutation Mutations::Notes::Create::Note, calls_gitaly: true
     mount_mutation Mutations::Notes::Create::DiffNote, calls_gitaly: true
     mount_mutation Mutations::Notes::Create::ImageDiffNote, calls_gitaly: true
-    mount_mutation Mutations::Notes::Update::Note,
-                   description: 'Updates a Note. If the body of the Note contains only quick actions, ' \
-                                'the Note will be destroyed during the update, and no Note will be ' \
-                                'returned'
-    mount_mutation Mutations::Notes::Update::ImageDiffNote,
-                   description: 'Updates a DiffNote on an image (a `Note` where the `position.positionType` is `"image"`). ' \
-                                'If the body of the Note contains only quick actions, the Note will be ' \
-                                'destroyed during the update, and no Note will be returned'
+    mount_mutation Mutations::Notes::Update::Note
+    mount_mutation Mutations::Notes::Update::ImageDiffNote
     mount_mutation Mutations::Notes::RepositionImageDiffNote
     mount_mutation Mutations::Notes::Destroy
     mount_mutation Mutations::Releases::Create

app/policies/merge_request_policy.rb

@@ -9,7 +9,10 @@ class MergeRequestPolicy < IssuablePolicy
   # Although :read_merge_request is computed in the policy context,
   # it would not be safe to prevent :create_note there, since
   # note permissions are shared, and this would apply too broadly.
-  rule { ~can?(:read_merge_request) }.prevent :create_note
+  rule { ~can?(:read_merge_request) }.policy do
+    prevent :create_note
+    prevent :accept_merge_request
+  end
 
   rule { can?(:update_merge_request) }.policy do
     enable :approve_merge_request
@@ -18,6 +21,12 @@ class MergeRequestPolicy < IssuablePolicy
   rule { ~anonymous & can?(:read_merge_request) }.policy do
     enable :create_todo
   end
+
+  condition(:can_merge) { @subject.can_be_merged_by?(@user) }
+
+  rule { can_merge }.policy do
+    enable :accept_merge_request
+  end
 end
 
 MergeRequestPolicy.prepend_if_ee('EE::MergeRequestPolicy')

changelogs/unreleased/ajk-graphql-accept-mr.yml

+---
+title: Add mutation to accept merge requests
+merge_request: 54758
+author:
+type: added

doc/api/graphql/reference/index.md

@@ -2768,6 +2768,16 @@ Autogenerated return type of MarkAsSpamSnippet.
 | `webUrl` | String | Web URL of the merge request. |
 | `workInProgress` | Boolean! | Indicates if the merge request is a draft. |
 
+### `MergeRequestAcceptPayload`
+
+Autogenerated return type of MergeRequestAccept.
+
+| Field | Type | Description |
+| ----- | ---- | ----------- |
+| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
+| `errors` | String! => Array | Errors encountered during execution of the mutation. |
+| `mergeRequest` | MergeRequest | The merge request after mutation. |
+
 ### `MergeRequestCreatePayload`
 
 Autogenerated return type of MergeRequestCreate.
@@ -5620,6 +5630,14 @@ State of a GitLab merge request.
 | `merged` | Merge Request has been merged. |
 | `opened` | In open state. |
 
+### `MergeStrategyEnum`
+
+| Value | Description |
+| ----- | ----------- |
+| `ADD_TO_MERGE_TRAIN_WHEN_PIPELINE_SUCCEEDS` | Use the add_to_merge_train_when_pipeline_succeeds merge strategy. |
+| `MERGE_TRAIN` | Use the merge_train merge strategy. |
+| `MERGE_WHEN_PIPELINE_SUCCEEDS` | Use the merge_when_pipeline_succeeds merge strategy. |
+
 ### `MilestoneStateEnum`
 
 Current state of milestone.

ee/spec/graphql/mutations/merge_requests/accept_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::MergeRequests::Accept do
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project, :public, :repository) }
+
+  subject(:mutation) { described_class.new(context: context, object: nil, field: nil) }
+
+  let_it_be(:context) do
+    GraphQL::Query::Context.new(
+      query: OpenStruct.new(schema: GitlabSchema),
+      values: { current_user: user },
+      object: nil
+    )
+  end
+
+  def mutation_arguments(merge_request)
+    {
+      project_path: project.full_path,
+      iid: merge_request.iid.to_s,
+      sha: merge_request.diff_head_sha,
+      squash: false
+    }
+  end
+
+  describe '#resolve' do
+    before do
+      project.add_maintainer(user)
+      stub_licensed_features(merge_pipelines: true, merge_trains: true)
+      stub_feature_flags(disable_merge_trains: false)
+      project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
+    end
+
+    it "can use the MERGE_TRAIN strategy" do
+      enum = ::Types::MergeStrategyEnum.values['MERGE_TRAIN']
+      merge_request = create(:merge_request, :with_test_reports,
+                             source_project: project)
+
+      args = mutation_arguments(merge_request).merge(
+        auto_merge_strategy: enum.value
+      )
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(merge_request: be_auto_merge_enabled)
+    end
+
+    it "can use the ADD_TO_MERGE_TRAIN_WHEN_PIPELINE_SUCCEEDS strategy" do
+      enum = ::Types::MergeStrategyEnum.values['ADD_TO_MERGE_TRAIN_WHEN_PIPELINE_SUCCEEDS']
+      merge_request = create(:merge_request, :with_head_pipeline,
+                             source_project: project)
+
+      args = mutation_arguments(merge_request).merge(
+        auto_merge_strategy: enum.value
+      )
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(merge_request: be_auto_merge_enabled)
+    end
+  end
+end

spec/graphql/mutations/merge_requests/accept_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::MergeRequests::Accept do
+  include AfterNextHelpers
+
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project, :public, :repository) }
+
+  subject(:mutation) { described_class.new(context: context, object: nil, field: nil) }
+
+  let_it_be(:context) do
+    GraphQL::Query::Context.new(
+      query: OpenStruct.new(schema: GitlabSchema),
+      values: { current_user: user },
+      object: nil
+    )
+  end
+
+  before do
+    project.repository.expire_all_method_caches
+  end
+
+  describe '#resolve' do
+    before do
+      project.add_maintainer(user)
+    end
+
+    def common_args(merge_request)
+      {
+        project_path: project.full_path,
+        iid: merge_request.iid.to_s,
+        sha: merge_request.diff_head_sha,
+        squash: false # default value
+      }
+    end
+
+    it 'merges the merge request' do
+      merge_request = create(:merge_request, source_project: project)
+
+      result = mutation.resolve(**common_args(merge_request))
+
+      expect(result).to include(errors: be_empty, merge_request: be_merged)
+    end
+
+    it 'rejects the mutation if the SHA is a mismatch' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request).merge(sha: 'not a good sha')
+
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(errors: [described_class::SHA_MISMATCH])
+    end
+
+    it 'respects the merge commit message' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request).merge(commit_message: 'my super custom message')
+
+      result = mutation.resolve(**args)
+
+      expect(result).to include(merge_request: be_merged)
+      expect(project.repository.commit(merge_request.target_branch)).to have_attributes(
+        message: args[:commit_message]
+      )
+    end
+
+    it 'respects the squash flag' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request).merge(squash: true)
+
+      result = mutation.resolve(**args)
+
+      expect(result).to include(merge_request: be_merged)
+      expect(result[:merge_request].squash_commit_sha).to be_present
+    end
+
+    it 'respects the squash_commit_message argument' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request).merge(squash: true, squash_commit_message: 'squish')
+
+      result = mutation.resolve(**args)
+      sha = result[:merge_request].squash_commit_sha
+
+      expect(result).to include(merge_request: be_merged)
+      expect(project.repository.commit(sha)).to have_attributes(message: "squish\n")
+    end
+
+    it 'respects the should_remove_source_branch argument when true' do
+      b = project.repository.add_branch(user, generate(:branch), 'master')
+      merge_request = create(:merge_request, source_branch: b.name, source_project: project)
+      args = common_args(merge_request).merge(should_remove_source_branch: true)
+
+      expect(::MergeRequests::DeleteSourceBranchWorker).to receive(:perform_async)
+
+      result = mutation.resolve(**args)
+
+      expect(result).to include(merge_request: be_merged)
+    end
+
+    it 'respects the should_remove_source_branch argument when false' do
+      b = project.repository.add_branch(user, generate(:branch), 'master')
+      merge_request = create(:merge_request, source_branch: b.name, source_project: project)
+      args = common_args(merge_request).merge(should_remove_source_branch: false)
+
+      expect(::MergeRequests::DeleteSourceBranchWorker).not_to receive(:perform_async)
+
+      result = mutation.resolve(**args)
+
+      expect(result).to include(merge_request: be_merged)
+    end
+
+    it 'rejects unmergeable MRs' do
+      merge_request = create(:merge_request, :closed, source_project: project)
+      args = common_args(merge_request)
+
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(errors: [described_class::NOT_MERGEABLE])
+    end
+
+    it 'rejects merges when we cannot validate the hooks' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request)
+      expect_next(::MergeRequests::MergeService)
+        .to receive(:hooks_validation_pass?).with(merge_request).and_return(false)
+
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(errors: [described_class::HOOKS_VALIDATION_ERROR])
+    end
+
+    it 'rejects merges when the merge service returns an error' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request)
+      expect_next(::MergeRequests::MergeService)
+        .to receive(:execute).with(merge_request).and_return(:failed)
+
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(errors: [described_class::MERGE_FAILED])
+    end
+
+    it 'rejects merges when the merge service raises merge error' do
+      merge_request = create(:merge_request, source_project: project)
+      args = common_args(merge_request)
+      expect_next(::MergeRequests::MergeService)
+        .to receive(:execute).and_raise(::MergeRequests::MergeBaseService::MergeError, 'boom')
+
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(errors: ['boom'])
+    end
+
+    it "can use the MERGE_WHEN_PIPELINE_SUCCEEDS strategy" do
+      enum = ::Types::MergeStrategyEnum.values['MERGE_WHEN_PIPELINE_SUCCEEDS']
+      merge_request = create(:merge_request, :with_head_pipeline, source_project: project)
+      args = common_args(merge_request).merge(auto_merge_strategy: enum.value)
+
+      result = mutation.resolve(**args)
+
+      expect(result).not_to include(merge_request: be_merged)
+      expect(result).to include(errors: be_empty, merge_request: be_auto_merge_enabled)
+    end
+  end
+end

spec/requests/api/graphql/mutations/merge_requests/accept_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'accepting a merge request', :request_store do
+  include GraphqlHelpers
+
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:project) { create(:project, :public, :repository) }
+  let!(:merge_request) { create(:merge_request, source_project: project) }
+  let(:input) do
+    {
+      project_path: project.full_path,
+      iid: merge_request.iid.to_s,
+      sha: merge_request.diff_head_sha
+    }
+  end
+
+  let(:mutation) { graphql_mutation(:merge_request_accept, input, 'mergeRequest { state }') }
+  let(:mutation_response) { graphql_mutation_response(:merge_request_accept) }
+
+  context 'when the user is not allowed to accept a merge request' do
+    before do
+      project.add_reporter(current_user)
+    end
+
+    it_behaves_like 'a mutation that returns a top-level access error'
+  end
+
+  context 'when user has permissions to create a merge request' do
+    before do
+      project.add_maintainer(current_user)
+    end
+
+    it 'merges the merge request' do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(response).to have_gitlab_http_status(:success)
+      expect(mutation_response['mergeRequest']).to include(
+        'state' => 'merged'
+      )
+    end
+  end
+end