Commit 2e2f9ce2 authored by Sean McGivern's avatar Sean McGivern

Ensure an MR never has negative approvals left

parent c5adebd6
...@@ -644,7 +644,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -644,7 +644,10 @@ class MergeRequest < ActiveRecord::Base
# choose the lower so the MR doesn't get 'stuck' in a state where it can't be approved. # choose the lower so the MR doesn't get 'stuck' in a state where it can't be approved.
# #
def approvals_left def approvals_left
[approvals_required - approvals.count, number_of_potential_approvers].min [
[approvals_required - approvals.count, number_of_potential_approvers].min,
0
].max
end end
def approvals_required def approvals_required
......
...@@ -1010,6 +1010,22 @@ describe MergeRequest, models: true do ...@@ -1010,6 +1010,22 @@ describe MergeRequest, models: true do
expect(merge_request.can_approve?(nil)).to be_falsey expect(merge_request.can_approve?(nil)).to be_falsey
end end
end end
context 'when more than the number of approvers have approved the MR' do
before do
create(:approval, user: approver, merge_request: merge_request)
create(:approval, user: approver_2, merge_request: merge_request)
create(:approval, user: developer, merge_request: merge_request)
end
it 'marks the MR as approved' do
expect(merge_request).to be_approved
end
it 'clamps the approvals left at zero' do
expect(merge_request.approvals_left).to eq(0)
end
end
end end
context 'when the approvers do not contain the MR author' do context 'when the approvers do not contain the MR author' do
......
...@@ -29,12 +29,16 @@ describe MergeRequests::ApprovalService, services: true do ...@@ -29,12 +29,16 @@ describe MergeRequests::ApprovalService, services: true do
context 'with valid approval' do context 'with valid approval' do
it 'creates an approval note' do it 'creates an approval note' do
allow(merge_request).to receive(:approvals_left).and_return(1)
expect(SystemNoteService).to receive(:approve_mr).with(merge_request, user) expect(SystemNoteService).to receive(:approve_mr).with(merge_request, user)
service.execute(merge_request) service.execute(merge_request)
end end
it 'marks pending todos as done' do it 'marks pending todos as done' do
allow(merge_request).to receive(:approvals_left).and_return(1)
service.execute(merge_request) service.execute(merge_request)
expect(todo.reload).to be_done expect(todo.reload).to be_done
......
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