Commit 415f0254 authored by James Fargher's avatar James Fargher

Merge branch 'ajk-graphql-accept-mr' into 'master'

Add mutation to accept merge requests

See merge request gitlab-org/gitlab!54758
parents 40f9d22d 91a81f2c
# 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
......@@ -6,6 +6,12 @@ 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,
......
......@@ -5,6 +5,10 @@ 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,
......@@ -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)
......
......@@ -5,6 +5,7 @@ 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,
......
# 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
......@@ -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
......
......@@ -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')
---
title: Add mutation to accept merge requests
merge_request: 54758
author:
type: added
......@@ -2784,6 +2784,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.
......@@ -5636,6 +5646,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.
......
# 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
# 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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment