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.

23 files changed, 217 insertions, 122 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 42d32e53685..a995c7010b9 100644
--- 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
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 2cf6a2dd1b3..ebb527d2e38 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -239,7 +239,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     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 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     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
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index e845cfcf66a..90dcfccee5f 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -387,19 +387,8 @@ class MergeRequest < ActiveRecord::Base
     !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 @@ class MergeRequest < ActiveRecord::Base
 
     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
 
diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb
index 96a25330af1..22168ae5b2c 100644
--- a/app/services/merge_requests/create_service.rb
+++ b/app/services/merge_requests/create_service.rb
@@ -9,13 +9,11 @@ module MergeRequests
 
       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)
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index a308136f236..4b133f8f4d7 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -56,22 +56,14 @@ module MergeRequests
     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
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 026a37997d4..477c64e7377 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -11,8 +11,6 @@ module MergeRequests
       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
 
diff --git a/app/views/projects/merge_requests/merge.js.haml b/app/views/projects/merge_requests/merge.js.haml
index 84b6c9ebc5c..0cf52fef16f 100644
--- a/app/views/projects/merge_requests/merge.js.haml
+++ b/app/views/projects/merge_requests/merge.js.haml
@@ -1,7 +1,7 @@
 - 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'))}");
diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml
index bf2e76f0083..d9f33059a7d 100644
--- 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
diff --git 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
index 2b6b5e05e86..aa88a07a33d 100644
--- 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
 
diff --git a/app/views/projects/merge_requests/widget/open/_not_allowed.html.haml b/app/views/projects/merge_requests/widget/open/_not_allowed.html.haml
index 57ce1959021..c4cca63a605 100644
--- 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.
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index c30bdb0ae91..78f38bbcde5 100644
--- 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?)
diff --git a/db/migrate/20160720102606_add_remove_source_branch_to_merge_requests.rb b/db/migrate/20160720102606_add_remove_source_branch_to_merge_requests.rb
new file mode 100644
index 00000000000..8abb7a7c0f1
--- /dev/null
+++ b/db/migrate/20160720102606_add_remove_source_branch_to_merge_requests.rb
@@ -0,0 +1,28 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index 71980a6d51f..2a6e701b5bc 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -632,6 +632,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     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
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 3e88a758936..e2e9e230817 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -1,5 +1,7 @@
 # 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.",
diff --git a/features/steps/project/merge_requests/acceptance.rb b/features/steps/project/merge_requests/acceptance.rb
index 4fda0731e2f..d3f7411f698 100644
--- 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
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).
diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
index 60bc07bd1a0..36c7d79a39d 100644
--- 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 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
   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 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
   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 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
       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
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 3270b877c1a..1f566be7059 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -331,7 +331,7 @@ describe MergeRequest, models: true do
   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 @@ describe MergeRequest, models: true do
       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
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 617600d6173..e8ee3febec6 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -216,11 +216,14 @@ describe API::API, api: true  do
              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 API::API, api: true  do
   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 @@ describe API::API, api: true  do
 
     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 @@ describe API::API, api: true  do
               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?'
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index b84a580967a..03d580323dd 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -13,7 +13,7 @@ describe MergeRequests::CreateService, services: true do
           description: 'please fix',
           source_branch: 'feature',
           target_branch: 'master',
-          force_remove_source_branch: '1'
+          remove_source_branch: true
         }
       end
 
@@ -30,7 +30,7 @@ describe MergeRequests::CreateService, services: true do
       it { expect(@merge_request).to be_valid }
       it { expect(@merge_request.title).to eq('Awesome merge_request') }
       it { expect(@merge_request.assignee).to be_nil }
-      it { expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') }
+      it { expect(@merge_request.remove_source_branch).to be true }
 
       it 'executes hooks with default action' do
         expect(service).to have_received(:execute_hooks).with(@merge_request)
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index df9d96c9493..1f8a8c7cb3d 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -56,7 +56,7 @@ describe MergeRequests::MergeService, services: true do
       context 'when another merge request has the same source branch' do
         it 'removes the source branch' do
           create(:merge_request, source_project: merge_request.source_project,
-                  target_project: merge_request.target_project, target_branch: 'mepmep')
+                                 target_project: merge_request.target_project, target_branch: 'mepmep')
 
           expect(DeleteBranchService).not_to receive(:new)
 
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 283a336afd9..ed67adbe7a7 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -40,7 +40,7 @@ describe MergeRequests::UpdateService, services: true do
           state_event: 'close',
           label_ids: [label.id],
           target_branch: 'target',
-          force_remove_source_branch: '1'
+          remove_source_branch: true
         }
       end
 
@@ -62,7 +62,7 @@ describe MergeRequests::UpdateService, services: true do
       it { expect(@merge_request.labels.count).to eq(1) }
       it { expect(@merge_request.labels.first.title).to eq(label.name) }
       it { expect(@merge_request.target_branch).to eq('target') }
-      it { expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') }
+      it { expect(@merge_request.remove_source_branch).to be_truthy }
 
       it 'executes hooks with update action' do
         expect(service).to have_received(:execute_hooks).