diff options
author | Z.J. van de Weg <zegerjan@gitlab.com> | 2016-07-21 12:13:50 +0200 |
---|---|---|
committer | Z.J. van de Weg <zegerjan@gitlab.com> | 2016-08-10 10:51:59 +0200 |
commit | 95c1cb9f2e9f7e3dc1e0458e0e7fd7c4fdb82a77 (patch) | |
tree | e778285ca2ca7a2da0c61b8a797319e68456d7df /lib | |
parent | 4ef6b1fbb31d4ab0b91c2bb3bd99ce18dc40b229 (diff) | |
download | gitlab-ce-zj-refactor-merge-params.tar.gz |
Create remove_source_branch field on MergeRequestzj-refactor-merge-params
A few releases back a checkbox was introduced to remove the source
branch once the MR was merged. With this checkbox the key
`force_remove_source_branch` was introduced, while
`should_remove_source_branch` was already introduced at the time that
merge when build succeeds was implemented. To me this warranted the
creation of a field `remove_source_branch`. A migration has been added,
and tested on PG only at this time. MySQL should be done still.
Also, this commit updates the style for the API docs on MR's. This was
done as I wanted to be a good boyscout, hope the MR stays foccussed
enough.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/entities.rb | 4 | ||||
-rw-r--r-- | lib/api/merge_requests.rb | 46 |
2 files changed, 32 insertions, 18 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e5b00dc45a5..f8bb7d215ee 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -218,8 +218,8 @@ module API merge_request.subscribed?(options[:current_user]) end expose :user_notes_count - expose :should_remove_source_branch?, as: :should_remove_source_branch - expose :force_remove_source_branch?, as: :force_remove_source_branch + expose(:should_remove_source_branch?) { |mr| mr.remove_source_branch } + expose(:force_remove_source_branch?) { |mr| mr.remove_source_branch } end class MergeRequestChanges < MergeRequest diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2b685621da9..02a0bc20354 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -18,6 +18,19 @@ module API render_api_error!(errors, 400) end + + def remove_source_branch(merge_request) + # We only need the update the value if it differs from what we've got + # the to_boolean calls could return `nil`, which can't be inserted into + # to the `remove_source_branch` field as the column is restrained + new_value = to_boolean(params[:should_remove_source_branch]) || + to_boolean(params[:force_remove_source_branch]) || + to_boolean(params[:remove_source_branch]) + + if new_value != merge_request.remove_source_branch + merge_request.remove_source_branch = !merge_request.remove_source_branch + end + end end # List merge requests @@ -63,15 +76,16 @@ module API # # Parameters: # - # id (required) - The ID of a project - this will be the source of the merge request - # source_branch (required) - The source branch - # target_branch (required) - The target branch - # target_project_id - The target project of the merge request defaults to the :id of the project - # assignee_id - Assignee user ID - # title (required) - Title of MR - # description - Description of MR - # labels (optional) - Labels for MR as a comma-separated list - # milestone_id (optional) - Milestone ID + # id (required) - The ID of a project - this will be the source of the merge request + # source_branch (required) - The source branch + # target_branch (required) - The target branch + # target_project_id - The target project of the merge request defaults to the :id of the project + # assignee_id - Assignee user ID + # title (required) - Title of MR + # description(optional) - Description of MR + # labels (optional) - Labels for MR as a comma-separated list + # milestone_id (optional) - Milestone ID + # remove_source_branch (optional) - Should the source branch be deleted if the MR is merged? # # Example: # POST /projects/:id/merge_requests @@ -79,7 +93,7 @@ module API post ":id/merge_requests" do authorize! :create_merge_request, user_project required_attributes! [:source_branch, :target_branch, :title] - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description, :milestone_id] + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description, :milestone_id, :remove_source_branch] # Validate label names in advance if (errors = validate_label_params(params)).any? @@ -177,11 +191,13 @@ module API # description - Description of MR # labels (optional) - Labels for a MR as a comma-separated list # milestone_id (optional) - Milestone ID + # remove_source_branch (optional) - Should the source branch be deleted if the MR is merged? + # # Example: # PUT /projects/:id/merge_requests/:merge_request_id # put path do - attrs = attributes_for_keys [:target_branch, :assignee_id, :title, :state_event, :description, :milestone_id] + attrs = attributes_for_keys [:target_branch, :assignee_id, :title, :state_event, :description, :milestone_id, :remove_source_branch] merge_request = user_project.merge_requests.find(params[:merge_request_id]) authorize! :update_merge_request, merge_request @@ -216,7 +232,7 @@ module API # id (required) - The ID of a project # merge_request_id (required) - ID of MR # merge_commit_message (optional) - Custom merge commit message - # should_remove_source_branch (optional) - When true, the source branch will be deleted if possible + # remove_source_branch (optional) - When true, the source branch will be deleted if possible # merge_when_build_succeeds (optional) - When true, this MR will be merged when the build succeeds # sha (optional) - When present, must have the HEAD SHA of the source branch # Example: @@ -237,10 +253,8 @@ module API render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409) end - merge_params = { - commit_message: params[:merge_commit_message], - should_remove_source_branch: params[:should_remove_source_branch] - } + merge_params = { commit_message: params[:merge_commit_message] } + remove_source_branch(merge_request) if to_boolean(params[:merge_when_build_succeeds]) && merge_request.pipeline && merge_request.pipeline.active? ::MergeRequests::MergeWhenBuildSucceedsService.new(merge_request.target_project, current_user, merge_params). |