Assign code owners in data migration

The sync_code_owners_with_approvers call uses
original implementation, as it requires full
MR implementation to compute code owners.
However the specs are copied over,
so in the future if its behavior changes,
spec failure would notify developer to copy
existing code over.

ee/app/models/ee/merge_request.rb

@@ -83,11 +83,19 @@ module EE
     end
 
     def sync_code_owners_with_approvers
-      ActiveRecord::Base.transaction do
-        rule = approval_rules.code_owner.first
-        rule ||= approval_rules.code_owner.create!(name: ApprovalMergeRequestRule::DEFAULT_NAME_FOR_CODE_OWNER)
+      return if merged?
 
-        rule.users = code_owners
+      owners = code_owners
+
+      if owners.present?
+        ActiveRecord::Base.transaction do
+          rule = approval_rules.code_owner.first
+          rule ||= approval_rules.code_owner.create!(name: ApprovalMergeRequestRule::DEFAULT_NAME_FOR_CODE_OWNER)
+
+          rule.users = code_owners
+        end
+      else
+        approval_rules.code_owner.delete_all
       end
     end
   end

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules.rb

@@ -102,6 +102,10 @@ module Gitlab
           state == 'merged'
         end
 
+        def sync_code_owners_with_approvers
+          ::MergeRequest.find(id).sync_code_owners_with_approvers
+        end
+
         def finalize_approvals
           return unless merged?
 
@@ -170,6 +174,8 @@ module Gitlab
             rule.approval_project_rule = target.target_project.approval_rules.regular.first
           end
 
+          target.sync_code_owners_with_approvers
+
           target.finalize_approvals if target.merged?
         end
       end

ee/spec/lib/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb

@@ -120,6 +120,56 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
           expect(target.approval_rules.exists?(approval_rule.id)).to eq(false)
         end
       end
+
+      context '#sync_code_owners_with_approvers' do
+        let(:owners) { create_list(:user, 2) }
+
+        before do
+          allow(::Gitlab::CodeOwners).to receive(:for_merge_request).and_return(owners)
+        end
+
+        context 'when merge request is merged' do
+          let(:target) { create(:merged_merge_request) }
+
+          it 'does nothing' do
+            expect do
+              described_class.new.perform(target_type, target.id)
+            end.not_to change { target.approval_rules.count }
+          end
+        end
+
+        context 'when code owner rule does not exist' do
+          it 'creates rule' do
+            expect do
+              described_class.new.perform(target_type, target.id)
+            end.to change { target.approval_rules.code_owner.count }.by(1)
+
+            expect(target.approval_rules.code_owner.first.users).to contain_exactly(*owners)
+          end
+        end
+
+        context 'when code owner rule exists' do
+          let!(:code_owner_rule) { create(:approval_merge_request_rule, merge_request: target, code_owner: true, users: [create(:user)]) }
+
+          it 'reuses and updates existing rule' do
+            expect do
+              described_class.new.perform(target_type, target.id)
+            end.not_to change { target.approval_rules.count }
+
+            expect(code_owner_rule.reload.users).to contain_exactly(*owners)
+          end
+
+          context 'when there is no code owner' do
+            let(:owners) { [] }
+
+            it 'removes rule' do
+              described_class.new.perform(target_type, target.id)
+
+              expect(target.approval_rules.exists?(code_owner_rule.id)).to eq(false)
+            end
+          end
+        end
+      end
     end
 
     context 'project' do

ee/spec/models/merge_request_spec.rb

@@ -71,36 +71,49 @@ describe MergeRequest do
   end
 
   describe '#sync_code_owners_with_approvers' do
-    let(:owners) { [create(:user), create(:user)] }
+    let(:owners) { create_list(:user, 2) }
 
     before do
       allow(subject).to receive(:code_owners).and_return(owners)
     end
 
-    it 'sync code owner to the code owner rule' do
+    it 'does nothing when merge request is merged' do
+      allow(subject).to receive(:merged?).and_return(true)
+
       expect do
         subject.sync_code_owners_with_approvers
-      end.to change { subject.approval_rules.count }.by(1)
-
-      expect(subject.approval_rules.code_owner.first.users).to contain_exactly(*owners)
+      end.not_to change { subject.approval_rules.count }
     end
 
-    context 'when code owner rule already exists' do
-      let!(:code_owner_rule) { subject.approval_rules.code_owner.create!(name: 'Code Owner') }
+    context 'when code owner rule does not exist' do
+      it 'creates rule' do
+        expect do
+          subject.sync_code_owners_with_approvers
+        end.to change { subject.approval_rules.code_owner.count }.by(1)
 
-      before do
-        code_owner_rule.users << create(:user)
+        expect(subject.approval_rules.code_owner.first.users).to contain_exactly(*owners)
       end
+    end
 
-      it 'reuses existing rule' do
+    context 'when code owner rule exists' do
+      let!(:code_owner_rule) { subject.approval_rules.code_owner.create!(name: 'Code Owner', users: [create(:user)]) }
+
+      it 'reuses and updates existing rule' do
         expect do
           subject.sync_code_owners_with_approvers
         end.not_to change { subject.approval_rules.count }
 
-        rule = subject.approval_rules.code_owner.first
+        expect(code_owner_rule.reload.users).to contain_exactly(*owners)
+      end
+
+      context 'when there is no code owner' do
+        let(:owners) { [] }
 
-        expect(rule).to eq(code_owner_rule)
-        expect(rule.users).to contain_exactly(*owners)
+        it 'removes rule' do
+          subject.sync_code_owners_with_approvers
+
+          expect(subject.approval_rules.exists?(code_owner_rule.id)).to eq(false)
+        end
       end
     end
   end