Commit 30415104 authored by Dylan Griffith's avatar Dylan Griffith Committed by Heinrich Lee Yu

Make searching issues/MRs by IID even easier

Prior to this change you were required to prepend your search with `#`
or `!` (like an Issue/MR link) to trigger the search by IID behaviour.
This changes things so that it's even simpler to search by IID. As long
as the input is just a number we'll ensure that the IID field is
included in search query. This also won't prevent you from searching for
the number as part of an issue title or description since we also query
those fields but we give higher weight to the IID so exact IID matches
should appear as the first result.

This MR also includes some refactoring to improve the tests.
Specifically some of the tests for searching by IID were passing by
accident before. It also refactors them to use `contain_exactly` since
this makes them more concise.
parent f9087286
---
title: Make searching issues/MRs by IID even easier
merge_request: 40467
author:
type: added
...@@ -8,7 +8,14 @@ module Elastic ...@@ -8,7 +8,14 @@ module Elastic
if query =~ /#(\d+)\z/ if query =~ /#(\d+)\z/
iid_query_hash(Regexp.last_match(1)) iid_query_hash(Regexp.last_match(1))
else else
basic_query_hash(%w(title^2 description), query) fields = %w(title^2 description)
# We can only allow searching the iid field if the query is
# just a number, otherwise Elasticsearch will error since this
# field is type integer.
fields << "iid^3" if query =~ /\A\d+\z/
basic_query_hash(fields, query)
end end
options[:features] = 'issues' options[:features] = 'issues'
......
...@@ -8,7 +8,14 @@ module Elastic ...@@ -8,7 +8,14 @@ module Elastic
if query =~ /\!(\d+)\z/ if query =~ /\!(\d+)\z/
iid_query_hash(Regexp.last_match(1)) iid_query_hash(Regexp.last_match(1))
else else
basic_query_hash(%w(title^2 description), query) fields = %w(title^2 description)
# We can only allow searching the iid field if the query is
# just a number, otherwise Elasticsearch will error since this
# field is type integer.
fields << "iid^3" if query =~ /\A\d+\z/
basic_query_hash(fields, query)
end end
options[:features] = 'merge_requests' options[:features] = 'merge_requests'
......
...@@ -118,14 +118,14 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -118,14 +118,14 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
@issue_2 = create( @issue_2 = create(
:issue, :issue,
project: project_1, project: project_1,
title: 'Issue 2', title: 'Issue Two',
description: 'Hello world, here I am!', description: 'Hello world, here I am!',
iid: 2 iid: 2
) )
@issue_3 = create( @issue_3 = create(
:issue, :issue,
project: project_2, project: project_2,
title: 'Issue 3', title: 'Issue Three',
iid: 2 iid: 2
) )
...@@ -138,9 +138,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -138,9 +138,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
results = described_class.new(user, 'hello world', limit_projects) results = described_class.new(user, 'hello world', limit_projects)
issues = results.objects('issues') issues = results.objects('issues')
expect(issues).to include @issue_1 expect(issues).to contain_exactly(@issue_1, @issue_2)
expect(issues).to include @issue_2
expect(issues).not_to include @issue_3
expect(results.issues_count).to eq 2 expect(results.issues_count).to eq 2
end end
...@@ -155,9 +153,15 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -155,9 +153,15 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
results = described_class.new(user, '#2', limit_projects, public_and_internal_projects: false) results = described_class.new(user, '#2', limit_projects, public_and_internal_projects: false)
issues = results.objects('issues') issues = results.objects('issues')
expect(issues).not_to include @issue_1 expect(issues).to contain_exactly(@issue_2)
expect(issues).to include @issue_2 expect(results.issues_count).to eq 1
expect(issues).not_to include @issue_3 end
it 'can also find an issue by iid without the prefixed #' do
results = described_class.new(user, '2', limit_projects, public_and_internal_projects: false)
issues = results.objects('issues')
expect(issues).to contain_exactly(@issue_2)
expect(results.issues_count).to eq 1 expect(results.issues_count).to eq 1
end end
...@@ -421,7 +425,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -421,7 +425,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
:conflict, :conflict,
source_project: project_1, source_project: project_1,
target_project: project_1, target_project: project_1,
title: 'Merge Request 2', title: 'Merge Request Two',
description: 'Hello world, here I am!', description: 'Hello world, here I am!',
iid: 2 iid: 2
) )
...@@ -429,7 +433,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -429,7 +433,7 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
:merge_request, :merge_request,
source_project: project_2, source_project: project_2,
target_project: project_2, target_project: project_2,
title: 'Merge Request 3', title: 'Merge Request Three',
iid: 2 iid: 2
) )
...@@ -439,12 +443,10 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -439,12 +443,10 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
it_behaves_like 'a paginated object', 'merge_requests' it_behaves_like 'a paginated object', 'merge_requests'
it 'lists found merge requests' do it 'lists found merge requests' do
results = described_class.new(user, 'hello world', limit_projects) results = described_class.new(user, 'hello world', limit_projects, public_and_internal_projects: false)
merge_requests = results.objects('merge_requests') merge_requests = results.objects('merge_requests')
expect(merge_requests).to include @merge_request_1 expect(merge_requests).to contain_exactly(@merge_request_1, @merge_request_2)
expect(merge_requests).to include @merge_request_2
expect(merge_requests).not_to include @merge_request_3
expect(results.merge_requests_count).to eq 2 expect(results.merge_requests_count).to eq 2
end end
...@@ -456,12 +458,18 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need ...@@ -456,12 +458,18 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
end end
it 'lists merge request when search by a valid iid' do it 'lists merge request when search by a valid iid' do
results = described_class.new(user, '#2', limit_projects) results = described_class.new(user, '!2', limit_projects, public_and_internal_projects: false)
merge_requests = results.objects('merge_requests')
expect(merge_requests).to contain_exactly(@merge_request_2)
expect(results.merge_requests_count).to eq 1
end
it 'can also find an issue by iid without the prefixed !' do
results = described_class.new(user, '2', limit_projects, public_and_internal_projects: false)
merge_requests = results.objects('merge_requests') merge_requests = results.objects('merge_requests')
expect(merge_requests).not_to include @merge_request_1 expect(merge_requests).to contain_exactly(@merge_request_2)
expect(merge_requests).to include @merge_request_2
expect(merge_requests).not_to include @merge_request_3
expect(results.merge_requests_count).to eq 1 expect(results.merge_requests_count).to eq 1
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment