Commit 363339c6 authored by Mark Chao's avatar Mark Chao

Cap approvals_required at 100

Fix migration to honor this cap
parent 24e9be42
...@@ -4,6 +4,7 @@ module ApprovalRuleLike ...@@ -4,6 +4,7 @@ module ApprovalRuleLike
extend ActiveSupport::Concern extend ActiveSupport::Concern
DEFAULT_NAME = 'Default' DEFAULT_NAME = 'Default'
APPROVALS_REQUIRED_MAX = 100
included do included do
has_and_belongs_to_many :users has_and_belongs_to_many :users
...@@ -11,6 +12,7 @@ module ApprovalRuleLike ...@@ -11,6 +12,7 @@ module ApprovalRuleLike
has_many :group_users, -> { distinct }, through: :groups, source: :users has_many :group_users, -> { distinct }, through: :groups, source: :users
validates :name, presence: true validates :name, presence: true
validates :approvals_required, numericality: { less_than_or_equal_to: APPROVALS_REQUIRED_MAX }
end end
# Users who are eligible to approve, including specified group members. # Users who are eligible to approve, including specified group members.
......
...@@ -162,7 +162,7 @@ module Gitlab ...@@ -162,7 +162,7 @@ module Gitlab
unless rule.persisted? unless rule.persisted?
rule.name ||= ApprovalRuleLike::DEFAULT_NAME rule.name ||= ApprovalRuleLike::DEFAULT_NAME
rule.approvals_required = target.approvals_required rule.approvals_required = [target.approvals_required, ApprovalRuleLike::APPROVALS_REQUIRED_MAX].min
rule.save! rule.save!
end end
......
...@@ -154,6 +154,18 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do ...@@ -154,6 +154,18 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
end end
end end
context 'when approvals_before_merge is too big' do
it "caps at allowed maximum" do
target.target_project.update(approvals_before_merge: ::ApprovalRuleLike::APPROVALS_REQUIRED_MAX + 1)
target.update(approvals_before_merge: nil)
create_member_in(create(:user), :old_schema)
described_class.new.perform(target_type, target.id)
expect(target.approval_rules.regular.first.approvals_required).to eq(::ApprovalRuleLike::APPROVALS_REQUIRED_MAX)
end
end
context 'when approver is no longer overwritten' do context 'when approver is no longer overwritten' do
before do before do
create_member_in(create(:user), :new_schema) create_member_in(create(:user), :new_schema)
......
...@@ -74,6 +74,25 @@ describe ApprovalRuleLike do ...@@ -74,6 +74,25 @@ describe ApprovalRuleLike do
end end
end end
end end
describe 'validation' do
context 'when value is too big' do
it 'is invalid' do
subject.approvals_required = described_class::APPROVALS_REQUIRED_MAX + 1
expect(subject).to be_invalid
expect(subject.errors.key?(:approvals_required)).to eq(true)
end
end
context 'when value is within limit' do
it 'is valid' do
subject.approvals_required = described_class::APPROVALS_REQUIRED_MAX
expect(subject).to be_valid
end
end
end
end end
context 'MergeRequest' do context 'MergeRequest' do
......
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