kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Fix updating merge_when_build_succeeds via merge API endpoint

Browse source 
When updating a merge request via the `/merge` endpoint we
check the `mergeable` and `mergeable_state` status, these will return
`false` if the application option only_allow_merge_if_pipeline_succeeds is
enabled. We should skip CI checks if the request uses the
merge_when_pipeline_succeeds param

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/22740
This commit is contained in:
Chris Wilson 2017-04-24 16:53:43 +10:00
parent c768026474
commit e670cb271e
3 changed files with 21 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
changelogs/unreleased/mrchrisw-22740-merge-api.yml Normal file
View file

@ -0,0 +1,4 @@
---
title: Fix updating merge_when_build_succeeds via merge API endpoint
merge_request: 10873
author:

7
lib/api/merge_requests.rb
View file

@ -197,14 +197,15 @@ module API
end end
put ':id/merge_requests/:merge_request_iid/merge' do put ':id/merge_requests/:merge_request_iid/merge' do
merge_request = find_project_merge_request(params[:merge_request_iid]) merge_request = find_project_merge_request(params[:merge_request_iid])
merge_when_pipeline_succeeds = to_boolean(params[:merge_when_pipeline_succeeds])
# Merge request can not be merged # Merge request can not be merged
# because user dont have permissions to push into target branch # because user dont have permissions to push into target branch
unauthorized! unless merge_request.can_be_merged_by?(current_user) unauthorized! unless merge_request.can_be_merged_by?(current_user)
not_allowed! unless merge_request.mergeable_state? not_allowed! unless merge_request.mergeable_state?(skip_ci_check: merge_when_pipeline_succeeds)
render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable? render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable?(skip_ci_check: merge_when_pipeline_succeeds)
if params[:sha] && merge_request.diff_head_sha != params[:sha] if params[:sha] && merge_request.diff_head_sha != params[:sha]
render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409) render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
@ -215,7 +216,7 @@ module API
should_remove_source_branch: params[:should_remove_source_branch] should_remove_source_branch: params[:should_remove_source_branch]
} }
if params[:merge_when_pipeline_succeeds] && merge_request.head_pipeline && merge_request.head_pipeline.active? if merge_when_pipeline_succeeds && merge_request.head_pipeline && merge_request.head_pipeline.active?
::MergeRequests::MergeWhenPipelineSucceedsService ::MergeRequests::MergeWhenPipelineSucceedsService
.new(merge_request.target_project, current_user, merge_params) .new(merge_request.target_project, current_user, merge_params)
.execute(merge_request) .execute(merge_request)

14
spec/requests/api/merge_requests_spec.rb
View file

@ -6,7 +6,7 @@ describe API::MergeRequests, api: true do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:admin) { create(:user, :admin) } let(:admin) { create(:user, :admin) }
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace) } let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, title: "Test", created_at: base_time) } let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, title: "Test", created_at: base_time) }
let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, title: "Closed test", created_at: base_time + 1.second) } let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, title: "Closed test", created_at: base_time + 1.second) }
let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') } let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') }
@ -527,6 +527,18 @@ describe API::MergeRequests, api: true do
expect(json_response['merge_when_pipeline_succeeds']).to eq(true) expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
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(:head_pipeline).and_return(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), merge_when_pipeline_succeeds: true
expect(response).to have_http_status(200)
expect(json_response['title']).to eq('Test')
expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
end
it "returns 404 for an invalid merge request IID" do it "returns 404 for an invalid merge request IID" do
put api("/projects/#{project.id}/merge_requests/12345/merge", user) put api("/projects/#{project.id}/merge_requests/12345/merge", user)