diff options
author | Paul Slaughter <pslaughter@gitlab.com> | 2019-08-09 16:32:18 +0000 |
---|---|---|
committer | Paul Slaughter <pslaughter@gitlab.com> | 2019-08-09 16:32:18 +0000 |
commit | 28388d61a2bbebe00f55c9d7037d9ff5391f90b3 (patch) | |
tree | 512763c294b4a7408f65efbf83af26e2c878f84f | |
parent | e0e6e9e7dbcff3ce9ea12fb0421704d7ef48ad15 (diff) | |
parent | aa137fd39f1e750ff4d02371b7cdf1322e84a343 (diff) | |
download | gitlab-ce-28388d61a2bbebe00f55c9d7037d9ff5391f90b3.tar.gz |
Merge branch '59712-resolve-the-search-problem-issue' into 'master'
Resolve "Search loses branch"
Closes #59712
See merge request gitlab-org/gitlab-ce!28282
-rw-r--r-- | app/assets/javascripts/lib/utils/forms.js | 12 | ||||
-rw-r--r-- | app/assets/javascripts/pages/projects/project.js | 23 | ||||
-rw-r--r-- | app/assets/javascripts/pages/search/show/search.js | 3 | ||||
-rw-r--r-- | app/views/search/_form.html.haml | 1 | ||||
-rw-r--r-- | app/views/search/_results.html.haml | 16 | ||||
-rw-r--r-- | app/views/shared/_ref_switcher.html.haml | 15 | ||||
-rw-r--r-- | changelogs/unreleased/59712-resolve-the-search-problem-issue.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 6 | ||||
-rw-r--r-- | spec/features/search/user_searches_for_code_spec.rb | 69 | ||||
-rw-r--r-- | spec/frontend/lib/utils/forms_spec.js | 74 |
10 files changed, 196 insertions, 28 deletions
diff --git a/app/assets/javascripts/lib/utils/forms.js b/app/assets/javascripts/lib/utils/forms.js new file mode 100644 index 00000000000..106209a2f3a --- /dev/null +++ b/app/assets/javascripts/lib/utils/forms.js @@ -0,0 +1,12 @@ +export const serializeFormEntries = entries => + entries.reduce((acc, { name, value }) => Object.assign(acc, { [name]: value }), {}); + +export const serializeForm = form => { + const fdata = new FormData(form); + const entries = Array.from(fdata.keys()).map(key => { + const val = fdata.getAll(key); + return { name: key, value: val.length === 1 ? val[0] : val }; + }); + + return serializeFormEntries(entries); +}; diff --git a/app/assets/javascripts/pages/projects/project.js b/app/assets/javascripts/pages/projects/project.js index f0d529758d5..332b6811af6 100644 --- a/app/assets/javascripts/pages/projects/project.js +++ b/app/assets/javascripts/pages/projects/project.js @@ -1,9 +1,10 @@ -/* eslint-disable func-names, no-var, no-return-assign, one-var, object-shorthand, vars-on-top */ +/* eslint-disable func-names, no-var, no-return-assign, object-shorthand, vars-on-top */ import $ from 'jquery'; import Cookies from 'js-cookie'; import { __ } from '~/locale'; -import { visitUrl } from '~/lib/utils/url_utility'; +import { visitUrl, mergeUrlParams } from '~/lib/utils/url_utility'; +import { serializeForm } from '~/lib/utils/forms'; import axios from '~/lib/utils/axios_utils'; import flash from '~/flash'; import projectSelect from '../../project_select'; @@ -107,9 +108,10 @@ export default class Project { refLink.href = '#'; return $('.js-project-refs-dropdown').each(function() { - var $dropdown, selected; - $dropdown = $(this); - selected = $dropdown.data('selected'); + var $dropdown = $(this); + var selected = $dropdown.data('selected'); + var fieldName = $dropdown.data('fieldName'); + var shouldVisit = Boolean($dropdown.data('visit')); return $dropdown.glDropdown({ data(term, callback) { axios @@ -127,7 +129,7 @@ export default class Project { filterRemote: true, filterByText: true, inputFieldName: $dropdown.data('inputFieldName'), - fieldName: $dropdown.data('fieldName'), + fieldName, renderRow: function(ref) { var li = refListItem.cloneNode(false); @@ -158,15 +160,12 @@ export default class Project { clicked: function(options) { const { e } = options; e.preventDefault(); - if ($('input[name="ref"]').length) { + if ($(`input[name="${fieldName}"]`).length) { var $form = $dropdown.closest('form'); - - var $visit = $dropdown.data('visit'); - var shouldVisit = $visit ? true : $visit; var action = $form.attr('action'); - var divider = action.indexOf('?') === -1 ? '?' : '&'; + if (shouldVisit) { - visitUrl(`${action}${divider}${$form.serialize()}`); + visitUrl(mergeUrlParams(serializeForm($form[0]), action)); } } }, diff --git a/app/assets/javascripts/pages/search/show/search.js b/app/assets/javascripts/pages/search/show/search.js index 7743e05e748..81b6225cb18 100644 --- a/app/assets/javascripts/pages/search/show/search.js +++ b/app/assets/javascripts/pages/search/show/search.js @@ -2,6 +2,7 @@ import $ from 'jquery'; import Flash from '~/flash'; import Api from '~/api'; import { __ } from '~/locale'; +import Project from '~/pages/projects/project'; export default class Search { constructor() { @@ -69,6 +70,8 @@ export default class Search { }, clicked: () => Search.submitSearch(), }); + + Project.initRefSwitcher(); } eventListeners() { diff --git a/app/views/search/_form.html.haml b/app/views/search/_form.html.haml index 4c4f3e0298b..464db94b7f4 100644 --- a/app/views/search/_form.html.haml +++ b/app/views/search/_form.html.haml @@ -1,6 +1,7 @@ = form_tag search_path, method: :get, class: 'search-page-form js-search-form' do |f| = hidden_field_tag :snippets, params[:snippets] = hidden_field_tag :scope, params[:scope] + = hidden_field_tag :repository_ref, params[:repository_ref] .d-lg-flex.align-items-end .search-field-holder.form-group.mr-lg-1.mb-lg-0 diff --git a/app/views/search/_results.html.haml b/app/views/search/_results.html.haml index cb8a8a24be8..de9947528cf 100644 --- a/app/views/search/_results.html.haml +++ b/app/views/search/_results.html.haml @@ -2,17 +2,25 @@ = render partial: "search/results/empty" = render_if_exists 'shared/promotions/promote_advanced_search' - else - .row-content-block + .row-content-block.d-md-flex.text-left.align-items-center - unless @search_objects.is_a?(Kaminari::PaginatableWithoutCount) = search_entries_info(@search_objects, @scope, @search_term) - unless @show_snippets - if @project - - link_to_project = link_to(@project.full_name, [@project.namespace.becomes(Namespace), @project]) - = _("in project %{link_to_project}").html_safe % { link_to_project: link_to_project } + - link_to_project = link_to(@project.full_name, [@project.namespace.becomes(Namespace), @project], class: 'ml-md-1') + - if @scope == 'blobs' + - repository_ref = params[:repository_ref].to_s.presence || @project.default_branch + = s_("SearchCodeResults|in") + .mx-md-1 + = render partial: "shared/ref_switcher", locals: { ref: repository_ref, form_path: request.fullpath, field_name: 'repository_ref' } + = s_('SearchCodeResults|of %{link_to_project}').html_safe % { link_to_project: link_to_project } + - else + = _("in project %{link_to_project}").html_safe % { link_to_project: link_to_project } - elsif @group - - link_to_group = link_to(@group.name, @group) + - link_to_group = link_to(@group.name, @group, class: 'ml-md-1') = _("in group %{link_to_group}").html_safe % { link_to_group: link_to_group } = render_if_exists 'shared/promotions/promote_advanced_search' + .results.prepend-top-10 - if @scope == 'commits' %ul.content-list.commit-list diff --git a/app/views/shared/_ref_switcher.html.haml b/app/views/shared/_ref_switcher.html.haml index 7cbc5810c10..74e0a088656 100644 --- a/app/views/shared/_ref_switcher.html.haml +++ b/app/views/shared/_ref_switcher.html.haml @@ -1,12 +1,19 @@ -- dropdown_toggle_text = @ref || @project.default_branch -= form_tag switch_project_refs_path(@project), method: :get, class: "project-refs-form" do - = hidden_field_tag :destination, destination +- return unless @project + +- ref = local_assigns.fetch(:ref, @ref) +- form_path = local_assigns.fetch(:form_path, switch_project_refs_path(@project)) +- dropdown_toggle_text = ref || @project.default_branch +- field_name = local_assigns.fetch(:field_name, 'ref') + += form_tag form_path, method: :get, class: "project-refs-form" do + - if defined?(destination) + = hidden_field_tag :destination, destination - if defined?(path) = hidden_field_tag :path, path - @options && @options.each do |key, value| = hidden_field_tag key, value, id: nil .dropdown - = dropdown_toggle dropdown_toggle_text, { toggle: "dropdown", selected: dropdown_toggle_text, ref: @ref, refs_url: refs_project_path(@project, sort: 'updated_desc'), field_name: 'ref', submit_form_on_click: true, visit: true }, { toggle_class: "js-project-refs-dropdown qa-branches-select" } + = dropdown_toggle dropdown_toggle_text, { toggle: "dropdown", selected: dropdown_toggle_text, ref: ref, refs_url: refs_project_path(@project, sort: 'updated_desc'), field_name: field_name, submit_form_on_click: true, visit: true }, { toggle_class: "js-project-refs-dropdown qa-branches-select" } .dropdown-menu.dropdown-menu-selectable.git-revision-dropdown.dropdown-menu-paging.qa-branches-dropdown{ class: ("dropdown-menu-right" if local_assigns[:align_right]) } .dropdown-page-one = dropdown_title _("Switch branch/tag") diff --git a/changelogs/unreleased/59712-resolve-the-search-problem-issue.yml b/changelogs/unreleased/59712-resolve-the-search-problem-issue.yml new file mode 100644 index 00000000000..964962cb817 --- /dev/null +++ b/changelogs/unreleased/59712-resolve-the-search-problem-issue.yml @@ -0,0 +1,5 @@ +--- +title: Add branch/tags/commits dropdown filter on the search page for searching codes +merge_request: 28282 +author: minghuan lei +type: changed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8c8985b398c..9b41f32042a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9666,6 +9666,12 @@ msgstr "" msgid "SearchAutocomplete|in this project" msgstr "" +msgid "SearchCodeResults|in" +msgstr "" + +msgid "SearchCodeResults|of %{link_to_project}" +msgstr "" + msgid "SearchResults|Showing %{from} - %{to} of %{count} %{scope} for \"%{term}\"" msgstr "" diff --git a/spec/features/search/user_searches_for_code_spec.rb b/spec/features/search/user_searches_for_code_spec.rb index 71af75640de..5a60991c1bf 100644 --- a/spec/features/search/user_searches_for_code_spec.rb +++ b/spec/features/search/user_searches_for_code_spec.rb @@ -6,6 +6,21 @@ 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) @@ -15,12 +30,7 @@ describe 'User searches for code' do it 'finds a file' do visit(project_path(project)) - page.within('.search') do - fill_in('search', with: 'application.js') - click_button('Go') - end - - click_link('Code') + submit_search('application.js') expect(page).to have_selector('.file-content .code') expect(page).to have_selector("span.line[lang='javascript']") @@ -48,6 +58,50 @@ describe 'User searches for code' do end end end + + context 'search code within refs', :js do + let(:ref_name) { 'v1.0.0' } + + before do + visit(project_tree_path(project, ref_name)) + submit_search('gitlab-grack', with_send_keys: true) + end + + it 'shows ref switcher in code result summary' do + expect(find('.js-project-refs-dropdown')).to have_text(ref_name) + end + it 'persists branch name across search' do + find('.btn-search').click + expect(find('.js-project-refs-dropdown')).to have_text(ref_name) + end + + # this example is use to test the desgine that the refs is not + # only repersent the branch as well as the tags. + it 'ref swither list all the branchs and tags' do + find('.js-project-refs-dropdown').click + expect(find('.dropdown-page-one .dropdown-content')).to have_link('sha-starting-with-large-number') + expect(find('.dropdown-page-one .dropdown-content')).to have_link('v1.0.0') + end + + it 'search result changes when refs switched' do + expect(find('.search-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') + 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) + 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) + expect(page).not_to have_selector('.js-project-refs-dropdown') + end end context 'when signed out' do @@ -58,8 +112,7 @@ describe 'User searches for code' do end it 'finds code' do - fill_in('search', with: 'rspec') - click_button('Go') + submit_search('rspec') page.within('.results') do expect(find(:css, '.search-results')).to have_content('Update capybara, rspec-rails, poltergeist to recent versions') diff --git a/spec/frontend/lib/utils/forms_spec.js b/spec/frontend/lib/utils/forms_spec.js new file mode 100644 index 00000000000..cac17235f0d --- /dev/null +++ b/spec/frontend/lib/utils/forms_spec.js @@ -0,0 +1,74 @@ +import { serializeForm } from '~/lib/utils/forms'; + +describe('lib/utils/forms', () => { + const createDummyForm = inputs => { + const form = document.createElement('form'); + + form.innerHTML = inputs + .map(({ type, name, value }) => { + let str = ``; + if (type === 'select') { + str = `<select name="${name}">`; + value.forEach(v => { + if (v.length > 0) { + str += `<option value="${v}"></option> `; + } + }); + str += `</select>`; + } else { + str = `<input type="${type}" name="${name}" value="${value}" checked/>`; + } + return str; + }) + .join(''); + + return form; + }; + + describe('serializeForm', () => { + it('returns an object of key values from inputs', () => { + const form = createDummyForm([ + { type: 'text', name: 'foo', value: 'foo-value' }, + { type: 'text', name: 'bar', value: 'bar-value' }, + ]); + + const data = serializeForm(form); + + expect(data).toEqual({ + foo: 'foo-value', + bar: 'bar-value', + }); + }); + + it('works with select', () => { + const form = createDummyForm([ + { type: 'select', name: 'foo', value: ['foo-value1', 'foo-value2'] }, + { type: 'text', name: 'bar', value: 'bar-value1' }, + ]); + + const data = serializeForm(form); + + expect(data).toEqual({ + foo: 'foo-value1', + bar: 'bar-value1', + }); + }); + + it('works with multiple inputs of the same name', () => { + const form = createDummyForm([ + { type: 'checkbox', name: 'foo', value: 'foo-value3' }, + { type: 'checkbox', name: 'foo', value: 'foo-value2' }, + { type: 'checkbox', name: 'foo', value: 'foo-value1' }, + { type: 'text', name: 'bar', value: 'bar-value2' }, + { type: 'text', name: 'bar', value: 'bar-value1' }, + ]); + + const data = serializeForm(form); + + expect(data).toEqual({ + foo: ['foo-value3', 'foo-value2', 'foo-value1'], + bar: ['bar-value2', 'bar-value1'], + }); + }); + }); +}); |