From 86d53b683ec30afb14ef983daea605bf0707e425 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 12 Jan 2021 17:45:37 +0100 Subject: [PATCH] [Search Sessions][Discover] Send to background integration improvements and fixes (#87311) --- .../public/application/angular/discover.js | 2 +- .../angular/discover_state.test.ts | 35 ++++++++++++++++++- .../application/angular/discover_state.ts | 27 +++++++++++--- .../discover/public/url_generator.test.ts | 2 +- src/plugins/discover/public/url_generator.ts | 2 +- x-pack/test/functional/apps/discover/index.ts | 1 - .../tests}/apps/discover/async_search.ts | 7 ++-- .../tests/apps/discover/index.ts | 1 + 8 files changed, 64 insertions(+), 13 deletions(-) rename x-pack/test/{functional => send_search_to_background_integration/tests}/apps/discover/async_search.ts (88%) diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index 4c5cb864b511..839add1aeb22 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -295,7 +295,7 @@ function discoverController($element, $route, $scope, $timeout, Promise, uiCapab createSearchSessionRestorationDataProvider({ appStateContainer, data, - getSavedSearchId: () => savedSearch.id, + getSavedSearch: () => savedSearch, }) ); diff --git a/src/plugins/discover/public/application/angular/discover_state.test.ts b/src/plugins/discover/public/application/angular/discover_state.test.ts index 2914ce8f17a0..e05a3028c5ca 100644 --- a/src/plugins/discover/public/application/angular/discover_state.test.ts +++ b/src/plugins/discover/public/application/angular/discover_state.test.ts @@ -17,8 +17,14 @@ * under the License. */ -import { getState, GetStateReturn } from './discover_state'; +import { + getState, + GetStateReturn, + createSearchSessionRestorationDataProvider, +} from './discover_state'; import { createBrowserHistory, History } from 'history'; +import { dataPluginMock } from '../../../../data/public/mocks'; +import { SavedSearch } from '../../saved_searches'; let history: History; let state: GetStateReturn; @@ -103,3 +109,30 @@ describe('Test discover state with legacy migration', () => { `); }); }); + +describe('createSearchSessionRestorationDataProvider', () => { + let mockSavedSearch: SavedSearch = ({} as unknown) as SavedSearch; + const searchSessionInfoProvider = createSearchSessionRestorationDataProvider({ + data: dataPluginMock.createStartContract(), + appStateContainer: getState({ + history: createBrowserHistory(), + }).appStateContainer, + getSavedSearch: () => mockSavedSearch, + }); + + describe('session name', () => { + test('No saved search returns default name', async () => { + expect(await searchSessionInfoProvider.getName()).toBe('Discover'); + }); + + test('Saved Search with a title returns saved search title', async () => { + mockSavedSearch = ({ id: 'id', title: 'Name' } as unknown) as SavedSearch; + expect(await searchSessionInfoProvider.getName()).toBe('Name'); + }); + + test('Saved Search without a title returns default name', async () => { + mockSavedSearch = ({ id: 'id', title: undefined } as unknown) as SavedSearch; + expect(await searchSessionInfoProvider.getName()).toBe('Discover'); + }); + }); +}); diff --git a/src/plugins/discover/public/application/angular/discover_state.ts b/src/plugins/discover/public/application/angular/discover_state.ts index 9bf3cc587a3d..69291872fa85 100644 --- a/src/plugins/discover/public/application/angular/discover_state.ts +++ b/src/plugins/discover/public/application/angular/discover_state.ts @@ -17,6 +17,7 @@ * under the License. */ import { isEqual } from 'lodash'; +import { i18n } from '@kbn/i18n'; import { History } from 'history'; import { NotificationsStart } from 'kibana/public'; import { @@ -38,6 +39,7 @@ import { import { migrateLegacyQuery } from '../helpers/migrate_legacy_query'; import { DiscoverGridSettings } from '../components/discover_grid/types'; import { DISCOVER_APP_URL_GENERATOR, DiscoverUrlGeneratorState } from '../../url_generator'; +import { SavedSearch } from '../../saved_searches'; export interface AppState { /** @@ -264,15 +266,32 @@ export function isEqualState(stateA: AppState, stateB: AppState) { export function createSearchSessionRestorationDataProvider(deps: { appStateContainer: StateContainer; data: DataPublicPluginStart; - getSavedSearchId: () => string | undefined; + getSavedSearch: () => SavedSearch; }): SearchSessionInfoProvider { + const getSavedSearchId = () => deps.getSavedSearch().id; return { - getName: async () => 'Discover', + getName: async () => { + const savedSearch = deps.getSavedSearch(); + return ( + (savedSearch.id && savedSearch.title) || + i18n.translate('discover.discoverDefaultSearchSessionName', { + defaultMessage: 'Discover', + }) + ); + }, getUrlGeneratorData: async () => { return { urlGeneratorId: DISCOVER_APP_URL_GENERATOR, - initialState: createUrlGeneratorState({ ...deps, forceAbsoluteTime: false }), - restoreState: createUrlGeneratorState({ ...deps, forceAbsoluteTime: true }), + initialState: createUrlGeneratorState({ + ...deps, + getSavedSearchId, + forceAbsoluteTime: false, + }), + restoreState: createUrlGeneratorState({ + ...deps, + getSavedSearchId, + forceAbsoluteTime: true, + }), }; }, }; diff --git a/src/plugins/discover/public/url_generator.test.ts b/src/plugins/discover/public/url_generator.test.ts index 95bff6b1fdc9..f87ef4f91108 100644 --- a/src/plugins/discover/public/url_generator.test.ts +++ b/src/plugins/discover/public/url_generator.test.ts @@ -62,7 +62,7 @@ describe('Discover url generator', () => { const url = await generator.createUrl({ savedSearchId }); const { _a, _g } = getStatesFromKbnUrl(url, ['_a', '_g']); - expect(url.startsWith(`${appBasePath}#/${savedSearchId}`)).toBe(true); + expect(url.startsWith(`${appBasePath}#/view/${savedSearchId}`)).toBe(true); expect(_a).toEqual({}); expect(_g).toEqual({}); }); diff --git a/src/plugins/discover/public/url_generator.ts b/src/plugins/discover/public/url_generator.ts index 6d86818910b1..d1b574c360d8 100644 --- a/src/plugins/discover/public/url_generator.ts +++ b/src/plugins/discover/public/url_generator.ts @@ -119,7 +119,7 @@ export class DiscoverUrlGenerator sort, interval, }: DiscoverUrlGeneratorState): Promise => { - const savedSearchPath = savedSearchId ? encodeURIComponent(savedSearchId) : ''; + const savedSearchPath = savedSearchId ? `view/${encodeURIComponent(savedSearchId)}` : ''; const appState: { query?: Query; filters?: Filter[]; diff --git a/x-pack/test/functional/apps/discover/index.ts b/x-pack/test/functional/apps/discover/index.ts index 13426da504bd..57599c82d8de 100644 --- a/x-pack/test/functional/apps/discover/index.ts +++ b/x-pack/test/functional/apps/discover/index.ts @@ -16,6 +16,5 @@ export default function ({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./error_handling')); loadTestFile(require.resolve('./visualize_field')); loadTestFile(require.resolve('./value_suggestions')); - loadTestFile(require.resolve('./async_search')); }); } diff --git a/x-pack/test/functional/apps/discover/async_search.ts b/x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts similarity index 88% rename from x-pack/test/functional/apps/discover/async_search.ts rename to x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts index 77d60f4036a3..d64df98c9860 100644 --- a/x-pack/test/functional/apps/discover/async_search.ts +++ b/x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import expect from '@kbn/expect'; -import { FtrProviderContext } from '../../ftr_provider_context'; +import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); @@ -31,14 +31,13 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { expect(searchSessionId2).not.to.be(searchSessionId1); }); - // NOTE: this test will be revised when - // `searchSessionId` functionality actually works - it('search session id should be picked up from the URL', async () => { + it('search session id should be picked up from the URL, non existing session id errors out', async () => { const url = await browser.getCurrentUrl(); const fakeSearchSessionId = '__test__'; const savedSessionURL = url + `&searchSessionId=${fakeSearchSessionId}`; await browser.navigateTo(savedSessionURL); await PageObjects.header.waitUntilLoadingHasFinished(); + await testSubjects.existOrFail('discoverNoResultsError'); // expect error because of fake searchSessionId const searchSessionId1 = await getSearchSessionId(); expect(searchSessionId1).to.be(fakeSearchSessionId); await queryBar.clickQuerySubmitButton(); diff --git a/x-pack/test/send_search_to_background_integration/tests/apps/discover/index.ts b/x-pack/test/send_search_to_background_integration/tests/apps/discover/index.ts index b4fc74072ce0..d4f254e552dc 100644 --- a/x-pack/test/send_search_to_background_integration/tests/apps/discover/index.ts +++ b/x-pack/test/send_search_to_background_integration/tests/apps/discover/index.ts @@ -17,6 +17,7 @@ export default function ({ loadTestFile, getService }: FtrProviderContext) { await kibanaServer.uiSettings.replace({ defaultIndex: 'logstash-*' }); }); + loadTestFile(require.resolve('./async_search')); loadTestFile(require.resolve('./sessions_in_space')); }); }