[Search Sessions] Improve session restoration back button (#87635) (#…

…89532)
elastic · Jan 28, 2021 · bba95e8 · bba95e8
1 parent ab1699d
commit bba95e8
Show file tree

Hide file tree

Showing 27 changed files with 560 additions and 148 deletions.
diff --git a/...ana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md b/...ana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md
diff --git a/...bana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md b/...bana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md
diff --git a/...in-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md b/...in-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-plugins-kibana\_utils-public-state\_sync](./kibana-plugin-plugins-kibana_utils-public-state_sync.md) &gt; [IKbnUrlStateStorage](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md) &gt; [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
+
+<b>Signature:</b>
+
+```typescript
+kbnUrlControls: IKbnUrlControls;
+```
diff --git a/...ync/kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md b/...ync/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) | <code>() =&gt; void</code> | cancels any pending url updates |
 |  [change$](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.change_.md) | <code>&lt;State = unknown&gt;(key: string) =&gt; Observable&lt;State &#124; null&gt;</code> |  |
-|  [flush](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md) | <code>(opts?: {</code><br/><code>        replace?: boolean;</code><br/><code>    }) =&gt; boolean</code> | Synchronously runs any pending url updates, returned boolean indicates if change occurred. |
 |  [get](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.get.md) | <code>&lt;State = unknown&gt;(key: string) =&gt; State &#124; null</code> |  |
+|  [kbnUrlControls](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md) | <code>IKbnUrlControls</code> | 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) | <code>&lt;State&gt;(key: string, state: State, opts?: {</code><br/><code>        replace: boolean;</code><br/><code>    }) =&gt; Promise&lt;string &#124; undefined&gt;</code> |  |
 
diff --git 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<DashboardAppServices>().services;
 
-  const [lastReloadTime, setLastReloadTime] = useState(0);
+  const triggerRefresh$ = useMemo(() => new Subject<{ force?: boolean }>(), []);
   const [indexPatterns, setIndexPatterns] = useState<IndexPattern[]>([]);
 
   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,19 +104,43 @@ 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);
         }
 
         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 (
     <div className="app-container dshAppContainer">
       {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
@@ -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/...lugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.tsx.snap b/...lugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.tsx.snap