Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
7f1c9cc7
Commit
7f1c9cc7
authored
Sep 14, 2021
by
Heinrich Lee Yu
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Remove optimized_issuable_label_filter flag
Also refactors the label filter to a separate class Changelog: other
parent
ab95a021
Changes
12
Hide whitespace changes
Inline
Side-by-side
Showing
12 changed files
with
288 additions
and
407 deletions
+288
-407
app/finders/issuable_finder.rb
app/finders/issuable_finder.rb
+23
-39
app/finders/issuable_finder/params.rb
app/finders/issuable_finder/params.rb
+0
-32
app/finders/issuables/label_filter.rb
app/finders/issuables/label_filter.rb
+155
-0
app/models/concerns/issuable.rb
app/models/concerns/issuable.rb
+0
-14
app/models/concerns/optimized_issuable_label_filter.rb
app/models/concerns/optimized_issuable_label_filter.rb
+0
-121
config/feature_flags/development/optimized_issuable_label_filter.yml
...ure_flags/development/optimized_issuable_label_filter.yml
+0
-8
ee/app/finders/epics_finder.rb
ee/app/finders/epics_finder.rb
+10
-24
ee/spec/graphql/resolvers/epics_resolver_spec.rb
ee/spec/graphql/resolvers/epics_resolver_spec.rb
+1
-1
spec/finders/issues_finder_spec.rb
spec/finders/issues_finder_spec.rb
+74
-92
spec/finders/merge_requests_finder_spec.rb
spec/finders/merge_requests_finder_spec.rb
+24
-42
spec/graphql/resolvers/merge_requests_resolver_spec.rb
spec/graphql/resolvers/merge_requests_resolver_spec.rb
+0
-10
spec/requests/api/issues/get_group_issues_spec.rb
spec/requests/api/issues/get_group_issues_spec.rb
+1
-24
No files found.
app/finders/issuable_finder.rb
View file @
7f1c9cc7
...
...
@@ -41,7 +41,6 @@ class IssuableFinder
include
FinderMethods
include
CreatedAtFilter
include
Gitlab
::
Utils
::
StrongMemoize
prepend
OptimizedIssuableLabelFilter
requires_cross_project_access
unless:
->
{
params
.
project?
}
...
...
@@ -149,7 +148,6 @@ class IssuableFinder
# Negates all params found in `negatable_params`
def
filter_negated_items
(
items
)
items
=
by_negated_label
(
items
)
items
=
by_negated_milestone
(
items
)
items
=
by_negated_release
(
items
)
items
=
by_negated_my_reaction_emoji
(
items
)
...
...
@@ -172,29 +170,19 @@ class IssuableFinder
count_params
=
params
.
merge
(
state:
nil
,
sort:
nil
,
force_cte:
true
)
finder
=
self
.
class
.
new
(
current_user
,
count_params
)
state_counts
=
finder
.
execute
.
reorder
(
nil
)
.
group
(
:state_id
)
.
count
counts
=
Hash
.
new
(
0
)
# Searching by label includes a GROUP BY in the query, but ours will be last
# because it is added last. Searching by multiple labels also includes a row
# per issuable, so we have to count those in Ruby - which is bad, but still
# better than performing multiple queries.
#
# This does not apply when we are using a CTE for the search, as the labels
# GROUP BY is inside the subquery in that case, so we set labels_count to 1.
#
# Groups and projects have separate feature flags to suggest the use
# of a CTE. The CTE will not be used if the sort doesn't support it,
# but will always be used for the counts here as we ignore sorting
# anyway.
labels_count
=
params
.
label_names
.
any?
?
params
.
label_names
.
count
:
1
labels_count
=
1
if
use_cte_for_search?
finder
.
execute
.
reorder
(
nil
).
group
(
:state_id
).
count
.
each
do
|
key
,
value
|
counts
[
count_key
(
key
)]
+=
value
/
labels_count
state_counts
.
each
do
|
key
,
value
|
counts
[
count_key
(
key
)]
+=
value
end
counts
[
:all
]
=
counts
.
values
.
sum
counts
.
with_indifferent_access
end
# rubocop: enable CodeReuse/ActiveRecord
...
...
@@ -360,7 +348,7 @@ class IssuableFinder
def
sort
(
items
)
# Ensure we always have an explicit sort order (instead of inheriting
# multiple orders when combining ActiveRecord::Relation objects).
params
[
:sort
]
?
items
.
sort_by_attribute
(
params
[
:sort
],
excluded_labels:
params
.
label_names
)
:
items
.
reorder
(
id: :desc
)
params
[
:sort
]
?
items
.
sort_by_attribute
(
params
[
:sort
],
excluded_labels:
label_filter
.
label_names_excluded_from_priority_sort
)
:
items
.
reorder
(
id: :desc
)
end
# rubocop: enable CodeReuse/ActiveRecord
...
...
@@ -384,6 +372,20 @@ class IssuableFinder
end
end
def
by_label
(
items
)
label_filter
.
filter
(
items
)
end
def
label_filter
strong_memoize
(
:label_filter
)
do
Issuables
::
LabelFilter
.
new
(
params:
original_params
,
project:
params
.
project
,
group:
params
.
group
)
end
end
# rubocop: disable CodeReuse/ActiveRecord
def
by_milestone
(
items
)
return
items
unless
params
.
milestones?
...
...
@@ -436,24 +438,6 @@ class IssuableFinder
items
.
without_particular_release
(
not_params
[
:release_tag
],
not_params
[
:project_id
])
end
def
by_label
(
items
)
return
items
unless
params
.
labels?
if
params
.
filter_by_no_label?
items
.
without_label
elsif
params
.
filter_by_any_label?
items
.
any_label
(
params
[
:sort
])
else
items
.
with_label
(
params
.
label_names
,
params
[
:sort
])
end
end
def
by_negated_label
(
items
)
return
items
unless
not_params
.
labels?
items
.
without_particular_labels
(
not_params
.
label_names
)
end
def
by_my_reaction_emoji
(
items
)
return
items
unless
params
[
:my_reaction_emoji
]
&&
current_user
...
...
app/finders/issuable_finder/params.rb
View file @
7f1c9cc7
...
...
@@ -29,20 +29,6 @@ class IssuableFinder
params
.
present?
end
def
filter_by_no_label?
downcased
=
label_names
.
map
(
&
:downcase
)
downcased
.
include?
(
FILTER_NONE
)
end
def
filter_by_any_label?
label_names
.
map
(
&
:downcase
).
include?
(
FILTER_ANY
)
end
def
labels?
params
[
:label_name
].
present?
end
def
milestones?
params
[
:milestone_title
].
present?
||
params
[
:milestone_wildcard_id
].
present?
end
...
...
@@ -160,24 +146,6 @@ class IssuableFinder
end
end
def
label_names
if
labels?
params
[
:label_name
].
is_a?
(
String
)
?
params
[
:label_name
].
split
(
','
)
:
params
[
:label_name
]
else
[]
end
end
def
labels
strong_memoize
(
:labels
)
do
if
labels?
&&
!
filter_by_no_label?
LabelsFinder
.
new
(
current_user
,
project_ids:
projects
,
title:
label_names
).
execute
(
skip_authorization:
true
)
# rubocop: disable CodeReuse/Finder
else
Label
.
none
end
end
end
def
milestones
strong_memoize
(
:milestones
)
do
if
milestones?
...
...
app/finders/issuables/label_filter.rb
0 → 100644
View file @
7f1c9cc7
# frozen_string_literal: true
module
Issuables
class
LabelFilter
<
BaseFilter
include
Gitlab
::
Utils
::
StrongMemoize
extend
Gitlab
::
Cache
::
RequestCache
def
initialize
(
project
:,
group
:,
**
kwargs
)
@project
=
project
@group
=
group
super
(
**
kwargs
)
end
def
filter
(
issuables
)
filtered
=
by_label
(
issuables
)
by_negated_label
(
filtered
)
end
def
label_names_excluded_from_priority_sort
label_names_from_params
end
private
# rubocop: disable CodeReuse/ActiveRecord
def
by_label
(
issuables
)
return
issuables
unless
label_names_from_params
.
present?
target_model
=
issuables
.
model
if
filter_by_no_label?
issuables
.
where
(
label_link_query
(
target_model
).
arel
.
exists
.
not
)
elsif
filter_by_any_label?
issuables
.
where
(
label_link_query
(
target_model
).
arel
.
exists
)
else
issuables_with_selected_labels
(
issuables
,
label_names_from_params
)
end
end
# rubocop: enable CodeReuse/ActiveRecord
def
by_negated_label
(
issuables
)
return
issuables
unless
label_names_from_not_params
.
present?
issuables_without_selected_labels
(
issuables
,
label_names_from_not_params
)
end
def
filter_by_no_label?
label_names_from_params
.
map
(
&
:downcase
).
include?
(
FILTER_NONE
)
end
def
filter_by_any_label?
label_names_from_params
.
map
(
&
:downcase
).
include?
(
FILTER_ANY
)
end
# rubocop: disable CodeReuse/ActiveRecord
def
issuables_with_selected_labels
(
issuables
,
label_names
)
target_model
=
issuables
.
model
if
root_namespace
all_label_ids
=
find_label_ids
(
label_names
)
# Found less labels in the DB than we were searching for. Return nothing.
return
issuables
.
none
if
all_label_ids
.
size
!=
label_names
.
size
all_label_ids
.
each
do
|
label_ids
|
issuables
=
issuables
.
where
(
label_link_query
(
target_model
,
label_ids:
label_ids
).
arel
.
exists
)
end
else
label_names
.
each
do
|
label_name
|
issuables
=
issuables
.
where
(
label_link_query
(
target_model
,
label_names:
label_name
).
arel
.
exists
)
end
end
issuables
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
issuables_without_selected_labels
(
issuables
,
label_names
)
target_model
=
issuables
.
model
if
root_namespace
label_ids
=
find_label_ids
(
label_names
).
flatten
(
1
)
issuables
.
where
(
label_link_query
(
target_model
,
label_ids:
label_ids
).
arel
.
exists
.
not
)
else
issuables
.
where
(
label_link_query
(
target_model
,
label_names:
label_names
).
arel
.
exists
.
not
)
end
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
find_label_ids
(
label_names
)
group_labels
=
Label
.
where
(
project_id:
nil
)
.
where
(
title:
label_names
)
.
where
(
group_id:
root_namespace
.
self_and_descendant_ids
)
project_labels
=
Label
.
where
(
group_id:
nil
)
.
where
(
title:
label_names
)
.
where
(
project_id:
Project
.
select
(
:id
).
where
(
namespace_id:
root_namespace
.
self_and_descendant_ids
))
Label
.
from_union
([
group_labels
,
project_labels
],
remove_duplicates:
false
)
.
reorder
(
nil
)
.
pluck
(
:title
,
:id
)
.
group_by
(
&
:first
)
.
values
.
map
{
|
labels
|
labels
.
map
(
&
:last
)
}
end
# Avoid repeating label queries times when the finder is instantiated multiple times during the request.
request_cache
(
:find_label_ids
)
{
root_namespace
.
id
}
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def
label_link_query
(
target_model
,
label_ids:
nil
,
label_names:
nil
)
relation
=
LabelLink
.
where
(
target_type:
target_model
.
name
)
.
where
(
LabelLink
.
arel_table
[
'target_id'
].
eq
(
target_model
.
arel_table
[
'id'
]))
relation
=
relation
.
where
(
label_id:
label_ids
)
if
label_ids
relation
=
relation
.
joins
(
:label
).
where
(
labels:
{
name:
label_names
})
if
label_names
relation
end
# rubocop: enable CodeReuse/ActiveRecord
def
label_names_from_params
return
if
params
[
:label_name
].
blank?
strong_memoize
(
:label_names_from_params
)
do
split_label_names
(
params
[
:label_name
])
end
end
def
label_names_from_not_params
return
if
not_params
.
blank?
||
not_params
[
:label_name
].
blank?
strong_memoize
(
:label_names_from_not_params
)
do
split_label_names
(
not_params
[
:label_name
])
end
end
def
split_label_names
(
label_name_param
)
label_name_param
.
is_a?
(
String
)
?
label_name_param
.
split
(
','
)
:
label_name_param
end
def
root_namespace
strong_memoize
(
:root_namespace
)
do
(
@project
||
@group
)
&
.
root_ancestor
end
end
end
end
app/models/concerns/issuable.rb
View file @
7f1c9cc7
...
...
@@ -117,20 +117,6 @@ module Issuable
end
# rubocop:enable GitlabSecurity/SqlInjection
scope
:without_particular_labels
,
->
(
label_names
)
do
labels_table
=
Label
.
arel_table
label_links_table
=
LabelLink
.
arel_table
issuables_table
=
klass
.
arel_table
inner_query
=
label_links_table
.
project
(
'true'
)
.
join
(
labels_table
,
Arel
::
Nodes
::
InnerJoin
).
on
(
labels_table
[
:id
].
eq
(
label_links_table
[
:label_id
]))
.
where
(
label_links_table
[
:target_type
].
eq
(
name
)
.
and
(
label_links_table
[
:target_id
].
eq
(
issuables_table
[
:id
]))
.
and
(
labels_table
[
:title
].
in
(
label_names
)))
.
exists
.
not
where
(
inner_query
)
end
scope
:without_label
,
->
{
joins
(
"LEFT OUTER JOIN label_links ON label_links.target_type = '
#{
name
}
' AND label_links.target_id =
#{
table_name
}
.id"
).
where
(
label_links:
{
id:
nil
})
}
scope
:with_label_ids
,
->
(
label_ids
)
{
joins
(
:label_links
).
where
(
label_links:
{
label_id:
label_ids
})
}
scope
:join_project
,
->
{
joins
(
:project
)
}
...
...
app/models/concerns/optimized_issuable_label_filter.rb
deleted
100644 → 0
View file @
ab95a021
# frozen_string_literal: true
module
OptimizedIssuableLabelFilter
extend
ActiveSupport
::
Concern
prepended
do
extend
Gitlab
::
Cache
::
RequestCache
# Avoid repeating label queries times when the finder is instantiated multiple times during the request.
request_cache
(
:find_label_ids
)
{
[
root_namespace
.
id
,
params
.
label_names
]
}
end
def
by_label
(
items
)
return
items
unless
params
.
labels?
return
super
if
Feature
.
disabled?
(
:optimized_issuable_label_filter
,
default_enabled: :yaml
)
target_model
=
items
.
model
if
params
.
filter_by_no_label?
items
.
where
(
'NOT EXISTS (?)'
,
optimized_any_label_query
(
target_model
))
elsif
params
.
filter_by_any_label?
items
.
where
(
'EXISTS (?)'
,
optimized_any_label_query
(
target_model
))
else
issuables_with_selected_labels
(
items
,
target_model
)
end
end
# Taken from IssuableFinder
def
count_by_state
return
super
if
Feature
.
disabled?
(
:optimized_issuable_label_filter
,
default_enabled: :yaml
)
count_params
=
params
.
merge
(
state:
nil
,
sort:
nil
,
force_cte:
true
)
finder
=
self
.
class
.
new
(
current_user
,
count_params
)
state_counts
=
finder
.
execute
.
reorder
(
nil
)
.
group
(
:state_id
)
.
count
counts
=
Hash
.
new
(
0
)
state_counts
.
each
do
|
key
,
value
|
counts
[
count_key
(
key
)]
+=
value
end
counts
[
:all
]
=
counts
.
values
.
sum
counts
.
with_indifferent_access
end
private
def
issuables_with_selected_labels
(
items
,
target_model
)
if
root_namespace
all_label_ids
=
find_label_ids
# Found less labels in the DB than we were searching for. Return nothing.
return
items
.
none
if
all_label_ids
.
size
!=
params
.
label_names
.
size
all_label_ids
.
each
do
|
label_ids
|
items
=
items
.
where
(
'EXISTS (?)'
,
optimized_label_query_by_label_ids
(
target_model
,
label_ids
))
end
else
params
.
label_names
.
each
do
|
label_name
|
items
=
items
.
where
(
'EXISTS (?)'
,
optimized_label_query_by_label_name
(
target_model
,
label_name
))
end
end
items
end
def
find_label_ids
group_labels
=
Label
.
where
(
project_id:
nil
)
.
where
(
title:
params
.
label_names
)
.
where
(
group_id:
root_namespace
.
self_and_descendants
.
select
(
:id
))
project_labels
=
Label
.
where
(
group_id:
nil
)
.
where
(
title:
params
.
label_names
)
.
where
(
project_id:
Project
.
select
(
:id
).
where
(
namespace_id:
root_namespace
.
self_and_descendants
.
select
(
:id
)))
Label
.
from_union
([
group_labels
,
project_labels
],
remove_duplicates:
false
)
.
reorder
(
nil
)
.
pluck
(
:title
,
:id
)
.
group_by
(
&
:first
)
.
values
.
map
{
|
labels
|
labels
.
map
(
&
:last
)
}
end
def
root_namespace
strong_memoize
(
:root_namespace
)
do
(
params
.
project
||
params
.
group
)
&
.
root_ancestor
end
end
def
optimized_any_label_query
(
target_model
)
LabelLink
.
where
(
target_type:
target_model
.
name
)
.
where
(
LabelLink
.
arel_table
[
'target_id'
].
eq
(
target_model
.
arel_table
[
'id'
]))
.
limit
(
1
)
end
def
optimized_label_query_by_label_ids
(
target_model
,
label_ids
)
LabelLink
.
where
(
target_type:
target_model
.
name
)
.
where
(
LabelLink
.
arel_table
[
'target_id'
].
eq
(
target_model
.
arel_table
[
'id'
]))
.
where
(
label_id:
label_ids
)
.
limit
(
1
)
end
def
optimized_label_query_by_label_name
(
target_model
,
label_name
)
LabelLink
.
joins
(
:label
)
.
where
(
target_type:
target_model
.
name
)
.
where
(
LabelLink
.
arel_table
[
'target_id'
].
eq
(
target_model
.
arel_table
[
'id'
]))
.
where
(
labels:
{
name:
label_name
})
.
limit
(
1
)
end
end
config/feature_flags/development/optimized_issuable_label_filter.yml
deleted
100644 → 0
View file @
ab95a021
---
name
:
optimized_issuable_label_filter
introduced_by_url
:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34503
rollout_issue_url
:
https://gitlab.com/gitlab-org/gitlab/-/issues/259719
milestone
:
'
13.4'
type
:
development
group
:
group::optimize
default_enabled
:
true
ee/app/finders/epics_finder.rb
View file @
7f1c9cc7
...
...
@@ -65,7 +65,7 @@ class EpicsFinder < IssuableFinder
@skip_visibility_check
=
skip_visibility_check
raise
ArgumentError
,
'group_id argument is missing'
unless
params
[
:group_id
]
return
Epic
.
none
unless
Ability
.
allowed?
(
current_user
,
:read_epic
,
group
)
return
Epic
.
none
unless
Ability
.
allowed?
(
current_user
,
:read_epic
,
params
.
group
)
items
=
init_collection
items
=
filter_items
(
items
)
...
...
@@ -83,8 +83,8 @@ class EpicsFinder < IssuableFinder
groups
=
if
params
[
:iids
].
present?
# If we are querying for specific iids, then we should only be looking at
# those in the group, not any sub-groups (which can have identical iids).
# The `group` method takes care of checking permissions
[
group
]
# The `
params.
group` method takes care of checking permissions
[
params
.
group
]
else
permissioned_related_groups
end
...
...
@@ -141,21 +141,7 @@ class EpicsFinder < IssuableFinder
# API endpoints send in `nil` values so we test if there are any non-nil
return
items
unless
not_params
&
.
values
&
.
any?
items
=
by_negated_my_reaction_emoji
(
items
)
by_negated_label
(
items
)
end
def
group
strong_memoize
(
:group
)
do
next
unless
params
[
:group_id
]
if
params
[
:group_id
].
is_a?
(
Group
)
params
[
:group_id
]
else
Group
.
find
(
params
[
:group_id
])
end
end
by_negated_my_reaction_emoji
(
items
)
end
def
starts_with_iid
(
items
)
...
...
@@ -172,13 +158,13 @@ class EpicsFinder < IssuableFinder
include_descendants
=
params
.
fetch
(
:include_descendant_groups
,
true
)
if
include_ancestors
&&
include_descendants
group
.
self_and_hierarchy
params
.
group
.
self_and_hierarchy
elsif
include_ancestors
group
.
self_and_ancestors
params
.
group
.
self_and_ancestors
elsif
include_descendants
group
.
self_and_descendants
params
.
group
.
self_and_descendants
else
Group
.
id_in
(
group
.
id
)
Group
.
id_in
(
params
.
group
.
id
)
end
end
...
...
@@ -251,7 +237,7 @@ class EpicsFinder < IssuableFinder
# `groups` is a list of groups in the same group hierarchy, group is
# highest in the group hierarchy except if we fetch ancestors - in that
# case top-level group is group's root parent
parent
=
params
.
fetch
(
:include_ancestor_groups
,
false
)
?
group
.
root_ancestor
:
group
parent
=
params
.
fetch
(
:include_ancestor_groups
,
false
)
?
params
.
group
.
root_ancestor
:
params
.
group
# If they can view confidential epics in this parent group they can
# definitely view confidential epics in subgroups.
...
...
@@ -290,6 +276,6 @@ class EpicsFinder < IssuableFinder
override
:feature_flag_scope
def
feature_flag_scope
group
params
.
group
end
end
ee/spec/graphql/resolvers/epics_resolver_spec.rb
View file @
7f1c9cc7
...
...
@@ -325,7 +325,7 @@ RSpec.describe Resolvers::EpicsResolver do
context
'with negated filters'
do
let_it_be
(
:group
)
{
create
(
:group
)
}
let_it_be
(
:author
)
{
create
(
:user
)
}
let_it_be
(
:label
)
{
create
(
:
label
)
}
let_it_be
(
:label
)
{
create
(
:
group_label
,
group:
group
)
}
let_it_be
(
:epic_1
)
{
create
(
:labeled_epic
,
group:
group
,
labels:
[
label
])
}
let_it_be
(
:epic_2
)
{
create
(
:epic
,
group:
group
,
author:
author
)
}
let_it_be
(
:epic_3
)
{
create
(
:epic
,
group:
group
)
}
...
...
spec/finders/issues_finder_spec.rb
View file @
7f1c9cc7
...
...
@@ -426,139 +426,121 @@ RSpec.describe IssuesFinder do
end
end
shared_examples
':label_name parameter'
do
context
'filtering by label'
do
let
(
:params
)
{
{
label_name:
label
.
title
}
}
context
'filtering by label'
do
let
(
:params
)
{
{
label_name:
label
.
title
}
}
it
'returns issues with that label'
do
expect
(
issues
).
to
contain_exactly
(
issue2
)
end
it
'returns issues with that label'
do
expect
(
issues
).
to
contain_exactly
(
issue2
)
end
context
'using NOT'
do
let
(
:params
)
{
{
not:
{
label_name:
label
.
title
}
}
}
context
'using NOT'
do
let
(
:params
)
{
{
not:
{
label_name:
label
.
title
}
}
}
it
'returns issues that do not have that label'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue3
,
issue4
,
issue5
)
end
it
'returns issues that do not have that label'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue3
,
issue4
,
issue5
)
end
# IssuableFinder first filters using the outer params (the ones not inside the `not` key.)
# Afterwards, it applies the `not` params to that resultset. This means that things inside the `not` param
# do not take precedence over the outer params with the same name.
context
'shadowing the same outside param'
do
let
(
:params
)
{
{
label_name:
label2
.
title
,
not:
{
label_name:
label
.
title
}
}
}
# IssuableFinder first filters using the outer params (the ones not inside the `not` key.)
# Afterwards, it applies the `not` params to that resultset. This means that things inside the `not` param
# do not take precedence over the outer params with the same name.
context
'shadowing the same outside param'
do
let
(
:params
)
{
{
label_name:
label2
.
title
,
not:
{
label_name:
label
.
title
}
}
}
it
'does not take precedence over labels outside NOT'
do
expect
(
issues
).
to
contain_exactly
(
issue3
)
end
it
'does not take precedence over labels outside NOT'
do
expect
(
issues
).
to
contain_exactly
(
issue3
)
end
end
context
'further filtering outside params'
do
let
(
:params
)
{
{
label_name:
label2
.
title
,
not:
{
assignee_username:
user2
.
username
}
}
}
context
'further filtering outside params'
do
let
(
:params
)
{
{
label_name:
label2
.
title
,
not:
{
assignee_username:
user2
.
username
}
}
}
it
'further filters on the returned resultset'
do
expect
(
issues
).
to
be_empty
end
it
'further filters on the returned resultset'
do
expect
(
issues
).
to
be_empty
end
end
end
end
context
'filtering by multiple labels'
do
let
(
:params
)
{
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
let
(
:label2
)
{
create
(
:label
,
project:
project2
)
}
before
do
create
(
:label_link
,
label:
label2
,
target:
issue2
)
end
it
'returns the unique issues with all those labels'
do
expect
(
issues
).
to
contain_exactly
(
issue2
)
end
context
'filtering by multiple labels'
do
let
(
:params
)
{
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
let
(
:label2
)
{
create
(
:label
,
project:
project2
)
}
context
'using NOT'
do
let
(
:params
)
{
{
not:
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
}
before
do
create
(
:label_link
,
label:
label2
,
target:
issue2
)
end
it
'returns issues that do not have any of the labels provided'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue4
,
issue5
)
end
end
it
'returns the unique issues with all those labels'
do
expect
(
issues
).
to
contain_exactly
(
issue2
)
end
context
'filtering by a label that includes any or none in the title'
do
let
(
:params
)
{
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
let
(
:label
)
{
create
(
:label
,
title:
'any foo'
,
project:
project2
)
}
let
(
:label2
)
{
create
(
:label
,
title:
'bar none'
,
project:
project2
)
}
context
'using NOT'
do
let
(
:params
)
{
{
not:
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
}
before
do
create
(
:label_link
,
label:
label2
,
target:
issue2
)
it
'returns issues that do not have any of the labels provided'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue4
,
issue5
)
end
end
end
it
'returns the unique issues with all those labels'
do
expect
(
issues
).
to
contain_exactly
(
issue2
)
end
context
'filtering by a label that includes any or none in the title'
do
let
(
:params
)
{
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
let
(
:label
)
{
create
(
:label
,
title:
'any foo'
,
project:
project2
)
}
let
(
:label2
)
{
create
(
:label
,
title:
'bar none'
,
project:
project2
)
}
context
'using NOT'
do
let
(
:params
)
{
{
not:
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
}
before
do
create
(
:label_link
,
label:
label2
,
target:
issue2
)
end
it
'returns issues that do not have ANY ONE of the labels provided'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue4
,
issue5
)
end
end
it
'returns the unique issues with all those labels'
do
expect
(
issues
).
to
contain_exactly
(
issue2
)
end
context
'
filtering by no label
'
do
let
(
:params
)
{
{
label_name:
described_class
::
Params
::
FILTER_NONE
}
}
context
'
using NOT
'
do
let
(
:params
)
{
{
not:
{
label_name:
[
label
.
title
,
label2
.
title
].
join
(
','
)
}
}
}
it
'returns issues
with no labels
'
do
it
'returns issues
that do not have ANY ONE of the labels provided
'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue4
,
issue5
)
end
end
end
context
'filtering by any label'
do
let
(
:params
)
{
{
label_name:
described_class
::
Params
::
FILTER_ANY
}
}
it
'returns issues that have one or more label'
do
create_list
(
:label_link
,
2
,
label:
create
(
:label
,
project:
project2
),
target:
issue3
)
context
'filtering by no label'
do
let
(
:params
)
{
{
label_name:
described_class
::
Params
::
FILTER_NONE
}
}
expect
(
issues
).
to
contain_exactly
(
issue2
,
issue3
)
e
nd
it
'returns issues with no labels'
do
e
xpect
(
issues
).
to
contain_exactly
(
issue1
,
issue4
,
issue5
)
end
end
context
'when the same label exists on project and group levels'
do
let
(
:issue1
)
{
create
(
:issue
,
project:
project1
)
}
let
(
:issue2
)
{
create
(
:issue
,
project:
project1
)
}
# Skipping validation to reproduce a "real-word" scenario.
# We still have legacy labels on PRD that have the same title on the group and project levels, example: `bug`
let
(
:project_label
)
{
build
(
:label
,
title:
'somelabel'
,
project:
project1
).
tap
{
|
r
|
r
.
save!
(
validate:
false
)
}
}
let
(
:group_label
)
{
create
(
:group_label
,
title:
'somelabel'
,
group:
project1
.
group
)
}
context
'filtering by any label'
do
let
(
:params
)
{
{
label_name:
described_class
::
Params
::
FILTER_ANY
}
}
let
(
:params
)
{
{
label_name:
'somelabel'
}
}
it
'returns issues that have one or more label'
do
create_list
(
:label_link
,
2
,
label:
create
(
:label
,
project:
project2
),
target:
issue3
)
before
do
create
(
:label_link
,
label:
group_label
,
target:
issue1
)
create
(
:label_link
,
label:
project_label
,
target:
issue2
)
end
it
'finds both issue records'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue2
)
end
expect
(
issues
).
to
contain_exactly
(
issue2
,
issue3
)
end
end
context
'when `optimized_issuable_label_filter` feature flag is off'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
false
)
end
context
'when the same label exists on project and group levels'
do
let
(
:issue1
)
{
create
(
:issue
,
project:
project1
)
}
let
(
:issue2
)
{
create
(
:issue
,
project:
project1
)
}
it_behaves_like
':label_name parameter'
end
# Skipping validation to reproduce a "real-word" scenario.
# We still have legacy labels on PRD that have the same title on the group and project levels, example: `bug`
let
(
:project_label
)
{
build
(
:label
,
title:
'somelabel'
,
project:
project1
).
tap
{
|
r
|
r
.
save!
(
validate:
false
)
}
}
let
(
:group_label
)
{
create
(
:group_label
,
title:
'somelabel'
,
group:
project1
.
group
)
}
let
(
:params
)
{
{
label_name:
'somelabel'
}
}
context
'when `optimized_issuable_label_filter` feature flag is on'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
true
)
create
(
:label_link
,
label:
group_label
,
target:
issue1
)
create
(
:label_link
,
label:
project_label
,
target:
issue2
)
end
it_behaves_like
':label_name parameter'
it
'finds both issue records'
do
expect
(
issues
).
to
contain_exactly
(
issue1
,
issue2
)
end
end
context
'filtering by issue term'
do
...
...
spec/finders/merge_requests_finder_spec.rb
View file @
7f1c9cc7
...
...
@@ -227,56 +227,38 @@ RSpec.describe MergeRequestsFinder do
end
end
shared_examples
':label_name parameter'
do
describe
':label_name parameter'
do
let
(
:common_labels
)
{
create_list
(
:label
,
3
)
}
let
(
:distinct_labels
)
{
create_list
(
:label
,
3
)
}
let
(
:merge_requests
)
do
common_attrs
=
{
source_project:
project1
,
target_project:
project1
,
author:
user
}
distinct_labels
.
map
do
|
label
|
labels
=
[
label
,
*
common_labels
]
create
(
:labeled_merge_request
,
:closed
,
labels:
labels
,
**
common_attrs
)
end
end
def
find
(
label_name
)
described_class
.
new
(
user
,
label_name:
label_name
).
execute
end
it
'accepts a single label'
do
found
=
find
(
distinct_labels
.
first
.
title
)
common
=
find
(
common_labels
.
first
.
title
)
expect
(
found
).
to
contain_exactly
(
merge_requests
.
first
)
expect
(
common
).
to
match_array
(
merge_requests
)
end
it
'accepts an array of labels, all of which must match'
do
all_distinct
=
find
(
distinct_labels
.
pluck
(
:title
))
all_common
=
find
(
common_labels
.
pluck
(
:title
))
expect
(
all_distinct
).
to
be_empty
expect
(
all_common
).
to
match_array
(
merge_requests
)
describe
':label_name parameter'
do
let
(
:common_labels
)
{
create_list
(
:label
,
3
)
}
let
(
:distinct_labels
)
{
create_list
(
:label
,
3
)
}
let
(
:merge_requests
)
do
common_attrs
=
{
source_project:
project1
,
target_project:
project1
,
author:
user
}
distinct_labels
.
map
do
|
label
|
labels
=
[
label
,
*
common_labels
]
create
(
:labeled_merge_request
,
:closed
,
labels:
labels
,
**
common_attrs
)
end
end
end
context
'when `optimized_issuable_label_filter` feature flag is off'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
false
)
def
find
(
label_name
)
described_class
.
new
(
user
,
label_name:
label_name
).
execute
end
it_behaves_like
':label_name parameter'
end
it
'accepts a single label'
do
found
=
find
(
distinct_labels
.
first
.
title
)
common
=
find
(
common_labels
.
first
.
title
)
context
'when `optimized_issuable_label_filter` feature flag is on'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
true
)
expect
(
found
).
to
contain_exactly
(
merge_requests
.
first
)
expect
(
common
).
to
match_array
(
merge_requests
)
end
it_behaves_like
':label_name parameter'
it
'accepts an array of labels, all of which must match'
do
all_distinct
=
find
(
distinct_labels
.
pluck
(
:title
))
all_common
=
find
(
common_labels
.
pluck
(
:title
))
expect
(
all_distinct
).
to
be_empty
expect
(
all_common
).
to
match_array
(
merge_requests
)
end
end
it
'filters by source project id'
do
...
...
spec/graphql/resolvers/merge_requests_resolver_spec.rb
View file @
7f1c9cc7
...
...
@@ -294,16 +294,6 @@ RSpec.describe Resolvers::MergeRequestsResolver do
nils_last
(
mr
.
metrics
.
merged_at
)
end
context
'when label filter is given and the optimized_issuable_label_filter feature flag is off'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
false
)
end
it
'does not raise PG::GroupingError'
do
expect
{
resolve_mr
(
project
,
sort: :merged_at_desc
,
labels:
%w[a b]
)
}.
not_to
raise_error
end
end
context
'when sorting by closed at'
do
before
do
merge_request_1
.
metrics
.
update!
(
latest_closed_at:
10
.
days
.
ago
)
...
...
spec/requests/api/issues/get_group_issues_spec.rb
View file @
7f1c9cc7
...
...
@@ -402,14 +402,7 @@ RSpec.describe API::Issues do
expect_paginated_array_response
([
group_closed_issue
.
id
,
group_issue
.
id
])
end
shared_examples
'labels parameter'
do
it
'returns an array of labeled group issues'
do
get
api
(
base_url
,
user
),
params:
{
labels:
group_label
.
title
}
expect_paginated_array_response
(
group_issue
.
id
)
expect
(
json_response
.
first
[
'labels'
]).
to
eq
([
group_label
.
title
])
end
context
'labels parameter'
do
it
'returns an array of labeled group issues'
do
get
api
(
base_url
,
user
),
params:
{
labels:
group_label
.
title
}
...
...
@@ -458,22 +451,6 @@ RSpec.describe API::Issues do
end
end
context
'when `optimized_issuable_label_filter` feature flag is off'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
false
)
end
it_behaves_like
'labels parameter'
end
context
'when `optimized_issuable_label_filter` feature flag is on'
do
before
do
stub_feature_flags
(
optimized_issuable_label_filter:
true
)
end
it_behaves_like
'labels parameter'
end
it
'returns issues matching given search string for title'
do
get
api
(
base_url
,
user
),
params:
{
search:
group_issue
.
title
}
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment