From dd3ddcd72bbfec3ba5bbcd871a9ac68064be7501 Mon Sep 17 00:00:00 2001 From: YarNayar Date: Thu, 8 Dec 2016 17:54:45 +0300 Subject: Allows to search within project by commit's hash Was proposed in #24833 --- app/models/commit.rb | 11 +- ...low-to-search-by-commit-hash-within-project.yml | 4 + lib/gitlab/project_search_results.rb | 20 +++- spec/features/search_spec.rb | 15 +++ spec/lib/gitlab/project_search_results_spec.rb | 115 +++++++++++++++++++++ spec/models/commit_spec.rb | 18 ++++ 6 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/24833-Allow-to-search-by-commit-hash-within-project.yml diff --git a/app/models/commit.rb b/app/models/commit.rb index 5d942cb0422..316bd2e512b 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -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", /(?\h{7,40})/) + @link_reference_pattern ||= super("commit", /(?#{COMMIT_SHA_PATTERN})/) end def to_reference(from_project = nil, full: false) diff --git a/changelogs/unreleased/24833-Allow-to-search-by-commit-hash-within-project.yml b/changelogs/unreleased/24833-Allow-to-search-by-commit-hash-within-project.yml new file mode 100644 index 00000000000..be66c370f36 --- /dev/null +++ b/changelogs/unreleased/24833-Allow-to-search-by-commit-hash-within-project.yml @@ -0,0 +1,4 @@ +--- +title: 'Allows to search within project by commit hash' +merge_request: +author: YarNayar diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 6bdf3db9cb8..a66719706be 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -114,7 +114,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.include?(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 diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index a05b83959fb..c64aeea0612 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -211,4 +211,19 @@ describe "Search", feature: true do end end end + + describe 'search for commits' do + before do + visit search_path(project_id: project.id) + 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 diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 14ee386dba6..d94eb52f838 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -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 diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 0d425ab7fd4..b2202f0fd44 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -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 -- cgit v1.2.1 From 99404a5851a4b8bbba8a5786d7351f2d4b024092 Mon Sep 17 00:00:00 2001 From: YarNayar Date: Wed, 11 Jan 2017 07:20:32 +0300 Subject: Search feature: redirects to commit page if query is commit sha and only commit found See !8028 and #24833 --- app/controllers/search_controller.rb | 14 ++++++++++++ .../redirect-to-commit-when-only-commit-found.yml | 5 +++++ lib/gitlab/project_search_results.rb | 10 ++++++++- lib/gitlab/search_results.rb | 4 ++++ spec/features/search_spec.rb | 25 ++++++++++++++++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/redirect-to-commit-when-only-commit-found.yml diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index b666aa01d6b..6576ebd5235 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -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 diff --git a/changelogs/unreleased/redirect-to-commit-when-only-commit-found.yml b/changelogs/unreleased/redirect-to-commit-when-only-commit-found.yml new file mode 100644 index 00000000000..e0f7e11b6d1 --- /dev/null +++ b/changelogs/unreleased/redirect-to-commit-when-only-commit-found.yml @@ -0,0 +1,5 @@ +--- +title: 'Search feature: redirects to commit page if query is commit sha and only commit + found' +merge_request: 8028 +author: YarNayar diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index a66719706be..db325c00705 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -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 @@ -122,7 +130,7 @@ module Gitlab commits = find_commits_by_message(query) commit_by_sha = find_commit_by_sha(query) - commits << commit_by_sha if commit_by_sha && !commits.include?(commit_by_sha) + commits |= [commit_by_sha] if commit_by_sha commits end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 35212992698..c9c65f76f4b 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -43,6 +43,10 @@ module Gitlab @milestones_count ||= milestones.count end + def single_commit_result? + false + end + private def projects diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index c64aeea0612..0fe5a897565 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -217,6 +217,31 @@ describe "Search", feature: true 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' -- cgit v1.2.1