Merge branch 'security-codeowner-diffs' into 'master'

Ensure MR diff exists before codeowner check

Closes #94

See merge request gitlab-org/security/gitlab!351

changelogs/unreleased/bug-codeowner-diffs.yml

+---
+title: Ensure MR diff exists before codeowner check
+merge_request:
+author:
+type: security

ee/app/services/ee/merge_requests/refresh_service.rb

@@ -9,11 +9,12 @@ module EE
 
       override :refresh_merge_requests!
       def refresh_merge_requests!
-        update_approvers
-        reset_approvals_for_merge_requests(push.ref, push.newrev)
         check_merge_train_status
 
         super
+
+        update_approvers
+        reset_approvals_for_merge_requests(push.ref, push.newrev)
       end
 
       # Note: Closed merge requests also need approvals reset.

ee/spec/services/ee/merge_requests/refresh_service_spec.rb

@@ -61,7 +61,7 @@ describe MergeRequests::RefreshService do
     end
 
     describe '#update_approvers' do
-      let(:owner) { create(:user) }
+      let(:owner) { create(:user, username: 'default-codeowner') }
       let(:current_user) { merge_request.author }
       let(:service) { described_class.new(project, current_user) }
       let(:enable_code_owner) { true }
@@ -86,6 +86,25 @@ describe MergeRequests::RefreshService do
         forked_merge_request
       end
 
+      it 'gets called in a specific order' do
+        allow_any_instance_of(MergeRequests::BaseService).to receive(:inspect).and_return(true)
+        expect(service).to receive(:reload_merge_requests).ordered
+        expect(service).to receive(:update_approvers).ordered
+        expect(service).to receive(:reset_approvals_for_merge_requests).ordered
+
+        subject
+      end
+
+      it 'creates an approval rule based on current diff' do
+        file = File.read(Rails.root.join('ee', 'spec', 'fixtures', 'codeowners_example'))
+        project.repository.create_file(owner, 'CODEOWNERS', file, { branch_name: 'test', message: 'codeowners' })
+
+        subject
+
+        expect(another_merge_request.approval_rules.size).to eq(3)
+        expect(another_merge_request.approval_rules.first.rule_type).to eq('code_owner')
+      end
+
       context 'when code owners disabled' do
         let(:enable_code_owner) { false }