Merge branch 'reset_approval_on_push_for_changed_target' into 'master'

Fix: Approvals not reset if changing target branch

See merge request !1941

app/services/merge_requests/refresh_service.rb

@@ -90,7 +90,7 @@ module MergeRequests
             target_project.reset_approvals_on_push &&
             merge_request.rebase_commit_sha != @newrev
 
-          merge_request.approvals.destroy_all
+          merge_request.approvals.delete_all
         end
       end
     end

app/services/merge_requests/update_service.rb

@@ -16,6 +16,7 @@ module MergeRequests
       if params[:force_remove_source_branch].present?
         merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
       end
+
       old_approvers = merge_request.overall_approvers.to_a
 
       handle_wip_event(merge_request)
@@ -48,6 +49,7 @@ module MergeRequests
         create_branch_change_note(merge_request, 'target',
                                   merge_request.previous_changes['target_branch'].first,
                                   merge_request.target_branch)
+        reset_approvals(merge_request)
       end
 
       if merge_request.previous_changes.include?('milestone_id')
@@ -111,6 +113,14 @@ module MergeRequests
 
     private
 
+    def reset_approvals(merge_request)
+      target_project = merge_request.target_project
+
+      if target_project.approvals_before_merge.nonzero? && target_project.reset_approvals_on_push
+        merge_request.approvals.delete_all
+      end
+    end
+
     def handle_wip_event(merge_request)
       if wip_event = params.delete(:wip_event)
         # We update the title that is provided in the params or we use the mr title

changelogs/unreleased-ee/reset_approval_on_push_for_changed_target.yml

+---
+title: 'Fix: Approvals not reset if changing target branch'
+merge_request:
+author:

spec/services/merge_requests/update_service_spec.rb

@@ -59,7 +59,7 @@ describe MergeRequests::UpdateService, services: true do
         end
       end
 
-      it 'mathces base expectations' do
+      it 'matches base expectations' do
         expect(@merge_request).to be_valid
         expect(@merge_request.title).to eq('New title')
         expect(@merge_request.assignee).to eq(user2)
@@ -531,6 +531,17 @@ describe MergeRequests::UpdateService, services: true do
       end
     end
 
+    context 'updating target_branch' do
+      it 'resets approvals when target_branch is changed' do
+        merge_request.target_project.update(reset_approvals_on_push: true, approvals_before_merge: 2)
+        merge_request.approvals.create(user_id: user2.id)
+
+        update_merge_request(target_branch: 'video')
+
+        expect(merge_request.reload.approvals).to be_empty
+      end
+    end
+
     context 'updating asssignee_id' do
       it 'does not update assignee when assignee_id is invalid' do
         merge_request.update(assignee_id: user.id)