Fix MR edit for forks of projects with approvals

CHANGELOG-EE

@@ -2,6 +2,10 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.10.0 (unreleased)
 
+v 8.9.1
+  - Fix MR creation from forks where target project has approvals enabled
+  - Fix MR edit where target project has approvals enabled
+
 v 8.9.0
   - Fix JenkinsService test button
   - Fix nil user handling in UpdateMirrorService

app/controllers/projects/merge_requests_controller.rb

@@ -136,11 +136,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def create
     @target_branches ||= []
-    create_params = merge_request_params
-
-    if create_params[:approvals_before_merge].to_i <= project.approvals_before_merge
-      create_params.delete(:approvals_before_merge)
-    end
+    create_params = clamp_approvals_before_merge(merge_request_params)
 
     @merge_request = MergeRequests::CreateService.new(project, current_user, create_params).execute
 
@@ -164,11 +160,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def update
-    update_params = merge_request_params
-
-    if update_params[:approvals_before_merge].to_i <= project.approvals_before_merge
-      update_params.delete(:approvals_before_merge)
-    end
+    update_params = clamp_approvals_before_merge(merge_request_params)
 
     @merge_request = MergeRequests::UpdateService.new(project, current_user, update_params).execute(@merge_request)
 
@@ -417,6 +409,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     )
   end
 
+  def clamp_approvals_before_merge(mr_params)
+    if mr_params[:approvals_before_merge].to_i <= selected_target_project.approvals_before_merge
+      mr_params.delete(:approvals_before_merge)
+    end
+
+    mr_params
+  end
+
   def merge_params
     params.permit(:should_remove_source_branch, :commit_message)
   end

spec/features/merge_requests/create_new_mr_spec.rb

@@ -40,13 +40,24 @@ feature 'Create New Merge Request', feature: true, js: true do
   end
 
   context 'when approvals are enabled for the target project' do
-    before { project.update_attributes(approvals_before_merge: 1) }
+    before do
+      project.update_attributes(approvals_before_merge: 1)
 
-    it 'shows approval settings' do
       visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { source_branch: 'feature_conflict' })
+    end
 
+    it 'shows approval settings' do
       expect(page).to have_content('Approvers')
     end
+
+    context 'saving the MR' do
+      it 'shows the saved MR' do
+        fill_in 'merge_request_title', with: 'Test'
+        click_button 'Submit merge request'
+
+        expect(page).to have_link('Close merge request')
+      end
+    end
   end
 
   context 'when target project cannot be viewed by the current user' do

spec/features/merge_requests/edit_mr_spec.rb

@@ -7,15 +7,24 @@ feature 'Edit Merge Request', feature: true do
 
   before do
     project.team << [user, :master]
+    project.update_attributes(approvals_before_merge: 2)
 
     login_as user
 
     visit edit_namespace_project_merge_request_path(project.namespace, project, merge_request)
   end
 
-  context 'editing a MR' do
+  context 'editing a MR that needs approvals' do
     it 'form should have class js-quick-submit' do
       expect(page).to have_selector('.js-quick-submit')
     end
+
+    context 'saving the MR' do
+      it 'shows the saved MR' do
+        click_button 'Save changes'
+
+        expect(page).to have_link('Close merge request')
+      end
+    end
   end
 end