diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 0380bc1c5480a0e551c36541e9c87655cf7f316d..ab1b0c69c46771d468f46a5416a6327f704e0b46 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -35,6 +35,8 @@ def show return unless search_term_valid? + return if check_single_commit_result? + @search_term = params[:search] @scope = search_service.scope @@ -47,8 +49,6 @@ def show eager_load_user_status if @scope == 'users' increment_search_counters - - check_single_commit_result end def count @@ -103,14 +103,23 @@ def eager_load_user_status @search_objects = @search_objects.eager_load(:status) # rubocop:disable CodeReuse/ActiveRecord end - def check_single_commit_result - if @search_results.single_commit_result? - only_commit = @search_results.objects('commits').first - query = params[:search].strip.downcase - found_by_commit_sha = Commit.valid_hash?(query) && only_commit.sha.start_with?(query) + def check_single_commit_result? + return false if params[:force_search_results] + return false unless @project.present? + # download_code project policy grants user the read_commit ability + return false unless Ability.allowed?(current_user, :download_code, @project) - redirect_to project_commit_path(@project, only_commit) if found_by_commit_sha - end + query = params[:search].strip.downcase + 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: "".html_safe, a_end: ''.html_safe } + redirect_to project_commit_path(@project, commit) + + true end def increment_search_counters diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 63601485daf59af571f5bdcde684f2267bfe0088..3fd2c30a8897d562a7848fe6951e250312160475 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -1,7 +1,18 @@ # frozen_string_literal: true 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) return unless current_user diff --git a/doc/user/search/img/project_search_sha_redirect.png b/doc/user/search/img/project_search_sha_redirect.png new file mode 100644 index 0000000000000000000000000000000000000000..718a859d4af3650ffece27a76c69a3c2ec6e3e08 Binary files /dev/null and b/doc/user/search/img/project_search_sha_redirect.png differ diff --git a/doc/user/search/index.md b/doc/user/search/index.md index f52106e4fa8876bbf9febc7a92b72bb3b1747ca2..85be047cd9f079904a26f0b9ba127d88d9bd6069 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -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 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)** Leverage Elasticsearch for faster, more advanced code search across your entire diff --git a/ee/changelogs/unreleased/233278-in-global-search-if-there-is-only-one-result-redirect-to-that-p.yml b/ee/changelogs/unreleased/233278-in-global-search-if-there-is-only-one-result-redirect-to-that-p.yml new file mode 100644 index 0000000000000000000000000000000000000000..84e0e45e8e35c2f27870be7143687f6ca95a5e62 --- /dev/null +++ b/ee/changelogs/unreleased/233278-in-global-search-if-there-is-only-one-result-redirect-to-that-p.yml @@ -0,0 +1,5 @@ +--- +title: Enable single result redirect for advanced commits search +merge_request: 44308 +author: +type: changed diff --git a/ee/lib/gitlab/elastic/search_results.rb b/ee/lib/gitlab/elastic/search_results.rb index 67d21b60ff65fb4127c8f386bb27b05f7bc5b30d..cac06058507cc0ebf914e6d9f8e3165ffa39a87a 100644 --- a/ee/lib/gitlab/elastic/search_results.rb +++ b/ee/lib/gitlab/elastic/search_results.rb @@ -130,10 +130,6 @@ def milestones_count alias_method :limited_merge_requests_count, :merge_requests_count alias_method :limited_milestones_count, :milestones_count - def single_commit_result? - false - end - def self.parse_search_result(result, project) ref = result["_source"]["blob"]["commit_sha"] path = result["_source"]["blob"]["path"] diff --git a/ee/spec/requests/search_controller_spec.rb b/ee/spec/requests/search_controller_spec.rb index 61d69fa12913884eeedd0e459333bd2166f90443..c467a76b172f483b80cf35d2ff9dc71e9a7df873 100644 --- a/ee/spec/requests/search_controller_spec.rb +++ b/ee/spec/requests/search_controller_spec.rb @@ -7,7 +7,7 @@ let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) } - before_all do + before do login_as(user) end diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 333564bee018dc604ac84a1bce09c234b49778bf..fd08b560e18ac5a2085e5d6e1b33d441b57a4f93 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -75,15 +75,6 @@ def commits_count @commits_count ||= commits(limit: count_limit).count 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 def paginated_commits(page, per_page) diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 3d4920456e2d6f104baa091aebb8e7029b5f6f01..f05bc9c8619e6acfb496b3160e24963faef90701 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -94,10 +94,6 @@ def limited_users_count @limited_users_count ||= limited_count(users) end - def single_commit_result? - false - end - def count_limit COUNT_LIMIT end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3a790d87539456092e48492a2572c9d5d6dbc803..3238bb69573a767ed830651f33b55d38a41c0fff 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,6 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2020-10-12 08:34+0200\n" -"PO-Revision-Date: 2020-10-12 08:34+0200\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" "Language: \n" @@ -29751,6 +29749,9 @@ msgstr "" msgid "You have been invited" 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." msgstr "" diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index 52bfc4803130a97f76f9b0fed359a975029f0891..700cdd96ba865755cff7e8a4298803e4cd7d7057 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -7,7 +7,7 @@ let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) } - before_all do + before do login_as(user) end @@ -31,9 +31,11 @@ def send_search_request(params) context 'for issues scope' do let(:object) { :issue } - let(:creation_args) { { project: project } } - let(:params) { { search: '*', scope: 'issues' } } - let(:threshold) { 0 } + let(:creation_args) { { project: project, title: 'foo' } } + let(:params) { { search: 'foo', scope: 'issues' } } + # 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' end @@ -41,9 +43,16 @@ def send_search_request(params) context 'for merge_request scope' do let(:creation_traits) { [:unique_branches] } let(:object) { :merge_request } - let(:creation_args) { { source_project: project } } - let(:params) { { search: '*', scope: 'merge_requests' } } - let(:threshold) { 0 } + let(:creation_args) { { source_project: project, title: 'bar' } } + let(:params) { { search: 'bar', scope: 'merge_requests' } } + # 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' end @@ -51,16 +60,74 @@ def send_search_request(params) context 'for project scope' do let(:creation_traits) { [:public] } let(:object) { :project } - let(:creation_args) { {} } - let(:params) { { search: '*', scope: 'projects' } } + let(:creation_args) { { name: 'project' } } + let(:params) { { search: 'project', scope: 'projects' } } # some N+1 queries still exist # each project requires 3 extra queries # - one count for forks # - one count for open MRs # - 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' 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 diff --git a/spec/support/helpers/search_helpers.rb b/spec/support/helpers/search_helpers.rb index 328f272724a261872138b91661727560b2c39313..57e605653a264acd2f764eae4d48412e4b89d177 100644 --- a/spec/support/helpers/search_helpers.rb +++ b/spec/support/helpers/search_helpers.rb @@ -9,7 +9,7 @@ def fill_in_search(text) wait_for_all_requests end - def submit_search(query, scope: nil) + def submit_search(query) page.within('.search-form, .search-page-form') do field = find_field('search') field.fill_in(with: query)