Update merge options for auto merge strategies
Currently, merge options is updated on #execute method, however, we should have #update interface to make it explicit.
This commit is contained in:
parent
9baff6f6b5
commit
b8c1317152
|
@ -273,9 +273,15 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
|
||||||
@merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false))
|
@merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false))
|
||||||
|
|
||||||
if auto_merge_requested?
|
if auto_merge_requested?
|
||||||
|
if merge_request.auto_merge_enabled?
|
||||||
|
# TODO: We should have a dedicated endpoint for updating merge params.
|
||||||
|
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/63130.
|
||||||
|
AutoMergeService.new(project, current_user, merge_params).update(merge_request)
|
||||||
|
else
|
||||||
AutoMergeService.new(project, current_user, merge_params)
|
AutoMergeService.new(project, current_user, merge_params)
|
||||||
.execute(merge_request,
|
.execute(merge_request,
|
||||||
params[:auto_merge_strategy] || AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
|
params[:auto_merge_strategy] || AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
|
||||||
|
end
|
||||||
else
|
else
|
||||||
@merge_request.merge_async(current_user.id, merge_params)
|
@merge_request.merge_async(current_user.id, merge_params)
|
||||||
|
|
||||||
|
|
|
@ -20,6 +20,14 @@ module AutoMerge
|
||||||
strategy.to_sym
|
strategy.to_sym
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def update(merge_request)
|
||||||
|
merge_request.merge_params.merge!(params)
|
||||||
|
|
||||||
|
return :failed unless merge_request.save
|
||||||
|
|
||||||
|
strategy.to_sym
|
||||||
|
end
|
||||||
|
|
||||||
def cancel(merge_request)
|
def cancel(merge_request)
|
||||||
if cancel_auto_merge(merge_request)
|
if cancel_auto_merge(merge_request)
|
||||||
yield if block_given?
|
yield if block_given?
|
||||||
|
|
|
@ -24,6 +24,12 @@ class AutoMergeService < BaseService
|
||||||
service.execute(merge_request)
|
service.execute(merge_request)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def update(merge_request)
|
||||||
|
return :failed unless merge_request.auto_merge_enabled?
|
||||||
|
|
||||||
|
get_service_instance(merge_request.auto_merge_strategy).update(merge_request)
|
||||||
|
end
|
||||||
|
|
||||||
def process(merge_request)
|
def process(merge_request)
|
||||||
return unless merge_request.auto_merge_enabled?
|
return unless merge_request.auto_merge_enabled?
|
||||||
|
|
||||||
|
|
|
@ -412,7 +412,7 @@ describe Projects::MergeRequestsController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when the pipeline succeeds is passed' do
|
context 'when merge when pipeline succeeds option is passed' do
|
||||||
let!(:head_pipeline) do
|
let!(:head_pipeline) do
|
||||||
create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request)
|
create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request)
|
||||||
end
|
end
|
||||||
|
@ -462,6 +462,30 @@ describe Projects::MergeRequestsController do
|
||||||
expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds')
|
expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when auto merge has not been enabled yet' do
|
||||||
|
it 'calls AutoMergeService#execute' do
|
||||||
|
expect_next_instance_of(AutoMergeService) do |service|
|
||||||
|
expect(service).to receive(:execute).with(merge_request, 'merge_when_pipeline_succeeds')
|
||||||
|
end
|
||||||
|
|
||||||
|
merge_when_pipeline_succeeds
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when auto merge has already been enabled' do
|
||||||
|
before do
|
||||||
|
merge_request.update!(auto_merge_enabled: true, merge_user: user)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'calls AutoMergeService#update' do
|
||||||
|
expect_next_instance_of(AutoMergeService) do |service|
|
||||||
|
expect(service).to receive(:update).with(merge_request)
|
||||||
|
end
|
||||||
|
|
||||||
|
merge_when_pipeline_succeeds
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do
|
describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do
|
||||||
|
|
|
@ -92,6 +92,30 @@ describe AutoMerge::BaseService do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#update' do
|
||||||
|
subject { service.update(merge_request) }
|
||||||
|
|
||||||
|
let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
|
||||||
|
|
||||||
|
context 'when merge params are specified' do
|
||||||
|
let(:params) do
|
||||||
|
{
|
||||||
|
'commit_message' => "Merge branch 'patch-12' into 'master'",
|
||||||
|
'sha' => "200fcc9c260f7219eaf0daba87d818f0922c5b18",
|
||||||
|
'should_remove_source_branch' => false,
|
||||||
|
'squash' => false,
|
||||||
|
'squash_commit_message' => "Update README.md"
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'updates merge params' do
|
||||||
|
expect { subject }.to change {
|
||||||
|
merge_request.reload.merge_params.slice(*params.keys)
|
||||||
|
}.from({}).to(params)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#cancel' do
|
describe '#cancel' do
|
||||||
subject { service.cancel(merge_request) }
|
subject { service.cancel(merge_request) }
|
||||||
|
|
||||||
|
|
|
@ -92,6 +92,30 @@ describe AutoMergeService do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#update' do
|
||||||
|
subject { service.update(merge_request) }
|
||||||
|
|
||||||
|
context 'when auto merge is enabled' do
|
||||||
|
let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
|
||||||
|
|
||||||
|
it 'delegates to a relevant service instance' do
|
||||||
|
expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service|
|
||||||
|
expect(service).to receive(:update).with(merge_request)
|
||||||
|
end
|
||||||
|
|
||||||
|
subject
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when auto merge is not enabled' do
|
||||||
|
let(:merge_request) { create(:merge_request) }
|
||||||
|
|
||||||
|
it 'returns failed' do
|
||||||
|
is_expected.to eq(:failed)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#process' do
|
describe '#process' do
|
||||||
subject { service.process(merge_request) }
|
subject { service.process(merge_request) }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue