Merge branch...

Merge branch '233278-in-global-search-if-there-is-only-one-result-redirect-to-that-page-instead-of-global-search' into 'master'

Advanced Search: redirect single sha result for project search

See merge request gitlab-org/gitlab!44308

app/controllers/search_controller.rb

@@ -35,6 +35,8 @@ class SearchController < ApplicationController
 
     return unless search_term_valid?
 
+    return if check_single_commit_result?
+
     @search_term = params[:search]
 
     @scope = search_service.scope
@@ -47,8 +49,6 @@ class SearchController < ApplicationController
     eager_load_user_status if @scope == 'users'
 
     increment_search_counters
-
-    check_single_commit_result
   end
 
   def count
@@ -103,14 +103,23 @@ class SearchController < ApplicationController
     @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
+  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)
+
     query = params[:search].strip.downcase
-      found_by_commit_sha = Commit.valid_hash?(query) && only_commit.sha.start_with?(query)
+    return false unless Commit.valid_hash?(query)
 
-      redirect_to project_commit_path(@project, only_commit) if found_by_commit_sha
-    end
+    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
 
   def increment_search_counters

app/helpers/search_helper.rb

 # 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

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

ee/changelogs/unreleased/233278-in-global-search-if-there-is-only-one-result-redirect-to-that-p.yml

+---
+title: Enable single result redirect for advanced commits search
+merge_request: 44308
+author:
+type: changed

ee/lib/gitlab/elastic/search_results.rb

@@ -130,10 +130,6 @@ module Gitlab
       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"]

ee/spec/requests/search_controller_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe SearchController, type: :request do
   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
 

lib/gitlab/project_search_results.rb

@@ -75,15 +75,6 @@ module Gitlab
       @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)

lib/gitlab/search_results.rb

@@ -94,10 +94,6 @@ module Gitlab
       @limited_users_count ||= limited_count(users)
     end
 
-    def single_commit_result?
-      false
-    end
-
     def count_limit
       COUNT_LIMIT
     end

locale/gitlab.pot

@@ -30303,6 +30303,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 ""
 

spec/requests/search_controller_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe SearchController, type: :request do
   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 @@ RSpec.describe SearchController, type: :request do
 
     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 @@ RSpec.describe SearchController, type: :request do
     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 @@ RSpec.describe SearchController, type: :request do
     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

spec/support/helpers/search_helpers.rb

@@ -9,7 +9,7 @@ module SearchHelpers
     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)