Skip to content
Snippets Groups Projects
Verified Commit 511fa306 authored by Terri Chu's avatar Terri Chu :nail_care: Committed by GitLab
Browse files

Fix bug with label_name search filter

The fix refactors the filter to take search level
into account (global, group, or project)

Changelog: fixed
EE: true
parent 4c418997
No related branches found
No related tags found
1 merge request!163353Fix bug with label_name search filter
......@@ -602,13 +602,28 @@ def filter_ids_by_feature(project_ids, user, feature)
end
def find_labels_by_name(names, options)
raise ArgumentError, 'search_scope is a required option' unless options.key?(:search_scope)
return [] unless names.any?
search_level = options[:search_scope].to_sym
project_ids = options[:project_ids]
group_ids = options[:group_ids]
finder_params = { name: names }
finder_params[:project_ids] = project_ids if project_ids&.any? && project_ids != :any
finder_params[:group_id] = group_ids.first if group_ids&.any?
case search_level
when :global
# no-op
# no group or project filtering applied
when :group
finder_params[:group_id] = group_ids.first if group_ids&.any?
when :project
finder_params[:group_id] = group_ids.first if group_ids&.any?
finder_params[:project_ids] = project_ids if project_ids != :any && project_ids&.any?
else
raise ArgumentError, 'Invalid search_scope option provided'
end
labels = LabelsFinder.new(nil, finder_params).execute(skip_authorization: true)
labels.each_with_object(Hash.new { |h, k| h[k] = [] }) do |label, hash|
......
......@@ -101,11 +101,11 @@
describe 'named queries' do
using RSpec::Parameterized::TableSyntax
where(:projects, :groups) do
[] | []
[ref(:project)] | []
[] | [ref(:group)]
[ref(:project)] | [ref(:group)]
where(:search_level, :projects, :groups) do
:global | [] | []
:project | [ref(:project)] | []
:group | [] | [ref(:group)]
:project | [ref(:project)] | [ref(:group)]
end
with_them do
......@@ -116,6 +116,7 @@
let(:base_options) do
{
current_user: user,
search_scope: search_level,
project_ids: project_ids,
group_ids: group_ids,
public_and_internal_projects: false,
......
......@@ -362,6 +362,8 @@
let_it_be(:project) { create(:project, group: group) }
# project label must be defined first or the title cannot match
let_it_be(:project_label) { create(:label, project: project, title: label_title) }
let_it_be(:project_2) { create(:project, group: group) }
let_it_be(:project_label_2) { create(:label, project: project_2, title: label_title) }
let_it_be(:group_label) { create(:group_label, group: group, title: label_title) }
subject(:by_label_ids) { described_class.by_label_ids(query_hash: query_hash, options: options) }
......@@ -397,7 +399,7 @@
end
context 'when options[:aggregation] is true' do
let(:options) { { labels: [project_label.id], aggregation: true } }
let(:options) { { labels: [project.id], aggregation: true } }
it_behaves_like 'does not modify the query_hash'
end
......@@ -410,47 +412,163 @@
end
context 'when options[:label_name] is provided' do
let(:options) { { label_name: [label_title] } }
let(:label_name) { [label_title] }
let(:aggregations) { false }
let(:count_only) { false }
let(:group_ids) { nil }
let(:project_ids) { nil }
let(:options) do
{
label_name: label_name, search_scope: search_level, count_only: count_only, aggregations: aggregations,
group_ids: group_ids, project_ids: project_ids
}
end
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id)
}
}]
}
]
context 'when search_level invalid' do
let(:search_level) { :not_supported }
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
it 'raises an exception' do
expect { by_label_ids }.to raise_exception(ArgumentError)
end
end
context 'when multiple label names are provided' do
let_it_be(:another_label) { create(:label, project: project, title: 'Another label') }
let(:options) { { label_name: [label_title, another_label.title] } }
context 'when search_level is not provided' do
let(:options) { { label_name: label_name } }
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: contain_exactly(
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id)
it 'raises an exception' do
expect { by_label_ids }.to raise_exception(ArgumentError)
end
end
context 'for global search' do
let(:search_level) { :global }
context 'when multiple label names are provided' do
let_it_be(:another_label) { create(:label, project: project, title: 'Another label') }
let(:label_name) { [label_title, another_label.title] }
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: contain_exactly(
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
},
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(another_label.id)
}
}
},
{
)
}
]
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
context 'when options[:group_ids] is provided' do
let(:group_ids) { [group.id] }
it 'adds the label_ids filter to query_hash with no group filtering' do
expected_filter = [
bool: {
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(another_label.id)
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
}]
}
]
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
context 'when options[:project_ids] is provided' do
let(:project_ids) { [project.id] }
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
}]
}
)
]
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
end
context 'when options[:project_ids] is provided' do
using RSpec::Parameterized::TableSyntax
let(:project_ids) { projects == :any ? projects : [projects.id] }
where(:projects) do
[:any, ref(:project), ref(:project_2)]
end
with_them do
it 'adds the label_ids filter to query_hash with no project filtering' do
expected_filter = [
bool: {
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
}]
}
]
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
end
context 'when options[:count_only] is true' do
let(:count_only) { true }
it_behaves_like 'does not modify the query_hash'
end
context 'when options[:aggregation] is true' do
let(:aggregation) { true }
it_behaves_like 'does not modify the query_hash'
end
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
}]
}
]
......@@ -461,8 +579,53 @@
end
end
context 'when options[:group_ids] is provided' do
let(:options) { { label_name: [label_title], group_ids: [group.id] } }
context 'for group search' do
let(:search_level) { :group }
let(:group_ids) { [group.id] }
let(:project_ids) { nil }
context 'when multiple label names are provided' do
let_it_be(:another_label) { create(:label, project: project, title: 'Another label') }
let(:label_name) { [label_title, another_label.title] }
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: contain_exactly(
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
},
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(another_label.id)
}
}
)
}
]
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
context 'when options[:count_only] is true' do
let(:count_only) { true }
it_behaves_like 'does not modify the query_hash'
end
context 'when options[:aggregation] is true' do
let(:aggregation) { true }
it_behaves_like 'does not modify the query_hash'
end
it 'adds the label_ids filter to query_hash' do
expected_filter = [
......@@ -470,7 +633,7 @@
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id)
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
}]
}
......@@ -481,13 +644,48 @@
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
context 'when options[:project_ids] is provided' do
let(:options) do
{ label_name: [label_title], group_id: [group.id], project_ids: [project.id] }
end
context 'for project search' do
let(:search_level) { :project }
let(:group_ids) { nil }
let(:project_ids) { [project.id] }
context 'when multiple label names are provided' do
let_it_be(:another_label) { create(:label, project: project, title: 'Another label') }
let(:label_name) { [label_title, another_label.title] }
it 'adds the label_ids filter to query_hash' do
expected_filter = [
bool: {
must: contain_exactly(
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id)
}
},
{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(another_label.id)
}
}
)
}
]
expect(by_label_ids.dig(:query, :bool, :filter)).to match(expected_filter)
expect(by_label_ids.dig(:query, :bool, :must)).to be_empty
expect(by_label_ids.dig(:query, :bool, :must_not)).to be_empty
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
context 'when options[:group_ids] is provided' do
let(:group_ids) { [group.id] }
it 'adds the label_ids filter to query_hash with no group filtering' do
expected_filter = [
bool: {
must: [{
......@@ -505,10 +703,18 @@
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
end
context 'when options[:project_ids] is provided' do
let(:options) { { label_name: [label_title], project_ids: [project.id] } }
context 'when options[:count_only] is true' do
let(:count_only) { true }
it_behaves_like 'does not modify the query_hash'
end
context 'when options[:aggregation] is true' do
let(:aggregation) { true }
it_behaves_like 'does not modify the query_hash'
end
it 'adds the label_ids filter to query_hash' do
expected_filter = [
......@@ -528,22 +734,10 @@
expect(by_label_ids.dig(:query, :bool, :should)).to be_empty
end
end
context 'when options[:count_only] is true' do
let(:options) { { label_name: [label_title], count_only: true } }
it_behaves_like 'does not modify the query_hash'
end
context 'when options[:aggregation] is true' do
let(:options) { { label_name: [label_title], aggregation: true } }
it_behaves_like 'does not modify the query_hash'
end
end
context 'when options[:labels] and options[:label_name] are provided' do
let(:options) { { labels: [project_label.id], label_name: [label_title] } }
let(:options) { { labels: [project_label.id], label_name: [label_title], search_scope: :global } }
it 'uses label_name option and adds the label_ids filter to query_hash' do
expected_filter = [
......@@ -551,7 +745,7 @@
must: [{
terms: {
_name: 'filters:label_ids',
label_ids: contain_exactly(group_label.id, project_label.id)
label_ids: contain_exactly(group_label.id, project_label.id, project_label_2.id)
}
}]
}
......
......@@ -7,6 +7,7 @@
let(:base_options) do
{
current_user: user,
search_scope: 'global',
project_ids: project_ids,
group_ids: [],
public_and_internal_projects: false
......
......@@ -7,6 +7,7 @@
let(:base_options) do
{
current_user: user,
search_scope: 'global',
project_ids: project_ids,
group_ids: [],
public_and_internal_projects: true
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment