Fix remove_source_branch merge request API handling
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
This commit is contained in:
		
							parent
							
								
									1a50801cd0
								
							
						
					
					
						commit
						d3fa9c9539
					
				|  | @ -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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -0,0 +1,5 @@ | |||
| --- | ||||
| title: Fix remove_source_branch merge request API handling | ||||
| merge_request: 27392 | ||||
| author: | ||||
| type: fixed | ||||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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" } | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue