From bba95e82696f97a0afe8f1fb3b12a6aec0b3d36b Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Thu, 28 Jan 2021 11:44:04 +0100 Subject: [PATCH] [Search Sessions] Improve session restoration back button (#87635) (#89532) --- ...c-state_sync.ikbnurlstatestorage.cancel.md | 13 --- ...ic-state_sync.ikbnurlstatestorage.flush.md | 15 --- ...sync.ikbnurlstatestorage.kbnurlcontrols.md | 13 +++ ...s-public-state_sync.ikbnurlstatestorage.md | 3 +- .../public/application/dashboard_app.tsx | 96 ++++++++++++++----- .../application/dashboard_state_manager.ts | 4 +- .../dashboard_listing.test.tsx.snap | 60 +++++++++--- .../state_sync/sync_state_with_url.test.ts | 4 +- .../application/angular/context_state.ts | 2 +- .../public/application/angular/discover.js | 58 ++++++----- .../angular/discover_search_session.test.ts | 96 +++++++++++++++++++ .../angular/discover_search_session.ts | 85 ++++++++++++++++ .../application/angular/discover_state.ts | 2 +- .../state_sync/storages/kbn_url_storage.md | 8 +- .../public/history/history_observable.test.ts | 89 +++++++++++++++++ .../public/history/history_observable.ts | 60 ++++++++++++ .../kibana_utils/public/history/index.ts | 1 + src/plugins/kibana_utils/public/index.ts | 9 +- .../public/state_sync/public.api.md | 6 +- .../public/state_sync/state_sync.test.ts | 4 +- .../create_kbn_url_state_storage.test.ts | 12 +-- .../create_kbn_url_state_storage.ts | 19 +--- test/functional/apps/discover/_discover.ts | 4 +- .../test_suites/data_plugin/session.ts | 5 +- .../routes/map_page/url_state/global_sync.ts | 2 +- .../async_search/send_to_background.ts | 18 +++- .../tests/apps/discover/async_search.ts | 20 +++- 27 files changed, 560 insertions(+), 148 deletions(-) delete mode 100644 docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md delete mode 100644 docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md create mode 100644 docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md create mode 100644 src/plugins/discover/public/application/angular/discover_search_session.test.ts create mode 100644 src/plugins/discover/public/application/angular/discover_search_session.ts create mode 100644 src/plugins/kibana_utils/public/history/history_observable.test.ts create mode 100644 src/plugins/kibana_utils/public/history/history_observable.ts diff --git a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md b/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md deleted file mode 100644 index 29a511d57d7b..000000000000 --- a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md +++ /dev/null @@ -1,13 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-plugins-kibana\_utils-public-state\_sync](./kibana-plugin-plugins-kibana_utils-public-state_sync.md) > [IKbnUrlStateStorage](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md) > [cancel](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md) - -## IKbnUrlStateStorage.cancel property - -cancels any pending url updates - -Signature: - -```typescript -cancel: () => void; -``` diff --git a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md b/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md deleted file mode 100644 index dfeef1cdce22..000000000000 --- a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md +++ /dev/null @@ -1,15 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-plugins-kibana\_utils-public-state\_sync](./kibana-plugin-plugins-kibana_utils-public-state_sync.md) > [IKbnUrlStateStorage](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md) > [flush](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md) - -## IKbnUrlStateStorage.flush property - -Synchronously runs any pending url updates, returned boolean indicates if change occurred. - -Signature: - -```typescript -flush: (opts?: { - replace?: boolean; - }) => boolean; -``` diff --git a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md b/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md new file mode 100644 index 000000000000..8e3b9a7bfeb3 --- /dev/null +++ b/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-kibana\_utils-public-state\_sync](./kibana-plugin-plugins-kibana_utils-public-state_sync.md) > [IKbnUrlStateStorage](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md) > [kbnUrlControls](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md) + +## IKbnUrlStateStorage.kbnUrlControls property + +Lower level wrapper around history library that handles batching multiple URL updates into one history change + +Signature: + +```typescript +kbnUrlControls: IKbnUrlControls; +``` diff --git a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md b/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md index 371f7b7c1536..7fb8717fae00 100644 --- a/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md +++ b/docs/development/plugins/kibana_utils/public/state_sync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md @@ -20,9 +20,8 @@ export interface IKbnUrlStateStorage extends IStateStorage | Property | Type | Description | | --- | --- | --- | -| [cancel](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md) | () => void | cancels any pending url updates | | [change$](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.change_.md) | <State = unknown>(key: string) => Observable<State | null> | | -| [flush](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md) | (opts?: {
replace?: boolean;
}) => boolean | Synchronously runs any pending url updates, returned boolean indicates if change occurred. | | [get](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.get.md) | <State = unknown>(key: string) => State | null | | +| [kbnUrlControls](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md) | IKbnUrlControls | Lower level wrapper around history library that handles batching multiple URL updates into one history change | | [set](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.set.md) | <State>(key: string, state: State, opts?: {
replace: boolean;
}) => Promise<string | undefined> | | diff --git a/src/plugins/dashboard/public/application/dashboard_app.tsx b/src/plugins/dashboard/public/application/dashboard_app.tsx index e1e2a49439de..7ea181715717 100644 --- a/src/plugins/dashboard/public/application/dashboard_app.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app.tsx @@ -6,22 +6,22 @@ * Public License, v 1. */ -import _ from 'lodash'; import { History } from 'history'; -import { merge, Subscription } from 'rxjs'; -import React, { useEffect, useCallback, useState } from 'react'; +import { merge, Subject, Subscription } from 'rxjs'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { debounceTime, tap } from 'rxjs/operators'; import { useKibana } from '../../../kibana_react/public'; import { DashboardConstants } from '../dashboard_constants'; import { DashboardTopNav } from './top_nav/dashboard_top_nav'; import { DashboardAppServices, DashboardEmbedSettings, DashboardRedirect } from './types'; import { + getChangesFromAppStateForContainerState, + getDashboardContainerInput, + getFiltersSubscription, getInputSubscription, getOutputSubscription, - getFiltersSubscription, getSearchSessionIdFromURL, - getDashboardContainerInput, - getChangesFromAppStateForContainerState, } from './dashboard_app_functions'; import { useDashboardBreadcrumbs, @@ -30,11 +30,11 @@ import { useSavedDashboard, } from './hooks'; -import { removeQueryParam } from '../services/kibana_utils'; import { IndexPattern } from '../services/data'; import { EmbeddableRenderer } from '../services/embeddable'; import { DashboardContainerInput } from '.'; import { leaveConfirmStrings } from '../dashboard_strings'; +import { createQueryParamObservable, replaceUrlHashQuery } from '../../../kibana_utils/public'; export interface DashboardAppProps { history: History; @@ -59,7 +59,7 @@ export function DashboardApp({ indexPatterns: indexPatternService, } = useKibana().services; - const [lastReloadTime, setLastReloadTime] = useState(0); + const triggerRefresh$ = useMemo(() => new Subject<{ force?: boolean }>(), []); const [indexPatterns, setIndexPatterns] = useState([]); const savedDashboard = useSavedDashboard(savedDashboardId, history); @@ -68,9 +68,13 @@ export function DashboardApp({ history ); const dashboardContainer = useDashboardContainer(dashboardStateManager, history, false); + const searchSessionIdQuery$ = useMemo( + () => createQueryParamObservable(history, DashboardConstants.SEARCH_SESSION_ID), + [history] + ); const refreshDashboardContainer = useCallback( - (lastReloadRequestTime?: number) => { + (force?: boolean) => { if (!dashboardContainer || !dashboardStateManager) { return; } @@ -80,7 +84,7 @@ export function DashboardApp({ appStateDashboardInput: getDashboardContainerInput({ isEmbeddedExternally: Boolean(embedSettings), dashboardStateManager, - lastReloadRequestTime, + lastReloadRequestTime: force ? Date.now() : undefined, dashboardCapabilities, query: data.query, }), @@ -100,10 +104,35 @@ export function DashboardApp({ const shouldRefetch = Object.keys(changes).some( (changeKey) => !noRefetchKeys.includes(changeKey as keyof DashboardContainerInput) ); - if (getSearchSessionIdFromURL(history)) { - // going away from a background search results - removeQueryParam(history, DashboardConstants.SEARCH_SESSION_ID, true); - } + + const newSearchSessionId: string | undefined = (() => { + // do not update session id if this is irrelevant state change to prevent excessive searches + if (!shouldRefetch) return; + + let searchSessionIdFromURL = getSearchSessionIdFromURL(history); + if (searchSessionIdFromURL) { + if ( + data.search.session.isRestore() && + data.search.session.isCurrentSession(searchSessionIdFromURL) + ) { + // navigating away from a restored session + dashboardStateManager.kbnUrlStateStorage.kbnUrlControls.updateAsync((nextUrl) => { + if (nextUrl.includes(DashboardConstants.SEARCH_SESSION_ID)) { + return replaceUrlHashQuery(nextUrl, (query) => { + delete query[DashboardConstants.SEARCH_SESSION_ID]; + return query; + }); + } + return nextUrl; + }); + searchSessionIdFromURL = undefined; + } else { + data.search.session.restore(searchSessionIdFromURL); + } + } + + return searchSessionIdFromURL ?? data.search.session.start(); + })(); if (changes.viewMode) { setViewMode(changes.viewMode); @@ -111,8 +140,7 @@ export function DashboardApp({ dashboardContainer.updateInput({ ...changes, - // do not start a new session if this is irrelevant state change to prevent excessive searches - ...(shouldRefetch && { searchSessionId: data.search.session.start() }), + ...(newSearchSessionId && { searchSessionId: newSearchSessionId }), }); } }, @@ -159,23 +187,42 @@ export function DashboardApp({ subscriptions.add( merge( ...[timeFilter.getRefreshIntervalUpdate$(), timeFilter.getTimeUpdate$()] - ).subscribe(() => refreshDashboardContainer()) + ).subscribe(() => triggerRefresh$.next()) ); + subscriptions.add( merge( data.search.session.onRefresh$, - data.query.timefilter.timefilter.getAutoRefreshFetch$() + data.query.timefilter.timefilter.getAutoRefreshFetch$(), + searchSessionIdQuery$ ).subscribe(() => { - setLastReloadTime(() => new Date().getTime()); + triggerRefresh$.next({ force: true }); }) ); dashboardStateManager.registerChangeListener(() => { // we aren't checking dirty state because there are changes the container needs to know about // that won't make the dashboard "dirty" - like a view mode change. - refreshDashboardContainer(); + triggerRefresh$.next(); }); + // debounce `refreshDashboardContainer()` + // use `forceRefresh=true` in case at least one debounced trigger asked for it + let forceRefresh: boolean = false; + subscriptions.add( + triggerRefresh$ + .pipe( + tap((trigger) => { + forceRefresh = forceRefresh || (trigger?.force ?? false); + }), + debounceTime(50) + ) + .subscribe(() => { + refreshDashboardContainer(forceRefresh); + forceRefresh = false; + }) + ); + return () => { subscriptions.unsubscribe(); }; @@ -187,6 +234,8 @@ export function DashboardApp({ data.search.session, indexPatternService, dashboardStateManager, + searchSessionIdQuery$, + triggerRefresh$, refreshDashboardContainer, ]); @@ -216,11 +265,6 @@ export function DashboardApp({ }; }, [dashboardStateManager, dashboardContainer, onAppLeave, embeddable]); - // Refresh the dashboard container when lastReloadTime changes - useEffect(() => { - refreshDashboardContainer(lastReloadTime); - }, [lastReloadTime, refreshDashboardContainer]); - return (
{savedDashboard && dashboardStateManager && dashboardContainer && viewMode && ( @@ -242,7 +286,7 @@ export function DashboardApp({ // The user can still request a reload in the query bar, even if the // query is the same, and in that case, we have to explicitly ask for // a reload, since no state changes will cause it. - setLastReloadTime(() => new Date().getTime()); + triggerRefresh$.next({ force: true }); } }} /> diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 90706a11b8ce..c52bd1b4d47b 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -72,7 +72,7 @@ export class DashboardStateManager { >; private readonly stateContainerChangeSub: Subscription; private readonly STATE_STORAGE_KEY = '_a'; - private readonly kbnUrlStateStorage: IKbnUrlStateStorage; + public readonly kbnUrlStateStorage: IKbnUrlStateStorage; private readonly stateSyncRef: ISyncStateRef; private readonly history: History; private readonly usageCollection: UsageCollectionSetup | undefined; @@ -596,7 +596,7 @@ export class DashboardStateManager { this.toUrlState(this.stateContainer.get()) ); // immediately forces scheduled updates and changes location - return this.kbnUrlStateStorage.flush({ replace }); + return !!this.kbnUrlStateStorage.kbnUrlControls.flush(replace); } // TODO: find nicer solution for this diff --git a/src/plugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.tsx.snap b/src/plugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.tsx.snap index bce8a661634f..faec6b4f6f24 100644 --- a/src/plugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.tsx.snap +++ b/src/plugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.tsx.snap @@ -4,10 +4,16 @@ exports[`after fetch When given a title that matches multiple dashboards, filter { test('url is actually changed when data in services changes', () => { const { stop } = syncQueryStateWithUrl(queryServiceStart, kbnUrlStateStorage); filterManager.setFilters([gF, aF]); - kbnUrlStateStorage.flush(); // sync force location change + kbnUrlStateStorage.kbnUrlControls.flush(); // sync force location change expect(history.location.hash).toMatchInlineSnapshot( `"#?_g=(filters:!(('$state':(store:globalState),meta:(alias:!n,disabled:!t,index:'logstash-*',key:query,negate:!t,type:custom,value:'%7B%22match%22:%7B%22key1%22:%22value1%22%7D%7D'),query:(match:(key1:value1)))),refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))"` ); @@ -126,7 +126,7 @@ describe('sync_query_state_with_url', () => { test('when url is changed, filters synced back to filterManager', () => { const { stop } = syncQueryStateWithUrl(queryServiceStart, kbnUrlStateStorage); - kbnUrlStateStorage.cancel(); // stop initial syncing pending update + kbnUrlStateStorage.kbnUrlControls.cancel(); // stop initial syncing pending update history.push(pathWithFilter); expect(filterManager.getGlobalFilters()).toHaveLength(1); stop(); diff --git a/src/plugins/discover/public/application/angular/context_state.ts b/src/plugins/discover/public/application/angular/context_state.ts index 73523b218df7..e8c2f1d397ba 100644 --- a/src/plugins/discover/public/application/angular/context_state.ts +++ b/src/plugins/discover/public/application/angular/context_state.ts @@ -206,7 +206,7 @@ export function getState({ } }, // helper function just needed for testing - flushToUrl: (replace?: boolean) => stateStorage.flush({ replace }), + flushToUrl: (replace?: boolean) => stateStorage.kbnUrlControls.flush(replace), }; } diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index 5c26680c7cc4..d492d7438695 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -48,8 +48,6 @@ import { popularizeField } from '../helpers/popularize_field'; import { getSwitchIndexPatternAppState } from '../helpers/get_switch_index_pattern_app_state'; import { addFatalError } from '../../../../kibana_legacy/public'; import { METRIC_TYPE } from '@kbn/analytics'; -import { SEARCH_SESSION_ID_QUERY_PARAM } from '../../url_generator'; -import { getQueryParams, removeQueryParam } from '../../../../kibana_utils/public'; import { DEFAULT_COLUMNS_SETTING, MODIFY_COLUMNS_ON_SWITCH, @@ -63,6 +61,7 @@ import { getTopNavLinks } from '../components/top_nav/get_top_nav_links'; import { updateSearchSource } from '../helpers/update_search_source'; import { calcFieldCounts } from '../helpers/calc_field_counts'; import { getDefaultSort } from './doc_table/lib/get_default_sort'; +import { DiscoverSearchSessionManager } from './discover_search_session'; const services = getServices(); @@ -87,9 +86,6 @@ const fetchStatuses = { ERROR: 'error', }; -const getSearchSessionIdFromURL = (history) => - getQueryParams(history.location)[SEARCH_SESSION_ID_QUERY_PARAM]; - const app = getAngularModule(); app.config(($routeProvider) => { @@ -180,7 +176,9 @@ function discoverController($route, $scope, Promise) { const { isDefault: isDefaultType } = indexPatternsUtils; const subscriptions = new Subscription(); const refetch$ = new Subject(); + let inspectorRequest; + let isChangingIndexPattern = false; const savedSearch = $route.current.locals.savedObjects.savedSearch; $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPattern( @@ -198,15 +196,10 @@ function discoverController($route, $scope, Promise) { }; const history = getHistory(); - // used for restoring a search session - let isInitialSearch = true; - - // search session requested a data refresh - subscriptions.add( - data.search.session.onRefresh$.subscribe(() => { - refetch$.next(); - }) - ); + const searchSessionManager = new DiscoverSearchSessionManager({ + history, + session: data.search.session, + }); const state = getState({ getStateDefaults, @@ -258,6 +251,7 @@ function discoverController($route, $scope, Promise) { $scope.$evalAsync(async () => { if (oldStatePartial.index !== newStatePartial.index) { //in case of index pattern switch the route has currently to be reloaded, legacy + isChangingIndexPattern = true; $route.reload(); return; } @@ -354,7 +348,12 @@ function discoverController($route, $scope, Promise) { if (abortController) abortController.abort(); savedSearch.destroy(); subscriptions.unsubscribe(); - data.search.session.clear(); + if (!isChangingIndexPattern) { + // HACK: + // do not clear session when changing index pattern due to how state management around it is setup + // it will be cleared by searchSessionManager on controller reload instead + data.search.session.clear(); + } appStateUnsubscribe(); stopStateSync(); stopSyncingGlobalStateWithUrl(); @@ -478,7 +477,8 @@ function discoverController($route, $scope, Promise) { return ( config.get(SEARCH_ON_PAGE_LOAD_SETTING) || savedSearch.id !== undefined || - timefilter.getRefreshInterval().pause === false + timefilter.getRefreshInterval().pause === false || + searchSessionManager.hasSearchSessionIdInURL() ); }; @@ -489,7 +489,8 @@ function discoverController($route, $scope, Promise) { filterManager.getFetches$(), timefilter.getFetch$(), timefilter.getAutoRefreshFetch$(), - data.query.queryString.getUpdates$() + data.query.queryString.getUpdates$(), + searchSessionManager.newSearchSessionIdFromURL$ ).pipe(debounceTime(100)); subscriptions.add( @@ -515,6 +516,13 @@ function discoverController($route, $scope, Promise) { ) ); + subscriptions.add( + data.search.session.onRefresh$.subscribe(() => { + searchSessionManager.removeSearchSessionIdFromURL({ replace: false }); + refetch$.next(); + }) + ); + $scope.changeInterval = (interval) => { if (interval) { setAppState({ interval }); @@ -594,20 +602,7 @@ function discoverController($route, $scope, Promise) { if (abortController) abortController.abort(); abortController = new AbortController(); - const searchSessionId = (() => { - const searchSessionIdFromURL = getSearchSessionIdFromURL(history); - if (searchSessionIdFromURL) { - if (isInitialSearch) { - data.search.session.restore(searchSessionIdFromURL); - isInitialSearch = false; - return searchSessionIdFromURL; - } else { - // navigating away from background search - removeQueryParam(history, SEARCH_SESSION_ID_QUERY_PARAM); - } - } - return data.search.session.start(); - })(); + const searchSessionId = searchSessionManager.getNextSearchSessionId(); $scope .updateDataSource() @@ -634,6 +629,7 @@ function discoverController($route, $scope, Promise) { $scope.handleRefresh = function (_payload, isUpdate) { if (isUpdate === false) { + searchSessionManager.removeSearchSessionIdFromURL({ replace: false }); refetch$.next(); } }; diff --git a/src/plugins/discover/public/application/angular/discover_search_session.test.ts b/src/plugins/discover/public/application/angular/discover_search_session.test.ts new file mode 100644 index 000000000000..abec6aedeaf5 --- /dev/null +++ b/src/plugins/discover/public/application/angular/discover_search_session.test.ts @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import { DiscoverSearchSessionManager } from './discover_search_session'; +import { createMemoryHistory } from 'history'; +import { dataPluginMock } from '../../../../data/public/mocks'; +import { DataPublicPluginStart } from '../../../../data/public'; + +describe('DiscoverSearchSessionManager', () => { + const history = createMemoryHistory(); + const session = dataPluginMock.createStartContract().search.session as jest.Mocked< + DataPublicPluginStart['search']['session'] + >; + const searchSessionManager = new DiscoverSearchSessionManager({ + history, + session, + }); + + beforeEach(() => { + history.push('/'); + session.start.mockReset(); + session.restore.mockReset(); + session.getSessionId.mockReset(); + session.isCurrentSession.mockReset(); + session.isRestore.mockReset(); + }); + + describe('getNextSearchSessionId', () => { + test('starts a new session', () => { + const nextId = 'id'; + session.start.mockImplementationOnce(() => nextId); + + const id = searchSessionManager.getNextSearchSessionId(); + expect(id).toEqual(nextId); + expect(session.start).toBeCalled(); + }); + + test('restores a session using query param from the URL', () => { + const nextId = 'id_from_url'; + history.push(`/?searchSessionId=${nextId}`); + + const id = searchSessionManager.getNextSearchSessionId(); + expect(id).toEqual(nextId); + expect(session.restore).toBeCalled(); + }); + + test('removes query param from the URL when navigating away from a restored session', () => { + const idFromUrl = 'id_from_url'; + history.push(`/?searchSessionId=${idFromUrl}`); + + const nextId = 'id'; + session.start.mockImplementationOnce(() => nextId); + session.isCurrentSession.mockImplementationOnce(() => true); + session.isRestore.mockImplementationOnce(() => true); + + const id = searchSessionManager.getNextSearchSessionId(); + expect(id).toEqual(nextId); + expect(session.start).toBeCalled(); + expect(history.location.search).toMatchInlineSnapshot(`""`); + }); + }); + + describe('newSearchSessionIdFromURL$', () => { + test('notifies about searchSessionId changes in the URL', () => { + const emits: Array = []; + + const sub = searchSessionManager.newSearchSessionIdFromURL$.subscribe((newId) => { + emits.push(newId); + }); + + history.push(`/?searchSessionId=id1`); + history.push(`/?searchSessionId=id1`); + session.isCurrentSession.mockImplementationOnce(() => true); + history.replace(`/?searchSessionId=id2`); // should skip current this + history.replace(`/`); + history.push(`/?searchSessionId=id1`); + history.push(`/`); + + expect(emits).toMatchInlineSnapshot(` + Array [ + "id1", + null, + "id1", + null, + ] + `); + + sub.unsubscribe(); + }); + }); +}); diff --git a/src/plugins/discover/public/application/angular/discover_search_session.ts b/src/plugins/discover/public/application/angular/discover_search_session.ts new file mode 100644 index 000000000000..a53d7d6d2c33 --- /dev/null +++ b/src/plugins/discover/public/application/angular/discover_search_session.ts @@ -0,0 +1,85 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import { History } from 'history'; +import { filter } from 'rxjs/operators'; +import { DataPublicPluginStart } from '../../../../data/public'; +import { + createQueryParamObservable, + getQueryParams, + removeQueryParam, +} from '../../../../kibana_utils/public'; +import { SEARCH_SESSION_ID_QUERY_PARAM } from '../../url_generator'; + +export interface DiscoverSearchSessionManagerDeps { + history: History; + session: DataPublicPluginStart['search']['session']; +} + +/** + * Helps with state management of search session and {@link SEARCH_SESSION_ID_QUERY_PARAM} in the URL + */ +export class DiscoverSearchSessionManager { + /** + * Notifies about `searchSessionId` changes in the URL, + * skips if `searchSessionId` matches current search session id + */ + readonly newSearchSessionIdFromURL$ = createQueryParamObservable( + this.deps.history, + SEARCH_SESSION_ID_QUERY_PARAM + ).pipe( + filter((searchSessionId) => { + if (!searchSessionId) return true; + return !this.deps.session.isCurrentSession(searchSessionId); + }) + ); + + constructor(private readonly deps: DiscoverSearchSessionManagerDeps) {} + + /** + * Get next session id by either starting or restoring a session. + * When navigating away from the restored session {@link SEARCH_SESSION_ID_QUERY_PARAM} is removed from the URL using history.replace + */ + getNextSearchSessionId() { + let searchSessionIdFromURL = this.getSearchSessionIdFromURL(); + if (searchSessionIdFromURL) { + if ( + this.deps.session.isRestore() && + this.deps.session.isCurrentSession(searchSessionIdFromURL) + ) { + // navigating away from a restored session + this.removeSearchSessionIdFromURL({ replace: true }); + searchSessionIdFromURL = undefined; + } else { + this.deps.session.restore(searchSessionIdFromURL); + } + } + + return searchSessionIdFromURL ?? this.deps.session.start(); + } + + /** + * Removes Discovers {@link SEARCH_SESSION_ID_QUERY_PARAM} from the URL + * @param replace - methods to change the URL + */ + removeSearchSessionIdFromURL({ replace = true }: { replace?: boolean } = { replace: true }) { + if (this.hasSearchSessionIdInURL()) { + removeQueryParam(this.deps.history, SEARCH_SESSION_ID_QUERY_PARAM, replace); + } + } + + /** + * If there is a {@link SEARCH_SESSION_ID_QUERY_PARAM} currently in the URL + */ + hasSearchSessionIdInURL(): boolean { + return !!this.getSearchSessionIdFromURL(); + } + + private getSearchSessionIdFromURL = () => + getQueryParams(this.deps.history.location)[SEARCH_SESSION_ID_QUERY_PARAM] as string | undefined; +} diff --git a/src/plugins/discover/public/application/angular/discover_state.ts b/src/plugins/discover/public/application/angular/discover_state.ts index c769e263655a..65a8dded1109 100644 --- a/src/plugins/discover/public/application/angular/discover_state.ts +++ b/src/plugins/discover/public/application/angular/discover_state.ts @@ -200,7 +200,7 @@ export function getState({ setState(appStateContainerModified, defaultState); }, getPreviousAppState: () => previousAppState, - flushToUrl: () => stateStorage.flush(), + flushToUrl: () => stateStorage.kbnUrlControls.flush(), isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()), }; } diff --git a/src/plugins/kibana_utils/docs/state_sync/storages/kbn_url_storage.md b/src/plugins/kibana_utils/docs/state_sync/storages/kbn_url_storage.md index ec27895eed66..36c7d7119ffe 100644 --- a/src/plugins/kibana_utils/docs/state_sync/storages/kbn_url_storage.md +++ b/src/plugins/kibana_utils/docs/state_sync/storages/kbn_url_storage.md @@ -96,11 +96,11 @@ setTimeout(() => { }, 0); ``` -For cases, where granular control over URL updates is needed, `kbnUrlStateStorage` provides these advanced apis: +For cases, where granular control over URL updates is needed, `kbnUrlStateStorage` exposes `kbnUrlStateStorage.kbnUrlControls` that exposes these advanced apis: -- `kbnUrlStateStorage.flush({replace: boolean})` - allows to synchronously apply any pending updates. - `replace` option allows to use `history.replace()` instead of `history.push()`. Returned boolean indicates if any update happened -- `kbnUrlStateStorage.cancel()` - cancels any pending updates +- `kbnUrlStateStorage.kbnUrlControls.flush({replace: boolean})` - allows to synchronously apply any pending updates. + `replace` option allows using `history.replace()` instead of `history.push()`. +- `kbnUrlStateStorage.kbnUrlControls.cancel()` - cancels any pending updates. ### Sharing one `kbnUrlStateStorage` instance diff --git a/src/plugins/kibana_utils/public/history/history_observable.test.ts b/src/plugins/kibana_utils/public/history/history_observable.test.ts new file mode 100644 index 000000000000..818c0d773928 --- /dev/null +++ b/src/plugins/kibana_utils/public/history/history_observable.test.ts @@ -0,0 +1,89 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import { + createHistoryObservable, + createQueryParamObservable, + createQueryParamsObservable, +} from './history_observable'; +import { createMemoryHistory, History } from 'history'; +import { ParsedQuery } from 'query-string'; + +let history: History; + +beforeEach(() => { + history = createMemoryHistory(); +}); + +test('createHistoryObservable', () => { + const obs$ = createHistoryObservable(history); + const emits: string[] = []; + obs$.subscribe(({ location }) => { + emits.push(location.pathname + location.search); + }); + + history.push('/test'); + history.push('/'); + + expect(emits.length).toEqual(2); + expect(emits).toMatchInlineSnapshot(` + Array [ + "/test", + "/", + ] + `); +}); + +test('createQueryParamsObservable', () => { + const obs$ = createQueryParamsObservable(history); + const emits: ParsedQuery[] = []; + obs$.subscribe((params) => { + emits.push(params); + }); + + history.push('/test'); + history.push('/test?foo=bar'); + history.push('/?foo=bar'); + history.push('/test?foo=bar&foo1=bar1'); + + expect(emits.length).toEqual(2); + expect(emits).toMatchInlineSnapshot(` + Array [ + Object { + "foo": "bar", + }, + Object { + "foo": "bar", + "foo1": "bar1", + }, + ] + `); +}); + +test('createQueryParamObservable', () => { + const obs$ = createQueryParamObservable(history, 'foo'); + const emits: unknown[] = []; + obs$.subscribe((param) => { + emits.push(param); + }); + + history.push('/test'); + history.push('/test?foo=bar'); + history.push('/?foo=bar'); + history.push('/test?foo=baaaar&foo1=bar1'); + history.push('/test?foo1=bar1'); + + expect(emits.length).toEqual(3); + expect(emits).toMatchInlineSnapshot(` + Array [ + "bar", + "baaaar", + null, + ] + `); +}); diff --git a/src/plugins/kibana_utils/public/history/history_observable.ts b/src/plugins/kibana_utils/public/history/history_observable.ts new file mode 100644 index 000000000000..f02a5e340b1a --- /dev/null +++ b/src/plugins/kibana_utils/public/history/history_observable.ts @@ -0,0 +1,60 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. + */ + +import { Action, History, Location } from 'history'; +import { Observable } from 'rxjs'; +import { ParsedQuery } from 'query-string'; +import deepEqual from 'fast-deep-equal'; +import { map } from 'rxjs/operators'; +import { getQueryParams } from './get_query_params'; +import { distinctUntilChangedWithInitialValue } from '../../common'; + +/** + * Convert history.listen into an observable + * @param history - {@link History} instance + */ +export function createHistoryObservable( + history: History +): Observable<{ location: Location; action: Action }> { + return new Observable((observer) => { + const unlisten = history.listen((location, action) => observer.next({ location, action })); + return () => { + unlisten(); + }; + }); +} + +/** + * Create an observable that emits every time any of query params change. + * Uses deepEqual check. + * @param history - {@link History} instance + */ +export function createQueryParamsObservable(history: History): Observable { + return createHistoryObservable(history).pipe( + map(({ location }) => ({ ...getQueryParams(location) })), + distinctUntilChangedWithInitialValue({ ...getQueryParams(history.location) }, deepEqual) + ); +} + +/** + * Create an observable that emits every time _paramKey_ changes + * @param history - {@link History} instance + * @param paramKey - query param key to observe + */ +export function createQueryParamObservable( + history: History, + paramKey: string +): Observable { + return createQueryParamsObservable(history).pipe( + map((params) => (params[paramKey] ?? null) as Param | null), + distinctUntilChangedWithInitialValue( + (getQueryParams(history.location)[paramKey] ?? null) as Param | null, + deepEqual + ) + ); +} diff --git a/src/plugins/kibana_utils/public/history/index.ts b/src/plugins/kibana_utils/public/history/index.ts index 4b1b610d560e..b2ac9ed6c739 100644 --- a/src/plugins/kibana_utils/public/history/index.ts +++ b/src/plugins/kibana_utils/public/history/index.ts @@ -9,3 +9,4 @@ export { removeQueryParam } from './remove_query_param'; export { redirectWhenMissing } from './redirect_when_missing'; export { getQueryParams } from './get_query_params'; +export * from './history_observable'; diff --git a/src/plugins/kibana_utils/public/index.ts b/src/plugins/kibana_utils/public/index.ts index fa9cf5a52371..29936da0117c 100644 --- a/src/plugins/kibana_utils/public/index.ts +++ b/src/plugins/kibana_utils/public/index.ts @@ -68,7 +68,14 @@ export { StopSyncStateFnType, } from './state_sync'; export { Configurable, CollectConfigProps } from './ui'; -export { removeQueryParam, redirectWhenMissing, getQueryParams } from './history'; +export { + removeQueryParam, + redirectWhenMissing, + getQueryParams, + createQueryParamsObservable, + createHistoryObservable, + createQueryParamObservable, +} from './history'; export { applyDiff } from './state_management/utils/diff_object'; export { createStartServicesGetter, StartServicesGetter } from './core/create_start_service_getter'; diff --git a/src/plugins/kibana_utils/public/state_sync/public.api.md b/src/plugins/kibana_utils/public/state_sync/public.api.md index a4dfea82cdb5..5524563c034a 100644 --- a/src/plugins/kibana_utils/public/state_sync/public.api.md +++ b/src/plugins/kibana_utils/public/state_sync/public.api.md @@ -22,14 +22,12 @@ export const createSessionStorageStateStorage: (storage?: Storage) => ISessionSt // @public export interface IKbnUrlStateStorage extends IStateStorage { - cancel: () => void; // (undocumented) change$: (key: string) => Observable; - flush: (opts?: { - replace?: boolean; - }) => boolean; // (undocumented) get: (key: string) => State | null; + // Warning: (ae-forgotten-export) The symbol "IKbnUrlControls" needs to be exported by the entry point index.d.ts + kbnUrlControls: IKbnUrlControls; // (undocumented) set: (key: string, state: State, opts?: { replace: boolean; diff --git a/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts b/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts index c7f04bc9cdbe..890de8f6ed6a 100644 --- a/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts +++ b/src/plugins/kibana_utils/public/state_sync/state_sync.test.ts @@ -255,7 +255,7 @@ describe('state_sync', () => { expect(history.length).toBe(startHistoryLength); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/"`); - urlSyncStrategy.flush(); + urlSyncStrategy.kbnUrlControls.flush(); expect(history.length).toBe(startHistoryLength + 1); expect(getCurrentUrl()).toMatchInlineSnapshot( @@ -290,7 +290,7 @@ describe('state_sync', () => { expect(history.length).toBe(startHistoryLength); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/"`); - urlSyncStrategy.cancel(); + urlSyncStrategy.kbnUrlControls.cancel(); expect(history.length).toBe(startHistoryLength); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/"`); diff --git a/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.test.ts b/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.test.ts index fbd3c3f93379..037c6f9fc666 100644 --- a/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.test.ts +++ b/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.test.ts @@ -39,11 +39,11 @@ describe('KbnUrlStateStorage', () => { const key = '_s'; urlStateStorage.set(key, state); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/"`); - expect(urlStateStorage.flush()).toBe(true); + expect(!!urlStateStorage.kbnUrlControls.flush()).toBe(true); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/#?_s=(ok:1,test:test)"`); expect(urlStateStorage.get(key)).toEqual(state); - expect(urlStateStorage.flush()).toBe(false); // nothing to flush, not update + expect(!!urlStateStorage.kbnUrlControls.flush()).toBe(false); // nothing to flush, not update }); it('should cancel url updates', async () => { @@ -51,7 +51,7 @@ describe('KbnUrlStateStorage', () => { const key = '_s'; const pr = urlStateStorage.set(key, state); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/"`); - urlStateStorage.cancel(); + urlStateStorage.kbnUrlControls.cancel(); await pr; expect(getCurrentUrl()).toMatchInlineSnapshot(`"/"`); expect(urlStateStorage.get(key)).toEqual(null); @@ -215,11 +215,11 @@ describe('KbnUrlStateStorage', () => { const key = '_s'; urlStateStorage.set(key, state); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/kibana/app/"`); - expect(urlStateStorage.flush()).toBe(true); + expect(!!urlStateStorage.kbnUrlControls.flush()).toBe(true); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/kibana/app/#?_s=(ok:1,test:test)"`); expect(urlStateStorage.get(key)).toEqual(state); - expect(urlStateStorage.flush()).toBe(false); // nothing to flush, not update + expect(!!urlStateStorage.kbnUrlControls.flush()).toBe(false); // nothing to flush, not update }); it('should cancel url updates', async () => { @@ -227,7 +227,7 @@ describe('KbnUrlStateStorage', () => { const key = '_s'; const pr = urlStateStorage.set(key, state); expect(getCurrentUrl()).toMatchInlineSnapshot(`"/kibana/app/"`); - urlStateStorage.cancel(); + urlStateStorage.kbnUrlControls.cancel(); await pr; expect(getCurrentUrl()).toMatchInlineSnapshot(`"/kibana/app/"`); expect(urlStateStorage.get(key)).toEqual(null); diff --git a/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.ts b/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.ts index 700420447bf4..0935ecd20111 100644 --- a/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.ts +++ b/src/plugins/kibana_utils/public/state_sync/state_sync_state_storage/create_kbn_url_state_storage.ts @@ -13,6 +13,7 @@ import { IStateStorage } from './types'; import { createKbnUrlControls, getStateFromKbnUrl, + IKbnUrlControls, setStateToKbnUrl, } from '../../state_management/url'; @@ -39,16 +40,9 @@ export interface IKbnUrlStateStorage extends IStateStorage { change$: (key: string) => Observable; /** - * cancels any pending url updates + * Lower level wrapper around history library that handles batching multiple URL updates into one history change */ - cancel: () => void; - - /** - * Synchronously runs any pending url updates, returned boolean indicates if change occurred. - * @param opts: {replace? boolean} - allows to specify if push or replace should be used for flushing update - * @returns boolean - indicates if there was an update to flush - */ - flush: (opts?: { replace?: boolean }) => boolean; + kbnUrlControls: IKbnUrlControls; } /** @@ -114,11 +108,6 @@ export const createKbnUrlStateStorage = ( }), share() ), - flush: ({ replace = false }: { replace?: boolean } = {}) => { - return !!url.flush(replace); - }, - cancel() { - url.cancel(); - }, + kbnUrlControls: url, }; }; diff --git a/test/functional/apps/discover/_discover.ts b/test/functional/apps/discover/_discover.ts index e2a3bcec4a2a..6f5f3281eece 100644 --- a/test/functional/apps/discover/_discover.ts +++ b/test/functional/apps/discover/_discover.ts @@ -250,7 +250,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); }); describe('usage of discover:searchOnPageLoad', () => { - it('should fetch data from ES initially when discover:searchOnPageLoad is false', async function () { + it('should not fetch data from ES initially when discover:searchOnPageLoad is false', async function () { await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': false }); await PageObjects.common.navigateToApp('discover'); await PageObjects.header.awaitKibanaChrome(); @@ -258,7 +258,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await PageObjects.discover.getNrOfFetches()).to.be(0); }); - it('should not fetch data from ES initially when discover:searchOnPageLoad is true', async function () { + it('should fetch data from ES initially when discover:searchOnPageLoad is true', async function () { await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': true }); await PageObjects.common.navigateToApp('discover'); await PageObjects.header.awaitKibanaChrome(); diff --git a/test/plugin_functional/test_suites/data_plugin/session.ts b/test/plugin_functional/test_suites/data_plugin/session.ts index ac958ead321b..5567958cfd87 100644 --- a/test/plugin_functional/test_suites/data_plugin/session.ts +++ b/test/plugin_functional/test_suites/data_plugin/session.ts @@ -42,10 +42,7 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide await PageObjects.header.waitUntilLoadingHasFinished(); const sessionIds = await getSessionIds(); - // Discover calls destroy on index pattern change, which explicitly closes a session - expect(sessionIds.length).to.be(2); - expect(sessionIds[0].length).to.be(0); - expect(sessionIds[1].length).not.to.be(0); + expect(sessionIds.length).to.be(1); }); it('Starts on a refresh', async () => { diff --git a/x-pack/plugins/maps/public/routes/map_page/url_state/global_sync.ts b/x-pack/plugins/maps/public/routes/map_page/url_state/global_sync.ts index 7fefc6662ada..398c05b8ed69 100644 --- a/x-pack/plugins/maps/public/routes/map_page/url_state/global_sync.ts +++ b/x-pack/plugins/maps/public/routes/map_page/url_state/global_sync.ts @@ -30,6 +30,6 @@ export function updateGlobalState(newState: MapsGlobalState, flushUrlState = fal ...newState, }); if (flushUrlState) { - kbnUrlStateStorage.flush({ replace: true }); + kbnUrlStateStorage.kbnUrlControls.flush(true); } } diff --git a/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/send_to_background.ts b/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/send_to_background.ts index 03635efb6113..7e878e763bfc 100644 --- a/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/send_to_background.ts +++ b/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/send_to_background.ts @@ -30,9 +30,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await searchSessions.deleteAllSearchSessions(); }); - it('Restore using non-existing sessionId errors out. Refresh starts a new session and completes.', async () => { + it('Restore using non-existing sessionId errors out. Refresh starts a new session and completes. Back button restores a session.', async () => { await PageObjects.dashboard.loadSavedDashboard('Not Delayed'); - const url = await browser.getCurrentUrl(); + let url = await browser.getCurrentUrl(); const fakeSessionId = '__fake__'; const savedSessionURL = `${url}&searchSessionId=${fakeSessionId}`; await browser.get(savedSessionURL); @@ -53,6 +53,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Sum of Bytes by Extension' ); expect(session2).not.to.be(fakeSessionId); + + // back button should restore the session: + url = await browser.getCurrentUrl(); + expect(url).not.to.contain('searchSessionId'); + + await browser.goBack(); + + url = await browser.getCurrentUrl(); + expect(url).to.contain('searchSessionId'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await searchSessions.expectState('restored'); + expect( + await dashboardPanelActions.getSearchSessionIdByTitle('Sum of Bytes by Extension') + ).to.be(fakeSessionId); }); it('Saves and restores a session', async () => { diff --git a/x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts b/x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts index d64df98c9860..b5e65158c573 100644 --- a/x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts +++ b/x-pack/test/send_search_to_background_integration/tests/apps/discover/async_search.ts @@ -13,6 +13,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const browser = getService('browser'); const inspector = getService('inspector'); const PageObjects = getPageObjects(['discover', 'common', 'timePicker', 'header']); + const searchSessions = getService('searchSessions'); describe('discover async search', () => { before(async () => { @@ -31,18 +32,33 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { expect(searchSessionId2).not.to.be(searchSessionId1); }); - it('search session id should be picked up from the URL, non existing session id errors out', async () => { - const url = await browser.getCurrentUrl(); + it('search session id should be picked up from the URL, non existing session id errors out, back button restores a session', async () => { + let url = await browser.getCurrentUrl(); const fakeSearchSessionId = '__test__'; const savedSessionURL = url + `&searchSessionId=${fakeSearchSessionId}`; await browser.navigateTo(savedSessionURL); await PageObjects.header.waitUntilLoadingHasFinished(); + await searchSessions.expectState('restored'); await testSubjects.existOrFail('discoverNoResultsError'); // expect error because of fake searchSessionId const searchSessionId1 = await getSearchSessionId(); expect(searchSessionId1).to.be(fakeSearchSessionId); await queryBar.clickQuerySubmitButton(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await searchSessions.expectState('completed'); const searchSessionId2 = await getSearchSessionId(); expect(searchSessionId2).not.to.be(searchSessionId1); + + // back button should restore the session: + url = await browser.getCurrentUrl(); + expect(url).not.to.contain('searchSessionId'); + + await browser.goBack(); + + url = await browser.getCurrentUrl(); + expect(url).to.contain('searchSessionId'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await searchSessions.expectState('restored'); + expect(await getSearchSessionId()).to.be(fakeSearchSessionId); }); });