From 40b8edeec1e92a8161005369a16359f78a0952ce Mon Sep 17 00:00:00 2001
From: Eugenia Grieff <egrieff@gitlab.com>
Date: Tue, 23 Aug 2022 23:40:37 +0000
Subject: [PATCH] Allow to specify the fields to search for when querying work
items
Refactor the existing logic for epics to apply it to all issuables
Changelog: added
EE: true
---
.../concerns/issue_resolver_arguments.rb | 61 ++++++------
.../resolvers/concerns/search_arguments.rb | 36 +++++++
app/graphql/resolvers/work_items_resolver.rb | 35 +++----
doc/api/graphql/reference/index.md | 17 ++--
ee/app/graphql/resolvers/epics_resolver.rb | 32 +++----
.../graphql/resolvers/epics_resolver_spec.rb | 86 ++---------------
.../requests/api/graphql/group/epics_spec.rb | 14 +--
.../graphql/resolvers/issues_resolver_spec.rb | 52 ++--------
.../resolvers/work_items_resolver_spec.rb | 52 ++--------
.../api/graphql/project/issues_spec.rb | 24 +++--
.../api/graphql/project/work_items_spec.rb | 24 +++--
.../issuable_resolvers_shared_examples.rb | 95 +++++++++++++++++++
.../issuable_search_shared_examples.rb | 14 +++
13 files changed, 285 insertions(+), 257 deletions(-)
create mode 100644 spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb
create mode 100644 spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb
diff --git a/app/graphql/resolvers/concerns/issue_resolver_arguments.rb b/app/graphql/resolvers/concerns/issue_resolver_arguments.rb
index fe213936f555..15ab44352045 100644
--- a/app/graphql/resolvers/concerns/issue_resolver_arguments.rb
+++ b/app/graphql/resolvers/concerns/issue_resolver_arguments.rb
@@ -76,34 +76,18 @@ module IssueResolverArguments
end
def resolve_with_lookahead(**args)
- # The project could have been loaded in batch by `BatchLoader`.
- # At this point we need the `id` of the project to query for issues, so
- # make sure it's loaded and not `nil` before continuing.
- parent = object.respond_to?(:sync) ? object.sync : object
- return Issue.none if parent.nil?
-
- # Will need to be made group & namespace aware with
- # https://gitlab.com/gitlab-org/gitlab-foss/issues/54520
- args[:not] = args[:not].to_h if args[:not].present?
- args[:iids] ||= [args.delete(:iid)].compact if args[:iid]
- args[:attempt_project_search_optimizations] = true if args[:search].present?
-
- prepare_assignee_username_params(args)
- prepare_release_tag_params(args)
+ return Issue.none if resource_parent.nil?
- finder = IssuesFinder.new(current_user, args)
+ finder = IssuesFinder.new(current_user, prepare_finder_params(args))
- continue_issue_resolve(parent, finder, **args)
+ continue_issue_resolve(resource_parent, finder, **args)
end
def ready?(**args)
- args[:not] = args[:not].to_h if args[:not].present?
-
params_not_mutually_exclusive(args, mutually_exclusive_assignee_username_args)
params_not_mutually_exclusive(args, mutually_exclusive_milestone_args)
params_not_mutually_exclusive(args.fetch(:not, {}), mutually_exclusive_milestone_args)
params_not_mutually_exclusive(args, mutually_exclusive_release_tag_args)
- validate_anonymous_search_access! if args[:search].present?
super
end
@@ -128,6 +112,18 @@ def accept_release_tag
private
+ def prepare_finder_params(args)
+ params = super(args)
+ params[:not] = params[:not].to_h if params[:not].present?
+ params[:iids] ||= [params.delete(:iid)].compact if params[:iid]
+ params[:attempt_project_search_optimizations] = true if params[:search].present?
+
+ prepare_assignee_username_params(params)
+ prepare_release_tag_params(params)
+
+ params
+ end
+
def prepare_release_tag_params(args)
release_tag_wildcard = args.delete(:release_tag_wildcard_id)
return if release_tag_wildcard.blank?
@@ -135,20 +131,13 @@ def prepare_release_tag_params(args)
args[:release_tag] ||= release_tag_wildcard
end
- def mutually_exclusive_release_tag_args
- [:release_tag, :release_tag_wildcard_id]
- 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?
end
- def params_not_mutually_exclusive(args, mutually_exclusive_args)
- if args.slice(*mutually_exclusive_args).compact.size > 1
- arg_str = mutually_exclusive_args.map { |x| x.to_s.camelize(:lower) }.join(', ')
- raise ::Gitlab::Graphql::Errors::ArgumentError, "only one of [#{arg_str}] arguments is allowed at the same time."
- end
+ def mutually_exclusive_release_tag_args
+ [:release_tag, :release_tag_wildcard_id]
end
def mutually_exclusive_milestone_args
@@ -158,4 +147,20 @@ def mutually_exclusive_milestone_args
def mutually_exclusive_assignee_username_args
[:assignee_usernames, :assignee_username]
end
+
+ def params_not_mutually_exclusive(args, mutually_exclusive_args)
+ if args.slice(*mutually_exclusive_args).compact.size > 1
+ arg_str = mutually_exclusive_args.map { |x| x.to_s.camelize(:lower) }.join(', ')
+ raise ::Gitlab::Graphql::Errors::ArgumentError, "only one of [#{arg_str}] arguments is allowed at the same time."
+ end
+ end
+
+ def resource_parent
+ # The project could have been loaded in batch by `BatchLoader`.
+ # At this point we need the `id` of the project to query for issues, so
+ # make sure it's loaded and not `nil` before continuing.
+ strong_memoize(:resource_parent) do
+ object.respond_to?(:sync) ? object.sync : object
+ end
+ end
end
diff --git a/app/graphql/resolvers/concerns/search_arguments.rb b/app/graphql/resolvers/concerns/search_arguments.rb
index 7f480f9d0b61..86c7140af0ab 100644
--- a/app/graphql/resolvers/concerns/search_arguments.rb
+++ b/app/graphql/resolvers/concerns/search_arguments.rb
@@ -7,12 +7,48 @@ module SearchArguments
argument :search, GraphQL::Types::String,
required: false,
description: 'Search query for title or description.'
+ argument :in, [Types::IssuableSearchableFieldEnum],
+ required: false,
+ description: <<~DESC
+ Specify the fields to perform the search in.
+ Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'
+ DESC
+ end
+
+ def ready?(**args)
+ validate_search_in_params!(args)
+ validate_anonymous_search_access!
+
+ super
end
+ private
+
def validate_anonymous_search_access!
return if current_user.present? || Feature.disabled?(:disable_anonymous_search, type: :ops)
raise ::Gitlab::Graphql::Errors::ArgumentError,
"User must be authenticated to include the `search` argument."
end
+
+ def validate_search_in_params!(args)
+ return unless args[:in].present? && args[:search].blank?
+
+ raise Gitlab::Graphql::Errors::ArgumentError,
+ '`search` should be present when including the `in` argument'
+ end
+
+ def prepare_finder_params(args)
+ prepare_search_params(args)
+ end
+
+ def prepare_search_params(args)
+ return args unless args[:search].present?
+
+ parent_type = resource_parent.is_a?(Project) ? :project : :group
+ args[:"attempt_#{parent_type}_search_optimizations"] = true
+ args[:in] = args[:in].join(',') if args[:in].present?
+
+ args
+ end
end
diff --git a/app/graphql/resolvers/work_items_resolver.rb b/app/graphql/resolvers/work_items_resolver.rb
index 055984db3cb1..49f9633967d5 100644
--- a/app/graphql/resolvers/work_items_resolver.rb
+++ b/app/graphql/resolvers/work_items_resolver.rb
@@ -26,24 +26,11 @@ class WorkItemsResolver < BaseResolver
required: false
def resolve_with_lookahead(**args)
- # The project could have been loaded in batch by `BatchLoader`.
- # At this point we need the `id` of the project to query for issues, so
- # make sure it's loaded and not `nil` before continuing.
- parent = object.respond_to?(:sync) ? object.sync : object
- return WorkItem.none if parent.nil? || !parent.work_items_feature_flag_enabled?
+ return WorkItem.none if resource_parent.nil? || !resource_parent.work_items_feature_flag_enabled?
- args[:iids] ||= [args.delete(:iid)].compact if args[:iid]
- args[:attempt_project_search_optimizations] = true if args[:search].present?
+ finder = ::WorkItems::WorkItemsFinder.new(current_user, prepare_finder_params(args))
- finder = ::WorkItems::WorkItemsFinder.new(current_user, args)
-
- Gitlab::Graphql::Loaders::IssuableLoader.new(parent, finder).batching_find_all { |q| apply_lookahead(q) }
- end
-
- def ready?(**args)
- validate_anonymous_search_access! if args[:search].present?
-
- super
+ Gitlab::Graphql::Loaders::IssuableLoader.new(resource_parent, finder).batching_find_all { |q| apply_lookahead(q) }
end
private
@@ -56,6 +43,22 @@ def unconditional_includes
:author
]
end
+
+ def prepare_finder_params(args)
+ params = super(args)
+ params[:iids] ||= [params.delete(:iid)].compact if params[:iid]
+
+ params
+ end
+
+ def resource_parent
+ # The project could have been loaded in batch by `BatchLoader`.
+ # At this point we need the `id` of the project to query for work items, so
+ # make sure it's loaded and not `nil` before continuing.
+ strong_memoize(:resource_parent) do
+ object.respond_to?(:sync) ? object.sync : object
+ end
+ end
end
end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 698412c470df..1a6b2ae34390 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -9843,7 +9843,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="boardepicancestorsiid"></a>`iid` | [`ID`](#id) | IID of the epic, e.g., "1". |
| <a id="boardepicancestorsiidstartswith"></a>`iidStartsWith` | [`String`](#string) | Filter epics by IID for autocomplete. |
| <a id="boardepicancestorsiids"></a>`iids` | [`[ID!]`](#id) | List of IIDs of epics, e.g., `[1, 2]`. |
-| <a id="boardepicancestorsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument. |
+| <a id="boardepicancestorsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="boardepicancestorsincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include epics from ancestor groups. |
| <a id="boardepicancestorsincludedescendantgroups"></a>`includeDescendantGroups` | [`Boolean`](#boolean) | Include epics from descendant groups. |
| <a id="boardepicancestorslabelname"></a>`labelName` | [`[String!]`](#string) | Filter epics by labels. |
@@ -9881,7 +9881,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="boardepicchildreniid"></a>`iid` | [`ID`](#id) | IID of the epic, e.g., "1". |
| <a id="boardepicchildreniidstartswith"></a>`iidStartsWith` | [`String`](#string) | Filter epics by IID for autocomplete. |
| <a id="boardepicchildreniids"></a>`iids` | [`[ID!]`](#id) | List of IIDs of epics, e.g., `[1, 2]`. |
-| <a id="boardepicchildrenin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument. |
+| <a id="boardepicchildrenin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="boardepicchildrenincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include epics from ancestor groups. |
| <a id="boardepicchildrenincludedescendantgroups"></a>`includeDescendantGroups` | [`Boolean`](#boolean) | Include epics from descendant groups. |
| <a id="boardepicchildrenlabelname"></a>`labelName` | [`[String!]`](#string) | Filter epics by labels. |
@@ -11580,7 +11580,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="epicancestorsiid"></a>`iid` | [`ID`](#id) | IID of the epic, e.g., "1". |
| <a id="epicancestorsiidstartswith"></a>`iidStartsWith` | [`String`](#string) | Filter epics by IID for autocomplete. |
| <a id="epicancestorsiids"></a>`iids` | [`[ID!]`](#id) | List of IIDs of epics, e.g., `[1, 2]`. |
-| <a id="epicancestorsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument. |
+| <a id="epicancestorsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="epicancestorsincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include epics from ancestor groups. |
| <a id="epicancestorsincludedescendantgroups"></a>`includeDescendantGroups` | [`Boolean`](#boolean) | Include epics from descendant groups. |
| <a id="epicancestorslabelname"></a>`labelName` | [`[String!]`](#string) | Filter epics by labels. |
@@ -11618,7 +11618,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="epicchildreniid"></a>`iid` | [`ID`](#id) | IID of the epic, e.g., "1". |
| <a id="epicchildreniidstartswith"></a>`iidStartsWith` | [`String`](#string) | Filter epics by IID for autocomplete. |
| <a id="epicchildreniids"></a>`iids` | [`[ID!]`](#id) | List of IIDs of epics, e.g., `[1, 2]`. |
-| <a id="epicchildrenin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument. |
+| <a id="epicchildrenin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="epicchildrenincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include epics from ancestor groups. |
| <a id="epicchildrenincludedescendantgroups"></a>`includeDescendantGroups` | [`Boolean`](#boolean) | Include epics from descendant groups. |
| <a id="epicchildrenlabelname"></a>`labelName` | [`[String!]`](#string) | Filter epics by labels. |
@@ -12468,7 +12468,7 @@ Returns [`Epic`](#epic).
| <a id="groupepiciid"></a>`iid` | [`ID`](#id) | IID of the epic, e.g., "1". |
| <a id="groupepiciidstartswith"></a>`iidStartsWith` | [`String`](#string) | Filter epics by IID for autocomplete. |
| <a id="groupepiciids"></a>`iids` | [`[ID!]`](#id) | List of IIDs of epics, e.g., `[1, 2]`. |
-| <a id="groupepicin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument. |
+| <a id="groupepicin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="groupepicincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include epics from ancestor groups. |
| <a id="groupepicincludedescendantgroups"></a>`includeDescendantGroups` | [`Boolean`](#boolean) | Include epics from descendant groups. |
| <a id="groupepiclabelname"></a>`labelName` | [`[String!]`](#string) | Filter epics by labels. |
@@ -12518,7 +12518,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="groupepicsiid"></a>`iid` | [`ID`](#id) | IID of the epic, e.g., "1". |
| <a id="groupepicsiidstartswith"></a>`iidStartsWith` | [`String`](#string) | Filter epics by IID for autocomplete. |
| <a id="groupepicsiids"></a>`iids` | [`[ID!]`](#id) | List of IIDs of epics, e.g., `[1, 2]`. |
-| <a id="groupepicsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument. |
+| <a id="groupepicsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="groupepicsincludeancestorgroups"></a>`includeAncestorGroups` | [`Boolean`](#boolean) | Include epics from ancestor groups. |
| <a id="groupepicsincludedescendantgroups"></a>`includeDescendantGroups` | [`Boolean`](#boolean) | Include epics from descendant groups. |
| <a id="groupepicslabelname"></a>`labelName` | [`[String!]`](#string) | Filter epics by labels. |
@@ -12580,6 +12580,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="groupissuesepicid"></a>`epicId` | [`String`](#string) | ID of an epic associated with the issues, "none" and "any" values are supported. |
| <a id="groupissuesiid"></a>`iid` | [`String`](#string) | IID of the issue. For example, "1". |
| <a id="groupissuesiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of issues. For example, `["1", "2"]`. |
+| <a id="groupissuesin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="groupissuesincludearchived"></a>`includeArchived` | [`Boolean`](#boolean) | Return issues from archived projects. |
| <a id="groupissuesincludesubepics"></a>`includeSubepics` | [`Boolean`](#boolean) | Whether to include subepics when filtering issues by epicId. |
| <a id="groupissuesincludesubgroups"></a>`includeSubgroups` | [`Boolean`](#boolean) | Include issues belonging to subgroups. |
@@ -16072,6 +16073,7 @@ Returns [`Issue`](#issue).
| <a id="projectissueepicid"></a>`epicId` | [`String`](#string) | ID of an epic associated with the issues, "none" and "any" values are supported. |
| <a id="projectissueiid"></a>`iid` | [`String`](#string) | IID of the issue. For example, "1". |
| <a id="projectissueiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of issues. For example, `["1", "2"]`. |
+| <a id="projectissuein"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="projectissueincludesubepics"></a>`includeSubepics` | [`Boolean`](#boolean) | Whether to include subepics when filtering issues by epicId. |
| <a id="projectissueiterationid"></a>`iterationId` | [`[ID]`](#id) | List of iteration Global IDs applied to the issue. |
| <a id="projectissueiterationwildcardid"></a>`iterationWildcardId` | [`IterationWildcardId`](#iterationwildcardid) | Filter by iteration ID wildcard. |
@@ -16113,6 +16115,7 @@ Returns [`IssueStatusCountsType`](#issuestatuscountstype).
| <a id="projectissuestatuscountscrmorganizationid"></a>`crmOrganizationId` | [`String`](#string) | ID of an organization assigned to the issues. |
| <a id="projectissuestatuscountsiid"></a>`iid` | [`String`](#string) | IID of the issue. For example, "1". |
| <a id="projectissuestatuscountsiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of issues. For example, `["1", "2"]`. |
+| <a id="projectissuestatuscountsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="projectissuestatuscountslabelname"></a>`labelName` | [`[String]`](#string) | Labels applied to this issue. |
| <a id="projectissuestatuscountsmilestonetitle"></a>`milestoneTitle` | [`[String]`](#string) | Milestone applied to this issue. |
| <a id="projectissuestatuscountsmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
@@ -16153,6 +16156,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| <a id="projectissuesepicid"></a>`epicId` | [`String`](#string) | ID of an epic associated with the issues, "none" and "any" values are supported. |
| <a id="projectissuesiid"></a>`iid` | [`String`](#string) | IID of the issue. For example, "1". |
| <a id="projectissuesiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of issues. For example, `["1", "2"]`. |
+| <a id="projectissuesin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="projectissuesincludesubepics"></a>`includeSubepics` | [`Boolean`](#boolean) | Whether to include subepics when filtering issues by epicId. |
| <a id="projectissuesiterationid"></a>`iterationId` | [`[ID]`](#id) | List of iteration Global IDs applied to the issue. |
| <a id="projectissuesiterationwildcardid"></a>`iterationWildcardId` | [`IterationWildcardId`](#iterationwildcardid) | Filter by iteration ID wildcard. |
@@ -16730,6 +16734,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
| ---- | ---- | ----------- |
| <a id="projectworkitemsiid"></a>`iid` | [`String`](#string) | IID of the issue. For example, "1". |
| <a id="projectworkitemsiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of work items. For example, `["1", "2"]`. |
+| <a id="projectworkitemsin"></a>`in` | [`[IssuableSearchableField!]`](#issuablesearchablefield) | Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'. |
| <a id="projectworkitemssearch"></a>`search` | [`String`](#string) | Search query for title or description. |
| <a id="projectworkitemssort"></a>`sort` | [`WorkItemSort`](#workitemsort) | Sort work items by this criteria. |
| <a id="projectworkitemsstate"></a>`state` | [`IssuableState`](#issuablestate) | Current state of this work item. |
diff --git a/ee/app/graphql/resolvers/epics_resolver.rb b/ee/app/graphql/resolvers/epics_resolver.rb
index 66c6706534ea..545c927dbe90 100644
--- a/ee/app/graphql/resolvers/epics_resolver.rb
+++ b/ee/app/graphql/resolvers/epics_resolver.rb
@@ -18,10 +18,6 @@ class EpicsResolver < BaseResolver
required: false,
description: 'Filter epics by state.'
- argument :in, [Types::IssuableSearchableFieldEnum],
- required: false,
- description: 'Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'
-
argument :sort, Types::EpicSortEnum,
required: false,
description: 'List epics by sort order.'
@@ -86,8 +82,6 @@ class EpicsResolver < BaseResolver
def ready?(**args)
validate_timeframe_params!(args)
validate_starts_with_iid!(args)
- validate_search_in_params!(args)
- validate_anonymous_search_access! if args[:search].present?
super(**args)
end
@@ -98,7 +92,7 @@ def resolve_with_lookahead(**args)
return [] unless resolver_object.present?
return [] unless epic_feature_enabled?
- find_epics(transform_args(args))
+ find_epics(prepare_finder_params(args))
end
private
@@ -130,14 +124,19 @@ def epic_feature_enabled?
end
def transform_args(args)
- transformed = args.dup
- transformed[:group_id] = group
- transformed[:iids] ||= [args[:iid]].compact
- transformed[:in] = args[:in].join(',') if args[:in].present?
+ transformed = args.dup
+ transformed[:group_id] = group
+ transformed[:iids] ||= [args[:iid]].compact
transformed.merge(transform_timeframe_parameters(args)).merge(relative_param)
end
+ def prepare_finder_params(args)
+ params = transform_args(args)
+
+ super(params)
+ end
+
def relative_param
return {} unless parent
@@ -159,6 +158,10 @@ def group
parent.group
end
+ def resource_parent
+ strong_memoize(:resource_parent) { group }
+ end
+
# If we're querying for multiple iids and selecting issues, then ideally
# we want to batch the epic and issue queries into one to reduce N+1 and memory.
# https://gitlab.com/gitlab-org/gitlab/issues/11841
@@ -183,12 +186,5 @@ def validate_starts_with_iid!(args)
raise Gitlab::Graphql::Errors::ArgumentError, 'Invalid `iidStartsWith` query'
end
end
-
- def validate_search_in_params!(args)
- return unless args[:in].present? && args[:search].blank?
-
- raise Gitlab::Graphql::Errors::ArgumentError,
- '`search` should be present when including the `in` argument'
- end
end
end
diff --git a/ee/spec/graphql/resolvers/epics_resolver_spec.rb b/ee/spec/graphql/resolvers/epics_resolver_spec.rb
index 5c0d01ec2445..25fb495c4ff7 100644
--- a/ee/spec/graphql/resolvers/epics_resolver_spec.rb
+++ b/ee/spec/graphql/resolvers/epics_resolver_spec.rb
@@ -99,83 +99,15 @@
end
end
- context 'with search' do
- let!(:epic3) { create(:epic, group: group, title: 'third', description: 'text 2') }
-
- it 'filters epics by title' do
- epics = resolve_epics(search: 'created')
-
- expect(epics).to match_array([epic1, epic2])
- end
-
- it 'filters epics by description' do
- epics = resolve_epics(search: 'text')
-
- expect(epics).to match_array([epic2, epic3])
- end
-
- context 'with in param' do
- it 'generates an error if param search is missing' do
- error_message = "`search` should be present when including the `in` argument"
-
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do
- resolve_epics(in: ['title'])
- end
- end
-
- it 'filters epics by description only' do
- epics_with_text = resolve_epics(search: 'text', in: ['description'])
- epics_with_created = resolve_epics(search: 'created', in: ['description'])
-
- expect(epics_with_created).to be_empty
- expect(epics_with_text).to match_array([epic2, epic3])
- end
-
- it 'filters epics by title only' do
- epics_with_text = resolve_epics(search: 'text', in: ['title'])
- epics_with_created = resolve_epics(search: 'created', in: ['title'])
-
- expect(epics_with_created).to match_array([epic1, epic2])
- expect(epics_with_text).to be_empty
- end
-
- it 'filters epics by title and description' do
- epic4 = create(:epic, group: group, title: 'fourth text', description: ['description'])
- epics = resolve_epics(search: 'text', in: %w(title description))
-
- expect(epics).to match_array([epic2, epic3, epic4])
- end
- end
-
- context 'with anonymous user' do
- let_it_be(:current_user) { nil }
-
- context 'with disable_anonymous_search enabled' do
- before do
- stub_feature_flags(disable_anonymous_search: true)
- end
-
- it 'returns an error' do
- error_message = "User must be authenticated to include the `search` argument."
-
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do
- resolve_epics(search: 'created')
- end
- end
- end
-
- context 'with disable_anonymous_search disabled' do
- before do
- stub_feature_flags(disable_anonymous_search: false)
- end
-
- it 'filters epics by search term' do
- epics = resolve_epics(search: 'created')
-
- expect(epics).to match_array([epic1, epic2])
- end
- end
- end
+ it_behaves_like 'graphql query for searching issuables' do
+ let_it_be(:parent) { group }
+ let_it_be(:issuable1) { epic1 }
+ let_it_be(:issuable2) { epic2 }
+ let_it_be(:issuable3) { create(:epic, group: group, title: 'third', description: 'text 2') }
+ let_it_be(:issuable4) { create(:epic, group: group) }
+
+ let_it_be(:finder_class) { EpicsFinder }
+ let_it_be(:optimization_param) { :attempt_group_search_optimizations }
end
context 'with author_username' do
diff --git a/ee/spec/requests/api/graphql/group/epics_spec.rb b/ee/spec/requests/api/graphql/group/epics_spec.rb
index 42042099535d..69ed0c829413 100644
--- a/ee/spec/requests/api/graphql/group/epics_spec.rb
+++ b/ee/spec/requests/api/graphql/group/epics_spec.rb
@@ -262,16 +262,10 @@ def global_ids(*epics)
end
end
- context 'filter' do
- context 'with search params' do
- it 'returns only matching epics' do
- filter_params = { search: 'bar', in: [:DESCRIPTION] }
- graphql_query = query(filter_params)
-
- post_graphql(graphql_query, current_user: user)
-
- expect_array_response([epic2.to_global_id.to_s])
- end
+ context 'with search params' do
+ it_behaves_like 'query with a search term' do
+ let(:issuable_data) { epics_data }
+ let(:issuable) { epic2 }
end
end
end
diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb
index 89e45810033f..a74b2a3f18cb 100644
--- a/spec/graphql/resolvers/issues_resolver_spec.rb
+++ b/spec/graphql/resolvers/issues_resolver_spec.rb
@@ -311,49 +311,15 @@
end
context 'when searching issues' do
- it 'returns correct issues' do
- expect(resolve_issues(search: 'foo')).to contain_exactly(issue2)
- end
-
- it 'uses project search optimization' do
- expected_arguments = a_hash_including(
- search: 'foo',
- attempt_project_search_optimizations: true
- )
- expect(IssuesFinder).to receive(:new).with(anything, expected_arguments).and_call_original
-
- resolve_issues(search: 'foo')
- end
-
- context 'with anonymous user' do
- let_it_be(:public_project) { create(:project, :public) }
- let_it_be(:public_issue) { create(:issue, project: public_project, title: 'Test issue') }
-
- context 'with disable_anonymous_search enabled' do
- before do
- stub_feature_flags(disable_anonymous_search: true)
- end
-
- it 'generates an error' do
- error_message = "User must be authenticated to include the `search` argument."
-
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do
- resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil })
- end
- end
- end
-
- context 'with disable_anonymous_search disabled' do
- before do
- stub_feature_flags(disable_anonymous_search: false)
- end
-
- it 'returns correct issues' do
- expect(
- resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil })
- ).to contain_exactly(public_issue)
- end
- end
+ it_behaves_like 'graphql query for searching issuables' do
+ let_it_be(:parent) { project }
+ let_it_be(:issuable1) { create(:issue, project: project, title: 'first created') }
+ let_it_be(:issuable2) { create(:issue, project: project, title: 'second created', description: 'text 1') }
+ let_it_be(:issuable3) { create(:issue, project: project, title: 'third', description: 'text 2') }
+ let_it_be(:issuable4) { create(:issue, project: project) }
+
+ let_it_be(:finder_class) { IssuesFinder }
+ let_it_be(:optimization_param) { :attempt_project_search_optimizations }
end
end
diff --git a/spec/graphql/resolvers/work_items_resolver_spec.rb b/spec/graphql/resolvers/work_items_resolver_spec.rb
index 29eac0ab46ed..ef7cc0daa0c2 100644
--- a/spec/graphql/resolvers/work_items_resolver_spec.rb
+++ b/spec/graphql/resolvers/work_items_resolver_spec.rb
@@ -52,49 +52,15 @@
end
context 'when searching items' do
- it 'returns correct items' do
- expect(resolve_items(search: 'foo')).to contain_exactly(item2)
- end
-
- it 'uses project search optimization' do
- expected_arguments = a_hash_including(
- search: 'foo',
- attempt_project_search_optimizations: true
- )
- expect(::WorkItems::WorkItemsFinder).to receive(:new).with(anything, expected_arguments).and_call_original
-
- resolve_items(search: 'foo')
- end
-
- context 'with anonymous user' do
- let_it_be(:public_project) { create(:project, :public) }
- let_it_be(:public_item) { create(:work_item, project: public_project, title: 'Test item') }
-
- context 'with disable_anonymous_search enabled' do
- before do
- stub_feature_flags(disable_anonymous_search: true)
- end
-
- it 'generates an error' do
- error_message = "User must be authenticated to include the `search` argument."
-
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do
- resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil })
- end
- end
- end
-
- context 'with disable_anonymous_search disabled' do
- before do
- stub_feature_flags(disable_anonymous_search: false)
- end
-
- it 'returns correct items' do
- expect(
- resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil })
- ).to contain_exactly(public_item)
- end
- end
+ it_behaves_like 'graphql query for searching issuables' do
+ let_it_be(:parent) { project }
+ let_it_be(:issuable1) { create(:work_item, project: project, title: 'first created') }
+ let_it_be(:issuable2) { create(:work_item, project: project, title: 'second created', description: 'text 1') }
+ let_it_be(:issuable3) { create(:work_item, project: project, title: 'third', description: 'text 2') }
+ let_it_be(:issuable4) { create(:work_item, project: project) }
+
+ let_it_be(:finder_class) { ::WorkItems::WorkItemsFinder }
+ let_it_be(:optimization_param) { :attempt_project_search_optimizations }
end
end
diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb
index 596e023a027d..d0ed42123491 100644
--- a/spec/requests/api/graphql/project/issues_spec.rb
+++ b/spec/requests/api/graphql/project/issues_spec.rb
@@ -27,14 +27,6 @@
QUERY
end
- let(:query) do
- graphql_query_for(
- 'project',
- { 'fullPath' => project.full_path },
- query_graphql_field('issues', issue_filter_params, fields)
- )
- end
-
it_behaves_like 'a working graphql query' do
before do
post_graphql(query, current_user: current_user)
@@ -89,6 +81,14 @@
end
end
+ context 'when filtering by search' do
+ it_behaves_like 'query with a search term' do
+ let(:issuable_data) { issues_data }
+ let(:user) { current_user }
+ let_it_be(:issuable) { create(:issue, project: project, description: 'bar') }
+ end
+ end
+
context 'when limiting the number of results' do
let(:query) do
<<~GQL
@@ -679,4 +679,12 @@ def execute_query
def issues_ids
graphql_dig_at(issues_data, :node, :id)
end
+
+ def query(params = issue_filter_params)
+ graphql_query_for(
+ 'project',
+ { 'fullPath' => project.full_path },
+ query_graphql_field('issues', params, fields)
+ )
+ end
end
diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb
index 6ef28392b8b1..1dd0046f782c 100644
--- a/spec/requests/api/graphql/project/work_items_spec.rb
+++ b/spec/requests/api/graphql/project/work_items_spec.rb
@@ -27,14 +27,6 @@
QUERY
end
- let(:query) do
- graphql_query_for(
- 'project',
- { 'fullPath' => project.full_path },
- query_graphql_field('workItems', item_filter_params, fields)
- )
- end
-
it_behaves_like 'a working graphql query' do
before do
post_graphql(query, current_user: current_user)
@@ -83,6 +75,14 @@
end
end
+ context 'when filtering by search' do
+ it_behaves_like 'query with a search term' do
+ let(:issuable_data) { items_data }
+ let(:user) { current_user }
+ let_it_be(:issuable) { create(:work_item, project: project, description: 'bar') }
+ end
+ end
+
describe 'sorting and pagination' do
let(:data_path) { [:project, :work_items] }
@@ -118,4 +118,12 @@ def pagination_query(params)
def item_ids
graphql_dig_at(items_data, :node, :id)
end
+
+ def query(params = item_filter_params)
+ graphql_query_for(
+ 'project',
+ { 'fullPath' => project.full_path },
+ query_graphql_field('workItems', params, fields)
+ )
+ end
end
diff --git a/spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb b/spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb
new file mode 100644
index 000000000000..feaa8070090e
--- /dev/null
+++ b/spec/support/shared_examples/graphql/resolvers/issuable_resolvers_shared_examples.rb
@@ -0,0 +1,95 @@
+# frozen_string_literal: true
+
+# Requires `parent`, issuable1`, `issuable2`, `issuable3`, `issuable4`,
+# `finder_class` and `optimization_param` bindings.
+RSpec.shared_examples 'graphql query for searching issuables' do
+ it 'uses search optimization' do
+ expected_arguments = a_hash_including(
+ search: 'text',
+ optimization_param => true
+ )
+ expect(finder_class).to receive(:new).with(anything, expected_arguments).and_call_original
+
+ resolve_issuables(search: 'text')
+ end
+
+ it 'filters issuables by title' do
+ issuables = resolve_issuables(search: 'created')
+
+ expect(issuables).to contain_exactly(issuable1, issuable2)
+ end
+
+ it 'filters issuables by description' do
+ issuables = resolve_issuables(search: 'text')
+
+ expect(issuables).to contain_exactly(issuable2, issuable3)
+ end
+
+ context 'with in param' do
+ it 'generates an error if param search is missing' do
+ error_message = "`search` should be present when including the `in` argument"
+
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do
+ resolve_issuables(in: ['title'])
+ end
+ end
+
+ it 'filters issuables by title and description' do
+ issuable4.update!(title: 'fourth text')
+ issuables = resolve_issuables(search: 'text', in: %w[title description])
+
+ expect(issuables).to contain_exactly(issuable2, issuable3, issuable4)
+ end
+
+ it 'filters issuables by description only' do
+ with_text = resolve_issuables(search: 'text', in: ['description'])
+ with_created = resolve_issuables(search: 'created', in: ['description'])
+
+ expect(with_created).to be_empty
+ expect(with_text).to contain_exactly(issuable2, issuable3)
+ end
+
+ it 'filters issuables by title only' do
+ with_text = resolve_issuables(search: 'text', in: ['title'])
+ with_created = resolve_issuables(search: 'created', in: ['title'])
+
+ expect(with_created).to contain_exactly(issuable1, issuable2)
+ expect(with_text).to be_empty
+ end
+ end
+
+ context 'with anonymous user' do
+ let_it_be(:current_user) { nil }
+
+ context 'with disable_anonymous_search as `true`' do
+ before do
+ stub_feature_flags(disable_anonymous_search: true)
+ end
+
+ it 'returns an error' do
+ error_message = "User must be authenticated to include the `search` argument."
+
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, error_message) do
+ resolve_issuables(search: 'created')
+ end
+ end
+ end
+
+ context 'with disable_anonymous_search as `false`' do
+ before do
+ stub_feature_flags(disable_anonymous_search: false)
+ parent.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+ end
+
+ it 'filters issuables by search term' do
+ issuables = resolve_issuables(search: 'created')
+
+ expect(issuables).to contain_exactly(issuable1, issuable2)
+ end
+ end
+ end
+
+ def resolve_issuables(args = {}, obj = parent, context = { current_user: current_user })
+ resolve(described_class, obj: obj, args: args, ctx: context, arg_style: :internal)
+ end
+end
diff --git a/spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb b/spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb
new file mode 100644
index 000000000000..22805cf7aedf
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/graphql/issuable_search_shared_examples.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+# Requires `query(params)` , `user`, `issuable_data` and `issuable` bindings
+RSpec.shared_examples 'query with a search term' do
+ it 'returns only matching issuables' do
+ filter_params = { search: 'bar', in: [:DESCRIPTION] }
+ graphql_query = query(filter_params)
+
+ post_graphql(graphql_query, current_user: user)
+ ids = graphql_dig_at(issuable_data, :node, :id)
+
+ expect(ids).to contain_exactly(issuable.to_global_id.to_s)
+ end
+end
--
GitLab