Merge branch 'id-validate-any-approval-rule-uniqueness' into 'master'

Add app validation for any-approver rule uniqueness

See merge request gitlab-org/gitlab!23241

ee/app/models/approval_merge_request_rule.rb

@@ -24,6 +24,7 @@ class ApprovalMergeRequestRule < ApplicationRecord
   end
 
   validates :name, uniqueness: { scope: [:merge_request, :code_owner] }
+  validates :rule_type, uniqueness: { scope: :merge_request_id, message: proc { _('any-approver for the merge request already exists') } }, if: :any_approver?
   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/issues/11834

ee/app/models/approval_project_rule.rb

@@ -17,6 +17,7 @@ class ApprovalProjectRule < ApplicationRecord
   validate :validate_default_license_report_name, on: :update, if: :report_approver?
 
   validates :name, uniqueness: { scope: :project_id }
+  validates :rule_type, uniqueness: { scope: :project_id, message: proc { _('any-approver for the project already exists') } }, if: :any_approver?
 
   def self.applicable_to_branch(branch)
     includes(:protected_branches).select { |rule| rule.applies_to_branch?(branch) }

ee/changelogs/unreleased/id-validate-any-approval-rule-uniqueness.yml

+---
+title: Add app validation for any-approver rule uniqueness
+merge_request: 23241
+author:
+type: fixed

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -92,14 +92,12 @@ describe ApprovalMergeRequestRule do
     context 'any_approver rules' do
       let(:rule) { build(:approval_merge_request_rule, merge_request: merge_request, rule_type: :any_approver) }
 
-      it 'is valid' do
-        expect(rule).to be_valid
-      end
-
-      it 'creating more than one any_approver rule raises an error' do
+      it 'creating only one any_approver rule is allowed' do
         create(:approval_merge_request_rule, merge_request: merge_request, rule_type: :any_approver)
 
-        expect { rule.save }.to raise_error(ActiveRecord::RecordNotUnique)
+        expect(rule).not_to be_valid
+        expect(rule.errors.messages).to eq(rule_type: ['any-approver for the merge request already exists'])
+        expect { rule.save(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
       end
     end
   end

ee/spec/models/approval_project_rule_spec.rb

@@ -141,10 +141,12 @@ describe ApprovalProjectRule do
     let(:project) { create(:project) }
     let(:rule) { build(:approval_project_rule, project: project, rule_type: :any_approver) }
 
-    it 'creating more than one any_approver rule raises an error' do
+    it 'creating only one any_approver rule is allowed' do
       create(:approval_project_rule, project: project, rule_type: :any_approver)
 
-      expect { rule.save }.to raise_error(ActiveRecord::RecordNotUnique)
+      expect(rule).not_to be_valid
+      expect(rule.errors.messages).to eq(rule_type: ['any-approver for the project already exists'])
+      expect { rule.save(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
     end
   end
 

locale/gitlab.pot

@@ -21766,6 +21766,12 @@ msgstr ""
 msgid "and"
 msgstr ""
 
+msgid "any-approver for the merge request already exists"
+msgstr ""
+
+msgid "any-approver for the project already exists"
+msgstr ""
+
 msgid "archived"
 msgstr ""