Merge branch '325691-add-sync_code_onwers_approval_rules_worker' into 'master'

Add new MergeRequests::SyncCodeOwnerApprovalRulesWorker

See merge request gitlab-org/gitlab!58512

changelogs/unreleased/325691-add-sync_code_onwers_approval_rules_worker.yml

+---
+title: Add new MergeRequests::SyncCodeOwnerApprovalRulesWorker
+merge_request: 58512
+author:
+type: performance

config/sidekiq_queues.yml

@@ -220,6 +220,8 @@
   - 1
 - - merge_requests_resolve_todos
   - 1
+- - merge_requests_sync_code_owner_approval_rules
+  - 1
 - - metrics_dashboard_prune_old_annotations
   - 1
 - - metrics_dashboard_sync_dashboards

ee/app/services/ee/merge_requests/update_service.rb

@@ -39,7 +39,9 @@ module EE
       def after_update(merge_request)
         super
 
-        ::MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute
+        merge_request.run_after_commit do
+          ::MergeRequests::SyncCodeOwnerApprovalRulesWorker.perform_async(merge_request)
+        end
       end
 
       override :create_branch_change_note

ee/app/workers/all_queues.yml

@@ -867,6 +867,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: merge_requests_sync_code_owner_approval_rules
+  :feature_category: :source_code_management
+  :has_external_dependencies:
+  :urgency: :high
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: new_epic
   :feature_category: :epics
   :has_external_dependencies:

ee/app/workers/merge_requests/sync_code_owner_approval_rules_worker.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  class SyncCodeOwnerApprovalRulesWorker
+    include ApplicationWorker
+
+    feature_category :source_code_management
+    urgency :high
+    deduplicate :until_executed
+    idempotent!
+
+    def perform(merge_request_id)
+      merge_request = MergeRequest.find_by_id(merge_request_id)
+      return unless merge_request
+
+      ::MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute
+    end
+  end
+end

ee/spec/services/ee/merge_requests/update_service_spec.rb

@@ -302,13 +302,14 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       end
     end
 
-    it 'updates code owner approval rules' do
-      expect_next_instance_of(::MergeRequests::SyncCodeOwnerApprovalRules) do |instance|
-        expect(instance).to receive(:execute)
-      end
+    context 'when called inside an ActiveRecord transaction' do
+      it 'does not attempt to update code owner approval rules' do
+        allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(true)
+        expect(::MergeRequests::SyncCodeOwnerApprovalRulesWorker).not_to receive(:perform_async)
 
         update_merge_request(title: 'Title')
       end
+    end
 
     context 'updating reviewers_ids' do
       it 'updates the tracking when user ids are valid' do

ee/spec/services/milestones/destroy_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Milestones::DestroyService do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :repository) }
+  let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) }
+
+  before do
+    project.add_maintainer(user)
+  end
+
+  def service
+    described_class.new(project, user, {})
+  end
+
+  describe '#execute' do
+    context 'with an existing merge request' do
+      let!(:issue) { create(:issue, project: project, milestone: milestone) }
+      let!(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
+
+      it 'manually queues MergeRequests::SyncCodeOwnerApprovalRulesWorker jobs' do
+        expect(::MergeRequests::SyncCodeOwnerApprovalRulesWorker).to receive(:perform_async)
+
+        service.execute(milestone)
+      end
+    end
+  end
+end

ee/spec/workers/merge_requests/sync_code_owner_approval_rules_worker_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe MergeRequests::SyncCodeOwnerApprovalRulesWorker do
+  let_it_be(:merge_request) { create(:merge_request) }
+
+  subject { described_class.new }
+
+  describe "#perform" do
+    it_behaves_like 'an idempotent worker' do
+      let(:job_args) { [merge_request.id] }
+    end
+
+    context "when merge request is not found" do
+      it "returns without attempting to sync code owner rules" do
+        expect(MergeRequests::SyncCodeOwnerApprovalRules).not_to receive(:new)
+
+        subject.perform(non_existing_record_id)
+      end
+    end
+
+    context "when merge request is found" do
+      it "attempts to sync code owner rules" do
+        expect_next_instance_of(::MergeRequests::SyncCodeOwnerApprovalRules) do |instance|
+          expect(instance).to receive(:execute)
+        end
+
+        subject.perform(merge_request.id)
+      end
+    end
+  end
+end

spec/services/milestones/destroy_service_spec.rb

@@ -22,15 +22,17 @@ RSpec.describe Milestones::DestroyService do
       expect { milestone.reload }.to raise_error ActiveRecord::RecordNotFound
     end
 
-    it 'deletes milestone id from issuables' do
-      issue = create(:issue, project: project, milestone: milestone)
-      merge_request = create(:merge_request, source_project: project, milestone: milestone)
+    context 'with an existing merge request' do
+      let!(:issue) { create(:issue, project: project, milestone: milestone) }
+      let!(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
 
+      it 'deletes milestone id from issuables' do
         service.execute(milestone)
 
         expect(issue.reload.milestone).to be_nil
         expect(merge_request.reload.milestone).to be_nil
       end
+    end
 
     it 'logs destroy event' do
       service.execute(milestone)