Commit 5c3c4ba6 authored by Igor Drozdov's avatar Igor Drozdov

Add app validation for any-approver rule uniqueness

This would save us from 500 error raised by database
parent 7467e0ae
......@@ -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
......
......@@ -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) }
......
---
title: Add app validation for any-approver rule uniqueness
merge_request: 23241
author:
type: fixed
......@@ -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
......
......@@ -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
......
......@@ -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 ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment