Merge branch 'disable-mr-source-branch-change' into 'master'

Disable changing of the source branch in merge request update API

As noted in: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/738#note_1307284

See merge request !745

5 files changed, 12 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index c6d3d5b801c..61e9084a39e 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 34c190bf621..4f6c6cba9a9 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 c1d82ad9576..7b0873a9111 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 2216a12a87a..d835dce2ded 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 dcd50f73326..0ed5883914b 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