Merge branch '232981-scan-commits-for-changes-to-codeowners' into 'master'

Resolve "Scan commits for changes to CODEOWNERS"

See merge request gitlab-org/gitlab!38575

ee/app/models/ee/merge_request.rb

@@ -67,6 +67,10 @@ module EE
           :author, :approved_by_users, :metrics,
           latest_merge_request_diff: :merge_request_diff_files, target_project: :namespace, milestone: :project)
       end
+
+      scope :including_merge_train, -> do
+        includes(:merge_train)
+      end
     end
 
     class_methods do

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

@@ -13,7 +13,8 @@ module EE
 
         super
 
-        update_approvers
+        update_approvers_for_source_branch_merge_requests
+        update_approvers_for_target_branch_merge_requests
         reset_approvals_for_merge_requests(push.ref, push.newrev)
       end
 
@@ -34,13 +35,33 @@ module EE
         merge_requests.map(&:latest_merge_request_diff)
       end
 
-      def update_approvers
+      def update_approvers_for_source_branch_merge_requests
         merge_requests_for_source_branch.each do |merge_request|
           ::MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute if project.feature_available?(:code_owners)
           ::MergeRequests::SyncReportApproverApprovalRules.new(merge_request).execute if project.feature_available?(:report_approver_rules)
         end
       end
 
+      def update_approvers_for_target_branch_merge_requests
+        if update_target_approvers_features_enabled? && branch_protected? && code_owners_updated?
+          merge_requests_for_target_branch.each do |merge_request|
+            ::MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute unless merge_request.on_train?
+          end
+        end
+      end
+
+      def update_target_approvers_features_enabled?
+        ::Feature.enabled?(:update_target_approvers, project) && project.feature_available?(:code_owners)
+      end
+
+      def branch_protected?
+        project.branch_requires_code_owner_approval?(push.branch_name)
+      end
+
+      def code_owners_updated?
+        push.modified_paths.find { |path| ::Gitlab::CodeOwners::FILE_PATHS.include?(path) }
+      end
+
       # rubocop:disable Gitlab/ModuleWithInstanceVariables
       def check_merge_train_status
         return unless @push.branch_updated?
@@ -48,6 +69,14 @@ module EE
         MergeTrains::CheckStatusService.new(project, current_user)
           .execute(project, @push.branch_name, @push.newrev)
       end
+
+      def merge_requests_for_target_branch(reload: false, mr_states: [:opened])
+        @target_merge_requests = nil if reload
+        @target_merge_requests ||= project.merge_requests
+          .with_state(mr_states)
+          .by_target_branch(push.branch_name)
+          .including_merge_train
+      end
       # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
       def reset_approvals?(merge_request, newrev)

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

@@ -63,7 +63,89 @@ RSpec.describe MergeRequests::RefreshService do
       end
     end
 
-    describe '#update_approvers' do
+    describe '#update_approvers_for_target_branch_merge_requests' do
+      shared_examples_for 'does not refresh the code owner rules' do
+        specify do
+          expect(::MergeRequests::SyncCodeOwnerApprovalRules).not_to receive(:new)
+          subject
+        end
+      end
+
+      subject { service.execute(oldrev, newrev, "refs/heads/master") }
+
+      let(:enable_code_owner) { true }
+      let(:enable_target_approvers) { true }
+      let!(:protected_branch) { create(:protected_branch, name: 'master', project: project, code_owner_approval_required: true) }
+      let(:newrev) { TestEnv::BRANCH_SHA['with-codeowners'] }
+
+      before do
+        stub_feature_flags(update_target_approvers: enable_target_approvers, code_owners: enable_code_owner)
+        stub_licensed_features(code_owner_approval_required: true)
+      end
+
+      context 'when the feature flags are enabled' do
+        context 'when the branch is protected' do
+          context 'when code owners file is updated' do
+            let(:irrelevant_merge_request) { another_merge_request }
+            let(:relevant_merge_request) { merge_request }
+
+            context 'when not on the merge train' do
+              it 'refreshes the code owner rules for all relevant merge requests' do
+                fake_refresh_service = instance_double(::MergeRequests::SyncCodeOwnerApprovalRules)
+
+                expect(::MergeRequests::SyncCodeOwnerApprovalRules)
+                  .to receive(:new).with(relevant_merge_request).and_return(fake_refresh_service)
+                expect(fake_refresh_service).to receive(:execute)
+
+                expect(::MergeRequests::SyncCodeOwnerApprovalRules)
+                  .not_to receive(:new).with(irrelevant_merge_request)
+
+                subject
+              end
+            end
+
+            context 'when on the merge train' do
+              let(:merge_request) do
+                create(:merge_request,
+                   :on_train,
+                   source_project: project,
+                   source_branch: source_branch,
+                   target_branch: 'master',
+                   target_project: project)
+              end
+
+              it_behaves_like 'does not refresh the code owner rules'
+            end
+          end
+
+          context 'when code owners file is not updated' do
+            let(:newrev) { TestEnv::BRANCH_SHA['after-create-delete-modify-move'] }
+
+            it_behaves_like 'does not refresh the code owner rules'
+          end
+        end
+
+        context 'when the branch is not protected' do
+          let(:protected_branch) { nil }
+
+          it_behaves_like 'does not refresh the code owner rules'
+        end
+      end
+
+      context 'when update_target_approvers is disabled' do
+        let(:enable_code_owner) { false }
+
+        it_behaves_like 'does not refresh the code owner rules'
+      end
+
+      context 'when code_owners is disabled' do
+        let(:enable_target_approvers) { false }
+
+        it_behaves_like 'does not refresh the code owner rules'
+      end
+    end
+
+    describe '#update_approvers_for_source_branch_merge_requests' do
       let(:owner) { create(:user, username: 'default-codeowner') }
       let(:current_user) { merge_request.author }
       let(:service) { described_class.new(project, current_user) }
@@ -92,7 +174,7 @@ RSpec.describe MergeRequests::RefreshService do
       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(:update_approvers_for_source_branch_merge_requests).ordered
         expect(service).to receive(:reset_approvals_for_merge_requests).ordered
 
         subject