Commit 1bccd36f authored by Markus Koller's avatar Markus Koller Committed by Lin Jen-Shin

Improve search result labels

- Use "results" instead of "blobs", "wiki blobs", "snippet blobs"
- Use "comments" instead of "notes"
- Use correct pluralization
- Don't add "1 - 10 of" if there's only one page
parent b76bc276
......@@ -30,7 +30,46 @@ module SearchHelper
to = collection.offset_value + collection.to_a.size
count = collection.total_count
s_("SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\"") % { from: from, to: to, count: count, scope: scope.humanize(capitalize: false), term: term }
search_entries_info_template(collection) % {
from: from,
to: to,
count: count,
scope: search_entries_info_label(scope, count),
term: term
}
end
def search_entries_info_label(scope, count)
case scope
when 'blobs', 'snippet_blobs', 'wiki_blobs'
ns_('SearchResults|result', 'SearchResults|results', count)
when 'commits'
ns_('SearchResults|commit', 'SearchResults|commits', count)
when 'issues'
ns_('SearchResults|issue', 'SearchResults|issues', count)
when 'merge_requests'
ns_('SearchResults|merge request', 'SearchResults|merge requests', count)
when 'milestones'
ns_('SearchResults|milestone', 'SearchResults|milestones', count)
when 'notes'
ns_('SearchResults|comment', 'SearchResults|comments', count)
when 'projects'
ns_('SearchResults|project', 'SearchResults|projects', count)
when 'snippet_titles'
ns_('SearchResults|snippet', 'SearchResults|snippets', count)
when 'users'
ns_('SearchResults|user', 'SearchResults|users', count)
else
raise "Unrecognized search scope '#{scope}'"
end
end
def search_entries_info_template(collection)
if collection.total_pages > 1
s_("SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\"")
else
s_("SearchResults|Showing %{count} %{scope} for \"%{term}\"")
end
end
def find_project_for_result_blob(projects, result)
......
......@@ -47,6 +47,7 @@
= hidden_field_tag :snippets, true
= hidden_field_tag :repository_ref, @ref
= hidden_field_tag :nav_source, 'navbar'
-# workaround for non-JS feature specs, for JS you need to use find('#search').send_keys(:enter)
= button_tag 'Go' if ENV['RAILS_ENV'] == 'test'
-# workaround for non-JS feature specs, see spec/support/helpers/search_helpers.rb
- if ENV['RAILS_ENV'] == 'test'
%noscript= button_tag 'Search'
.search-autocomplete-opts.hide{ :'data-autocomplete-path' => search_autocomplete_path, :'data-autocomplete-project-id' => @project.try(:id), :'data-autocomplete-project-ref' => @ref }
---
title: Improve search result labels
merge_request: 32101
author:
type: changed
......@@ -10097,9 +10097,57 @@ msgstr ""
msgid "SearchCodeResults|of %{link_to_project}"
msgstr ""
msgid "SearchResults|Showing %{count} %{scope} for \"%{term}\""
msgstr ""
msgid "SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\""
msgstr ""
msgid "SearchResults|comment"
msgid_plural "SearchResults|comments"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|commit"
msgid_plural "SearchResults|commits"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|issue"
msgid_plural "SearchResults|issues"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|merge request"
msgid_plural "SearchResults|merge requests"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|milestone"
msgid_plural "SearchResults|milestones"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|project"
msgid_plural "SearchResults|projects"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|result"
msgid_plural "SearchResults|results"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|snippet"
msgid_plural "SearchResults|snippets"
msgstr[0] ""
msgstr[1] ""
msgid "SearchResults|user"
msgid_plural "SearchResults|users"
msgstr[0] ""
msgstr[1] ""
msgid "Secret"
msgstr ""
......
......@@ -16,8 +16,7 @@ describe 'Global search' do
it 'increases usage ping searches counter' do
expect(Gitlab::UsageDataCounters::SearchCounter).to receive(:increment_navbar_searches_count)
fill_in "search", with: "foobar"
click_button "Go"
submit_search('foobar')
end
describe 'I search through the issues and I see pagination' do
......@@ -27,10 +26,9 @@ describe 'Global search' do
end
it "has a pagination" do
fill_in "search", with: "initial"
click_button "Go"
submit_search('initial')
select_search_scope('Issues')
select_filter("Issues")
expect(page).to have_selector('.gl-pagination .next')
end
end
......
......@@ -18,8 +18,7 @@ describe 'Projects > Files > User searches for files' do
end
it 'does not show any result' do
fill_in('search', with: 'coffee')
click_button('Go')
submit_search('coffee')
expect(page).to have_content("We couldn't find any")
end
......@@ -50,8 +49,7 @@ describe 'Projects > Files > User searches for files' do
it 'shows found files' do
expect(page).to have_selector('.tree-controls .shortcuts-find-file')
fill_in('search', with: 'coffee')
click_button('Go')
submit_search('coffee')
expect(page).to have_content('coffee')
expect(page).to have_content('CONTRIBUTING.md')
......
......@@ -6,21 +6,6 @@ describe 'User searches for code' do
let(:user) { create(:user) }
let(:project) { create(:project, :repository, namespace: user.namespace) }
def submit_search(search, with_send_keys: false)
page.within('.search') do
field = find_field('search')
field.fill_in(with: search)
if with_send_keys
field.send_keys(:enter)
else
click_button("Go")
end
end
click_link('Code')
end
context 'when signed in' do
before do
project.add_maintainer(user)
......@@ -31,7 +16,9 @@ describe 'User searches for code' do
visit(project_path(project))
submit_search('application.js')
select_search_scope('Code')
expect(page).to have_selector('.results', text: 'application.js')
expect(page).to have_selector('.file-content .code')
expect(page).to have_selector("span.line[lang='javascript']")
end
......@@ -52,9 +39,7 @@ describe 'User searches for code' do
fill_in('dashboard_search', with: 'rspec')
find('.btn-search').click
page.within('.results') do
expect(find(:css, '.search-results')).to have_content('Update capybara, rspec-rails, poltergeist to recent versions')
end
expect(page).to have_selector('.results', text: 'Update capybara, rspec-rails, poltergeist to recent versions')
end
it 'search mutiple words with refs switching' do
......@@ -64,16 +49,12 @@ describe 'User searches for code' do
fill_in('dashboard_search', with: search)
find('.btn-search').click
page.within('.results') do
expect(find('.search-results')).to have_content(expected_result)
end
expect(page).to have_selector('.results', text: expected_result)
find('.js-project-refs-dropdown').click
find('.dropdown-page-one .dropdown-content').click_link('v1.0.0')
page.within('.results') do
expect(find(:css, '.search-results')).to have_content(expected_result)
end
expect(page).to have_selector('.results', text: expected_result)
expect(find_field('dashboard_search').value).to eq(search)
end
......@@ -84,7 +65,9 @@ describe 'User searches for code' do
before do
visit(project_tree_path(project, ref_name))
submit_search('gitlab-grack', with_send_keys: true)
submit_search('gitlab-grack')
select_search_scope('Code')
end
it 'shows ref switcher in code result summary' do
......@@ -104,22 +87,27 @@ describe 'User searches for code' do
end
it 'search result changes when refs switched' do
expect(find('.search-results')).not_to have_content('path = gitlab-grack')
expect(find('.results')).not_to have_content('path = gitlab-grack')
find('.js-project-refs-dropdown').click
find('.dropdown-page-one .dropdown-content').click_link('master')
expect(find('.search-results')).to have_content('path = gitlab-grack')
expect(page).to have_selector('.results', text: 'path = gitlab-grack')
end
end
it 'no ref switcher shown in issue result summary', :js do
issue = create(:issue, title: 'test', project: project)
visit(project_tree_path(project))
submit_search('test', with_send_keys: true)
submit_search('test')
select_search_scope('Code')
expect(page).to have_selector('.js-project-refs-dropdown')
page.within('.search-filter') do
click_link('Issues')
end
expect(find(:css, '.search-results')).to have_link(issue.title)
select_search_scope('Issues')
expect(find(:css, '.results')).to have_link(issue.title)
expect(page).not_to have_selector('.js-project-refs-dropdown')
end
end
......@@ -133,10 +121,9 @@ describe 'User searches for code' do
it 'finds code' do
submit_search('rspec')
select_search_scope('Code')
page.within('.results') do
expect(find(:css, '.search-results')).to have_content('Update capybara, rspec-rails, poltergeist to recent versions')
end
expect(page).to have_selector('.results', text: 'Update capybara, rspec-rails, poltergeist to recent versions')
end
end
end
......@@ -18,15 +18,13 @@ describe 'User searches for comments' do
let(:comment) { create(:note_on_commit, author: user, project: project, commit_id: 12345678, note: 'Bug here') }
it 'finds a commit' do
page.within('.search') do
fill_in('search', with: comment.note)
click_button('Go')
end
click_link('Comments')
submit_search(comment.note)
select_search_scope('Comments')
expect(page).to have_text('Commit deleted')
expect(page).to have_text('12345678')
page.within('.results') do
expect(page).to have_content('Commit deleted')
expect(page).to have_content('12345678')
end
end
end
end
......@@ -36,14 +34,10 @@ describe 'User searches for comments' do
let(:comment) { create(:note, noteable: snippet, author: user, note: 'Supercalifragilisticexpialidocious', project: project) }
it 'finds a snippet' do
page.within('.search') do
fill_in('search', with: comment.note)
click_button('Go')
end
click_link('Comments')
submit_search(comment.note)
select_search_scope('Comments')
expect(page).to have_link(snippet.title)
expect(page).to have_selector('.results', text: snippet.title)
end
end
end
......@@ -16,15 +16,13 @@ describe 'User searches for commits' do
context 'when searching by SHA' do
it 'finds a commit and redirects to its page' do
fill_in('search', with: sha)
click_button('Search')
submit_search(sha)
expect(page).to have_current_path(project_commit_path(project, sha))
end
it 'finds a commit in uppercase and redirects to its page' do
fill_in('search', with: sha.upcase)
click_button('Search')
submit_search(sha.upcase)
expect(page).to have_current_path(project_commit_path(project, sha))
end
......@@ -34,16 +32,14 @@ describe 'User searches for commits' do
it 'finds a commit and holds on /search page' do
create_commit('Message referencing another sha: "deadbeef"', project, user, 'master')
fill_in('search', with: 'deadbeef')
click_button('Search')
submit_search('deadbeef')
expect(page).to have_current_path('/search', ignore_query: true)
end
it 'finds multiple commits' do
fill_in('search', with: 'See merge request')
click_button('Search')
click_link('Commits')
submit_search('See merge request')
select_search_scope('Commits')
expect(page).to have_selector('.commit-row-description', count: 9)
end
......
......@@ -21,13 +21,11 @@ describe 'User searches for issues', :js do
it 'finds an issue' do
fill_in('dashboard_search', with: issue1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Issues')
end
select_search_scope('Issues')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(issue1.title).and have_no_link(issue2.title)
expect(page).to have_link(issue1.title)
expect(page).not_to have_link(issue2.title)
end
end
......@@ -41,13 +39,11 @@ describe 'User searches for issues', :js do
fill_in('dashboard_search', with: issue1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Issues')
end
select_search_scope('Issues')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(issue1.title).and have_no_link(issue2.title)
expect(page).to have_link(issue1.title)
expect(page).not_to have_link(issue2.title)
end
end
end
......@@ -65,13 +61,11 @@ describe 'User searches for issues', :js do
it 'finds an issue' do
fill_in('dashboard_search', with: issue1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Issues')
end
select_search_scope('Issues')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(issue1.title).and have_no_link(issue2.title)
expect(page).to have_link(issue1.title)
expect(page).not_to have_link(issue2.title)
end
end
end
......
......@@ -20,13 +20,11 @@ describe 'User searches for merge requests', :js do
it 'finds a merge request' do
fill_in('dashboard_search', with: merge_request1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Merge requests')
end
select_search_scope('Merge requests')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(merge_request1.title).and have_no_link(merge_request2.title)
expect(page).to have_link(merge_request1.title)
expect(page).not_to have_link(merge_request2.title)
end
end
......@@ -40,13 +38,11 @@ describe 'User searches for merge requests', :js do
fill_in('dashboard_search', with: merge_request1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Merge requests')
end
select_search_scope('Merge requests')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(merge_request1.title).and have_no_link(merge_request2.title)
expect(page).to have_link(merge_request1.title)
expect(page).not_to have_link(merge_request2.title)
end
end
end
......
......@@ -20,13 +20,11 @@ describe 'User searches for milestones', :js do
it 'finds a milestone' do
fill_in('dashboard_search', with: milestone1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Milestones')
end
select_search_scope('Milestones')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(milestone1.title).and have_no_link(milestone2.title)
expect(page).to have_link(milestone1.title)
expect(page).not_to have_link(milestone2.title)
end
end
......@@ -40,13 +38,11 @@ describe 'User searches for milestones', :js do
fill_in('dashboard_search', with: milestone1.title)
find('.btn-search').click
page.within('.search-filter') do
click_link('Milestones')
end
select_search_scope('Milestones')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(milestone1.title).and have_no_link(milestone2.title)
expect(page).to have_link(milestone1.title)
expect(page).not_to have_link(milestone2.title)
end
end
end
......
......@@ -20,8 +20,7 @@ describe 'User searches for projects' do
it 'preserves the group being searched in' do
visit(search_path(group_id: project.namespace.id))
fill_in('search', with: 'foo')
click_button('Search')
submit_search('foo')
expect(find('#group_id', visible: false).value).to eq(project.namespace.id.to_s)
end
......@@ -29,8 +28,7 @@ describe 'User searches for projects' do
it 'preserves the project being searched in' do
visit(search_path(project_id: project.id))
fill_in('search', with: 'foo')
click_button('Search')
submit_search('foo')
expect(find('#project_id', visible: false).value).to eq(project.id.to_s)
end
......
......@@ -3,83 +3,81 @@
require 'spec_helper'
describe 'User searches for users' do
context 'when on the dashboard' do
it 'finds the user', :js do
create(:user, username: 'gob_bluth', name: 'Gob Bluth')
let(:user1) { create(:user, username: 'gob_bluth', name: 'Gob Bluth') }
let(:user2) { create(:user, username: 'michael_bluth', name: 'Michael Bluth') }
let(:user3) { create(:user, username: 'gob_2018', name: 'George Oscar Bluth') }
sign_in(create(:user))
before do
sign_in(user1)
end
context 'when on the dashboard' do
it 'finds the user', :js do
visit dashboard_projects_path
fill_in 'search', with: 'gob'
find('#search').send_keys(:enter)
submit_search('gob')
select_search_scope('Users')
expect(page).to have_content('Users 1')
click_on('Users 1')
expect(page).to have_content('Gob Bluth')
expect(page).to have_content('@gob_bluth')
page.within('.results') do
expect(page).to have_content('Gob Bluth')
expect(page).to have_content('@gob_bluth')
end
end
end
context 'when on the project page' do
it 'finds the user belonging to the project' do
project = create(:project)
let(:project) { create(:project) }
user1 = create(:user, username: 'gob_bluth', name: 'Gob Bluth')
before do
create(:project_member, :developer, user: user1, project: project)
user2 = create(:user, username: 'michael_bluth', name: 'Michael Bluth')
create(:project_member, :developer, user: user2, project: project)
user3
end
create(:user, username: 'gob_2018', name: 'George Oscar Bluth')
sign_in(user1)
visit projects_path(project)
it 'finds the user belonging to the project' do
visit project_path(project)
fill_in 'search', with: 'gob'
click_button 'Go'
submit_search('gob')
select_search_scope('Users')
expect(page).to have_content('Gob Bluth')
expect(page).to have_content('@gob_bluth')
page.within('.results') do
expect(page).to have_content('Gob Bluth')
expect(page).to have_content('@gob_bluth')
expect(page).not_to have_content('Michael Bluth')
expect(page).not_to have_content('@michael_bluth')
expect(page).not_to have_content('Michael Bluth')
expect(page).not_to have_content('@michael_bluth')
expect(page).not_to have_content('George Oscar Bluth')
expect(page).not_to have_content('@gob_2018')
expect(page).not_to have_content('George Oscar Bluth')
expect(page).not_to have_content('@gob_2018')
end
end
end
context 'when on the group page' do
it 'finds the user belonging to the group' do
group = create(:group)
let(:group) { create(:group) }
user1 = create(:user, username: 'gob_bluth', name: 'Gob Bluth')
before do
create(:group_member, :developer, user: user1, group: group)
user2 = create(:user, username: 'michael_bluth', name: 'Michael Bluth')
create(:group_member, :developer, user: user2, group: group)
user3
end
create(:user, username: 'gob_2018', name: 'George Oscar Bluth')
sign_in(user1)
it 'finds the user belonging to the group' do
visit group_path(group)
fill_in 'search', with: 'gob'
click_button 'Go'
submit_search('gob')
select_search_scope('Users')
expect(page).to have_content('Gob Bluth')
expect(page).to have_content('@gob_bluth')
page.within('.results') do
expect(page).to have_content('Gob Bluth')
expect(page).to have_content('@gob_bluth')
expect(page).not_to have_content('Michael Bluth')
expect(page).not_to have_content('@michael_bluth')
expect(page).not_to have_content('Michael Bluth')
expect(page).not_to have_content('@michael_bluth')
expect(page).not_to have_content('George Oscar Bluth')
expect(page).not_to have_content('@gob_2018')
expect(page).not_to have_content('George Oscar Bluth')
expect(page).not_to have_content('@gob_2018')
end
end
end
end
......@@ -26,13 +26,10 @@ describe 'User searches for wiki pages', :js do
fill_in('dashboard_search', with: search_term)
find('.btn-search').click
page.within('.search-filter') do
click_link('Wiki')
end
select_search_scope('Wiki')
page.within('.results') do
expect(find(:css, '.search-results')).to have_link(wiki_page.title, href: project_wiki_path(project, wiki_page.slug))
expect(page).to have_link(wiki_page.title, href: project_wiki_path(project, wiki_page.slug))
end
end
end
......
......@@ -19,8 +19,7 @@ describe 'User uses header search field', :js do
end
it 'starts searching by pressing the enter key' do
fill_in('search', with: 'gitlab')
find('#search').native.send_keys(:enter)
submit_search('gitlab')
page.within('.page-title') do
expect(page).to have_content('Search')
......@@ -101,8 +100,7 @@ describe 'User uses header search field', :js do
before do
create(:issue, project: project, title: 'project issue')
fill_in('search', with: 'project')
find('#search').send_keys(:enter)
submit_search('project')
end
it 'displays result counts for all categories' do
......
......@@ -10,12 +10,8 @@ describe 'Search Snippets' do
sign_in private_snippet.author
visit dashboard_snippets_path
page.within '.search' do
fill_in 'search', with: 'Middle'
click_button 'Go'
end
click_link 'Titles and Filenames'
submit_search('Middle')
select_search_scope('Titles and Filenames')
expect(page).to have_link(public_snippet.title)
expect(page).to have_link(private_snippet.title)
......@@ -45,11 +41,7 @@ describe 'Search Snippets' do
sign_in create(:user)
visit dashboard_snippets_path
page.within '.search' do
fill_in 'search', with: 'line seven'
click_button 'Go'
end
submit_search('line seven')
expect(page).to have_content('line seven')
......
......@@ -6,7 +6,7 @@ describe SearchHelper do
str
end
describe 'search_autocomplete_source' do
describe 'search_autocomplete_opts' do
context "with no current user" do
before do
allow(self).to receive(:current_user).and_return(nil)
......@@ -99,6 +99,47 @@ describe SearchHelper do
end
end
describe 'search_entries_info' do
using RSpec::Parameterized::TableSyntax
where(:scope, :label) do
'commits' | 'commit'
'issues' | 'issue'
'merge_requests' | 'merge request'
'milestones' | 'milestone'
'projects' | 'project'
'snippet_titles' | 'snippet'
'users' | 'user'
'blobs' | 'result'
'snippet_blobs' | 'result'
'wiki_blobs' | 'result'
'notes' | 'comment'
end
with_them do
it 'uses the correct singular label' do
collection = Kaminari.paginate_array([:foo]).page(1).per(10)
expect(search_entries_info(collection, scope, 'foo')).to eq("Showing 1 #{label} for \"foo\"")
end
it 'uses the correct plural label' do
collection = Kaminari.paginate_array([:foo] * 23).page(1).per(10)
expect(search_entries_info(collection, scope, 'foo')).to eq("Showing 1 - 10 of 23 #{label.pluralize} for \"foo\"")
end
end
it 'raises an error for unrecognized scopes' do
expect do
collection = Kaminari.paginate_array([:foo]).page(1).per(10)
search_entries_info(collection, 'unknown', 'foo')
end.to raise_error(RuntimeError)
end
end
describe 'search_filter_input_options' do
context 'project' do
before do
......
......@@ -42,4 +42,8 @@ module CapybaraHelpers
def clear_browser_session
page.driver.browser.manage.delete_cookie('_gitlab_session')
end
def javascript_test?
Capybara.current_driver == Capybara.javascript_driver
end
end
# frozen_string_literal: true
module SearchHelpers
def select_filter(name)
find(:xpath, "//ul[contains(@class, 'search-filter')]//a[contains(.,'#{name}')]").click
def submit_search(query, scope: nil)
page.within('.search-form, .search-page-form') do
field = find_field('search')
field.fill_in(with: query)
if javascript_test?
field.send_keys(:enter)
else
click_button('Search')
end
end
end
def select_search_scope(scope)
page.within '.search-filter' do
click_link scope
end
end
end
......@@ -61,8 +61,4 @@ module WaitForRequests
Capybara.page.evaluate_script('jQuery.active').zero?
end
def javascript_test?
Capybara.current_driver == Capybara.javascript_driver
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'search/_results' do
before do
controller.params[:action] = 'show'
3.times { create(:issue) }
@search_objects = Issue.page(1).per(2)
@scope = 'issues'
@search_term = 'foo'
end
it 'displays the page size' do
render
expect(rendered).to have_content('Showing 1 - 2 of 3 issues for "foo"')
end
context 'when search results do not have a count' do
before do
@search_objects = @search_objects.without_count
end
it 'does not display the page size' do
render
expect(rendered).not_to have_content(/Showing .* of .*/)
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