Commit 9d12db03 authored by Gary Holtz's avatar Gary Holtz

Using a more efficient way to check for group/user equality

parent cd53569c
...@@ -140,11 +140,17 @@ class ApprovalMergeRequestRule < ApplicationRecord ...@@ -140,11 +140,17 @@ class ApprovalMergeRequestRule < ApplicationRecord
private private
def different_groups? def different_groups?
groups.sort != approval_project_rule.groups.sort group_ids = groups.collect(&:id)
apr_group_ids = approval_project_rule.groups.pluck(:id)
group_ids & apr_group_ids == group_ids
end end
def different_users? def different_users?
users.sort != approval_project_rule.users.sort user_ids = users.collect(&:id)
apr_user_ids = approval_project_rule.users.pluck(:id)
user_ids & apr_user_ids == user_ids
end end
def different_name_or_approvals_required? def different_name_or_approvals_required?
......
...@@ -36,6 +36,32 @@ RSpec.describe ApprovalMergeRequestRule do ...@@ -36,6 +36,32 @@ RSpec.describe ApprovalMergeRequestRule do
it 'is valid' do it 'is valid' do
expect(merge_request_rule).to be_valid expect(merge_request_rule).to be_valid
end end
context 'the MR rule shows that its modified from the original' do
it 'by having different groups' do
first_group = create(:group)
second_group = create(:group)
third_group = create(:group)
approval_project_rule.groups = [first_group, second_group]
approval_project_rule.save!
merge_request_rule.groups = [first_group, third_group]
expect(merge_request_rule.different_from_project_rule?).to be true
end
it 'by having different users' do
first_user = create(:user)
second_user = create(:user)
third_user = create(:user)
approval_project_rule.users = [first_user, second_user]
approval_project_rule.save!
merge_request_rule.users = [first_user, third_user]
expect(merge_request_rule.different_from_project_rule?).to be true
end
end
end end
context 'when the project of approval_project_rule and merge request does not match' do context 'when the project of approval_project_rule and merge request does not match' do
......
...@@ -33,16 +33,16 @@ RSpec.describe Gitlab::BackgroundMigration::AddModifiedToApprovalMergeRequestRul ...@@ -33,16 +33,16 @@ RSpec.describe Gitlab::BackgroundMigration::AddModifiedToApprovalMergeRequestRul
sources.create!(approval_project_rule_id: overridden_project_rule.id, approval_merge_request_rule_id: overridden_merge_rule.id) sources.create!(approval_project_rule_id: overridden_project_rule.id, approval_merge_request_rule_id: overridden_merge_rule.id)
sources.create!(approval_project_rule_id: overridden_project_rule_two.id, approval_merge_request_rule_id: overridden_merge_rule_two.id) sources.create!(approval_project_rule_id: overridden_project_rule_two.id, approval_merge_request_rule_id: overridden_merge_rule_two.id)
group1 = groups.create!(name: "1", path: "test_group1", type: 'Group') group1 = groups.create!(name: "group1", path: "test_group1", type: 'Group')
group2 = groups.create!(name: "2", path: "test_group2", type: 'Group') group2 = groups.create!(name: "group2", path: "test_group2", type: 'Group')
group3 = groups.create!(name: "3", path: "test_group3", type: 'Group') group3 = groups.create!(name: "group3", path: "test_group3", type: 'Group')
project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group1.id) project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group1.id)
project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group2.id) project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group2.id)
project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group3.id) project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group3.id)
mr_groups.create!(approval_merge_request_rule_id: overridden_project_rule_two.id, group_id: group1.id) mr_groups.create!(approval_merge_request_rule_id: overridden_merge_rule.id, group_id: group1.id)
mr_groups.create!(approval_merge_request_rule_id: overridden_project_rule_two.id, group_id: group2.id) mr_groups.create!(approval_merge_request_rule_id: overridden_merge_rule_two.id, group_id: group2.id)
end end
describe '#perform' do describe '#perform' 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