Ensure merge request's "remove branch" accessors return booleans24102-cannot-unselect-remove-source-branch-when-editing-merge-request

Signed-off-by: Rémy Coutable <remy@rymai.me>

4 files changed, 20 insertions, 3 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 0397c57f935..6b8ac3fb48b 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -441,11 +441,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def should_remove_source_branch?
-    merge_params['should_remove_source_branch'].present?
+    Gitlab::Utils.to_boolean(merge_params['should_remove_source_branch'])
   end
 
   def force_remove_source_branch?
-    merge_params['force_remove_source_branch'].present?
+    Gitlab::Utils.to_boolean(merge_params['force_remove_source_branch'])
   end
 
   def remove_source_branch?
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index d410755cad1..0ace6be8f4e 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -142,6 +142,7 @@
       .col-sm-10.col-sm-offset-2
         .checkbox
           = label_tag 'merge_request[force_remove_source_branch]' do
+            = hidden_field_tag 'merge_request[force_remove_source_branch]', '0'
             = check_box_tag 'merge_request[force_remove_source_branch]', '1', @merge_request.force_remove_source_branch?
             Remove source branch when merge request is accepted.
 
diff --git a/changelogs/unreleased/24102-cannot-unselect-remove-source-branch-when-editing-merge-request.yml b/changelogs/unreleased/24102-cannot-unselect-remove-source-branch-when-editing-merge-request.yml
new file mode 100644
index 00000000000..50d018170f1
--- /dev/null
+++ b/changelogs/unreleased/24102-cannot-unselect-remove-source-branch-when-editing-merge-request.yml
@@ -0,0 +1,4 @@
+---
+title: Ensure merge request's "remove branch" accessors return booleans
+merge_request: 7267
+author:
diff --git a/spec/features/merge_requests/edit_mr_spec.rb b/spec/features/merge_requests/edit_mr_spec.rb
index c77e719c5df..c46bd8d449f 100644
--- a/spec/features/merge_requests/edit_mr_spec.rb
+++ b/spec/features/merge_requests/edit_mr_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 feature 'Edit Merge Request', feature: true do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
-  let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) }
+  let(:merge_request) { create(:merge_request, :simple, source_project: project) }
 
   before do
     project.team << [user, :master]
@@ -28,5 +28,17 @@ feature 'Edit Merge Request', feature: true do
 
       expect(page).to have_content 'Someone edited the merge request the same time you did'
     end
+
+    it 'allows to unselect "Remove source branch"' do
+      merge_request.update(merge_params: { 'force_remove_source_branch' => '1' })
+      expect(merge_request.merge_params['force_remove_source_branch']).to be_truthy
+
+      visit edit_namespace_project_merge_request_path(project.namespace, project, merge_request)
+      uncheck 'Remove source branch when merge request is accepted'
+
+      click_button 'Save changes'
+
+      expect(page).to have_content 'Remove source branch'
+    end
   end
 end