Fix updating merge_when_build_succeeds via merge API endpointmrchrisw-22740-merge-api

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

3 files changed, 21 insertions, 4 deletions

diff --git a/changelogs/unreleased/mrchrisw-22740-merge-api.yml b/changelogs/unreleased/mrchrisw-22740-merge-api.yml
new file mode 100644
index 00000000000..e75160aec70
--- /dev/null
+++ b/changelogs/unreleased/mrchrisw-22740-merge-api.yml
@@ -0,0 +1,4 @@
+---
+title: Fix updating merge_when_build_succeeds via merge API endpoint
+merge_request: 10873
+author: 
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index cb7aec47cf0..c7dc2ea336f 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -197,14 +197,15 @@ module API
       end
       put ':id/merge_requests/:merge_request_iid/merge' do
         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
         # because user dont have permissions to push into target branch
         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]
           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]
         }
 
-        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
             .new(merge_request.target_project, current_user, merge_params)
             .execute(merge_request)
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 61d965e8974..41885659190 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -6,7 +6,7 @@ describe API::MergeRequests, api: true  do
   let(:user)        { create(:user) }
   let(:admin)       { create(:user, :admin) }
   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_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') }
@@ -527,6 +527,18 @@ describe API::MergeRequests, api: true  do
       expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
     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
       put api("/projects/#{project.id}/merge_requests/12345/merge", user)