Commit f714860a authored by Mark Chao's avatar Mark Chao

Check overall_approvals_required for approved?

This is useful for maintainer to set project-wide restriction on
number of approvals required, but let MR author choose whom to approve.

Change any_approval_allowed? as it now has different meaning
to approved?
parent 812f5106
......@@ -10,7 +10,7 @@ class ApprovalState
attr_reader :merge_request, :project
def_delegators :@merge_request, :merge_status, :approved_by_users
def_delegators :@merge_request, :merge_status, :approved_by_users, :approvals
alias_method :approved_approvers, :approved_by_users
def initialize(merge_request)
......@@ -48,12 +48,19 @@ class ApprovalState
!approved?
end
def overall_approvals_required
@overall_approvals_required ||= project.approvals_before_merge
end
def approved?
strong_memoize(:approved) do
wrapped_approval_rules.all?(&:approved?)
(overall_approvals_required == 0 || approvals.size >= overall_approvals_required) && wrapped_approval_rules.all?(&:approved?)
end
end
alias_method :any_approver_allowed?, :approved?
def any_approver_allowed?
approved? || overall_approvals_required > approvers.size
end
# Number of approvals remaining (excluding existing approvals) before the MR is
# considered approved.
......
......@@ -97,6 +97,16 @@ describe ApprovalState do
expect(subject.approval_needed?).to eq(false)
end
context 'when overall_approvals_required is not met' do
before do
project.update(approvals_before_merge: 1)
end
it 'returns false' do
expect(subject.approved?).to eq(false)
end
end
end
context 'when not approved' do
......@@ -136,6 +146,38 @@ describe ApprovalState do
end
end
describe '#any_approver_allowed?' do
context 'when approved' do
before do
allow(subject).to receive(:approved?).and_return(true)
end
it 'returns true' do
expect(subject.any_approver_allowed?).to eq(true)
end
end
context 'when not approved' do
before do
allow(subject).to receive(:approved?).and_return(false)
end
it 'returns false' do
expect(subject.approved?).to eq(false)
end
context 'when overall_approvals_required cannot be met' do
before do
project.update(approvals_before_merge: 1)
end
it 'returns false' do
expect(subject.any_approver_allowed?).to eq(true)
end
end
end
end
describe '#approvals_left' do
before do
create_rule(approvals_required: 5)
......@@ -585,6 +627,16 @@ describe ApprovalState do
it 'returns true' do
expect(subject.approved?).to eq(true)
end
context 'when overall_approvals_required is not met' do
before do
project.update(approvals_before_merge: 1)
end
it 'returns false' do
expect(subject.approved?).to eq(false)
end
end
end
context 'when some rules are not approved' do
......@@ -598,6 +650,38 @@ describe ApprovalState do
end
end
describe '#any_approver_allowed?' do
context 'when approved' do
before do
allow(subject).to receive(:approved?).and_return(true)
end
it 'returns true' do
expect(subject.any_approver_allowed?).to eq(true)
end
end
context 'when not approved' do
before do
allow(subject).to receive(:approved?).and_return(false)
end
it 'returns false' do
expect(subject.approved?).to eq(false)
end
context 'when overall_approvals_required cannot be met' do
before do
project.update(approvals_before_merge: 1)
end
it 'returns false' do
expect(subject.any_approver_allowed?).to eq(true)
end
end
end
end
describe '#approvals_left' do
let(:rule1) { create_unapproved_rule(approvals_required: 5) }
let(:rule2) { create_unapproved_rule(approvals_required: 7) }
......
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