Merge branch '1979-1-2-multi-block-approval-data-migration' into 'master'

[Data migration] Sync Approver/ApproverGroup with ApprovalRule/ApprovalRuleMember

Closes #1979

See merge request gitlab-org/gitlab-ee!8669

app/services/merge_requests/create_service.rb

@@ -66,3 +66,5 @@ module MergeRequests
     end
   end
 end
+
+MergeRequests::CreateService.include(EE::MergeRequests::CreateService)

app/services/merge_requests/post_merge_service.rb

@@ -52,3 +52,5 @@ module MergeRequests
     end
   end
 end
+
+MergeRequests::PostMergeService.prepend(EE::MergeRequests::PostMergeService)

ee/app/models/approval_merge_request_rule.rb

@@ -3,6 +3,8 @@
 class ApprovalMergeRequestRule < ApplicationRecord
   include ApprovalRuleLike
 
+  DEFAULT_NAME_FOR_CODE_OWNER = 'Code Owner'
+
   scope :regular, -> { where(code_owner: false) }
   scope :code_owner, -> { where(code_owner: true) } # special code owner rules, updated internally when code changes
 

ee/app/models/approver.rb

@@ -4,6 +4,8 @@ class Approver < ActiveRecord::Base
   belongs_to :target, polymorphic: true  # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :user
 
+  include ApproverMigrateHook
+
   validates :user, presence: true
 
   def find_by_user_id(user_id)

ee/app/models/approver_group.rb

@@ -4,6 +4,8 @@ class ApproverGroup < ActiveRecord::Base
   belongs_to :target, polymorphic: true  # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :group
 
+  include ApproverMigrateHook
+
   validates :group, presence: true
 
   delegate :users, to: :group

ee/app/models/concerns/approval_rule_like.rb

@@ -8,7 +8,7 @@ module ApprovalRuleLike
   included do
     has_and_belongs_to_many :users
     has_and_belongs_to_many :groups, class_name: 'Group', join_table: "#{self.table_name}_groups"
-    has_many :group_users, through: :groups, source: :users
+    has_many :group_users, -> { distinct }, through: :groups, source: :users
 
     validates :name, presence: true
   end

ee/app/models/concerns/approver_migrate_hook.rb

+# frozen_string_literal: true
+#
+# Sync up old models (Approver and ApproverGroup)
+# to new models (ApprovalMergeRequestRule and ApprovalProjectRule)
+#
+# TODO: remove once #1979 is closed
+
+module ApproverMigrateHook
+  extend ActiveSupport::Concern
+
+  included do
+    after_commit :schedule_approval_migration, on: [:create, :destroy]
+  end
+
+  def schedule_approval_migration
+    # After merge, approval information is frozen
+    return if target.is_a?(MergeRequest) && target.merged?
+
+    Gitlab::BackgroundMigration::MigrateApproverToApprovalRules.new.perform(target.class.name, target.id, sync_code_owner_rule: false)
+  end
+end

ee/app/models/ee/merge_request.rb

@@ -81,5 +81,22 @@ module EE
 
       compare_reports(::Ci::CompareLicenseManagementReportsService)
     end
+
+    def sync_code_owners_with_approvers
+      return if merged?
+
+      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.uniq
+        end
+      else
+        approval_rules.code_owner.delete_all
+      end
+    end
   end
 end

ee/app/services/ee/merge_requests/create_service.rb

+# frozen_string_literal: true
+
+module EE
+  module MergeRequests
+    module CreateService
+      extend ::Gitlab::Utils::Override
+
+      override :after_create
+      def after_create(issuable)
+        super
+
+        issuable.sync_code_owners_with_approvers
+      end
+    end
+  end
+end

ee/app/services/ee/merge_requests/post_merge_service.rb

+# frozen_string_literal: true
+
+module EE
+  module MergeRequests
+    module PostMergeService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(merge_request)
+        super
+        ApprovalRules::FinalizeService.new(merge_request).execute
+      end
+    end
+  end
+end

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

@@ -53,6 +53,8 @@ module EE
           new_code_owners = merge_request.code_owners - previous_code_owners
 
           create_approvers(merge_request, new_code_owners)
+
+          merge_request.sync_code_owners_with_approvers
         end
 
         results

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

@@ -25,6 +25,8 @@ module EE
           cleanup_approvers(merge_request, reload: true)
         end
 
+        sync_approval_rules(merge_request)
+
         merge_request
       end
 
@@ -42,6 +44,14 @@ module EE
 
         merge_request.approvals.delete_all if target_project.reset_approvals_on_push
       end
+
+      # TODO remove after #1979 is closed
+      def sync_approval_rules(merge_request)
+        return if merge_request.merged?
+        return unless merge_request.previous_changes.include?(:approvals_before_merge)
+
+        merge_request.approval_rules.regular.update_all(approvals_required: merge_request.approvals_before_merge)
+      end
     end
   end
 end

ee/app/services/ee/projects/update_service.rb

@@ -37,6 +37,8 @@ module EE
 
           sync_wiki_on_enable if !wiki_was_enabled && project.wiki_enabled?
           project.import_state.force_import_job! if params[:mirror].present? && project.mirror?
+
+          sync_approval_rules
         end
 
         result
@@ -67,6 +69,13 @@ module EE
       def sync_wiki_on_enable
         ::Geo::RepositoryUpdatedService.new(project.wiki.repository).execute
       end
+
+      # TODO remove after #1979 is closed
+      def sync_approval_rules
+        return unless project.previous_changes.include?(:approvals_before_merge)
+
+        project.approval_rules.update_all(approvals_required: project.approvals_before_merge)
+      end
     end
   end
 end

ee/db/post_migrate/20181204040404_migrate_project_approvers.rb

+# frozen_string_literal: true
+
+class MigrateProjectApprovers < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 3000
+
+  class MergeRequest < ActiveRecord::Base
+    include ::EachBatch
+    self.table_name = 'merge_requests'
+  end
+
+  class Approver < ActiveRecord::Base
+    self.table_name = 'approvers'
+  end
+
+  class ApproverGroup < ActiveRecord::Base
+    self.table_name = 'approver_groups'
+  end
+
+  def up
+    get_project_ids.each do |project_id|
+      Gitlab::BackgroundMigration::MigrateApproverToApprovalRules.new.perform('Project', project_id)
+    end
+
+    bulk_queue_background_migration_jobs_by_range(MergeRequest, 'MigrateApproverToApprovalRulesInBatch', batch_size: BATCH_SIZE)
+
+    check_time = Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesCheckProgress::RESCHEDULE_DELAY
+    BackgroundMigrationWorker.bulk_perform_in(check_time, [['MigrateApproverToApprovalRulesCheckProgress']])
+  end
+
+  def down
+  end
+
+  private
+
+  def get_project_ids
+    results = ActiveRecord::Base.connection.exec_query <<~SQL
+      SELECT DISTINCT target_id FROM (
+        SELECT target_id FROM approvers WHERE target_type='Project'
+        UNION
+        SELECT target_id FROM approver_groups WHERE target_type='Project'
+      ) t
+    SQL
+
+    results.rows.flatten
+  end
+end

ee/lib/api/merge_request_approvals.rb

@@ -77,6 +77,8 @@ module API
           requires :approver_group_ids, type: Array[String], coerce_with: ARRAY_COERCION_LAMBDA, desc: 'Array of Group IDs to set as approvers.'
         end
         put 'approvers' do
+          Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ee/issues/8883')
+
           merge_request = find_merge_request_with_access(params[:merge_request_iid], :update_approvers)
 
           merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, declared(params, include_parent_namespaces: false).merge(remove_old_approvers: true)).execute(merge_request)

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # A Project/MergeRequest level migration, aiming to convert existing data
+    # (from approvers, approver_groups tables)
+    # to new rule based schema.
+    class MigrateApproverToApprovalRules
+      include Gitlab::Utils::StrongMemoize
+
+      class Approver < ActiveRecord::Base
+        self.table_name = 'approvers'
+      end
+
+      class ApproverGroup < ActiveRecord::Base
+        self.table_name = 'approver_groups'
+      end
+
+      class ApprovalMergeRequestRule < ActiveRecord::Base
+        self.table_name = 'approval_merge_request_rules'
+
+        belongs_to :merge_request
+        scope :code_owner, -> { where(code_owner: true) }
+        scope :regular, -> { where(code_owner: false) } # Non code owner rule
+
+        has_and_belongs_to_many :users
+        has_and_belongs_to_many :groups, class_name: 'Group', join_table: :approval_merge_request_rules_groups
+        has_one :approval_merge_request_rule_source
+        has_one :approval_project_rule, through: :approval_merge_request_rule_source
+
+        def project
+          merge_request.target_project
+        end
+      end
+
+      class ApprovalMergeRequestRuleSource < ActiveRecord::Base
+        self.table_name = 'approval_merge_request_rule_sources'
+        belongs_to :approval_merge_request_rule
+        belongs_to :approval_project_rule
+      end
+
+      class ApprovalProjectRule < ActiveRecord::Base
+        self.table_name = 'approval_project_rules'
+
+        belongs_to :project
+        has_and_belongs_to_many :users
+        has_and_belongs_to_many :groups, class_name: 'Group', join_table: :approval_project_rules_groups
+
+        scope :regular, -> { all }
+      end
+
+      class MergeRequest < ActiveRecord::Base
+        self.table_name = 'merge_requests'
+
+        belongs_to :target_project, class_name: "Project"
+        has_many :approval_rules, class_name: 'ApprovalMergeRequestRule'
+
+        def approver_ids
+          @approver_ids ||= Approver.where(target_type: 'MergeRequest', target_id: id).pluck('distinct user_id')
+        end
+
+        def approver_group_ids
+          @approver_group_ids ||= ApproverGroup.where(target_type: 'MergeRequest', target_id: id).pluck('distinct group_id')
+        end
+
+        def sync_code_owners_with_approvers
+          return if state == 'merged' || state == 'closed'
+
+          Gitlab::GitalyClient.allow_n_plus_1_calls do
+            ::MergeRequest.find(id).sync_code_owners_with_approvers
+          end
+        end
+      end
+
+      class Project < ActiveRecord::Base
+        self.table_name = 'projects'
+
+        has_many :approval_rules, class_name: 'ApprovalProjectRule'
+
+        def approver_ids
+          @approver_ids ||= Approver.where(target_type: 'Project', target_id: id).pluck('distinct user_id')
+        end
+
+        def approver_group_ids
+          @approver_group_ids ||= ApproverGroup.where(target_type: 'Project', target_id: id).pluck('distinct group_id')
+        end
+      end
+
+      class User < ActiveRecord::Base
+        self.table_name = 'users'
+      end
+
+      ALLOWED_TARGET_TYPES = %w{MergeRequest Project}.freeze
+
+      # @param target_type [String] class of target, either 'MergeRequest' or 'Project'
+      # @param target_id [Integer] id of target
+      def perform(target_type, target_id, sync_code_owner_rule: true)
+        @target_type = target_type
+        @target_id = target_id
+        @sync_code_owner_rule = sync_code_owner_rule
+
+        raise "Incorrect target_type #{target_type}" unless ALLOWED_TARGET_TYPES.include?(@target_type)
+
+        ActiveRecord::Base.transaction do
+          case target
+          when MergeRequest
+            handle_merge_request
+          when Project
+            handle_project
+          end
+        end
+      end
+
+      private
+
+      def handle_merge_request
+        if rule = sync_rule
+          rule.approval_project_rule = target.target_project.approval_rules.regular.first
+        end
+
+        target.sync_code_owners_with_approvers if @sync_code_owner_rule
+      end
+
+      def handle_project
+        sync_rule
+      end
+
+      def sync_rule
+        unless approvers_exists?
+          target.approval_rules.regular.delete_all
+          return
+        end
+
+        rule = first_or_initialize
+        rule.update(user_ids: target.approver_ids, group_ids: target.approver_group_ids)
+        rule
+      end
+
+      def target
+        strong_memoize(:target) do
+          case @target_type
+          when 'MergeRequest'
+            MergeRequest.find_by(id: @target_id)
+          when 'Project'
+            Project.find_by(id: @target_id)
+          end
+        end
+      end
+
+      def first_or_initialize
+        rule = target.approval_rules.regular.first_or_initialize
+
+        unless rule.persisted?
+          rule.name ||= ApprovalRuleLike::DEFAULT_NAME
+          rule.approvals_required = target.approvals_before_merge || 0
+          rule.save!
+        end
+
+        rule
+      end
+
+      def approvers_exists?
+        target.approver_ids.any? || target.approver_group_ids.any?
+      end
+    end
+  end
+end

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules_check_progress.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class MigrateApproverToApprovalRulesCheckProgress
+      RESCHEDULE_DELAY = 1.day
+
+      def perform
+        if remaining?
+          BackgroundMigrationWorker.perform_in(RESCHEDULE_DELAY, self.class.name)
+        else
+          Feature.enable(:approval_rule)
+        end
+      end
+
+      private
+
+      def remaining?
+        Gitlab::BackgroundMigration.exists?('MigrateApproverToApprovalRulesInBatch')
+      end
+    end
+  end
+end

ee/lib/gitlab/background_migration/migrate_approver_to_approval_rules_in_batch.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class MigrateApproverToApprovalRulesInBatch
+      class MergeRequest < ActiveRecord::Base
+        self.table_name = 'merge_requests'
+      end
+
+      def perform(start_id, end_id)
+        merge_request_ids = MergeRequest.where('id >= ? AND id <= ?', start_id, end_id).pluck(:id)
+        merge_request_ids.each do |merge_request_id|
+          MigrateApproverToApprovalRules.new.perform('MergeRequest', merge_request_id)
+        end
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesCheckProgress do
+  context 'when there is MigrateApproverToApprovalRulesInBatch jobs' do
+    it 'reschedules check' do
+      allow(Gitlab::BackgroundMigration).to receive(:exists?).with('MigrateApproverToApprovalRulesInBatch').and_return(true)
+
+      expect(BackgroundMigrationWorker).to receive(:perform_in).with(described_class::RESCHEDULE_DELAY, described_class.name)
+
+      described_class.new.perform
+    end
+  end
+
+  context 'when there is no more MigrateApproverToApprovalRulesInBatch jobs' do
+    it 'enables feature' do
+      allow(Gitlab::BackgroundMigration).to receive(:exists?).with('MigrateApproverToApprovalRulesInBatch').and_return(false)
+
+      expect(Feature).to receive(:enable).with(:approval_rule)
+
+      described_class.new.perform
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRulesInBatch do
+  context 'when there is no more MigrateApproverToApprovalRules jobs' do
+    let(:job) { double(:job) }
+    let(:project) { create(:project) }
+
+    it 'migrates individual target' do
+      allow(Gitlab::BackgroundMigration::MigrateApproverToApprovalRules).to receive(:new).and_return(job)
+
+      merge_requests = create_list(:merge_request, 3)
+
+      expect(job).to receive(:perform).exactly(3).times
+
+      described_class.new.perform(merge_requests.first.id, merge_requests.last.id)
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
+  def create_skip_sync(*args)
+    build(*args) do |record|
+      allow(record).to receive(:schedule_approval_migration)
+      record.save!
+    end
+  end
+
+  def create_member_in(member, *populate_in)
+    if populate_in.include?(:old_schema)
+      case member
+      when User
+        create_skip_sync(:approver, target_type: target_type, target_id: target.id, user_id: member.id)
+      when Group
+        create_skip_sync(:approver_group, target_type: target_type, target_id: target.id, group_id: member.id)
+      end
+    end
+
+    if populate_in.include?(:new_schema)
+      approval_rule.add_member(member)
+    end
+  end
+
+  context 'sync approval rule and its members' do
+    shared_examples 'sync approval member' do
+      let(:user1) { create(:user) }
+      let(:user2) { create(:user) }
+      let(:group1) { create(:group) }
+      let(:group2) { create(:group) }
+
+      context 'when member in old schema but not in new schema' do
+        before do
+          create_member_in(user1, :old_schema)
+          create_member_in(group1, :old_schema)
+        end
+
+        it 'creates in new schema' do
+          expect do
+            described_class.new.perform(target_type, target.id)
+          end.to change { approval_rule.users.count }.by(1)
+            .and change { approval_rule.groups.count }.by(1)
+
+          approval_rule = target.approval_rules.first
+
+          expect(approval_rule.approvals_required).to eq(0)
+          expect(approval_rule.name).to eq('Default')
+
+          expect(approval_rule.users).to contain_exactly(user1)
+          expect(approval_rule.groups).to contain_exactly(group1)
+        end
+      end
+
+      context 'when member not in old schema but in new schema' do
+        before do
+          create_member_in(user1, :new_schema)
+          create_member_in(user2, :old_schema, :new_schema)
+          create_member_in(group1, :new_schema)
+          create_member_in(group2, :old_schema, :new_schema)
+        end
+
+        it 'removes in new schema' do
+          expect do
+            described_class.new.perform(target_type, target.id)
+          end.to change { approval_rule.users.count }.by(-1)
+            .and change { approval_rule.groups.count }.by(-1)
+
+          approval_rule = target.approval_rules.first
+
+          expect(approval_rule.users).to contain_exactly(user2)
+          expect(approval_rule.groups).to contain_exactly(group2)
+        end
+      end
+    end
+
+    context 'merge request' do
+      let(:target) { create(:merge_request) }
+      let(:target_type) { 'MergeRequest' }
+      let(:approval_rule) { create(:approval_merge_request_rule, merge_request: target) }
+
+      it_behaves_like 'sync approval member'
+
+      context 'when project rule is present' do
+        let!(:project_rule) { create(:approval_project_rule, project: target.target_project) }
+
+        it "sets MR rule's source to project rule without duplication" do
+          user = create(:user)
+          create_member_in(user, :old_schema)
+          create_member_in(user, :old_schema)
+
+          described_class.new.perform(target_type, target.id)
+
+          expect(target.approval_rules.regular.first.approval_project_rule).to eq(project_rule)
+        end
+      end
+
+      context 'when project rule is absent' do
+        it "has MR rule's source as nil" do
+          create_member_in(create(:user), :old_schema)
+
+          described_class.new.perform(target_type, target.id)
+
+          expect(target.approval_rules.regular.first.approval_project_rule).to eq(nil)
+        end
+      end
+
+      context 'when approver is no longer overwritten' do
+        before do
+          create_member_in(create(:user), :new_schema)
+          create_member_in(create(:group), :new_schema)
+        end
+
+        it 'removes rule' do
+          expect do
+            described_class.new.perform(target_type, target.id)
+          end.to change { approval_rule.users.count }.by(-1)
+            .and change { approval_rule.groups.count }.by(-1)
+
+          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
+      let(:target) { create(:project) }
+      let(:target_type) { 'Project' }
+      let(:approval_rule) { create(:approval_project_rule, project: target) }
+
+      it_behaves_like 'sync approval member'
+    end
+  end
+
+  context 'when target is deleted' do
+    let(:target) { create(:project) }
+    let(:target_type) { 'Project' }
+
+    it "does not err" do
+      target.destroy
+
+      expect do
+        described_class.new.perform(target_type, target.id)
+      end.not_to raise_error
+    end
+  end
+end

ee/spec/migrations/migrate_project_approvers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('ee', 'db', 'post_migrate', '20181204040404_migrate_project_approvers.rb')
+
+describe MigrateProjectApprovers, :migration do
+  let(:migration) { described_class.new }
+
+  describe '#up' do
+    let(:projects) { table(:projects) }
+    let(:namespaces) { table(:namespaces) }
+    let(:approvers) { table(:approvers) }
+    let(:approval_project_rules) { table(:approval_project_rules) }
+    let(:approval_project_rules_users) { table(:approval_project_rules_users) }
+    let(:users) { table(:users) }
+    let(:features) { table(:features) }
+
+    before do
+      namespaces.create(id: 1, name: 'gitlab-org', path: 'gitlab-org')
+      projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 1)
+      projects.create!(id: 124, name: 'gitlab2', path: 'gitlab2', namespace_id: 1)
+
+      users.create(id: 1, name: 'user1', email: 'user1@example.com', projects_limit: 0)
+      users.create(id: 2, name: 'user2', email: 'user2@example.com', projects_limit: 0)
+
+      approvers.create!(target_id: 123, target_type: 'Project', user_id: 1)
+      approvers.create!(target_id: 124, target_type: 'Project', user_id: 2)
+    end
+
+    it 'creates approval rules and its associations' do
+      migrate!
+
+      expect(approval_project_rules.pluck(:project_id)).to eq([123, 124])
+
+      rule_ids = approval_project_rules.pluck(:id)
+
+      expect(approval_project_rules_users.where(approval_project_rule_id: rule_ids.first).pluck(:user_id)).to contain_exactly(1)
+      expect(approval_project_rules_users.where(approval_project_rule_id: rule_ids.last).pluck(:user_id)).to contain_exactly(2)
+      expect(features.where(key: 'approval_rule').exists?).to eq(true)
+    end
+  end
+end

ee/spec/models/concerns/approval_rule_like_spec.rb

@@ -87,4 +87,16 @@ describe ApprovalRuleLike do
 
     it_behaves_like 'approval rule like'
   end
+
+  context '.group_users' do
+    subject { create(:approval_project_rule) }
+
+    it 'returns distinct users' do
+      group1.add_guest(user1)
+      group2.add_guest(user1)
+      subject.groups = [group1, group2]
+
+      expect(subject.group_users).to eq([user1])
+    end
+  end
 end

ee/spec/models/concerns/approver_migrate_hook_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ApproverMigrateHook do
+  def members(rule)
+    rule.users.reload + rule.groups.reload
+  end
+
+  context 'create rule and member mapping' do
+    shared_examples 'migrating approver' do
+      context 'when approver is created' do
+        it 'creates rule and member mapping' do
+          expect do
+            approver
+          end.to change { target.approval_rules.count }.by(1)
+
+          rule = target.approval_rules.regular.last
+
+          expect(rule.approvals_required).to eq(0)
+          expect(rule.name).to eq('Default')
+          expect(members(rule)).to contain_exactly(approver.member)
+        end
+
+        context 'when rule already exists' do
+          let!(:approval_rule) { target.approval_rules.create(name: 'foo') }
+
+          it 'reuses rule' do
+            expect do
+              approver
+            end.not_to change { target.approval_rules.regular.count }
+
+            rule = target.approval_rules.regular.last
+
+            expect(rule).to eq(approval_rule)
+            expect(members(rule)).to contain_exactly(approver.member)
+          end
+
+          context 'when member mapping already exists' do
+            before do
+              approval_rule.add_member approver.member
+            end
+
+            it 'does nothing' do
+              approver
+
+              expect(members(approval_rule)).to contain_exactly(approver.member)
+            end
+          end
+        end
+      end
+
+      context 'when approver is destroyed' do
+        it 'destroys rule member' do
+          approver
+
+          rule = target.approval_rules.regular.first
+
+          expect do
+            approver.destroy!
+          end.to change { members(rule).count }.by(-1)
+        end
+      end
+    end
+
+    context 'User' do
+      let(:approver) { create(:approver, target: target) }
+
+      context 'merge request' do
+        let(:target) { create(:merge_request) }
+
+        it_behaves_like 'migrating approver'
+      end
+
+      context 'project' do
+        let(:target) { create(:project) }
+
+        it_behaves_like 'migrating approver'
+      end
+    end
+
+    context 'Group' do
+      let(:approver) { create(:approver_group, target: target) }
+
+      context 'merge request' do
+        let(:target) { create(:merge_request) }
+
+        it_behaves_like 'migrating approver'
+      end
+
+      context 'project' do
+        let(:target) { create(:project) }
+
+        it_behaves_like 'migrating approver'
+      end
+    end
+  end
+end

ee/spec/models/merge_request_spec.rb

@@ -70,6 +70,54 @@ describe MergeRequest do
     end
   end
 
+  describe '#sync_code_owners_with_approvers' do
+    let(:owners) { create_list(:user, 2) }
+
+    before do
+      allow(subject).to receive(:code_owners).and_return(owners)
+    end
+
+    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.not_to change { subject.approval_rules.count }
+    end
+
+    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)
+
+        expect(subject.approval_rules.code_owner.first.users).to contain_exactly(*owners)
+      end
+    end
+
+    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 }
+
+        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
+          subject.sync_code_owners_with_approvers
+
+          expect(subject.approval_rules.exists?(code_owner_rule.id)).to eq(false)
+        end
+      end
+    end
+  end
+
   describe '#base_pipeline' do
     let!(:pipeline) { create(:ci_empty_pipeline, project: subject.project, sha: subject.diff_base_sha) }
 

ee/spec/services/ee/merge_requests/create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequests::CreateService do
+  include ProjectForksHelper
+
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+  let(:service) { described_class.new(project, user, opts) }
+  let(:opts) do
+    {
+      title: 'Awesome merge_request',
+      description: 'please fix',
+      source_branch: 'feature',
+      target_branch: 'master',
+      force_remove_source_branch: '1'
+    }
+  end
+
+  before do
+    project.add_maintainer(user)
+    allow(service).to receive(:execute_hooks)
+  end
+
+  describe '#execute' do
+    context 'code owners' do
+      let!(:owners) { create_list(:user, 2) }
+
+      before do
+        allow(::Gitlab::CodeOwners).to receive(:for_merge_request).and_return(owners)
+      end
+
+      it 'syncs code owner to approval rule' do
+        merge_request = service.execute
+
+        rule = merge_request.approval_rules.code_owner.first
+
+        expect(rule.users).to contain_exactly(*owners)
+      end
+    end
+  end
+end

ee/spec/services/ee/merge_requests/post_merge_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequests::PostMergeService do
+  let(:project) { merge_request.target_project }
+  let(:merge_request) { create(:merge_request) }
+  let(:current_user) { merge_request.author }
+  let(:service) { described_class.new(project, current_user) }
+
+  describe '#execute' do
+    context 'finalize approvals' do
+      let(:finalize_service) { double(:finalize_service) }
+
+      it 'executes ApprovalRules::FinalizeService' do
+        expect(ApprovalRules::FinalizeService).to receive(:new).and_return(finalize_service)
+        expect(finalize_service).to receive(:execute)
+
+        service.execute(merge_request)
+      end
+    end
+  end
+end

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

@@ -102,6 +102,9 @@ describe MergeRequests::RefreshService do
 
             it 'does not create Approver' do
               expect { subject }.not_to change { Approver.count }
+
+              rule = merge_request.approval_rules.code_owner.first
+              expect(rule.users).to eq(new_owners)
             end
           end
         end
@@ -125,6 +128,9 @@ describe MergeRequests::RefreshService do
               expect(new_approver.user).to eq(owner)
               expect(new_approver.created_at).to be_present
               expect(new_approver.updated_at).to be_present
+
+              rule = merge_request.approval_rules.code_owner.first
+              expect(rule.users).to eq(new_owners)
             end
           end
         end

ee/spec/services/merge_requests/update_service_spec.rb

@@ -56,5 +56,17 @@ describe MergeRequests::UpdateService, :mailer do
         end.not_to change { ActionMailer::Base.deliveries.count }
       end
     end
+
+    context 'when approvals_before_merge changes' do
+      it "updates approval_rules' approvals_required" do
+        rule = create(:approval_merge_request_rule, merge_request: merge_request)
+        code_owner_rule = create(:approval_merge_request_rule, merge_request: merge_request, code_owner: true)
+
+        update_merge_request(approvals_before_merge: 42)
+
+        expect(rule.reload.approvals_required).to eq(42)
+        expect(code_owner_rule.reload.approvals_required).to eq(0)
+      end
+    end
   end
 end

ee/spec/services/projects/update_service_spec.rb

@@ -215,6 +215,16 @@ describe Projects::UpdateService, '#execute' do
     end
   end
 
+  context 'with approval_rules' do
+    it "updates approval_rules' approvals_required" do
+      rule = create(:approval_project_rule, project: project)
+
+      update_project(project, user, approvals_before_merge: 42)
+
+      expect(rule.reload.approvals_required).to eq(42)
+    end
+  end
+
   def update_project(project, user, opts)
     Projects::UpdateService.new(project, user, opts).execute
   end

lib/gitlab/background_migration.rb

@@ -51,6 +51,19 @@ module Gitlab
       migration_class_for(class_name).new.perform(*arguments)
     end
 
+    def self.exists?(migration_class)
+      enqueued = Sidekiq::Queue.new(self.queue)
+      scheduled = Sidekiq::ScheduledSet.new
+
+      [enqueued, scheduled].each do |queue|
+        queue.each do |job|
+          return true if job.queue == self.queue && job.args.first == migration_class
+        end
+      end
+
+      false
+    end
+
     def self.migration_class_for(class_name)
       const_get(class_name)
     end

spec/lib/gitlab/background_migration_spec.rb

@@ -119,4 +119,48 @@ describe Gitlab::BackgroundMigration do
       described_class.perform('Foo', [10, 20])
     end
   end
+
+  describe '.exists?' do
+    context 'when there are enqueued jobs present' do
+      let(:queue) do
+        [double(args: ['Foo', [10, 20]], queue: described_class.queue)]
+      end
+
+      before do
+        allow(Sidekiq::Queue).to receive(:new)
+          .with(described_class.queue)
+          .and_return(queue)
+      end
+
+      it 'returns true if specific job exists' do
+        expect(described_class.exists?('Foo')).to eq(true)
+      end
+
+      it 'returns false if specific job does not exist' do
+        expect(described_class.exists?('Bar')).to eq(false)
+      end
+    end
+
+    context 'when there are scheduled jobs present', :sidekiq, :redis do
+      before do
+        Sidekiq::Testing.disable! do
+          BackgroundMigrationWorker.perform_in(10.minutes, 'Foo')
+
+          expect(Sidekiq::ScheduledSet.new).to be_one
+        end
+      end
+
+      after do
+        Sidekiq::ScheduledSet.new.clear
+      end
+
+      it 'returns true if specific job exists' do
+        expect(described_class.exists?('Foo')).to eq(true)
+      end
+
+      it 'returns false if specific job does not exist' do
+        expect(described_class.exists?('Bar')).to eq(false)
+      end
+    end
+  end
 end