diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2521a34e1c04b6569eaf74945a8629578d10a985..f725d8c78ebf4f52bf0904be11720429630ad320 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -98,8 +98,10 @@ def merge_request_pipelines_with_access end def automatically_mergeable?(merge_when_pipeline_succeeds, merge_request) - pipeline_active = merge_request.head_pipeline_active? || merge_request.diff_head_pipeline_active? - merge_when_pipeline_succeeds && merge_request.mergeable_state?(skip_ci_check: true) && pipeline_active + available_strategies = AutoMergeService.new(merge_request.project, + current_user).available_strategies(merge_request) + + merge_when_pipeline_succeeds && available_strategies.include?(merge_request.default_auto_merge_strategy) end def immediately_mergeable?(merge_when_pipeline_succeeds, merge_request) @@ -709,7 +711,7 @@ def order_merge_requests(merge_requests) not_allowed! if !immediately_mergeable && !automatically_mergeable - render_api_error!('Branch cannot be merged', 422) unless merge_request.mergeable?(skip_ci_check: automatically_mergeable) + render_api_error!('Branch cannot be merged', 422) unless automatically_mergeable || merge_request.mergeable?(skip_ci_check: automatically_mergeable) check_sha_param!(params, merge_request) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 2cb70a2866bda1306000bb2f6dcbc8bd4b381dec..9c2d54acfd0e94c6e06a07bbc7b6c2c9f441617e 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -3006,6 +3006,52 @@ end end + shared_examples 'merging with auto merge strategies' do + it 'does not merge if merge_when_pipeline_succeeds is passed and the pipeline has failed' do + create(:ci_pipeline, + :failed, + sha: merge_request.diff_head_sha, + merge_requests_as_head_pipeline: [merge_request]) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + + expect(response).to have_gitlab_http_status(:method_not_allowed) + expect(merge_request.reload.state).to eq('opened') + end + + it 'merges if the head pipeline already succeeded and `merge_when_pipeline_succeeds` is passed' do + create(:ci_pipeline, :success, sha: merge_request.diff_head_sha, merge_requests_as_head_pipeline: [merge_request]) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['state']).to eq('merged') + end + + it "enables auto merge if the pipeline is active" do + allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) + allow(pipeline).to receive(:active?).and_return(true) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq('Test') + expect(json_response['merge_when_pipeline_succeeds']).to eq(true) + end + + it "enables auto merge if the pipeline is active and only_allow_merge_if_pipeline_succeeds is true" do + allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) + allow(pipeline).to receive(:active?).and_return(true) + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq('Test') + expect(json_response['merge_when_pipeline_succeeds']).to eq(true) + end + end + describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge", :clean_gitlab_redis_cache do let(:project) { create(:project, :repository, namespace: user.namespace) } let(:merge_request) { create(:merge_request, :simple, author: user, source_project: project, source_branch: 'markdown', title: 'Test') } @@ -3117,50 +3163,80 @@ expect(response).to have_gitlab_http_status(:ok) end - it 'does not merge if merge_when_pipeline_succeeds is passed and the pipeline has failed' do - create(:ci_pipeline, - :failed, - sha: merge_request.diff_head_sha, - merge_requests_as_head_pipeline: [merge_request]) + context 'when merge_when_checks_pass is off' do + before do + stub_feature_flags(merge_when_checks_pass: false) + end - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + it_behaves_like 'merging with auto merge strategies' - expect(response).to have_gitlab_http_status(:method_not_allowed) - expect(merge_request.reload.state).to eq('opened') - end + it 'does not enable auto merge if MR is not mergeable and only_allow_merge_if_pipeline_succeeds is true' do + allow_any_instance_of(MergeRequest) + .to receive_messages( + head_pipeline: pipeline, + diff_head_pipeline: pipeline + ) - it 'merges if the head pipeline already succeeded and `merge_when_pipeline_succeeds` is passed' do - create(:ci_pipeline, :success, sha: merge_request.diff_head_sha, merge_requests_as_head_pipeline: [merge_request]) + merge_request.update!(title: 'Draft: 1234') - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['state']).to eq('merged') - end + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } - it "enables merge when pipeline succeeds if the pipeline is active" do - allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) - allow(pipeline).to receive(:active?).and_return(true) + expect(response).to have_gitlab_http_status(:method_not_allowed) + expect(merge_request.reload.state).to eq('opened') + end - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + context 'when the pipeline failed' do + let(:pipeline) { create(:ci_pipeline, :failed, project: project) } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to eq('Test') - expect(json_response['merge_when_pipeline_succeeds']).to eq(true) + it 'does not enable auto merge if the pipeline failed and only_allow_merge_if_pipeline_succeeds is true' do + allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + + expect(response).to have_gitlab_http_status(:method_not_allowed) + expect(merge_request.reload.state).to eq('opened') + end + end end - it "enables merge when pipeline succeeds if the pipeline is active and only_allow_merge_if_pipeline_succeeds is true" do - allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) - allow(pipeline).to receive(:active?).and_return(true) + it_behaves_like 'merging with auto merge strategies' + + it 'enables auto merge if the MR is not mergeable and only_allow_merge_if_pipeline_succeeds is true' do + allow_any_instance_of(MergeRequest) + .to receive_messages( + head_pipeline: pipeline, + diff_head_pipeline: pipeline + ) + + merge_request.update!(title: 'Draft: 1234') + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to eq('Test') + expect(json_response['title']).to eq('Draft: 1234') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end + context 'when the pipeline failed' do + let(:pipeline) { create(:ci_pipeline, :failed, project: project) } + + it "enables auto merge if the pipeline is failed and only_allow_merge_if_pipeline_succeeds is true" do + allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, diff_head_pipeline: pipeline) + project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq('Test') + expect(json_response['merge_when_pipeline_succeeds']).to eq(true) + end + end + it "returns 404 for an invalid merge request IID" do put api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/merge", user)