Create remove_source_branch field on MergeRequest

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.

Create remove_source_branch field on MergeRequest
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.
95c1cb9f · Z.J. van de Weg · 4ef6b1fb · 95c1cb9f · 95c1cb9f · 95c1cb9f
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,7 @@ v 8.11.0 (unreleased)
  - Environments have an url to link to
  - Update `timeago` plugin to use multiple string/locale settings
  - Remove unused images (ClemMakesApps)
+  - Check remove source branch by default on a new MR
  - Limit git rev-list output count to one in forced push check
  - Clean up unused routes (Josef Strzibny)
  - Fix issue on empty project to allow developers to only push to protected branches if given permission

--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -239,7 +239,7 @@ def merge

    TodoService.new.merge_merge_request(merge_request, current_user)

-    @merge_request.update(merge_error: nil)
+    @merge_request.update(merge_error: nil, remove_source_branch: !!params[:remove_source_branch])

    if params[:merge_when_build_succeeds].present?
      unless @merge_request.pipeline
@@ -434,13 +434,13 @@ def merge_request_params
    params.require(:merge_request).permit(
      :title, :assignee_id, :source_project_id, :source_branch,
      :target_project_id, :target_branch, :milestone_id,
-      :state_event, :description, :task_num, :force_remove_source_branch,
+      :state_event, :description, :task_num, :remove_source_branch,
      label_ids: []
    )
  end

  def merge_params
-    params.permit(:should_remove_source_branch, :commit_message)
+    params.permit(:commit_message, :remove_source_branch)
  end

  # Make sure merge requests created before 8.0

--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -387,19 +387,8 @@ def can_remove_source_branch?(current_user)
    !source_project.protected_branch?(source_branch) &&
      !source_project.root_ref?(source_branch) &&
      Ability.abilities.allowed?(current_user, :push_code, source_project) &&
-      diff_head_commit == source_branch_head
-  end
-
-  def should_remove_source_branch?
-    merge_params['should_remove_source_branch'].present?
-  end
-
-  def force_remove_source_branch?
-    merge_params['force_remove_source_branch'].present?
-  end
-
-  def remove_source_branch?
-    should_remove_source_branch? || force_remove_source_branch?
+      diff_head_commit == source_branch_head &&
+      !same_source_branch_merge_requests?
  end

  def mr_and_commit_notes
@@ -532,11 +521,7 @@ def reset_merge_when_build_succeeds

    self.merge_when_build_succeeds = false
    self.merge_user = nil
-    if merge_params
-      merge_params.delete('should_remove_source_branch')
-      merge_params.delete('commit_message')
-    end
-
+    self.merge_params = nil
    self.save
  end


--- a/app/services/merge_requests/create_service.rb
+++ b/app/services/merge_requests/create_service.rb
@@ -9,13 +9,11 @@ def execute

      filter_params
      label_params = params.delete(:label_ids)
-      force_remove_source_branch = params.delete(:force_remove_source_branch)

      merge_request = MergeRequest.new(params)
      merge_request.source_project = source_project
      merge_request.target_project ||= source_project
      merge_request.author = current_user
-      merge_request.merge_params['force_remove_source_branch'] = force_remove_source_branch

      if merge_request.save
        merge_request.update_attributes(label_ids: label_params)

--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -56,22 +56,14 @@ def commit
    def after_merge
      MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)

-      if remove_source_branch?
+      if merge_request.can_remove_source_branch?(branch_deletion_user)
        DeleteBranchService.new(@merge_request.source_project, branch_deletion_user).
          execute(merge_request.source_branch)
      end
    end

    def branch_deletion_user
-      @merge_request.force_remove_source_branch? ? @merge_request.author : current_user
-    end
-
-    def remove_source_branch?
-      return false unless @merge_request.remove_source_branch?
-
-      # If another MR in this project has the same source branch, we should not
-      # remove this branch
-      !@merge_request.same_source_branch_merge_requests?
+      @merge_request.remove_source_branch? ? @merge_request.author : current_user
    end
  end
 end
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -11,8 +11,6 @@ def execute(merge_request)
      params.except!(:target_project_id)
      params.except!(:source_branch)

-      merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
-
      update(merge_request)
    end


--- a/app/views/projects/merge_requests/merge.js.haml
+++ b/app/views/projects/merge_requests/merge.js.haml
 - case @status
 - when :success
  :plain
-    merge_request_widget.mergeInProgress(#{params[:should_remove_source_branch] == '1'});
+    merge_request_widget.mergeInProgress(#{params[:remove_source_branch] == '1'});
 - when :merge_when_build_succeeds
  :plain
    $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/merge_when_build_succeeds'))}");

--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -27,13 +27,13 @@
        - else
          = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do
            Accept Merge Request
-      - if @merge_request.force_remove_source_branch?
+      - if @merge_request.remove_source_branch?
        .accept-control
          The source branch will be removed.
      - elsif @merge_request.can_remove_source_branch?(current_user)
        .accept-control.checkbox
-          = label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
-            = check_box_tag :should_remove_source_branch
+          = f.label :remove_source_branch, class: "remove_source_checkbox" do
+            = f.check_box :remove_source_branch
            Remove source branch
      .accept-control.right
        = link_to "#", class: "modify-merge-commit-link js-toggle-button" do

--- a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
@@ -16,7 +16,7 @@
  - if remove_source_branch_button || user_can_cancel_automatic_merge
    .clearfix.prepend-top-10
      - if remove_source_branch_button
-        = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.diff_head_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
+        = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, remove_source_branch: true, sha: @merge_request.diff_head_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
          = icon('times')
          Remove Source Branch When Merged


--- a/app/views/projects/merge_requests/widget/open/_not_allowed.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_not_allowed.html.haml
@@ -2,5 +2,5 @@
  Ready to be merged automatically
 %p
  Ask someone with write access to this repository to merge this request.
-  - if @merge_request.force_remove_source_branch?
+  - if @merge_request.remove_source_branch?
    The source branch will be removed.
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -124,8 +124,8 @@
    .form-group
      .col-sm-10.col-sm-offset-2
        .checkbox
-          = label_tag 'merge_request[force_remove_source_branch]' do
-            = check_box_tag 'merge_request[force_remove_source_branch]', '1', @merge_request.force_remove_source_branch?
+          = f.label :remove_source_branch do
+            = f.check_box :remove_source_branch
            Remove source branch when merge request is accepted.

 - is_footer = !(issuable.is_a?(MergeRequest) && issuable.new_record?)

--- a/db/migrate/20160720102606_add_remove_source_branch_to_merge_requests.rb
+++ b/db/migrate/20160720102606_add_remove_source_branch_to_merge_requests.rb
+# online without errors
+# This migration makes an actual field now there are more usecases for this field
+# Migrating the data between the `merge_params` hash and this field will happen as
+# part of this migration, but cleaning up the merge_params field will not. So some
+# keys will be there but never used.
+class AddRemoveSourceBranchToMergeRequests < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:merge_requests, :remove_source_branch, :boolean, default: true)
+
+    set_to_false = if Gitlab::Database.postgresql?
+                     execute "SELECT id FROM merge_requests WHERE merge_params !~* 'remove_source_branch:.{0,2}1';"
+                   else
+                     execute "SELECT id FROM merge_requests WHERE merge_params NOT REGEXP 'remove_source_branch:.{0,2}1;'"
+                   end
+
+    set_to_false.each_slice(1000) do |ids|
+      MergeRequest.where(id: ids).update_all(remove_source_branch: false)
+    end
+  end
+
+  def down
+    # noop - We don't want to serialize and deserialize, also, the keys are still there
+  end
+end
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -632,6 +632,7 @@
    t.string   "merge_commit_sha"
    t.datetime "deleted_at"
    t.string   "in_progress_merge_commit_sha"
+    t.boolean  "remove_source_branch",         default: true,  null: false
  end

  add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
 # Merge requests

+Merge requests provide a way to contribute your work into the specified branch so your changes can be combined with others.
+
 ## List merge requests

 Get all merge requests for this project.
@@ -15,11 +17,17 @@ GET /projects/:id/merge_requests?iid=42

 Parameters:

- `id` (required) - The ID of a project
- `iid` (optional) - Return the request having the given `iid`
- `state` (optional) - Return `all` requests or just those that are `merged`, `opened` or `closed`
- `order_by` (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at`
- `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc`
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `iid` | integer | no | Return the MRs having the given `iid` |
+| `state` | string | no | Return `all` MRs or just those that are `merged`, `opened` or `closed` |
+| `order_by` | string | no | Return MRs ordered by `created_at` or `updated_at` fields. Default is `created_at` |
+| `sort` | string | no | Return MRs sorted in `asc` or `desc` order. Default is `desc` |
+
+```bash
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests/
+```

 ```json
 [
@@ -70,12 +78,12 @@ Parameters:
    "subscribed" : false,
    "user_notes_count": 1,
    "should_remove_source_branch": true,
-    "force_remove_source_branch": false
+    "force_remove_source_branch": true
  }
 ]
 ```

-## Get single MR
+## Get a single Merge Request

 Shows information about a single merge request.

@@ -85,8 +93,14 @@ GET /projects/:id/merge_requests/:merge_request_id

 Parameters:

- `id` (required) - The ID of a project
- `merge_request_id` (required) - The ID of MR
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `merge_request_id` | integer | no | Return the MR having the given `merge_request_id` |
+
+```bash
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests/1
+```

 ```json
 {
@@ -136,7 +150,7 @@ Parameters:
  "subscribed" : true,
  "user_notes_count": 1,
  "should_remove_source_branch": true,
-  "force_remove_source_branch": false
+  "force_remove_source_branch": true
 }
 ```

@@ -150,9 +164,14 @@ GET /projects/:id/merge_requests/:merge_request_id/commits

 Parameters:

- `id` (required) - The ID of a project
- `merge_request_id` (required) - The ID of MR
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `merge_request_id` | integer | no | Return the MR having the given `merge_request_id` |

+```bash
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests/1/commits
+```

 ```json
 [
@@ -187,8 +206,14 @@ GET /projects/:id/merge_requests/:merge_request_id/changes

 Parameters:

- `id` (required) - The ID of a project
- `merge_request_id` (required) - The ID of MR
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `merge_request_id` | integer | no | Return the MR having the given `merge_request_id` |
+
+```bash
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/21/merge_requests/1/changes
+```

 ```json
 {
@@ -238,7 +263,7 @@ Parameters:
  "subscribed" : true,
  "user_notes_count": 1,
  "should_remove_source_branch": true,
-  "force_remove_source_branch": false,
+  "force_remove_source_branch": true,
  "changes": [
    {
    "old_path": "VERSION",
@@ -257,21 +282,29 @@ Parameters:
 ## Create MR

 Creates a new merge request.
+
 ```
 POST /projects/:id/merge_requests
 ```

 Parameters:

- `id` (required)                - The ID of a project
- `source_branch` (required)     - The source branch
- `target_branch` (required)     - The target branch
- `assignee_id` (optional)       - Assignee user ID
- `title` (required)             - Title of MR
- `description` (optional)       - Description of MR
- `target_project_id` (optional) - The target project (numeric id)
- `labels` (optional)            - Labels for MR as a comma-separated list
- `milestone_id` (optional)      - Milestone ID
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `source_branch` | string | yes | The source branch |
+| `target_branch` | string | yes | The target branch |
+| `assignee_id` | integer | no | Assignee user ID |
+| `title` | string | yes |  Title of the new MR |
+| `description` | string | no |  Description of MR |
+| `target_project_id` | integer | no | The target project |
+| `labels` | string | no | Labels for MR as a comma-separated list |
+| `milestone_id` | string | no | Milestone ID |
+| `remove_source_branch` | boolean | no | Remove the source branch when the MR is merged |
+
+```bash
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests?source_branch=test1&target_branch=master&title=test1
+```

 ```json
 {
@@ -338,15 +371,22 @@ PUT /projects/:id/merge_requests/:merge_request_id

 Parameters:

- `id` (required)               - The ID of a project
- `merge_request_id` (required) - ID of MR
- `target_branch`               - The target branch
- `assignee_id`                 - Assignee user ID
- `title`                       - Title of MR
- `description`                 - Description of MR
- `state_event`                 - New state (close|reopen|merge)
- `labels` (optional)           - Labels for MR as a comma-separated list
- `milestone_id` (optional)     - Milestone ID
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `merge_request_id` | integer | yes | ID of MR |
+| `target_branch` | string | no | The target branch |
+| `assignee_id` | integer | no | Assignee user ID |
+| `title` | string | no |  Title of the new MR |
+| `description` | string | no |  Description of MR |
+| `target_project_id` | integer | no | The target project |
+| `labels` | string | no | Labels for MR as a comma-separated list |
+| `milestone_id` | string | no | Milestone ID |
+| `remove_source_branch` | boolean | no | Remove the source branch when the MR is merged |
+
+```bash
+curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests/1&title=test11
+```

 ```json
 {
@@ -354,7 +394,7 @@ Parameters:
  "iid": 1,
  "target_branch": "master",
  "project_id": 3,
-  "title": "test1",
+  "title": "test11",
  "state": "opened",
  "upvotes": 0,
  "downvotes": 0,
@@ -395,7 +435,7 @@ Parameters:
  "subscribed" : true,
  "user_notes_count": 1,
  "should_remove_source_branch": true,
-  "force_remove_source_branch": false
+  "force_remove_source_branch": true
 }
 ```

@@ -441,12 +481,18 @@ PUT /projects/:id/merge_requests/:merge_request_id/merge

 Parameters:

- `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)  - if `true` removes the source branch
- `merge_when_build_succeeds` (optional)    - if `true` the MR is merged when the build succeeds
- `sha` (optional)                          - if present, then this SHA must match the HEAD of the source branch, otherwise the merge will fail
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `merge_request_id` | integer | yes | ID of MR |
+| `merge_commit_message` | string | no | Custom merge commit message |
+| `remove_source_branch` | boolean | no | if `true` removes the source branch |
+| `merge_when_build_succeeds` | boolean | no | if `true` the MR is merged when the build succeeds |
+| `sha` | string | no | if present, then this SHA must match the HEAD of the source branch, otherwise the merge will fail |
+
+```bash
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests/1/merge
+```

 ```json
 {
@@ -496,7 +542,7 @@ Parameters:
  "subscribed" : true,
  "user_notes_count": 1,
  "should_remove_source_branch": true,
-  "force_remove_source_branch": false
+  "force_remove_source_branch": true
 }
 ```

@@ -509,13 +555,22 @@ If you don't have permissions to accept this merge request - you'll get a 401
 If the merge request is already merged or closed - you get 405 and error message 'Method Not Allowed'

 In case the merge request is not set to be merged when the build succeeds, you'll also get a 406 error.
+
 ```
 PUT /projects/:id/merge_requests/:merge_request_id/cancel_merge_when_build_succeeds
 ```
+
 Parameters:

- `id` (required)                           - The ID of a project
- `merge_request_id` (required)             - ID of MR
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `merge_request_id` | integer | yes | ID of MR |
+
+
+```bash
+curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/merge_requests/1/cancel_merge_when_build_succeeds
+```

 ```json
 {
@@ -565,7 +620,7 @@ Parameters:
  "subscribed" : true,
  "user_notes_count": 1,
  "should_remove_source_branch": true,
-  "force_remove_source_branch": false
+  "force_remove_source_branch": true
 }
 ```

@@ -886,7 +941,7 @@ Example response:
    "subscribed": true,
    "user_notes_count": 7,
    "should_remove_source_branch": true,
-    "force_remove_source_branch": false
+    "force_remove_source_branch": true
  },
  "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7",
  "body": "Et voluptas laudantium minus nihil recusandae ut accusamus earum aut non.",

--- a/features/steps/project/merge_requests/acceptance.rb
+++ b/features/steps/project/merge_requests/acceptance.rb
@@ -30,7 +30,7 @@ class Spinach::Features::ProjectMergeRequestsAcceptance < Spinach::FeatureSteps
    @user = create(:user)
    @project = create(:project, :public)
    @project_member = create(:project_member, :developer, user: @user, project: @project)
-    @merge_request = create(:merge_request, :with_diffs, :simple, source_project: @project)
+    @merge_request = create(:merge_request, :with_diffs, :simple, source_project: @project, remove_source_branch: false)
  end

  step 'I am signed in as a developer of the project' do

--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -218,8 +218,8 @@ class MergeRequest < ProjectEntity
        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

--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -18,6 +18,19 @@ def handle_merge_request_errors!(errors)

          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 @@ def handle_merge_request_errors!(errors)
      #
      # 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 @@ def handle_merge_request_errors!(errors)
      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 @@ def handle_merge_request_errors!(errors)
        #   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 @@ def handle_merge_request_errors!(errors)
        #   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 @@ def handle_merge_request_errors!(errors)
            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).

--- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
+++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
@@ -4,7 +4,7 @@
  let(:user) { create(:user) }

  let(:project)       { create(:project, :public) }
-  let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") }
+  let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user) }

  before do
    project.team << [user, :master]
@@ -42,15 +42,16 @@
  end

  context 'When it is enabled' do
-    let(:merge_request) do
-      create(:merge_request_with_diffs, :simple,  source_project: project, author: user,
-                                                  merge_user: user, title: "MepMep", merge_when_build_succeeds: true)
-    end
-
    let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) }
    let!(:ci_build) { create(:ci_build, pipeline: pipeline) }

    before do
+      # Do not create a new MR here in this scope, this will yield a second MR with the same source branch
+      merge_request.remove_source_branch = false
+      merge_request.merge_when_build_succeeds = true
+      merge_request.merge_user = user
+      merge_request.save
+
      login_as user
      visit_merge_request(merge_request)
    end
@@ -68,6 +69,7 @@
      expect(page).to have_link "Remove Source Branch When Merged"

      click_link "Remove Source Branch When Merged"
+
      expect(page).to have_content "The source branch will be removed"
    end
  end

--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -331,7 +331,7 @@
  describe "#reset_merge_when_build_succeeds" do
    let(:merge_if_green) do
      create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user),
-                             merge_params: { "should_remove_source_branch" => "1", "commit_message" => "msg" }
+                             remove_source_branch: true, merge_params: { "commit_message" => "msg" }
    end

    it "sets the item to false" do
@@ -339,7 +339,6 @@
      merge_if_green.reload

      expect(merge_if_green.merge_when_build_succeeds).to be_falsey
-      expect(merge_if_green.merge_params["should_remove_source_branch"]).to be_nil
      expect(merge_if_green.merge_params["commit_message"]).to be_nil
    end
  end

--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -216,11 +216,14 @@
             target_branch: 'master',
             author: user,
             labels: 'label, label2',
-             milestone_id: milestone.id
+             milestone_id: milestone.id,
+             remove_source_branch: false
+
        expect(response).to have_http_status(201)
        expect(json_response['title']).to eq('Test merge_request')
        expect(json_response['labels']).to eq(['label', 'label2'])
        expect(json_response['milestone']['id']).to eq(milestone.id)
+        expect(json_response['should_remove_source_branch?']).to be false
      end

      it "returns 422 when source_branch equals target_branch" do
@@ -470,10 +473,25 @@
  describe "PUT /projects/:id/merge_requests/:merge_request_id" do
    it "updates title and returns merge_request" do
      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), title: "New title"
+
      expect(response).to have_http_status(200)
      expect(json_response['title']).to eq('New title')
    end

+    it "updates the state of the merge_request" do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close"
+
+      expect(response).to have_http_status(200)
+      expect(json_response['state']).to eq('closed')
+    end
+
+    it 'updates the remove source branch setting' do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), remove_source_branch: false
+
+      expect(response).to have_http_status(200)
+      expect(json_response['should_remove_source_branch?']).to be false
+    end
+
    it "updates description and returns merge_request" do
      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), description: "New description"
      expect(response).to have_http_status(200)
@@ -482,18 +500,21 @@

    it "updates milestone_id and returns merge_request" do
      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), milestone_id: milestone.id
+
      expect(response).to have_http_status(200)
      expect(json_response['milestone']['id']).to eq(milestone.id)
    end

    it "returns 400 when source_branch is specified" do
      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user),
+
      source_branch: "master", target_branch: "master"
      expect(response).to have_http_status(400)
    end

    it "returns merge_request with renamed target_branch" do
      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), target_branch: "wiki"
+
      expect(response).to have_http_status(200)
      expect(json_response['target_branch']).to eq('wiki')
    end
@@ -503,6 +524,7 @@
              user),
          title: 'new issue',
          labels: 'label, label?, label&foo, ?, &'
+
      expect(response.status).to eq(200)
      expect(json_response['labels']).to include 'label'
      expect(json_response['labels']).to include 'label?'