From 85145d1d77ed919949d59c83cccecd43789cc781 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 29 May 2015 09:40:35 -0700 Subject: Disable changing of the source branch in merge request update API --- CHANGELOG | 1 + app/services/merge_requests/update_service.rb | 3 ++- doc/api/merge_requests.md | 4 +--- lib/api/merge_requests.rb | 8 ++++++-- spec/requests/api/merge_requests_spec.rb | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 66d23dcfd4f..9d4a15c7df8 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 -- cgit v1.2.1