Use debounce instead of async url update to remove app state from URL (…

…#127083)
elastic · Mar 8, 2022 · ed4c19c · ed4c19c
1 parent a76fef1
commit ed4c19c
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 57 deletions.
diff --git a/src/plugins/dashboard/public/application/lib/sync_dashboard_url_state.ts b/src/plugins/dashboard/public/application/lib/sync_dashboard_url_state.ts
@@ -7,6 +7,7 @@
  */
 
 import _ from 'lodash';
+import { debounceTime } from 'rxjs/operators';
 
 import { migrateAppState } from '.';
 import { DashboardSavedObject } from '../..';
@@ -25,8 +26,6 @@ import { convertSavedPanelsToPanelMap } from './convert_dashboard_panels';
 
 type SyncDashboardUrlStateProps = DashboardBuildContext & { savedDashboard: DashboardSavedObject };
 
-let awaitingRemoval = false;
-
 export const syncDashboardUrlState = ({
   dispatchDashboardStateChange,
   getLatestDashboardState,
@@ -36,14 +35,44 @@ export const syncDashboardUrlState = ({
   savedDashboard,
   kibanaVersion,
 }: SyncDashboardUrlStateProps) => {
+  /**
+   * Loads any dashboard state from the URL, and removes the state from the URL.
+   */
+  const loadAndRemoveDashboardState = (): Partial<DashboardState> => {
+    const rawAppStateInUrl = kbnUrlStateStorage.get<RawDashboardState>(DASHBOARD_STATE_STORAGE_KEY);
+    if (!rawAppStateInUrl) return {};
+
+    let panelsMap: DashboardPanelMap = {};
+    if (rawAppStateInUrl.panels && rawAppStateInUrl.panels.length > 0) {
+      const rawState = migrateAppState(rawAppStateInUrl, kibanaVersion, usageCollection);
+      panelsMap = convertSavedPanelsToPanelMap(rawState.panels);
+    }
+
+    const migratedQuery = rawAppStateInUrl.query
+      ? migrateLegacyQuery(rawAppStateInUrl.query)
+      : undefined;
+
+    const nextUrl = replaceUrlHashQuery(window.location.href, (query) => {
+      delete query[DASHBOARD_STATE_STORAGE_KEY];
+      return query;
+    });
+    kbnUrlStateStorage.kbnUrlControls.update(nextUrl, true);
+
+    return {
+      ..._.omit(rawAppStateInUrl, ['panels', 'query']),
+      ...(migratedQuery ? { query: migratedQuery } : {}),
+      ...(rawAppStateInUrl.panels ? { panels: panelsMap } : {}),
+    };
+  };
+
   // load initial state before subscribing to avoid state removal triggering update.
-  const loadDashboardStateProps = { kbnUrlStateStorage, usageCollection, kibanaVersion };
-  const initialDashboardStateFromUrl = loadDashboardUrlState(loadDashboardStateProps);
+  const initialDashboardStateFromUrl = loadAndRemoveDashboardState();
 
   const appStateSubscription = kbnUrlStateStorage
     .change$(DASHBOARD_STATE_STORAGE_KEY)
+    .pipe(debounceTime(10)) // debounce URL updates so react has time to unsubscribe when changing URLs
     .subscribe(() => {
-      const stateFromUrl = loadDashboardUrlState(loadDashboardStateProps);
+      const stateFromUrl = loadAndRemoveDashboardState();
 
       const updatedDashboardState = { ...getLatestDashboardState(), ...stateFromUrl };
       applyDashboardFilterState({
@@ -57,57 +86,6 @@ export const syncDashboardUrlState = ({
       dispatchDashboardStateChange(setDashboardState(updatedDashboardState));
     });
 
-  const stopWatchingAppStateInUrl = () => {
-    appStateSubscription.unsubscribe();
-  };
+  const stopWatchingAppStateInUrl = () => appStateSubscription.unsubscribe();
   return { initialDashboardStateFromUrl, stopWatchingAppStateInUrl };
 };
-
-interface LoadDashboardUrlStateProps {
-  kibanaVersion: DashboardBuildContext['kibanaVersion'];
-  usageCollection: DashboardBuildContext['usageCollection'];
-  kbnUrlStateStorage: DashboardBuildContext['kbnUrlStateStorage'];
-}
-
-/**
- * Loads any dashboard state from the URL, and removes the state from the URL.
- */
-const loadDashboardUrlState = ({
-  kibanaVersion,
-  usageCollection,
-  kbnUrlStateStorage,
-}: LoadDashboardUrlStateProps): Partial<DashboardState> => {
-  const rawAppStateInUrl = kbnUrlStateStorage.get<RawDashboardState>(DASHBOARD_STATE_STORAGE_KEY);
-  if (!rawAppStateInUrl) return {};
-
-  let panelsMap: DashboardPanelMap = {};
-  if (rawAppStateInUrl.panels && rawAppStateInUrl.panels.length > 0) {
-    const rawState = migrateAppState(rawAppStateInUrl, kibanaVersion, usageCollection);
-    panelsMap = convertSavedPanelsToPanelMap(rawState.panels);
-  }
-
-  const migratedQuery = rawAppStateInUrl.query
-    ? migrateLegacyQuery(rawAppStateInUrl.query)
-    : undefined;
-
-  // remove state from URL
-  if (!awaitingRemoval) {
-    awaitingRemoval = true;
-    kbnUrlStateStorage.kbnUrlControls.updateAsync((nextUrl) => {
-      awaitingRemoval = false;
-      if (nextUrl.includes(DASHBOARD_STATE_STORAGE_KEY)) {
-        return replaceUrlHashQuery(nextUrl, (query) => {
-          delete query[DASHBOARD_STATE_STORAGE_KEY];
-          return query;
-        });
-      }
-      return nextUrl;
-    }, true);
-  }
-
-  return {
-    ..._.omit(rawAppStateInUrl, ['panels', 'query']),
-    ...(migratedQuery ? { query: migratedQuery } : {}),
-    ...(rawAppStateInUrl.panels ? { panels: panelsMap } : {}),
-  };
-};
diff --git a/test/functional/apps/dashboard/dashboard_save.ts b/test/functional/apps/dashboard/dashboard_save.ts
@@ -58,6 +58,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       // wait till it finishes reloading or it might reload the url after simulating the
       // dashboard landing page click.
       await PageObjects.header.waitUntilLoadingHasFinished();
+
+      // after saving a new dashboard, the app state must be removed
+      await await PageObjects.dashboard.expectAppStateRemovedFromURL();
+
       await PageObjects.dashboard.gotoDashboardLandingPage();
 
       await listingTable.searchAndExpectItemsCount('dashboard', dashboardName, 2);

diff --git a/test/functional/apps/dashboard/dashboard_state.ts b/test/functional/apps/dashboard/dashboard_state.ts
@@ -195,12 +195,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
       it('for query parameter with soft refresh', async function () {
         await changeQuery(false, 'hi:goodbye');
+        await PageObjects.dashboard.expectAppStateRemovedFromURL();
       });
 
       it('for query parameter with hard refresh', async function () {
         await changeQuery(true, 'hi:hello');
         await queryBar.clearQuery();
         await queryBar.clickQuerySubmitButton();
+        await PageObjects.dashboard.expectAppStateRemovedFromURL();
       });
 
       it('for panel size parameters', async function () {

diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts
@@ -9,6 +9,8 @@
 export const PIE_CHART_VIS_NAME = 'Visualization PieChart';
 export const AREA_CHART_VIS_NAME = 'Visualization漢字 AreaChart';
 export const LINE_CHART_VIS_NAME = 'Visualization漢字 LineChart';
+
+import expect from '@kbn/expect';
 import { FtrService } from '../ftr_provider_context';
 
 interface SaveDashboardOptions {
@@ -51,6 +53,13 @@ export class DashboardPageObject extends FtrService {
     await this.common.navigateToApp('dashboard');
   }
 
+  public async expectAppStateRemovedFromURL() {
+    this.retry.try(async () => {
+      const url = await this.browser.getCurrentUrl();
+      expect(url.indexOf('_a')).to.be(-1);
+    });
+  }
+
   public async preserveCrossAppState() {
     const url = await this.browser.getCurrentUrl();
     await this.browser.get(url, false);