Limit to 5 the usernames/ids you can filter for approved by MRs

doc/api/merge_requests.md

@@ -54,7 +54,7 @@ Parameters:
 | `author_id`                     | integer        | no       | Returns merge requests created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me`              |
 | `assignee_id`                   | integer        | no       | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. |
 | `approver_ids` **(STARTER)**    | integer array  | no       | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. |
-| `approved_by_ids` **(STARTER)** | integer array  | no       | Returns merge requests which have been approved by all the users with the given `id`s. `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
+| `approved_by_ids` **(STARTER)** | integer array  | no       | Returns merge requests which have been approved by all the users with the given `id`s (Max: 5). `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
 | `my_reaction_emoji`             | string         | no       | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ |
 | `source_branch`                 | string         | no       | Return merge requests with the given source branch                                                                     |
 | `target_branch`                 | string         | no       | Return merge requests with the given target branch                                                                     |
@@ -230,7 +230,7 @@ Parameters:
 | `author_id`                     | integer        | no       | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_                                 |
 | `assignee_id`                   | integer        | no       | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ |
 | `approver_ids` **(STARTER)**    | integer array  | no       | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. |
-| `approved_by_ids` **(STARTER)** | integer array  | no       | Returns merge requests which have been approved by all the users with the given `id`s. `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
+| `approved_by_ids` **(STARTER)** | integer array  | no       | Returns merge requests which have been approved by all the users with the given `id`s (Max: 5). `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
 | `my_reaction_emoji`             | string         | no       | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ |
 | `source_branch`                 | string         | no       | Return merge requests with the given source branch                                                                             |
 | `target_branch`                 | string         | no       | Return merge requests with the given target branch                                                                             |
@@ -392,7 +392,7 @@ Parameters:
 | `author_id`                     | integer        | no       | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_                                 |
 | `assignee_id`                   | integer        | no       | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ |
 | `approver_ids` **(STARTER)**    | integer array  | no       | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. |
-| `approved_by_ids` **(STARTER)** | integer array  | no       | Returns merge requests which have been approved by all the users with the given `id`s. `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
+| `approved_by_ids` **(STARTER)** | integer array  | no       | Returns merge requests which have been approved by all the users with the given `id`s (Max: 5). `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
 | `my_reaction_emoji`             | string         | no       | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ |
 | `source_branch`                 | string         | no       | Return merge requests with the given source branch                                                                             |
 | `target_branch`                 | string         | no       | Return merge requests with the given target branch                                                                             |

ee/app/finders/merge_requests/by_approvals_finder.rb

@@ -5,9 +5,12 @@ module MergeRequests
   class ByApprovalsFinder
     attr_reader :usernames, :ids
 
+    # We apply a limitation to the amount of elements that can be part of the filter condition
+    MAX_FILTER_ELEMENTS = 5
+
     def initialize(usernames, ids)
-      @usernames = Array(usernames).map(&:to_s).uniq
-      @ids = Array(ids).uniq
+      @usernames = Array(usernames).map(&:to_s).uniq.take(MAX_FILTER_ELEMENTS)
+      @ids = Array(ids).uniq.take(MAX_FILTER_ELEMENTS)
     end
 
     def execute(items)

ee/spec/finders/merge_requests/by_approvals_finder_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 describe MergeRequests::ByApprovalsFinder do
   set(:first_user) { create(:user) }
   set(:second_user) { create(:user) }
+  let(:third_user) { create(:user) }
 
   set(:merge_request_without_approvals) { create(:merge_request) }
   set(:merge_request_with_first_user_approval) do
@@ -57,6 +58,17 @@ describe MergeRequests::ByApprovalsFinder do
       expect(merge_requests(ids: [first_user.id, second_user.id])).to match_array(expected_result)
       expect(merge_requests(names: [first_user.username, second_user.username])).to match_array(expected_result)
     end
+
+    context 'limiting max conditional elements' do
+      it 'returns merge requests approved by both users, considering limit of 2 being defined' do
+        stub_const('MergeRequests::ByApprovalsFinder::MAX_FILTER_ELEMENTS', 2)
+
+        expected_result = [merge_request_with_both_approvals]
+
+        expect(merge_requests(ids: [first_user.id, second_user.id, third_user.id])).to match_array(expected_result)
+        expect(merge_requests(names: [first_user.username, second_user.username, third_user.username])).to match_array(expected_result)
+      end
+    end
   end
 
   context 'with empty params' do