Commit 65bf7e0d authored by Sean McGivern's avatar Sean McGivern

Merge branch '24833-Allow-to-search-by-commit-hash-within-project' into 'master'

Allows to search within project by commit's hash #24833

Closes #24833

See merge request !8028
parents a89aab9c 99404a58
......@@ -45,6 +45,8 @@ class SearchController < ApplicationController
end
@search_objects = @search_results.objects(@scope, params[:page])
check_single_commit_result
end
def autocomplete
......@@ -59,4 +61,16 @@ class SearchController < ApplicationController
render json: search_autocomplete_opts(term).to_json
end
private
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)
redirect_to namespace_project_commit_path(@project.namespace, @project, only_commit) if found_by_commit_sha
end
end
end
......@@ -21,6 +21,9 @@ class Commit
DIFF_HARD_LIMIT_FILES = 1000
DIFF_HARD_LIMIT_LINES = 50000
# The SHA can be between 7 and 40 hex characters.
COMMIT_SHA_PATTERN = '\h{7,40}'
class << self
def decorate(commits, project)
commits.map do |commit|
......@@ -52,6 +55,10 @@ class Commit
def from_hash(hash, project)
new(Gitlab::Git::Commit.new(hash), project)
end
def valid_hash?(key)
!!(/\A#{COMMIT_SHA_PATTERN}\z/ =~ key)
end
end
attr_accessor :raw
......@@ -77,8 +84,6 @@ class Commit
# Pattern used to extract commit references from text
#
# The SHA can be between 7 and 40 hex characters.
#
# This pattern supports cross-project references.
def self.reference_pattern
@reference_pattern ||= %r{
......@@ -88,7 +93,7 @@ class Commit
end
def self.link_reference_pattern
@link_reference_pattern ||= super("commit", /(?<commit>\h{7,40})/)
@link_reference_pattern ||= super("commit", /(?<commit>#{COMMIT_SHA_PATTERN})/)
end
def to_reference(from_project = nil, full: false)
......
---
title: 'Allows to search within project by commit hash'
merge_request:
author: YarNayar
---
title: 'Search feature: redirects to commit page if query is commit sha and only commit
found'
merge_request: 8028
author: YarNayar
......@@ -71,6 +71,14 @@ module Gitlab
)
end
def single_commit_result?
commits_count == 1 && total_result_count == 1
end
def total_result_count
issues_count + merge_requests_count + milestones_count + notes_count + blobs_count + wiki_blobs_count + commits_count
end
private
def blobs
......@@ -114,7 +122,25 @@ module Gitlab
end
def commits
@commits ||= project.repository.find_commits_by_message(query)
@commits ||= find_commits(query)
end
def find_commits(query)
return [] unless Ability.allowed?(@current_user, :download_code, @project)
commits = find_commits_by_message(query)
commit_by_sha = find_commit_by_sha(query)
commits |= [commit_by_sha] if commit_by_sha
commits
end
def find_commits_by_message(query)
project.repository.find_commits_by_message(query)
end
def find_commit_by_sha(query)
key = query.strip
project.repository.commit(key) if Commit.valid_hash?(key)
end
def project_ids_relation
......
......@@ -43,6 +43,10 @@ module Gitlab
@milestones_count ||= milestones.count
end
def single_commit_result?
false
end
private
def projects
......
......@@ -211,4 +211,44 @@ describe "Search", feature: true do
end
end
end
describe 'search for commits' do
before do
visit search_path(project_id: project.id)
end
it 'redirects to commit page when search by sha and only commit found' do
fill_in 'search', with: '6d394385cf567f80a8fd85055db1ab4c5295806f'
click_button 'Search'
expect(page).to have_current_path(namespace_project_commit_path(project.namespace, project, '6d394385cf567f80a8fd85055db1ab4c5295806f'))
end
it 'redirects to single commit regardless of query case' do
fill_in 'search', with: '6D394385cf'
click_button 'Search'
expect(page).to have_current_path(namespace_project_commit_path(project.namespace, project, '6d394385cf567f80a8fd85055db1ab4c5295806f'))
end
it 'holds on /search page when the only commit is found by message' do
create_commit('Message referencing another sha: "deadbeef" ', project, user, 'master')
fill_in 'search', with: 'deadbeef'
click_button 'Search'
expect(page).to have_current_path('/search', only_path: true)
end
it 'shows multiple matching commits' do
fill_in 'search', with: 'See merge request'
click_button 'Search'
click_link 'Commits'
expect(page).to have_selector('.commit-row-description', count: 9)
end
end
end
......@@ -178,4 +178,119 @@ describe Gitlab::ProjectSearchResults, lib: true do
expect(results.objects('notes')).not_to include note
end
end
# Examples for commit access level test
#
# params:
# * search_phrase
# * commit
#
shared_examples 'access restricted commits' do
context 'when project is internal' do
let(:project) { create(:project, :internal) }
it 'does not search if user is not authenticated' do
commits = described_class.new(nil, project, search_phrase).objects('commits')
expect(commits).to be_empty
end
it 'searches if user is authenticated' do
commits = described_class.new(user, project, search_phrase).objects('commits')
expect(commits).to contain_exactly commit
end
end
context 'when project is private' do
let!(:creator) { create(:user, username: 'private-project-author') }
let!(:private_project) { create(:project, :private, creator: creator, namespace: creator.namespace) }
let(:team_master) do
user = create(:user, username: 'private-project-master')
private_project.team << [user, :master]
user
end
let(:team_reporter) do
user = create(:user, username: 'private-project-reporter')
private_project.team << [user, :reporter]
user
end
it 'does not show commit to stranger' do
commits = described_class.new(nil, private_project, search_phrase).objects('commits')
expect(commits).to be_empty
end
context 'team access' do
it 'shows commit to creator' do
commits = described_class.new(creator, private_project, search_phrase).objects('commits')
expect(commits).to contain_exactly commit
end
it 'shows commit to master' do
commits = described_class.new(team_master, private_project, search_phrase).objects('commits')
expect(commits).to contain_exactly commit
end
it 'shows commit to reporter' do
commits = described_class.new(team_reporter, private_project, search_phrase).objects('commits')
expect(commits).to contain_exactly commit
end
end
end
end
describe 'commit search' do
context 'by commit message' do
let(:project) { create(:project, :public) }
let(:commit) { project.repository.commit('59e29889be61e6e0e5e223bfa9ac2721d31605b8') }
let(:message) { 'Sorry, I did a mistake' }
it 'finds commit by message' do
commits = described_class.new(user, project, message).objects('commits')
expect(commits).to contain_exactly commit
end
it 'handles when no commit match' do
commits = described_class.new(user, project, 'not really an existing description').objects('commits')
expect(commits).to be_empty
end
it_behaves_like 'access restricted commits' do
let(:search_phrase) { message }
let(:commit) { project.repository.commit('59e29889be61e6e0e5e223bfa9ac2721d31605b8') }
end
end
context 'by commit hash' do
let(:project) { create(:project, :public) }
let(:commit) { project.repository.commit('0b4bc9a') }
commit_hashes = { short: '0b4bc9a', full: '0b4bc9a49b562e85de7cc9e834518ea6828729b9' }
commit_hashes.each do |type, commit_hash|
it "shows commit by #{type} hash id" do
commits = described_class.new(user, project, commit_hash).objects('commits')
expect(commits).to contain_exactly commit
end
end
it 'handles not existing commit hash correctly' do
commits = described_class.new(user, project, 'deadbeef').objects('commits')
expect(commits).to be_empty
end
it_behaves_like 'access restricted commits' do
let(:search_phrase) { '0b4bc9a49' }
let(:commit) { project.repository.commit('0b4bc9a') }
end
end
end
end
......@@ -351,4 +351,22 @@ eos
expect(commit).not_to be_work_in_progress
end
end
describe '.valid_hash?' do
it 'checks hash contents' do
expect(described_class.valid_hash?('abcdef01239ABCDEF')).to be true
expect(described_class.valid_hash?("abcdef01239ABCD\nEF")).to be false
expect(described_class.valid_hash?(' abcdef01239ABCDEF ')).to be false
expect(described_class.valid_hash?('Gabcdef01239ABCDEF')).to be false
expect(described_class.valid_hash?('gabcdef01239ABCDEF')).to be false
expect(described_class.valid_hash?('-abcdef01239ABCDEF')).to be false
end
it 'checks hash length' do
expect(described_class.valid_hash?('a' * 6)).to be false
expect(described_class.valid_hash?('a' * 7)).to be true
expect(described_class.valid_hash?('a' * 40)).to be true
expect(described_class.valid_hash?('a' * 41)).to be false
end
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