Merge branch '296031-or-params-api' into 'master'

Allow OR params in GraphQL issue resolvers

See merge request !102148



Merged-by: charlie ablett <cablett@gitlab.com>
Approved-by: Marcin Sedlak-Jakubowski <msedlakjakubowski@gitlab.com>
Approved-by: Qingyu Zhao <qzhao@gitlab.com>
Approved-by: charlie ablett <cablett@gitlab.com>
Co-authored-by: Heinrich Lee Yu <heinrich@gitlab.com>

app/graphql/resolvers/concerns/board_item_filterable.rb

@@ -14,6 +14,16 @@ def item_filters(args)
       set_filter_values(filters[:not])
     end
 
+    if filters[:or]
+      if ::Feature.disabled?(:or_issuable_queries, resource_parent)
+        raise ::Gitlab::Graphql::Errors::ArgumentError,
+              "'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled."
+      end
+
+      rewrite_param_name(filters[:or], :author_usernames, :author_username)
+      rewrite_param_name(filters[:or], :assignee_usernames, :assignee_username)
+    end
+
     filters
   end
 
@@ -30,6 +40,14 @@ def filter_by_assignee(filters)
       filters[:assignee_id] = filters.delete(:assignee_wildcard_id)
     end
   end
+
+  def rewrite_param_name(filters, old_name, new_name)
+    filters[new_name] = filters.delete(old_name) if filters[old_name].present?
+  end
+
+  def resource_parent
+    respond_to?(:board) ? board.resource_parent : list.board.resource_parent
+  end
 end
 
 ::BoardItemFilterable.prepend_mod_with('Resolvers::BoardItemFilterable')

app/graphql/resolvers/concerns/issue_resolver_arguments.rb

@@ -67,6 +67,9 @@ module IssueResolverArguments
     argument :not, Types::Issues::NegatedIssueFilterInputType,
              description: 'Negated arguments.',
              required: false
+    argument :or, Types::Issues::UnionedIssueFilterInputType,
+             description: 'List of arguments with inclusive OR.',
+             required: false
     argument :crm_contact_id, GraphQL::Types::String,
              required: false,
              description: 'ID of a contact assigned to the issues.'
@@ -84,7 +87,12 @@ def resolve_with_lookahead(**args)
   end
 
   def ready?(**args)
-    args[:not] = args[:not].to_h if args[:not].present?
+    if args[:or].present? && ::Feature.disabled?(:or_issuable_queries, resource_parent)
+      raise ::Gitlab::Graphql::Errors::ArgumentError, "'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled."
+    end
+
+    args[:not] = args[:not].to_h if args[:not]
+    args[:or] = args[:or].to_h if args[:or]
 
     params_not_mutually_exclusive(args, mutually_exclusive_assignee_username_args)
     params_not_mutually_exclusive(args, mutually_exclusive_milestone_args)
@@ -116,9 +124,12 @@ def accept_release_tag
 
   def prepare_finder_params(args)
     params = super(args)
+    params[:not] = params[:not].to_h if params[:not]
+    params[:or] = params[:or].to_h if params[:or]
     params[:iids] ||= [params.delete(:iid)].compact if params[:iid]
     params[:attempt_project_search_optimizations] = true if params[:search].present?
 
+    prepare_author_username_params(params)
     prepare_assignee_username_params(params)
     prepare_release_tag_params(params)
 
@@ -132,9 +143,14 @@ def prepare_release_tag_params(args)
     args[:release_tag] ||= release_tag_wildcard
   end
 
+  def prepare_author_username_params(args)
+    args[:or][:author_username] = args[:or].delete(:author_usernames) if args.dig(:or, :author_usernames).present?
+  end
+
   def prepare_assignee_username_params(args)
     args[:assignee_username] = args.delete(:assignee_usernames) if args[:assignee_usernames].present?
     args[:not][:assignee_username] = args[:not].delete(:assignee_usernames) if args.dig(:not, :assignee_usernames).present?
+    args[:or][:assignee_username] = args[:or].delete(:assignee_usernames) if args.dig(:or, :assignee_usernames).present?
   end
 
   def mutually_exclusive_release_tag_args

app/graphql/types/boards/board_issue_input_type.rb

@@ -9,6 +9,10 @@ class BoardIssueInputType < BoardIssueInputBaseType
                required: false,
                description: 'List of negated arguments.'
 
+      argument :or, Types::Issues::UnionedIssueFilterInputType,
+               required: false,
+               description: 'List of arguments with inclusive OR.'
+
       argument :search, GraphQL::Types::String,
                required: false,
                description: 'Search query for issue title or description.'

app/graphql/types/issues/unioned_issue_filter_input_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Issues
+    class UnionedIssueFilterInputType < BaseInputObject
+      graphql_name 'UnionedIssueFilterInput'
+
+      argument :assignee_usernames, [GraphQL::Types::String],
+               required: false,
+               description: 'Filters issues that are assigned to at least one of the given users.'
+      argument :author_usernames, [GraphQL::Types::String],
+               required: false,
+               description: 'Filters issues that are authored by one of the given users.'
+    end
+  end
+end

doc/api/graphql/reference/index.md

@@ -13207,6 +13207,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="groupissuesmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="groupissuesmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="groupissuesnot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
+| <a id="groupissuesor"></a>`or` | [`UnionedIssueFilterInput`](#unionedissuefilterinput) | List of arguments with inclusive OR. |
 | <a id="groupissuessearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="groupissuessort"></a>`sort` | [`IssueSort`](#issuesort) | Sort issues by this criteria. |
 | <a id="groupissuesstate"></a>`state` | [`IssuableState`](#issuablestate) | Current state of this issue. |
@@ -16871,6 +16872,7 @@ Returns [`Issue`](#issue).
 | <a id="projectissuemilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="projectissuemyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="projectissuenot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
+| <a id="projectissueor"></a>`or` | [`UnionedIssueFilterInput`](#unionedissuefilterinput) | List of arguments with inclusive OR. |
 | <a id="projectissuereleasetag"></a>`releaseTag` | [`[String!]`](#string) | Release tag associated with the issue's milestone. |
 | <a id="projectissuereleasetagwildcardid"></a>`releaseTagWildcardId` | [`ReleaseTagWildcardId`](#releasetagwildcardid) | Filter issues by release tag ID wildcard. |
 | <a id="projectissuesearch"></a>`search` | [`String`](#string) | Search query for title or description. |
@@ -16910,6 +16912,7 @@ Returns [`IssueStatusCountsType`](#issuestatuscountstype).
 | <a id="projectissuestatuscountsmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="projectissuestatuscountsmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="projectissuestatuscountsnot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
+| <a id="projectissuestatuscountsor"></a>`or` | [`UnionedIssueFilterInput`](#unionedissuefilterinput) | List of arguments with inclusive OR. |
 | <a id="projectissuestatuscountsreleasetag"></a>`releaseTag` | [`[String!]`](#string) | Release tag associated with the issue's milestone. |
 | <a id="projectissuestatuscountsreleasetagwildcardid"></a>`releaseTagWildcardId` | [`ReleaseTagWildcardId`](#releasetagwildcardid) | Filter issues by release tag ID wildcard. |
 | <a id="projectissuestatuscountssearch"></a>`search` | [`String`](#string) | Search query for title or description. |
@@ -16956,6 +16959,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="projectissuesmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="projectissuesmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="projectissuesnot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
+| <a id="projectissuesor"></a>`or` | [`UnionedIssueFilterInput`](#unionedissuefilterinput) | List of arguments with inclusive OR. |
 | <a id="projectissuesreleasetag"></a>`releaseTag` | [`[String!]`](#string) | Release tag associated with the issue's milestone. |
 | <a id="projectissuesreleasetagwildcardid"></a>`releaseTagWildcardId` | [`ReleaseTagWildcardId`](#releasetagwildcardid) | Filter issues by release tag ID wildcard. |
 | <a id="projectissuessearch"></a>`search` | [`String`](#string) | Search query for title or description. |
@@ -23541,6 +23545,7 @@ Field that are available while modifying the custom mapping attributes for an HT
 | <a id="boardissueinputmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter by milestone ID wildcard. |
 | <a id="boardissueinputmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="boardissueinputnot"></a>`not` | [`NegatedBoardIssueInput`](#negatedboardissueinput) | List of negated arguments. |
+| <a id="boardissueinputor"></a>`or` | [`UnionedIssueFilterInput`](#unionedissuefilterinput) | List of arguments with inclusive OR. |
 | <a id="boardissueinputreleasetag"></a>`releaseTag` | [`String`](#string) | Filter by release tag. |
 | <a id="boardissueinputsearch"></a>`search` | [`String`](#string) | Search query for issue title or description. |
 | <a id="boardissueinputtypes"></a>`types` | [`[IssueType!]`](#issuetype) | Filter by the given issue types. |
@@ -23945,6 +23950,15 @@ A time-frame defined as a closed inclusive range of two dates.
 | <a id="timeframeend"></a>`end` | [`Date!`](#date) | End of the range. |
 | <a id="timeframestart"></a>`start` | [`Date!`](#date) | Start of the range. |
+### `UnionedIssueFilterInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="unionedissuefilterinputassigneeusernames"></a>`assigneeUsernames` | [`[String!]`](#string) | Filters issues that are assigned to at least one of the given users. |
+| <a id="unionedissuefilterinputauthorusernames"></a>`authorUsernames` | [`[String!]`](#string) | Filters issues that are authored by one of the given users. |
+
 ### `UpdateDiffImagePositionInput`
 #### Arguments

ee/app/graphql/ee/resolvers/base_issues_resolver.rb

@@ -33,7 +33,7 @@ module BaseIssuesResolver
 
       override :resolve_with_lookahead
       def resolve_with_lookahead(**args)
-        args[:not] = args[:not].to_h if args[:not].present?
+        args[:not] = args[:not].to_h if args[:not]
         args[:iteration_id] = iteration_ids_from_args(args) if args[:iteration_id].present?
         args[:not][:iteration_id] = iteration_ids_from_args(args[:not]) if args.dig(:not, :iteration_id).present?
         prepare_iteration_wildcard_params(args)
@@ -43,7 +43,7 @@ def resolve_with_lookahead(**args)
       end
 
       def ready?(**args)
-        args[:not] = args[:not].to_h if args[:not].present?
+        args[:not] = args[:not].to_h if args[:not]
 
         if iteration_params_not_mutually_exclusive?(args) || iteration_params_not_mutually_exclusive?(args.fetch(:not, {}))
           arg_str = mutually_exclusive_iteration_args.map { |x| x.to_s.camelize(:lower) }.join(', ')

spec/requests/api/graphql/boards/board_list_issues_query_spec.rb

@@ -21,6 +21,7 @@
   let(:board_data)        { graphql_data[board_parent_type]['boards']['nodes'][0] }
   let(:lists_data)        { board_data['lists']['nodes'][0] }
   let(:issues_data)       { lists_data['issues']['nodes'] }
+  let(:issue_params)      { { filters: { label_name: label2.title, confidential: confidential }, first: 3 } }
 
   def query(list_params = params)
     graphql_query_for(
@@ -31,7 +32,7 @@ def query(list_params = params)
           nodes {
             lists {
               nodes {
-                issues(filters: {labelName: "#{label2.title}", confidential: #{confidential}}, first: 3) {
+                issues(#{attributes_to_graphql(issue_params)}) {
                   count
                   nodes {
                     #{all_graphql_fields_for('issues'.classify)}
@@ -77,18 +78,23 @@ def issue_relative_positions
     end
 
     context 'when user can read the board' do
-      before do
+      before_all do
         board_parent.add_reporter(user)
-        post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user)
       end
 
+      subject { post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) }
+
       it 'can access the issues', :aggregate_failures do
+        subject
+
         # ties for relative positions are broken by id in ascending order by default
         expect(issue_titles).to eq([issue2.title, issue1.title, issue3.title])
         expect(issue_relative_positions).not_to include(nil)
       end
 
       it 'does not set the relative positions of the issues not being returned', :aggregate_failures do
+        subject
+
         expect(issue_id).not_to include(issue6.id)
         expect(issue3.relative_position).to be_nil
       end
@@ -97,10 +103,36 @@ def issue_relative_positions
         let(:confidential) { true }
 
         it 'returns matching issue' do
+          subject
+
           expect(issue_titles).to match_array([issue7.title])
           expect(issue_relative_positions).not_to include(nil)
         end
       end
+
+      context 'when filtering by a unioned argument' do
+        let(:another_user) { create(:user) }
+        let(:issue_params) { { filters: { or: { assignee_usernames: [user.username, another_user.username] } } } }
+
+        it 'returns correctly filtered issues' do
+          issue1.assignee_ids = user.id
+          issue2.assignee_ids = another_user.id
+
+          subject
+
+          expect(issue_id).to contain_exactly(issue1.to_gid.to_s, issue2.to_gid.to_s)
+        end
+
+        context 'when feature flag is disabled' do
+          it 'returns an error' do
+            stub_feature_flags(or_issuable_queries: false)
+
+            subject
+
+            expect_graphql_errors_to_include("'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled.")
+          end
+        end
+      end
     end
   end
 

spec/requests/api/graphql/boards/board_lists_query_spec.rb

@@ -49,7 +49,7 @@ def query(list_params = params)
   end
 
   shared_examples 'group and project board lists query' do
-    let!(:board) { create(:board, resource_parent: board_parent) }
+    let_it_be(:board) { create(:board, resource_parent: board_parent) }
 
     context 'when the user does not have access to the board' do
       it 'returns nil' do
@@ -107,16 +107,20 @@ def pagination_query(params)
     end
 
     context 'when querying for a single list' do
+      let_it_be(:label_list) { create(:list, board: board, label: label, position: 10) }
+      let_it_be(:issues) do
+        [
+          create(:issue, project: project, labels: [label, label2]),
+          create(:issue, project: project, labels: [label, label2], confidential: true),
+          create(:issue, project: project, labels: [label])
+        ]
+      end
+
       before do
         board_parent.add_reporter(user)
       end
 
       it 'returns the correct list with issue count for matching issue filters' do
-        label_list = create(:list, board: board, label: label, position: 10)
-        create(:issue, project: project, labels: [label, label2])
-        create(:issue, project: project, labels: [label, label2], confidential: true)
-        create(:issue, project: project, labels: [label])
-
         post_graphql(
           query(
             id: global_id_of(label_list),
@@ -131,21 +135,56 @@ def pagination_query(params)
           expect(list_node['issuesCount']).to eq 1
         end
       end
+
+      context 'when filtering by a unioned argument' do
+        let_it_be(:another_user) { create(:user) }
+
+        it 'returns correctly filtered issues' do
+          issues[0].assignee_ids = user.id
+          issues[1].assignee_ids = another_user.id
+
+          post_graphql(
+            query(
+              id: global_id_of(label_list),
+              issueFilters: { or: { assignee_usernames: [user.username, another_user.username] } }
+            ), current_user: user
+          )
+
+          expect(lists_data[0]['node']['issuesCount']).to eq 2
+        end
+
+        context 'when feature flag is disabled' do
+          it 'returns an error' do
+            stub_feature_flags(or_issuable_queries: false)
+
+            post_graphql(
+              query(
+                id: global_id_of(label_list),
+                issueFilters: { or: { assignee_usernames: [user.username, another_user.username] } }
+              ), current_user: user
+            )
+
+            expect_graphql_errors_to_include(
+              "'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled."
+            )
+          end
+        end
+      end
     end
   end
 
   describe 'for a project' do
-    let(:board_parent) { project }
-    let(:label) { project_label }
-    let(:label2) { project_label2 }
+    let_it_be(:board_parent) { project }
+    let_it_be(:label) { project_label }
+    let_it_be(:label2) { project_label2 }
 
     it_behaves_like 'group and project board lists query'
   end
 
   describe 'for a group' do
-    let(:board_parent) { group }
-    let(:label) { group_label }
-    let(:label2) { group_label2 }
+    let_it_be(:board_parent) { group }
+    let_it_be(:label) { group_label }
+    let_it_be(:label2) { group_label2 }
 
     before do
       allow(board_parent).to receive(:multiple_issue_boards_available?).and_return(false)

spec/requests/api/graphql/project/issues_spec.rb

@@ -56,6 +56,62 @@
     end
   end
 
+  context 'when filtering by a negated argument' do
+    let(:issue_filter_params) { { not: { assignee_usernames: current_user.username } } }
+
+    it 'returns correctly filtered issues' do
+      issue_a.assignee_ids = current_user.id
+
+      post_graphql(query, current_user: current_user)
+
+      expect(issues_ids).to contain_exactly(issue_b_gid)
+    end
+
+    context 'when argument is blank' do
+      let(:issue_filter_params) { { not: {} } }
+
+      it 'does not raise an error' do
+        post_graphql(query, current_user: current_user)
+
+        expect_graphql_errors_to_be_empty
+      end
+    end
+  end
+
+  context 'when filtering by a unioned argument' do
+    let(:another_user) { create(:user) }
+    let(:issue_filter_params) { { or: { assignee_usernames: [current_user.username, another_user.username] } } }
+
+    it 'returns correctly filtered issues' do
+      issue_a.assignee_ids = current_user.id
+      issue_b.assignee_ids = another_user.id
+
+      post_graphql(query, current_user: current_user)
+
+      expect(issues_ids).to contain_exactly(issue_a_gid, issue_b_gid)
+    end
+
+    context 'when argument is blank' do
+      let(:issue_filter_params) { { or: {} } }
+
+      it 'does not raise an error' do
+        post_graphql(query, current_user: current_user)
+
+        expect_graphql_errors_to_be_empty
+      end
+    end
+
+    context 'when feature flag is disabled' do
+      it 'returns an error' do
+        stub_feature_flags(or_issuable_queries: false)
+
+        post_graphql(query, current_user: current_user)
+
+        expect_graphql_errors_to_include("'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled.")
+      end
+    end
+  end
+
   context 'filtering by my_reaction_emoji' do
     using RSpec::Parameterized::TableSyntax