Commit f245629d authored by Terri Chu's avatar Terri Chu Committed by Dylan Griffith

Enable single result redirect for advanced commits search

If a user searches for a commit in a project and only finds
one result, redirect the user to that result. This already
exists for basic search and now is enabled for advanced
search. Added a notice explaining the redirect with a
link back to search results.
parent 3bef636a
...@@ -35,6 +35,8 @@ class SearchController < ApplicationController ...@@ -35,6 +35,8 @@ class SearchController < ApplicationController
return unless search_term_valid? return unless search_term_valid?
return if check_single_commit_result?
@search_term = params[:search] @search_term = params[:search]
@scope = search_service.scope @scope = search_service.scope
...@@ -47,8 +49,6 @@ class SearchController < ApplicationController ...@@ -47,8 +49,6 @@ class SearchController < ApplicationController
eager_load_user_status if @scope == 'users' eager_load_user_status if @scope == 'users'
increment_search_counters increment_search_counters
check_single_commit_result
end end
def count def count
...@@ -103,14 +103,23 @@ class SearchController < ApplicationController ...@@ -103,14 +103,23 @@ class SearchController < ApplicationController
@search_objects = @search_objects.eager_load(:status) # rubocop:disable CodeReuse/ActiveRecord @search_objects = @search_objects.eager_load(:status) # rubocop:disable CodeReuse/ActiveRecord
end end
def check_single_commit_result def check_single_commit_result?
if @search_results.single_commit_result? return false if params[:force_search_results]
only_commit = @search_results.objects('commits').first return false unless @project.present?
query = params[:search].strip.downcase # download_code project policy grants user the read_commit ability
found_by_commit_sha = Commit.valid_hash?(query) && only_commit.sha.start_with?(query) return false unless Ability.allowed?(current_user, :download_code, @project)
redirect_to project_commit_path(@project, only_commit) if found_by_commit_sha query = params[:search].strip.downcase
end return false unless Commit.valid_hash?(query)
commit = @project.commit_by(oid: query)
return false unless commit.present?
link = search_path(safe_params.merge(force_search_results: true))
flash[:notice] = html_escape(_("You have been redirected to the only result; see the %{a_start}search results%{a_end} instead.")) % { a_start: "<a href=\"#{link}\"><u>".html_safe, a_end: '</u></a>'.html_safe }
redirect_to project_commit_path(@project, commit)
true
end end
def increment_search_counters def increment_search_counters
......
# frozen_string_literal: true # frozen_string_literal: true
module SearchHelper module SearchHelper
SEARCH_PERMITTED_PARAMS = [:search, :scope, :project_id, :group_id, :repository_ref, :snippets, :sort, :state, :confidential].freeze SEARCH_PERMITTED_PARAMS = [
:search,
:scope,
:project_id,
:group_id,
:repository_ref,
:snippets,
:sort,
:state,
:confidential,
:force_search_results
].freeze
def search_autocomplete_opts(term) def search_autocomplete_opts(term)
return unless current_user return unless current_user
......
...@@ -248,6 +248,14 @@ the search field on the top-right of your screen while the project page is open. ...@@ -248,6 +248,14 @@ the search field on the top-right of your screen while the project page is open.
![code search dropdown](img/project_search_dropdown.png) ![code search dropdown](img/project_search_dropdown.png)
![code search results](img/project_code_search.png) ![code search results](img/project_code_search.png)
### SHA search
You can quickly access a commit from within the project dashboard by entering the SHA
into the search field on the top right of the screen. If a single result is found, you will be
redirected to the commit result and given the option to return to the search results page.
![project sha search redirect](img/project_search_sha_redirect.png)
## Advanced Search **(STARTER)** ## Advanced Search **(STARTER)**
Leverage Elasticsearch for faster, more advanced code search across your entire Leverage Elasticsearch for faster, more advanced code search across your entire
......
---
title: Enable single result redirect for advanced commits search
merge_request: 44308
author:
type: changed
...@@ -130,10 +130,6 @@ module Gitlab ...@@ -130,10 +130,6 @@ module Gitlab
alias_method :limited_merge_requests_count, :merge_requests_count alias_method :limited_merge_requests_count, :merge_requests_count
alias_method :limited_milestones_count, :milestones_count alias_method :limited_milestones_count, :milestones_count
def single_commit_result?
false
end
def self.parse_search_result(result, project) def self.parse_search_result(result, project)
ref = result["_source"]["blob"]["commit_sha"] ref = result["_source"]["blob"]["commit_sha"]
path = result["_source"]["blob"]["path"] path = result["_source"]["blob"]["path"]
......
...@@ -7,7 +7,7 @@ RSpec.describe SearchController, type: :request do ...@@ -7,7 +7,7 @@ RSpec.describe SearchController, type: :request do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) } let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) }
before_all do before do
login_as(user) login_as(user)
end end
......
...@@ -75,15 +75,6 @@ module Gitlab ...@@ -75,15 +75,6 @@ module Gitlab
@commits_count ||= commits(limit: count_limit).count @commits_count ||= commits(limit: count_limit).count
end end
def single_commit_result?
return false if commits_count != 1
counts = %i(limited_milestones_count limited_notes_count
limited_merge_requests_count limited_issues_count
limited_blobs_count wiki_blobs_count)
counts.all? { |count_method| public_send(count_method) == 0 } # rubocop:disable GitlabSecurity/PublicSend
end
private private
def paginated_commits(page, per_page) def paginated_commits(page, per_page)
......
...@@ -94,10 +94,6 @@ module Gitlab ...@@ -94,10 +94,6 @@ module Gitlab
@limited_users_count ||= limited_count(users) @limited_users_count ||= limited_count(users)
end end
def single_commit_result?
false
end
def count_limit def count_limit
COUNT_LIMIT COUNT_LIMIT
end end
......
...@@ -30282,6 +30282,9 @@ msgstr "" ...@@ -30282,6 +30282,9 @@ msgstr ""
msgid "You have been invited" msgid "You have been invited"
msgstr "" msgstr ""
msgid "You have been redirected to the only result; see the %{a_start}search results%{a_end} instead."
msgstr ""
msgid "You have been unsubscribed from this thread." msgid "You have been unsubscribed from this thread."
msgstr "" msgstr ""
......
...@@ -7,7 +7,7 @@ RSpec.describe SearchController, type: :request do ...@@ -7,7 +7,7 @@ RSpec.describe SearchController, type: :request do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) } let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) }
before_all do before do
login_as(user) login_as(user)
end end
...@@ -31,9 +31,11 @@ RSpec.describe SearchController, type: :request do ...@@ -31,9 +31,11 @@ RSpec.describe SearchController, type: :request do
context 'for issues scope' do context 'for issues scope' do
let(:object) { :issue } let(:object) { :issue }
let(:creation_args) { { project: project } } let(:creation_args) { { project: project, title: 'foo' } }
let(:params) { { search: '*', scope: 'issues' } } let(:params) { { search: 'foo', scope: 'issues' } }
let(:threshold) { 0 } # there are 4 additional queries run for the logged in user:
# (1) geo_nodes, (1) users, (2) broadcast_messages
let(:threshold) { 4 }
it_behaves_like 'an efficient database result' it_behaves_like 'an efficient database result'
end end
...@@ -41,9 +43,16 @@ RSpec.describe SearchController, type: :request do ...@@ -41,9 +43,16 @@ RSpec.describe SearchController, type: :request do
context 'for merge_request scope' do context 'for merge_request scope' do
let(:creation_traits) { [:unique_branches] } let(:creation_traits) { [:unique_branches] }
let(:object) { :merge_request } let(:object) { :merge_request }
let(:creation_args) { { source_project: project } } let(:creation_args) { { source_project: project, title: 'bar' } }
let(:params) { { search: '*', scope: 'merge_requests' } } let(:params) { { search: 'bar', scope: 'merge_requests' } }
let(:threshold) { 0 } # some N+1 queries exist
# each merge request require 4 extra queries for:
# - one for projects
# - one for namespaces
# - two for routes
# plus 4 additional queries run for the logged in user:
# - (1) geo_nodes, (1) users, (2) broadcast_messages
let(:threshold) { 16 }
it_behaves_like 'an efficient database result' it_behaves_like 'an efficient database result'
end end
...@@ -51,16 +60,74 @@ RSpec.describe SearchController, type: :request do ...@@ -51,16 +60,74 @@ RSpec.describe SearchController, type: :request do
context 'for project scope' do context 'for project scope' do
let(:creation_traits) { [:public] } let(:creation_traits) { [:public] }
let(:object) { :project } let(:object) { :project }
let(:creation_args) { {} } let(:creation_args) { { name: 'project' } }
let(:params) { { search: '*', scope: 'projects' } } let(:params) { { search: 'project', scope: 'projects' } }
# some N+1 queries still exist # some N+1 queries still exist
# each project requires 3 extra queries # each project requires 3 extra queries
# - one count for forks # - one count for forks
# - one count for open MRs # - one count for open MRs
# - one count for open Issues # - one count for open Issues
let(:threshold) { 9 } # there are 4 additional queries run for the logged in user:
# (1) geo_nodes, (1) users, (2) broadcast_messages
let(:threshold) { 13 }
it_behaves_like 'an efficient database result' it_behaves_like 'an efficient database result'
end end
context 'when searching by SHA' do
let(:sha) { '6d394385cf567f80a8fd85055db1ab4c5295806f' }
it 'finds a commit and redirects to its page' do
send_search_request({ search: sha, scope: 'projects', project_id: project.id })
expect(response).to redirect_to(project_commit_path(project, sha))
end
it 'finds a commit in uppercase and redirects to its page' do
send_search_request( { search: sha.upcase, scope: 'projects', project_id: project.id })
expect(response).to redirect_to(project_commit_path(project, sha))
end
it 'finds a commit with a partial sha and redirects to its page' do
send_search_request( { search: sha[0..10], scope: 'projects', project_id: project.id })
expect(response).to redirect_to(project_commit_path(project, sha))
end
it 'redirects to the commit even if another scope result is returned' do
create(:note, project: project, note: "This is the #{sha}")
send_search_request( { search: sha, scope: 'projects', project_id: project.id })
expect(response).to redirect_to(project_commit_path(project, sha))
end
it 'goes to search results with the force_search_results param set' do
send_search_request({ search: sha, force_search_results: true, project_id: project.id })
expect(response).not_to redirect_to(project_commit_path(project, sha))
end
it 'does not redirect if user cannot download_code from project' do
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(user, :download_code, project).and_return(false)
send_search_request({ search: sha, project_id: project.id })
expect(response).not_to redirect_to(project_commit_path(project, sha))
end
it 'does not redirect if commit sha not found in project' do
send_search_request({ search: '23594bc765e25c5b22c17a8cca25ebd50f792598', project_id: project.id })
expect(response).not_to redirect_to(project_commit_path(project, sha))
end
it 'does not redirect if not using project scope' do
send_search_request({ search: sha, group_id: project.root_namespace.id })
expect(response).not_to redirect_to(project_commit_path(project, sha))
end
end
end end
end end
...@@ -9,7 +9,7 @@ module SearchHelpers ...@@ -9,7 +9,7 @@ module SearchHelpers
wait_for_all_requests wait_for_all_requests
end end
def submit_search(query, scope: nil) def submit_search(query)
page.within('.search-form, .search-page-form') do page.within('.search-form, .search-page-form') do
field = find_field('search') field = find_field('search')
field.fill_in(with: query) field.fill_in(with: query)
......
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