Merge branch '1591_approvals_update_fix' into 'master'

Fix updating approvals count when editing an MR

Closes #1591

See merge request !1106

app/controllers/projects/merge_requests_controller.rb

@@ -706,8 +706,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     target_project = @project.forked_from_project if @project.id.to_s != mr_params[:target_project_id]
     target_project ||= @project
 
+    # If the number of approvals is not greater than the project default, set to nil,
+    # so that we fall back to the project default.
     if mr_params[:approvals_before_merge].to_i <= target_project.approvals_before_merge
-      mr_params.delete(:approvals_before_merge)
+      mr_params[:approvals_before_merge] = nil
     end
 
     mr_params

changelogs/unreleased-ee/1591_approvals_update_fix.yml

+---
+title: Fix updating approvals count when editing an MR
+merge_request: 1106
+author:

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -400,6 +400,7 @@ describe Projects::MergeRequestsController do
     context 'the approvals_before_merge param' do
       before { project.update_attributes(approvals_before_merge: 2) }
 
+      context 'approvals_before_merge not set for the existing MR' do
         context 'when it is less than the one in the target project' do
           before { update_merge_request(approvals_before_merge: 1) }
 
@@ -439,6 +440,50 @@ describe Projects::MergeRequestsController do
           end
         end
       end
+
+      context 'approvals_before_merge set for the existing MR' do
+        before { merge_request.update_attribute(:approvals_before_merge, 4) }
+
+        context 'when it is less than the one in the target project' do
+          before { update_merge_request(approvals_before_merge: 1) }
+
+          it 'sets the param to nil' do
+            expect(merge_request.reload.approvals_before_merge).to eq(nil)
+          end
+
+          it 'updates the merge request' do
+            expect(merge_request.reload).to be_valid
+            expect(response).to redirect_to(namespace_project_merge_request_path(id: merge_request.iid, project_id: project.to_param))
+          end
+        end
+
+        context 'when it is equal to the one in the target project' do
+          before { update_merge_request(approvals_before_merge: 2) }
+
+          it 'sets the param to nil' do
+            expect(merge_request.reload.approvals_before_merge).to eq(nil)
+          end
+
+          it 'updates the merge request' do
+            expect(merge_request.reload).to be_valid
+            expect(response).to redirect_to(namespace_project_merge_request_path(id: merge_request.iid, project_id: project.to_param))
+          end
+        end
+
+        context 'when it is greater than the one in the target project' do
+          before { update_merge_request(approvals_before_merge: 3) }
+
+          it 'saves the param in the merge request' do
+            expect(merge_request.reload.approvals_before_merge).to eq(3)
+          end
+
+          it 'updates the merge request' do
+            expect(merge_request.reload).to be_valid
+            expect(response).to redirect_to(namespace_project_merge_request_path(id: merge_request.iid, project_id: project.to_param))
+          end
+        end
+      end
+    end
   end
 
   describe 'POST merge' do