Merge branch 'approvals_reset_for_fork' into 'master'

Approvals are not reset after a new push is made if the request is coming from a fork

https://gitlab.com/gitlab-org/gitlab-ee/issues/83

See merge request !105

CHANGELOG-EE

 v 8.4.0 (unreleased)
   - Add ability to create a note for user by admin
   - Fix "Commit was rejected by git hook", when max_file_size was set null in project's Git hooks
+  - Fix "Approvals are not reset after a new push is made if the request is coming from a fork"
 
 v 8.3.2
   - No EE-specific changes

app/services/merge_requests/refresh_service.rb

@@ -78,14 +78,11 @@ module MergeRequests
     end
 
     # Reset approvals for merge request
-    # Note: we should reset approvals for merge requests from forks too
     def reset_approvals_for_merge_requests
-      if @project.approvals_before_merge.nonzero? && @project.reset_approvals_on_push
-        merge_requests = @project.merge_requests.opened.where(source_branch: @branch_name).to_a
-        merge_requests += @fork_merge_requests.where(source_branch: @branch_name).to_a
-        merge_requests = filter_merge_requests(merge_requests)
+      merge_requests_for_source_branch.each do |merge_request|
+        target_project = merge_request.target_project
 
-        merge_requests.each do |merge_request|
+        if target_project.approvals_before_merge.nonzero? && target_project.reset_approvals_on_push
           merge_request.approvals.destroy_all
         end
       end

spec/services/merge_requests/refresh_service_spec.rb

@@ -55,7 +55,7 @@ describe MergeRequests::RefreshService, services: true do
       it { expect(@merge_request.merge_when_build_succeeds).to be_falsey}
       it { expect(@fork_merge_request).to be_open }
       it { expect(@fork_merge_request.notes).to be_empty }
-      it { expect(@fork_merge_request.approvals).to be_empty }
+      it { expect(@fork_merge_request.approvals).not_to be_empty }
     end
 
     context 'push to origin repo target branch' do
@@ -109,7 +109,7 @@ describe MergeRequests::RefreshService, services: true do
       it { expect(@merge_request.approvals).not_to be_empty }
       it { expect(@fork_merge_request.notes.last.note).to include('Added 4 commits') }
       it { expect(@fork_merge_request).to be_open }
-      it { expect(@fork_merge_request.approvals).not_to be_empty }
+      it { expect(@fork_merge_request.approvals).to be_empty }
     end
 
     context 'push to fork repo target branch' do
@@ -142,7 +142,7 @@ describe MergeRequests::RefreshService, services: true do
     end
 
     context 'resetting approvals if they are enabled' do
-      it "does not reset approvals if approvals_before_merge si disabled" do
+      it "does not reset approvals if approvals_before_merge is disabled" do
         @project.update(approvals_before_merge: 0)
         refresh_service = service.new(@project, @user)
         allow(refresh_service).to receive(:execute_hooks)
@@ -152,7 +152,7 @@ describe MergeRequests::RefreshService, services: true do
         expect(@merge_request.approvals).not_to be_empty
       end
 
-      it "does not reset approvals if reset_approvals_on_push si disabled" do
+      it "does not reset approvals if reset_approvals_on_push is disabled" do
         @project.update(reset_approvals_on_push: false)
         refresh_service = service.new(@project, @user)
         allow(refresh_service).to receive(:execute_hooks)