Add report_approver to approval_merge_request_rules

Updates ApprovalMergeRequestRules with new enum for the `rule_type`
subtype of 'report_approver'.

Initially there is only 1 report_type ('security') however this can
later be expanded to include other rules tied to report_approvers;
i.e.  code_quality or license_management.

This also includes some shims to the ApprovalRuleLike logic ensuring
compatibility between the existing ApprovalProjectRule and
ApprovalMergeRequestRule types

db/schema.rb

@@ -247,6 +247,7 @@ ActiveRecord::Schema.define(version: 20190611161641) do
     t.boolean "code_owner", default: false, null: false
     t.string "name", null: false
     t.integer "rule_type", limit: 2, default: 1, null: false
+    t.integer "report_type", limit: 2
     t.index ["merge_request_id", "code_owner", "name"], name: "approval_rule_name_index_for_code_owners", unique: true, where: "(code_owner = true)", using: :btree
     t.index ["merge_request_id", "code_owner"], name: "index_approval_merge_request_rules_1", using: :btree
     t.index ["merge_request_id", "rule_type", "name"], name: "index_approval_rule_name_for_code_owners_rule_type", unique: true, where: "(rule_type = 2)", using: :btree

ee/app/models/approval_merge_request_rule.rb

@@ -10,6 +10,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
   scope :matching_pattern, -> (pattern) { code_owner.where(name: pattern) }
 
   validates :name, uniqueness: { scope: [:merge_request, :code_owner] }
+  validates :report_type, presence: true, if: :report_approver?
   # Temporary validations until `code_owner` can be dropped in favor of `rule_type`
   # To be removed with https://gitlab.com/gitlab-org/gitlab-ee/issues/11834
   validates :code_owner, inclusion: { in: [true], if: :code_owner? }
@@ -27,7 +28,12 @@ class ApprovalMergeRequestRule < ApplicationRecord
 
   enum rule_type: {
     regular: 1,
-    code_owner: 2
+    code_owner: 2,
+    report_approver: 3
+  }
+
+  enum report_type: {
+    security: 1
   }
 
   # Deprecated scope until code_owner column has been migrated to rule_type

ee/app/models/approval_project_rule.rb

@@ -19,7 +19,16 @@ class ApprovalProjectRule < ApplicationRecord
   end
   alias_method :code_owner?, :code_owner
 
+  def report_approver
+    false
+  end
+  alias_method :report_approver?, :report_approver
+
   def source_rule
     nil
   end
+
+  def rule_type
+    'regular'
+  end
 end

ee/app/models/concerns/approval_rule_like.rb

@@ -45,7 +45,15 @@ module ApprovalRuleLike
     end
   end
 
-  def rule_type
-    @rule_type ||= code_owner? ? :code_owner : :regular
+  def code_owner?
+    raise NotImplementedError
+  end
+
+  def regular?
+    raise NotImplementedError
+  end
+
+  def report_approver?
+    raise NotImplementedError
   end
 end

ee/changelogs/unreleased/9928-add-report_approver-to-approval_rules.yml

+---
+title: Add report_approver to approval_merge_request_rules
+merge_request: 14050
+author:
+type: added

ee/db/migrate/20190521174505_add_report_type_to_approval_merge_request_rules.rb

+# frozen_string_literal: true
+
+class AddReportTypeToApprovalMergeRequestRules < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    change_table :approval_merge_request_rules do |t|
+      t.integer :report_type, limit: 2
+    end
+  end
+end

ee/spec/factories/approval_rules.rb

@@ -13,6 +13,13 @@ FactoryBot.define do
     sequence(:name) { |n| "*-#{n}.js" }
   end
 
+  factory :report_approver_rule, parent: :approval_merge_request_rule do
+    merge_request
+    rule_type :report_approver
+    report_type :security
+    sequence(:name) { |n| "*-#{n}.js" }
+  end
+
   factory :approval_project_rule do
     project
     name ApprovalRuleLike::DEFAULT_NAME

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -55,6 +55,20 @@ describe ApprovalMergeRequestRule do
         expect(new).not_to be_valid
       end
     end
+
+    context 'report_approver rules' do
+      it 'is valid' do
+        expect(build(:report_approver_rule)).to be_valid
+      end
+
+      it 'validates presence of report_type' do
+        rule = build(:report_approver_rule)
+        expect(rule).to be_valid
+
+        rule.report_type = nil
+        expect(rule).not_to be_valid
+      end
+    end
   end
 
   context 'scopes'  do

ee/spec/models/approval_project_rule_spec.rb

@@ -34,4 +34,10 @@ describe ApprovalProjectRule do
       expect(subject.code_owner?).to eq(false)
     end
   end
+
+  describe '#rule_type' do
+    it 'returns the correct type' do
+      expect(build(:approval_project_rule).rule_type).to eq('regular')
+    end
+  end
 end

ee/spec/models/concerns/approval_rule_like_spec.rb

@@ -135,12 +135,4 @@ describe ApprovalRuleLike do
       expect(subject.group_users).to eq([user1])
     end
   end
-
-  context '#rule_type' do
-    it 'returns the correct type' do
-      expect(build(:code_owner_rule).rule_type).to eq(:code_owner)
-      expect(build(:approval_merge_request_rule).rule_type).to eq(:regular)
-      expect(build(:approval_project_rule).rule_type).to eq(:regular)
-    end
-  end
 end