Commit 7981eae9 authored by Gabriel Mazetto's avatar Gabriel Mazetto

Use uniq values when filtering for approvals

parent e0518bdc
......@@ -6,8 +6,8 @@ module MergeRequests
attr_reader :usernames, :ids
def initialize(usernames, ids)
@usernames = usernames.to_a.map(&:to_s)
@ids = ids
@usernames = Array(usernames).map(&:to_s).uniq
@ids = Array(ids).uniq
end
def execute(items)
......@@ -37,7 +37,7 @@ module MergeRequests
end
def includes_custom_label?(label)
ids.to_s.downcase == label || usernames.map(&:downcase).include?(label)
ids.first.to_s.downcase == label || usernames.map(&:downcase).include?(label)
end
# Merge Requests without any approval
......@@ -54,12 +54,12 @@ module MergeRequests
# Merge Requests approved by given usernames
def find_approved_by_names(items)
items.approved_by_users_with_usernames(usernames)
items.approved_by_users_with_usernames(*usernames)
end
# Merge Requests approved by given user IDs
def find_approved_by_ids(items)
items.approved_by_users_with_ids(ids)
items.approved_by_users_with_ids(*ids)
end
end
end
......@@ -43,14 +43,14 @@ module EE
scope :without_approvals, -> { left_outer_joins(:approvals).where(approvals: { id: nil }) }
scope :with_approvals, -> { joins(:approvals) }
scope :approved_by_users_with_ids, -> (user_ids) do
scope :approved_by_users_with_ids, -> (*user_ids) do
with_approvals
.merge(Approval.with_user)
.where(users: { id: user_ids })
.group(:id)
.having("COUNT(users.id) = ?", user_ids.size)
end
scope :approved_by_users_with_usernames, -> (usernames) do
scope :approved_by_users_with_usernames, -> (*usernames) do
with_approvals
.merge(Approval.with_user)
.where(users: { username: usernames })
......
......@@ -27,8 +27,8 @@ describe MergeRequests::ByApprovalsFinder do
it 'returns merge requests without approvals' do
expected_result = [merge_request_without_approvals]
expect(merge_requests(ids: 'None')).to eq(expected_result)
expect(merge_requests(names: ['None'])).to eq(expected_result)
expect(merge_requests(ids: 'None')).to match_array(expected_result)
expect(merge_requests(names: ['None'])).to match_array(expected_result)
end
end
......@@ -36,8 +36,8 @@ describe MergeRequests::ByApprovalsFinder do
it 'returns merge requests approved by at least one user' do
expected_result = [merge_request_with_first_user_approval, merge_request_with_both_approvals]
expect(merge_requests(ids: 'Any')).to eq(expected_result)
expect(merge_requests(names: ['Any'])).to eq(expected_result)
expect(merge_requests(ids: 'Any')).to match_array(expected_result)
expect(merge_requests(names: ['Any'])).to match_array(expected_result)
end
end
......@@ -45,8 +45,8 @@ describe MergeRequests::ByApprovalsFinder do
it 'returns merge requests approved by specific user' do
expected_result = [merge_request_with_first_user_approval, merge_request_with_both_approvals]
expect(merge_requests(ids: [first_user.id])).to eq(expected_result)
expect(merge_requests(names: [first_user.username])).to eq(expected_result)
expect(merge_requests(ids: [first_user.id])).to match_array(expected_result)
expect(merge_requests(names: [first_user.username])).to match_array(expected_result)
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