diff --git a/CHANGELOG b/CHANGELOG index 66d23dcfd4f6313b5768db11ddbc42b00cfc9475..9d4a15c7df8f7e696c1eecd1450565040e7045d6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.12.0 (unreleased) + - Disable changing of the source branch in merge request update API (Stan Hu) - Shorten merge request WIP text. - Add option to disallow users from registering any application to use GitLab as an OAuth provider - Support editing target branch of merge request (Stan Hu) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 34c190bf62125eb28ec15f3bdc7c7e39b0a0470f..4f6c6cba9a90dde625cd992a9bad89f169765f4e 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -5,10 +5,11 @@ require_relative 'close_service' module MergeRequests class UpdateService < MergeRequests::BaseService def execute(merge_request) - # We don't allow change of source/target projects + # We don't allow change of source/target projects and source branch # after merge request was created params.except!(:source_project_id) params.except!(:target_project_id) + params.except!(:source_branch) state = params[:state_event] diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index c1d82ad9576a1043fde82c98d645bb0a5fada8f6..7b0873a9111c4e26d6fa28fa95185700280fc994 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -221,7 +221,7 @@ If an error occurs, an error number and a message explaining the reason is retur ## Update MR -Updates an existing merge request. You can change branches, title, or even close the MR. +Updates an existing merge request. You can change the target branch, title, or even close the MR. ``` PUT /projects/:id/merge_request/:merge_request_id @@ -231,7 +231,6 @@ Parameters: - `id` (required) - The ID of a project - `merge_request_id` (required) - ID of MR -- `source_branch` - The source branch - `target_branch` - The target branch - `assignee_id` - Assignee user ID - `title` - Title of MR @@ -242,7 +241,6 @@ Parameters: { "id": 1, "target_branch": "master", - "source_branch": "test1", "project_id": 3, "title": "test1", "description": "description1", diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2216a12a87a5638f59c843736570bb1b211c2011..d835dce2deda516937a8aa646361b2faa5606ac4 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -137,7 +137,6 @@ module API # Parameters: # id (required) - The ID of a project # merge_request_id (required) - ID of MR - # source_branch - The source branch # target_branch - The target branch # assignee_id - Assignee user ID # title - Title of MR @@ -148,10 +147,15 @@ module API # PUT /projects/:id/merge_request/:merge_request_id # put ":id/merge_request/:merge_request_id" do - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] + attrs = attributes_for_keys [:target_branch, :assignee_id, :title, :state_event, :description] merge_request = user_project.merge_requests.find(params[:merge_request_id]) authorize! :modify_merge_request, merge_request + # Ensure source_branch is not specified + if params[:source_branch].present? + render_api_error!('Source branch cannot be changed', 400) + end + # Validate label names in advance if (errors = validate_label_params(params)).any? render_api_error!({ labels: errors }, 400) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index dcd50f73326d21e5331b4c819ba315b041127906..0ed5883914b377fe84b8625d9688f72d738e3063 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -349,10 +349,10 @@ describe API::API, api: true do expect(json_response['description']).to eq('New description') end - it "should return 422 when source_branch and target_branch are renamed the same" do + it "should return 400 when source_branch is specified" do put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), source_branch: "master", target_branch: "master" - expect(response.status).to eq(422) + expect(response.status).to eq(400) end it "should return merge_request with renamed target_branch" do