Fix remove_source_branch merge request API handlingsh-fix-merge-requests-api-remove-branch-param

Users attempting to set merge requests to `remove_source_branch` to
`false` would encounter an Error 500 because the UpdateService and API
checked `present?`, which would always return `false`. We now just
use `has_key?` to decide whether the parameter is present.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/60530

4 files changed, 33 insertions, 2 deletions

diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index faaa4d66726..55546432ce4 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -16,7 +16,7 @@ module MergeRequests
         params.delete(:force_remove_source_branch)
       end
 
-      if params[:force_remove_source_branch].present?
+      if params.has_key?(:force_remove_source_branch)
         merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
       end
 
diff --git a/changelogs/unreleased/sh-fix-merge-requests-api-remove-branch-param.yml b/changelogs/unreleased/sh-fix-merge-requests-api-remove-branch-param.yml
new file mode 100644
index 00000000000..d13c972ccc9
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-merge-requests-api-remove-branch-param.yml
@@ -0,0 +1,5 @@
+---
+title: Fix remove_source_branch merge request API handling
+merge_request: 27392
+author:
+type: fixed
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 1cc0ecc6df8..ce85772e4ed 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -336,7 +336,7 @@ module API
         merge_request = find_merge_request_with_access(params.delete(:merge_request_iid), :update_merge_request)
 
         mr_params = declared_params(include_missing: false)
-        mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
+        mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params.has_key?(:remove_source_branch)
         mr_params = convert_parameters_from_legacy_format(mr_params)
 
         merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 45818edbf68..5c94a87529b 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1596,6 +1596,32 @@ describe API::MergeRequests do
   end
 
   describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
+    context 'updates force_remove_source_branch properly' do
+      it 'sets to false' do
+        merge_request.update(merge_params: { 'force_remove_source_branch' => true } )
+
+        expect(merge_request.force_remove_source_branch?).to be_truthy
+
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: "close", remove_source_branch: false }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['state']).to eq('closed')
+        expect(json_response['force_remove_source_branch']).to be_falsey
+      end
+
+      it 'sets to true' do
+        merge_request.update(merge_params: { 'force_remove_source_branch' => false } )
+
+        expect(merge_request.force_remove_source_branch?).to be_falsey
+
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: "close", remove_source_branch: true }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['state']).to eq('closed')
+        expect(json_response['force_remove_source_branch']).to be_truthy
+      end
+    end
+
     context "to close a MR" do
       it "returns merge_request" do
         put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: "close" }