Validate MR approval_rules references all project rules

Allow assigning project rule for MR rule

When no MR approval rule, we don't validate as project rule will be
used.

ee/app/models/approval_merge_request_rule.rb

@@ -21,6 +21,11 @@ class ApprovalMergeRequestRule < ApplicationRecord
     merge_request.target_project
   end
 
+  def approval_project_rule_id=(approval_project_rule_id)
+    self.approval_merge_request_rule_source ||= build_approval_merge_request_rule_source
+    self.approval_merge_request_rule_source.approval_project_rule_id = approval_project_rule_id
+  end
+
   # Users who are eligible to approve, including specified group members.
   # Excludes the author if 'self-approval' isn't explicitly
   # enabled on project settings.

ee/app/models/approval_merge_request_rule_source.rb

@@ -4,4 +4,16 @@
 class ApprovalMergeRequestRuleSource < ApplicationRecord
   belongs_to :approval_merge_request_rule
   belongs_to :approval_project_rule
+
+  validate :validate_project_rule
+
+  private
+
+  def validate_project_rule
+    project = approval_merge_request_rule.merge_request.target_project
+
+    unless project.approval_rules.where(id: approval_project_rule_id).exists?
+      errors.add(:approval_project_rule, :invalid)
+    end
+  end
 end

ee/app/models/ee/merge_request.rb

@@ -22,6 +22,7 @@ module EE
       has_many :draft_notes
 
       validate :validate_approvals_before_merge, unless: :importing?
+      validate :validate_approval_rule_source
 
       delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true
       delegate :sha, to: :base_pipeline, prefix: :base_pipeline, allow_nil: true
@@ -56,6 +57,21 @@ module EE
       end
     end
 
+    def validate_approval_rule_source
+      return unless approval_rules.any?
+
+      local_project_rule_ids = approval_rules.map { |rule| rule.approval_merge_request_rule_source&.approval_project_rule_id }
+      local_project_rule_ids.compact!
+
+      invalid = if new_record?
+                  local_project_rule_ids.to_set != project.approval_rule_ids.to_set
+                else
+                  (local_project_rule_ids - project.approval_rule_ids).present?
+                end
+
+      errors.add(:approval_rules, :invalid_sourcing_to_project_rules) if invalid
+    end
+
     def participant_approvers
       strong_memoize(:participant_approvers) do
         next [] unless approval_needed?

ee/spec/models/merge_request_spec.rb

@@ -17,6 +17,88 @@ describe MergeRequest do
     it { is_expected.to have_many(:approved_by_users) }
   end
 
+  describe 'approval_rules' do
+    context 'when project contains approval_rules' do
+      let!(:project_rule1) {  project.approval_rules.create(name: 'p1') }
+      let!(:project_rule2) {  project.approval_rules.create(name: 'p2') }
+
+      context "when creating" do
+        subject(:merge_request) { build(:merge_request, source_project: project, target_project: project) }
+
+        context "when MR has no rule" do
+          it 'is valid as project rule will be active' do
+            expect(merge_request).to be_valid
+          end
+        end
+
+        context "when MR rules exists but do not reference all project rules" do
+          it 'is invalid' do
+            merge_request.approval_rules.build(name: 'mr1', approval_project_rule_id: project_rule1.id)
+
+            expect(merge_request).to be_invalid
+            expect(merge_request.errors.added?(:approval_rules, :invalid_sourcing_to_project_rules)).to eq(true)
+          end
+        end
+
+        context "when MR rules exists but reference rules other than the project's" do
+          let(:other_project_rule) { create(:approval_project_rule) }
+
+          it 'is invalid' do
+            merge_request.approval_rules.build(name: 'mr1', approval_project_rule_id: project_rule1.id)
+            merge_request.approval_rules.build(name: 'mr2', approval_project_rule_id: project_rule2.id)
+            merge_request.approval_rules.build(name: 'mr3', approval_project_rule_id: other_project_rule.id)
+
+            expect(merge_request).to be_invalid
+            expect(merge_request.errors.added?(:approval_rules, :invalid_sourcing_to_project_rules)).to eq(true)
+          end
+        end
+
+        context "when MR's rules exists and reference all project's rules" do
+          it 'is valid' do
+            merge_request.approval_rules.build(name: 'mr1', approval_project_rule_id: project_rule1.id)
+            merge_request.approval_rules.build(name: 'mr2', approval_project_rule_id: project_rule2.id)
+
+            expect(merge_request).to be_valid
+          end
+        end
+      end
+
+      context "when updating" do
+        subject!(:merge_request) do
+          merge_request = build(:merge_request, source_project: project, target_project: project)
+          merge_request.approval_rules.build(name: 'mr1', approval_project_rule_id: project_rule1.id)
+          merge_request.approval_rules.build(name: 'mr2', approval_project_rule_id: project_rule2.id)
+          merge_request.save!
+          merge_request
+        end
+
+        context "when MR rules reference rules other than the project's" do
+          let(:other_project_rule) { create(:approval_project_rule) }
+
+          it 'is invalid' do
+            merge_request.approval_rules.build(name: 'mr3', approval_project_rule_id: other_project_rule.id)
+
+            expect(merge_request).to be_invalid
+            expect(merge_request.errors.added?(:approval_rules, :invalid_sourcing_to_project_rules)).to eq(true)
+          end
+        end
+
+        context 'when project later added a new rule' do
+          before do
+            project.approval_rules.create(name: 'p3')
+          end
+
+          it 'can still be saved' do
+            subject.reload
+            subject.title = 'foobar'
+
+            expect(subject.save).to eq(true)
+          end
+        end
+      end
+    end
+  end
+
   describe 'approvals' do
     shared_examples_for 'authors self-approval authorization' do
       context 'when authors are authorized to approve their own MRs' do

ee/spec/services/approval_rules/create_service_spec.rb

@@ -88,5 +88,41 @@ describe ApprovalRules::CreateService do
     let(:target) { create(:merge_request, source_project: project, target_project: project) }
 
     it_behaves_like "creatable"
+
+    context 'when project rule id is present' do
+      let(:project_rule) { create(:approval_project_rule, project: project) }
+
+      it 'associates with project rule' do
+        result = described_class.new(target, user, {
+          name: 'foo',
+          approvals_required: 1,
+          approval_project_rule_id: project_rule.id
+        }).execute
+
+        expect(result[:status]).to eq(:success)
+
+        rule = result[:rule]
+
+        expect(rule.approval_project_rule).to eq(project_rule)
+      end
+    end
+
+    context "when project rule id is not the same as MR's project" do
+      let(:project_rule) { create(:approval_project_rule) }
+
+      it 'ignores assignment' do
+        result = described_class.new(target, user, {
+          name: 'foo',
+          approvals_required: 1,
+          approval_project_rule_id: project_rule.id
+        }).execute
+
+        expect(result[:status]).to eq(:success)
+
+        rule = result[:rule]
+
+        expect(rule.approval_project_rule).to eq(nil)
+      end
+    end
   end
 end