Add ApprovalMergeRequestRule and ApprovalProjectRule

These represents rules associated with MRs and projects.
Rules are further associated with User and Group as members.

ApprovalMergeRequestRule is associated with Approvals,
as a record of whom approved what when MR is merged.
This is because recomputation result can change in
the future when project approvers/group members changes.

db/schema.rb

@@ -219,6 +219,60 @@ ActiveRecord::Schema.define(version: 20190103140724) do
     t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
   end
 
+  create_table "approval_merge_request_rules", id: :bigserial, force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.integer "merge_request_id", null: false
+    t.integer "approvals_required", limit: 2, default: 0, null: false
+    t.boolean "code_owner", default: false, null: false
+    t.string "name", null: false
+    t.index ["merge_request_id"], name: "index_approval_merge_request_rules_on_merge_request_id", using: :btree
+  end
+
+  create_table "approval_merge_request_rules_approvals", id: :bigserial, force: :cascade do |t|
+    t.bigint "approval_merge_request_rule_id", null: false
+    t.integer "approval_id", null: false
+    t.index ["approval_id"], name: "index_approval_merge_request_rules_approvals_2", using: :btree
+    t.index ["approval_merge_request_rule_id", "approval_id"], name: "index_approval_merge_request_rules_approvals_1", unique: true, using: :btree
+  end
+
+  create_table "approval_merge_request_rules_groups", id: :bigserial, force: :cascade do |t|
+    t.bigint "approval_merge_request_rule_id", null: false
+    t.integer "group_id", null: false
+    t.index ["approval_merge_request_rule_id", "group_id"], name: "index_approval_merge_request_rules_groups_1", unique: true, using: :btree
+    t.index ["group_id"], name: "index_approval_merge_request_rules_groups_2", using: :btree
+  end
+
+  create_table "approval_merge_request_rules_users", id: :bigserial, force: :cascade do |t|
+    t.bigint "approval_merge_request_rule_id", null: false
+    t.integer "user_id", null: false
+    t.index ["approval_merge_request_rule_id", "user_id"], name: "index_approval_merge_request_rules_users_1", unique: true, using: :btree
+    t.index ["user_id"], name: "index_approval_merge_request_rules_users_2", using: :btree
+  end
+
+  create_table "approval_project_rules", id: :bigserial, force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.integer "project_id", null: false
+    t.integer "approvals_required", limit: 2, default: 0, null: false
+    t.string "name", null: false
+    t.index ["project_id"], name: "index_approval_project_rules_on_project_id", using: :btree
+  end
+
+  create_table "approval_project_rules_groups", id: :bigserial, force: :cascade do |t|
+    t.bigint "approval_project_rule_id", null: false
+    t.integer "group_id", null: false
+    t.index ["approval_project_rule_id", "group_id"], name: "index_approval_project_rules_groups_1", unique: true, using: :btree
+    t.index ["group_id"], name: "index_approval_project_rules_groups_2", using: :btree
+  end
+
+  create_table "approval_project_rules_users", id: :bigserial, force: :cascade do |t|
+    t.bigint "approval_project_rule_id", null: false
+    t.integer "user_id", null: false
+    t.index ["approval_project_rule_id", "user_id"], name: "index_approval_project_rules_users_1", unique: true, using: :btree
+    t.index ["user_id"], name: "index_approval_project_rules_users_2", using: :btree
+  end
+
   create_table "approvals", force: :cascade do |t|
     t.integer "merge_request_id", null: false
     t.integer "user_id", null: false
@@ -3163,6 +3217,18 @@ ActiveRecord::Schema.define(version: 20190103140724) do
   add_foreign_key "application_settings", "namespaces", column: "custom_project_templates_group_id", on_delete: :nullify
   add_foreign_key "application_settings", "projects", column: "file_template_project_id", name: "fk_ec757bd087", on_delete: :nullify
   add_foreign_key "application_settings", "users", column: "usage_stats_set_by_user_id", name: "fk_964370041d", on_delete: :nullify
+  add_foreign_key "approval_merge_request_rules", "merge_requests", on_delete: :cascade
+  add_foreign_key "approval_merge_request_rules_approvals", "approval_merge_request_rules", on_delete: :cascade
+  add_foreign_key "approval_merge_request_rules_approvals", "approvals", on_delete: :cascade
+  add_foreign_key "approval_merge_request_rules_groups", "approval_merge_request_rules", on_delete: :cascade
+  add_foreign_key "approval_merge_request_rules_groups", "namespaces", column: "group_id", on_delete: :cascade
+  add_foreign_key "approval_merge_request_rules_users", "approval_merge_request_rules", on_delete: :cascade
+  add_foreign_key "approval_merge_request_rules_users", "users", on_delete: :cascade
+  add_foreign_key "approval_project_rules", "projects", on_delete: :cascade
+  add_foreign_key "approval_project_rules_groups", "approval_project_rules", on_delete: :cascade
+  add_foreign_key "approval_project_rules_groups", "namespaces", column: "group_id", on_delete: :cascade
+  add_foreign_key "approval_project_rules_users", "approval_project_rules", on_delete: :cascade
+  add_foreign_key "approval_project_rules_users", "users", on_delete: :cascade
   add_foreign_key "approvals", "merge_requests", name: "fk_310d714958", on_delete: :cascade
   add_foreign_key "approver_groups", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "badges", "namespaces", column: "group_id", on_delete: :cascade

ee/app/models/approval.rb

@@ -3,6 +3,7 @@
 class Approval < ActiveRecord::Base
   belongs_to :user
   belongs_to :merge_request
+  has_and_belongs_to_many :approval_rules, class_name: 'ApprovalMergeRequestRule', join_table: :approval_merge_request_rules_approvals
 
   validates :merge_request_id, presence: true
   validates :user_id, presence: true, uniqueness: { scope: [:merge_request_id] }

ee/app/models/approval_merge_request_rule.rb

+# frozen_string_literal: true
+
+class ApprovalMergeRequestRule < ApplicationRecord
+  include ApprovalRuleLike
+
+  scope :regular, -> { where(code_owner: false) }
+  scope :code_owner, -> { where(code_owner: true) } # special code owner rules, updated internally when code changes
+
+  belongs_to :merge_request
+
+  has_and_belongs_to_many :approvals # This is only populated after merge request is merged
+  has_many :approved_approvers, through: :approvals, source: :user
+
+  def project
+    merge_request.target_project
+  end
+
+  # Users who are eligible to approve, including specified group members.
+  # Excludes the author if 'self-approval' isn't explicitly
+  # enabled on project settings.
+  # @return [Array<User>]
+  def approvers
+    scope = super
+
+    if merge_request.author && !project.merge_requests_author_approval?
+      scope = scope.where.not(id: merge_request.author)
+    end
+
+    scope
+  end
+
+  def sync_approvals
+    # Before being merged, approvals are dynamically calculated instead of being persisted.
+    return unless merge_request.merged?
+
+    self.approvals = merge_request.approvals.where(user_id: approvers.map(&:id))
+  end
+end

ee/app/models/approval_project_rule.rb

+# frozen_string_literal: true
+
+class ApprovalProjectRule < ApplicationRecord
+  include ApprovalRuleLike
+
+  belongs_to :project
+
+  # To allow easier duck typing
+  scope :regular, -> { all }
+  scope :code_owner, -> { none }
+
+  def code_owner
+    false
+  end
+  alias_method :code_owner?, :code_owner
+end

ee/app/models/concerns/approval_rule_like.rb

+# frozen_string_literal: true
+
+module ApprovalRuleLike
+  extend ActiveSupport::Concern
+  include Gitlab::Utils::StrongMemoize
+
+  DEFAULT_NAME = 'Default'
+
+  included do
+    has_and_belongs_to_many :users
+    has_and_belongs_to_many :groups, class_name: 'Group', join_table: "#{self.table_name}_groups"
+
+    validates :name, presence: true
+  end
+
+  # Users who are eligible to approve, including specified group members.
+  # @return [Array<User>]
+  def approvers
+    strong_memoize(:approvers) do
+      User.from_union(
+        [
+          users,
+          User.joins(:group_members).where(members: { source_id: groups })
+        ]
+      )
+    end
+  end
+
+  def add_member(member)
+    case member
+    when User
+      users << member unless users.exists?(member.id)
+    when Group
+      groups << member unless groups.exists?(member.id)
+    end
+  end
+
+  def remove_member(member)
+    case member
+    when User
+      users.delete(member)
+    when Group
+      groups.delete(member)
+    end
+  end
+end

ee/app/models/ee/merge_request.rb

@@ -16,6 +16,7 @@ module EE
       has_many :approved_by_users, through: :approvals, source: :user
       has_many :approvers, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
       has_many :approver_groups, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+      has_many :approval_rules, class_name: 'ApprovalMergeRequestRule'
       has_many :draft_notes
 
       validate :validate_approvals_before_merge, unless: :importing?

ee/app/models/ee/project.rb

@@ -44,6 +44,7 @@ module EE
       has_many :reviews, inverse_of: :project
       has_many :approvers, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
       has_many :approver_groups, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+      has_many :approval_rules, class_name: 'ApprovalProjectRule'
       has_many :audit_events, as: :entity
       has_many :path_locks
       has_many :vulnerability_feedback, class_name: 'Vulnerabilities::Feedback'

ee/db/migrate/20181204031328_create_approval_rules.rb

+# frozen_string_literal: true
+
+class CreateApprovalRules < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :approval_project_rules, id: :bigserial do |t|
+      t.timestamps_with_timezone
+      t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false
+      t.integer :approvals_required, limit: 2, default: 0, null: false
+      t.string :name, null: false
+    end
+
+    create_table :approval_merge_request_rules, id: :bigserial do |t|
+      t.timestamps_with_timezone
+      t.references :merge_request, index: true, foreign_key: { on_delete: :cascade }, null: false
+      t.integer :approvals_required, limit: 2, default: 0, null: false
+      t.boolean :code_owner, default: false, null: false
+      t.string :name, null: false
+    end
+  end
+end

ee/db/migrate/20181204031329_create_approval_rules_approvals.rb

+# frozen_string_literal: true
+
+class CreateApprovalRulesApprovals < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table(:approval_merge_request_rules_approvals, id: :bigserial) do |t|
+      t.references(
+        :approval_merge_request_rule,
+        type: :bigint,
+        null: false,
+        foreign_key: { on_delete: :cascade },
+        index: false
+      )
+      t.references(
+        :approval,
+        type: :integer,
+        null: false,
+        foreign_key: { on_delete: :cascade },
+        index: { name: 'index_approval_merge_request_rules_approvals_2' }
+      )
+
+      t.index [:approval_merge_request_rule_id, :approval_id], unique: true, name: 'index_approval_merge_request_rules_approvals_1'
+    end
+  end
+end

ee/db/migrate/20181204031330_create_approval_rule_members.rb

+# frozen_string_literal: true
+
+class CreateApprovalRuleMembers < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  TABLES = [
+    { table: 'approval_merge_request_rules_users',  rule: 'approval_merge_request_rule', member: 'user',  member_table: 'users' },
+    { table: 'approval_merge_request_rules_groups', rule: 'approval_merge_request_rule', member: 'group', member_table: 'namespaces' },
+    { table: 'approval_project_rules_users',        rule: 'approval_project_rule',       member: 'user',  member_table: 'users' },
+    { table: 'approval_project_rules_groups',       rule: 'approval_project_rule',       member: 'group', member_table: 'namespaces' }
+  ].freeze
+
+  def up
+    TABLES.each do |params|
+      member_id = "#{params[:member]}_id"
+      rule_id = "#{params[:rule]}_id"
+
+      create_table(params[:table], id: :bigserial) do |t|
+        t.references params[:rule],   null: false, type: :bigint,  index: false, foreign_key: { on_delete: :cascade }
+        t.references params[:member], null: false, type: :integer, index: { name: "index_#{params[:table]}_2" }
+
+        # To accommodate Group being in the `namespaces` table
+        t.foreign_key params[:member_table], column: member_id, on_delete: :cascade
+
+        t.index [rule_id, member_id], unique: true, name: "index_#{params[:table]}_1"
+      end
+    end
+  end
+
+  def down
+    TABLES.each { |params| drop_table(params[:table]) }
+  end
+end

ee/spec/factories/approval_rules.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :approval_merge_request_rule do
+    merge_request
+    name ApprovalRuleLike::DEFAULT_NAME
+  end
+
+  factory :approval_project_rule do
+    project
+    name ApprovalRuleLike::DEFAULT_NAME
+  end
+end

ee/spec/lib/gitlab/import_export/all_models.yml

@@ -6,6 +6,7 @@ milestone:
 - boards
 merge_requests:
 - reviews
+- approval_rules
 - approvals
 - approvers
 - approver_groups
@@ -50,6 +51,7 @@ project:
 - jenkins_service
 - jenkins_deprecated_service
 - index_status
+- approval_rules
 - approvers
 - pages_domains
 - audit_events

ee/spec/models/approval_merge_request_rule_spec.rb

+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ApprovalMergeRequestRule, type: :model do
+  let(:merge_request) { create(:merge_request) }
+
+  subject { create(:approval_merge_request_rule, merge_request: merge_request) }
+
+  describe '#project' do
+    it 'returns project of MergeRequest' do
+      expect(subject.project).to eq(merge_request.project)
+    end
+  end
+
+  describe '#approvers' do
+    context 'when project setting includes author' do
+      before do
+        merge_request.target_project.update(merge_requests_author_approval: true)
+
+        create(:group) do |group|
+          group.add_guest(merge_request.author)
+          subject.groups << group
+        end
+      end
+
+      it 'contains author' do
+        expect(subject.approvers).to contain_exactly(merge_request.author)
+      end
+    end
+  end
+
+  describe '#sync_approvals' do
+    let(:member1) { create(:user) }
+    let(:member2) { create(:user) }
+    let(:member3) { create(:user) }
+    let!(:approval1) { create(:approval, merge_request: merge_request, user: member1) }
+    let!(:approval2) { create(:approval, merge_request: merge_request, user: member2) }
+    let!(:approval3) { create(:approval, merge_request: merge_request, user: member3) }
+
+    before do
+      subject.users = [member1, member2]
+    end
+
+    context 'when not merged' do
+      it 'does nothing' do
+        subject.sync_approvals
+
+        expect(subject.approvals).to be_empty
+      end
+    end
+
+    context 'when merged' do
+      let(:merge_request) { create(:merged_merge_request) }
+
+      it 'updates mapping' do
+        subject.sync_approvals
+
+        expect(subject.approvals.reload).to contain_exactly(approval1, approval2)
+      end
+    end
+  end
+end

ee/spec/models/approval_project_rule_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ApprovalProjectRule do
+  subject { create(:approval_project_rule) }
+
+  describe '.regular' do
+    it 'returns all records' do
+      rules = create_list(:approval_project_rule, 2)
+
+      expect(described_class.regular).to contain_exactly(*rules)
+    end
+  end
+
+  describe '.code_ownerscope' do
+    it 'returns nothing' do
+      create_list(:approval_project_rule, 2)
+
+      expect(described_class.code_owner).to be_empty
+    end
+  end
+
+  describe '#code_owner' do
+    it 'returns false' do
+      expect(subject.code_owner).to eq(false)
+      expect(subject.code_owner?).to eq(false)
+    end
+  end
+end

ee/spec/models/concerns/approval_rule_like_spec.rb

+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ApprovalRuleLike, type: :model do
+  let(:member1) { create(:user) }
+  let(:member2) { create(:user) }
+  let(:member3) { create(:user) }
+  let(:group1) { create(:group) }
+  let(:group2) { create(:group) }
+
+  let(:merge_request) { create(:merge_request) }
+
+  shared_examples 'approval rule like' do
+    describe '#add_member' do
+      it 'adds as a member of the rule' do
+        expect do
+          subject.add_member(member1)
+        end.to change { subject.users.count }.by(1)
+      end
+
+      it 'does nothing if already a member' do
+        subject.add_member(member1)
+
+        expect do
+          subject.add_member(member1)
+        end.not_to change { subject.users.count }
+      end
+    end
+
+    describe '#remove_member' do
+      it 'removes a member from the rule' do
+        subject.add_member(group1)
+
+        expect do
+          subject.remove_member(group1)
+        end.to change { subject.groups.count }.by(-1)
+      end
+
+      it 'does nothing if not a member' do
+        expect do
+          subject.remove_member(group1)
+        end.not_to change { subject.groups.count }
+      end
+    end
+
+    describe '#approvers' do
+      let(:group1_member1) { create(:user) }
+      let(:group2_member1) { create(:user) }
+
+      before do
+        subject.users << member1
+        subject.users << member2
+        subject.groups << group1
+        subject.groups << group2
+
+        group1.add_guest(group1_member1)
+        group2.add_guest(group2_member1)
+      end
+
+      it 'contains users as direct members and group members' do
+        expect(subject.approvers).to contain_exactly(member1, member2, group1_member1, group2_member1)
+      end
+
+      context 'when user is both a direct member and a group member' do
+        before do
+          group1.add_guest(member1)
+          group2.add_guest(member2)
+        end
+
+        it 'contains only unique users' do
+          expect(subject.approvers).to contain_exactly(member1, member2, group1_member1, group2_member1)
+        end
+      end
+    end
+  end
+
+  context 'MergeRequest' do
+    subject { create(:approval_merge_request_rule, merge_request: merge_request) }
+
+    it_behaves_like 'approval rule like'
+  end
+
+  context 'Project' do
+    subject { create(:approval_project_rule) }
+
+    it_behaves_like 'approval rule like'
+  end
+end