Skip to content

G
gitlab-ce
Commit 2e9739f1 authored by Zack Cuddy's avatar Zack Cuddy Committed by Tomas Bulva
Browse files
Options

Global Search - Header Search Snippets 

This change fixes a bug with the new
header search.  The issue being that
snippet searches are not properly scoped.

With this change the search_context is
used to properly pass the snippets param.

Changelog: fixed
parent c999069b
Show whitespace changes
Inline Side-by-side
Showing with 98 additions and 54 deletions
app/assets/javascripts/header_search/components/header_search_default_items.vue
View file @ 2e9739f1
...@@ -24,8 +24,8 @@ export default { ...@@ -24,8 +24,8 @@ export default {
...mapGetters(['defaultSearchOptions']), ...mapGetters(['defaultSearchOptions']),
sectionHeader() { sectionHeader() {
return ( return (
this.searchContext.project?.name || this.searchContext?.project?.name ||
this.searchContext.group?.name || this.searchContext?.group?.name ||
this.$options.i18n.allGitLab this.$options.i18n.allGitLab
); );
}, },
......
app/assets/javascripts/header_search/store/getters.js
View file @ 2e9739f1
...@@ -17,9 +17,10 @@ export const searchQuery = (state) => { ...@@ -17,9 +17,10 @@ export const searchQuery = (state) => {
{ {
search: state.search, search: state.search,
nav_source: 'navbar', nav_source: 'navbar',
project_id: state.searchContext.project?.id, project_id: state.searchContext?.project?.id,
group_id: state.searchContext.group?.id, group_id: state.searchContext?.group?.id,
scope: state.searchContext?.scope, scope: state.searchContext?.scope,
snippets: state.searchContext?.for_snippets ? true : null,
}, },
isNil, isNil,
); );
...@@ -31,7 +32,7 @@ export const autocompleteQuery = (state) => { ...@@ -31,7 +32,7 @@ export const autocompleteQuery = (state) => {
const query = omitBy( const query = omitBy(
{ {
term: state.search, term: state.search,
project_id: state.searchContext.project?.id, project_id: state.searchContext?.project?.id,
project_ref: state.searchContext?.ref, project_ref: state.searchContext?.ref,
}, },
isNil, isNil,
...@@ -42,16 +43,16 @@ export const autocompleteQuery = (state) => { ...@@ -42,16 +43,16 @@ export const autocompleteQuery = (state) => {
export const scopedIssuesPath = (state) => { export const scopedIssuesPath = (state) => {
return ( return (
state.searchContext.project_metadata?.issues_path || state.searchContext?.project_metadata?.issues_path ||
state.searchContext.group_metadata?.issues_path || state.searchContext?.group_metadata?.issues_path ||
state.issuesPath state.issuesPath
); );
}; };
export const scopedMRPath = (state) => { export const scopedMRPath = (state) => {
return ( return (
state.searchContext.project_metadata?.mr_path || state.searchContext?.project_metadata?.mr_path ||
state.searchContext.group_metadata?.mr_path || state.searchContext?.group_metadata?.mr_path ||
state.mrPath state.mrPath
); );
}; };
...@@ -96,6 +97,7 @@ export const projectUrl = (state) => { ...@@ -96,6 +97,7 @@ export const projectUrl = (state) => {
project_id: state.searchContext?.project?.id, project_id: state.searchContext?.project?.id,
group_id: state.searchContext?.group?.id, group_id: state.searchContext?.group?.id,
scope: state.searchContext?.scope, scope: state.searchContext?.scope,
snippets: state.searchContext?.for_snippets ? true : null,
}, },
isNil, isNil,
); );
...@@ -110,6 +112,7 @@ export const groupUrl = (state) => { ...@@ -110,6 +112,7 @@ export const groupUrl = (state) => {
nav_source: 'navbar', nav_source: 'navbar',
group_id: state.searchContext?.group?.id, group_id: state.searchContext?.group?.id,
scope: state.searchContext?.scope, scope: state.searchContext?.scope,
snippets: state.searchContext?.for_snippets ? true : null,
}, },
isNil, isNil,
); );
...@@ -123,6 +126,7 @@ export const allUrl = (state) => { ...@@ -123,6 +126,7 @@ export const allUrl = (state) => {
search: state.search, search: state.search,
nav_source: 'navbar', nav_source: 'navbar',
scope: state.searchContext?.scope, scope: state.searchContext?.scope,
snippets: state.searchContext?.for_snippets ? true : null,
}, },
isNil, isNil,
); );
...@@ -133,19 +137,19 @@ export const allUrl = (state) => { ...@@ -133,19 +137,19 @@ export const allUrl = (state) => {
export const scopedSearchOptions = (state, getters) => { export const scopedSearchOptions = (state, getters) => {
const options = []; const options = [];
if (state.searchContext.project) { if (state.searchContext?.project) {
options.push({ options.push({
html_id: 'scoped-in-project', html_id: 'scoped-in-project',
scope: state.searchContext.project.name, scope: state.searchContext?.project.name,
description: MSG_IN_PROJECT, description: MSG_IN_PROJECT,
url: getters.projectUrl, url: getters.projectUrl,
}); });
} }
if (state.searchContext.group) { if (state.searchContext?.group) {
options.push({ options.push({
html_id: 'scoped-in-group', html_id: 'scoped-in-group',
scope: state.searchContext.group.name, scope: state.searchContext?.group.name,
description: MSG_IN_GROUP, description: MSG_IN_GROUP,
url: getters.groupUrl, url: getters.groupUrl,
}); });
......
ee/spec/features/search/elastic/snippet_search_spec.rb
View file @ 2e9739f1
...@@ -6,8 +6,14 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si ...@@ -6,8 +6,14 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si
let(:public_project) { create(:project, :public) } let(:public_project) { create(:project, :public) }
let(:authorized_user) { create(:user) } let(:authorized_user) { create(:user) }
let(:authorized_project) { create(:project, namespace: authorized_user.namespace) } let(:authorized_project) { create(:project, namespace: authorized_user.namespace) }
let(:new_header_search) { false }
before do before do
# This feature is diabled by default in spec_helper.rb.
# We missed a feature breaking bug, so to prevent this regression, testing both scenarios for this spec.
# This can be removed as part of closing https://gitlab.com/gitlab-org/gitlab/-/issues/339348.
stub_feature_flags(new_header_search: new_header_search)
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true) stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
authorized_project.add_maintainer(authorized_user) authorized_project.add_maintainer(authorized_user)
...@@ -147,6 +153,27 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si ...@@ -147,6 +153,27 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si
end end
end end
context 'when :new_header_search is false' do
context 'when searching titles' do
before do
submit_search('snippet')
end
it_behaves_like 'expected snippet search results'
end
context 'when searching descriptions' do
before do
submit_search('snippet description')
end
it_behaves_like 'expected snippet search results'
end
end
context 'when :new_header_search is true' do
let(:new_header_search) { true }
context 'when searching titles' do context 'when searching titles' do
before do before do
submit_search('snippet') submit_search('snippet')
...@@ -162,4 +189,5 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si ...@@ -162,4 +189,5 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si
it_behaves_like 'expected snippet search results' it_behaves_like 'expected snippet search results'
end end
end
end end
spec/frontend/header_search/store/getters_spec.js
View file @ 2e9739f1
...@@ -37,20 +37,22 @@ describe('Header Search Store Getters', () => { ...@@ -37,20 +37,22 @@ describe('Header Search Store Getters', () => {
}); });
describe.each` describe.each`
group | project | scope | expectedPath group | project | scope | forSnippets | expectedPath
${null} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${null} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`} ${null} | ${null} | ${null} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&snippets=true`}
${null} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}`} ${MOCK_GROUP} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}`} ${null} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}&scope=issues`} ${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}`}
`('searchQuery', ({ group, project, scope, expectedPath }) => { ${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}&scope=issues&snippets=true`}
describe(`when group is ${group?.name}, project is ${project?.name}, and scope is ${scope}`, () => { `('searchQuery', ({ group, project, scope, forSnippets, expectedPath }) => {
describe(`when group is ${group?.name}, project is ${project?.name}, scope is ${scope}, and for_snippets is ${forSnippets}`, () => {
beforeEach(() => { beforeEach(() => {
createState({ createState({
searchContext: { searchContext: {
group, group,
project, project,
scope, scope,
for_snippets: forSnippets,
}, },
}); });
state.search = MOCK_SEARCH; state.search = MOCK_SEARCH;
...@@ -135,20 +137,22 @@ describe('Header Search Store Getters', () => { ...@@ -135,20 +137,22 @@ describe('Header Search Store Getters', () => {
}); });
describe.each` describe.each`
group | project | scope | expectedPath group | project | scope | forSnippets | expectedPath
${null} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${null} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`} ${null} | ${null} | ${null} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&snippets=true`}
${null} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}`} ${MOCK_GROUP} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}`} ${null} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}&scope=issues`} ${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}`}
`('projectUrl', ({ group, project, scope, expectedPath }) => { ${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&project_id=${MOCK_PROJECT.id}&group_id=${MOCK_GROUP.id}&scope=issues&snippets=true`}
describe(`when group is ${group?.name}, project is ${project?.name}, and scope is ${scope}`, () => { `('projectUrl', ({ group, project, scope, forSnippets, expectedPath }) => {
describe(`when group is ${group?.name}, project is ${project?.name}, scope is ${scope}, and for_snippets is ${forSnippets}`, () => {
beforeEach(() => { beforeEach(() => {
createState({ createState({
searchContext: { searchContext: {
group, group,
project, project,
scope, scope,
for_snippets: forSnippets,
}, },
}); });
state.search = MOCK_SEARCH; state.search = MOCK_SEARCH;
...@@ -161,20 +165,22 @@ describe('Header Search Store Getters', () => { ...@@ -161,20 +165,22 @@ describe('Header Search Store Getters', () => {
}); });
describe.each` describe.each`
group | project | scope | expectedPath group | project | scope | forSnippets | expectedPath
${null} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${null} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`} ${null} | ${null} | ${null} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&snippets=true`}
${null} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${MOCK_GROUP} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`} ${null} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}&scope=issues`} ${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}`}
`('groupUrl', ({ group, project, scope, expectedPath }) => { ${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&group_id=${MOCK_GROUP.id}&scope=issues&snippets=true`}
describe(`when group is ${group?.name}, project is ${project?.name}, and scope is ${scope}`, () => { `('groupUrl', ({ group, project, scope, forSnippets, expectedPath }) => {
describe(`when group is ${group?.name}, project is ${project?.name}, scope is ${scope}, and for_snippets is ${forSnippets}`, () => {
beforeEach(() => { beforeEach(() => {
createState({ createState({
searchContext: { searchContext: {
group, group,
project, project,
scope, scope,
for_snippets: forSnippets,
}, },
}); });
state.search = MOCK_SEARCH; state.search = MOCK_SEARCH;
...@@ -187,20 +193,22 @@ describe('Header Search Store Getters', () => { ...@@ -187,20 +193,22 @@ describe('Header Search Store Getters', () => {
}); });
describe.each` describe.each`
group | project | scope | expectedPath group | project | scope | forSnippets | expectedPath
${null} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${null} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${null} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${null} | ${null} | ${null} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&snippets=true`}
${null} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${MOCK_GROUP} | ${null} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`} ${null} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&scope=issues`} ${MOCK_GROUP} | ${MOCK_PROJECT} | ${null} | ${false} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar`}
`('allUrl', ({ group, project, scope, expectedPath }) => { ${MOCK_GROUP} | ${MOCK_PROJECT} | ${'issues'} | ${true} | ${`${MOCK_SEARCH_PATH}?search=${MOCK_SEARCH}&nav_source=navbar&scope=issues&snippets=true`}
describe(`when group is ${group?.name}, project is ${project?.name}, and scope is ${scope}`, () => { `('allUrl', ({ group, project, scope, forSnippets, expectedPath }) => {
describe(`when group is ${group?.name}, project is ${project?.name}, scope is ${scope}, and for_snippets is ${forSnippets}`, () => {
beforeEach(() => { beforeEach(() => {
createState({ createState({
searchContext: { searchContext: {
group, group,
project, project,
scope, scope,
for_snippets: forSnippets,
}, },
}); });
state.search = MOCK_SEARCH; state.search = MOCK_SEARCH;
......
spec/support/helpers/search_helpers.rb
View file @ 2e9739f1
...@@ -11,8 +11,12 @@ module SearchHelpers ...@@ -11,8 +11,12 @@ module SearchHelpers
end end
def submit_search(query) def submit_search(query)
page.within('.search-form, .search-page-form') do # Once the `new_header_search` feature flag has been removed
# We can remove the `.search-form` selector
# https://gitlab.com/gitlab-org/gitlab/-/issues/339348
page.within('.header-search, .search-form, .search-page-form') do
field = find_field('search') field = find_field('search')
field.click
field.fill_in(with: query) field.fill_in(with: query)
if javascript_test? if javascript_test?
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7