Commit 341838ac authored by Mark Chao's avatar Mark Chao

Only check overall approvals required when with no rule

This excludes code owner rule
parent 2d2ea563
...@@ -43,16 +43,24 @@ class ApprovalState ...@@ -43,16 +43,24 @@ class ApprovalState
def approval_needed? def approval_needed?
return false unless project.feature_available?(:merge_request_approvers) return false unless project.feature_available?(:merge_request_approvers)
overall_approvals_required > 0 || wrapped_approval_rules.any? { |rule| rule.approvals_required > 0 } if regular_rules.empty?
overall_approvals_required > 0
else
wrapped_approval_rules.any? { |rule| rule.approvals_required > 0 }
end
end end
def overall_approvals_required def overall_approvals_required
@overall_approvals_required ||= project.approvals_before_merge @overall_approvals_required ||= [project.approvals_before_merge, merge_request.approvals_before_merge].max
end end
def approved? def approved?
strong_memoize(:approved) do strong_memoize(:approved) do
(overall_approvals_required == 0 || approvals.size >= overall_approvals_required) && wrapped_approval_rules.all?(&:approved?) if regular_rules.empty?
approvals.size >= overall_approvals_required
else
wrapped_approval_rules.all?(&:approved?)
end
end end
end end
...@@ -64,7 +72,11 @@ class ApprovalState ...@@ -64,7 +72,11 @@ class ApprovalState
# considered approved. # considered approved.
def approvals_left def approvals_left
strong_memoize(:approvals_left) do strong_memoize(:approvals_left) do
wrapped_approval_rules.sum(&:approvals_left) if regular_rules.empty?
[overall_approvals_required - approved_approvers.size, 0].max
else
wrapped_approval_rules.sum(&:approvals_left)
end
end end
end end
......
...@@ -170,8 +170,8 @@ describe ApprovalState do ...@@ -170,8 +170,8 @@ describe ApprovalState do
project.update(approvals_before_merge: 3) project.update(approvals_before_merge: 3)
end end
it 'returns false' do it 'returns true as overall approvals_required is ignored' do
expect(subject.approved?).to eq(false) expect(subject.approved?).to eq(true)
end end
end end
end end
...@@ -714,8 +714,8 @@ describe ApprovalState do ...@@ -714,8 +714,8 @@ describe ApprovalState do
project.update(approvals_before_merge: 3) project.update(approvals_before_merge: 3)
end end
it 'returns false' do it 'returns true as overall approvals_required is ignored' do
expect(subject.approved?).to eq(false) expect(subject.approved?).to eq(true)
end end
end end
end end
......
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