Commit 6e4a4465 authored by Stan Hu's avatar Stan Hu

Fix error when duplicate users are merged in approvers list

If an user who belongs to an approval group and also is an individual in
the approval rule, `MergeService` will fail in the `after_merge`, which
will cause `MergeWorker` to retry again. Since the merge has been
successfully merged, `MergeWorker` will encounter an error since
`MergeService` is not idempotent.

This change fixes the issue by using the Array |= join method to add an
element to the Array, unless it is already present. This fixes the
immediate bug, but we will have to address the idempotency issues later.

Closes https://gitlab.com/gitlab-org/gitlab/issues/13488
parent ebb9f2ce
---
title: Fix error when duplicate users are merged in approvers list
merge_request: 17406
author:
type: fixed
......@@ -26,7 +26,7 @@ module ApprovalRules
def merge_group_members_into_users
merge_request.approval_rules.each do |rule|
rule.users += rule.group_users
rule.users |= rule.group_users
end
end
......
......@@ -94,6 +94,20 @@ describe ApprovalRules::FinalizeService do
expect(rule.approved_approvers).to contain_exactly(user3, group2_user)
expect(subject).not_to have_received(:copy_project_approval_rules)
end
# Test for https://gitlab.com/gitlab-org/gitlab/issues/13488
it 'gracefully merges duplicate users' do
group2.add_developer(user2)
expect do
subject.execute
end.not_to change { ApprovalMergeRequestRule.count }
rule = merge_request.approval_rules.regular.first
expect(rule.name).to eq('bar')
expect(rule.users).to contain_exactly(user2, user3, group2_user)
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