[Time to Visualize] Fix Dashboard OnAppLeave (#86193) (#86525)

Added isTransferInProgress to embeddable_state_transfer for apps to determine whether or not to show onAppLeave confirm
elastic · Dec 18, 2020 · d20bb3d · d20bb3d
1 parent 22a8360
commit d20bb3d
Show file tree

Hide file tree

Showing 15 changed files with 102 additions and 21 deletions.
diff --git a/...ibana-plugin-plugins-embeddable-public.embeddablestatetransfer._constructor_.md b/...ibana-plugin-plugins-embeddable-public.embeddablestatetransfer._constructor_.md
@@ -9,14 +9,15 @@ Constructs a new instance of the `EmbeddableStateTransfer` class
 <b>Signature:</b>
 
 ```typescript
-constructor(navigateToApp: ApplicationStart['navigateToApp'], appList?: ReadonlyMap<string, PublicAppInfo> | undefined, customStorage?: Storage);
+constructor(navigateToApp: ApplicationStart['navigateToApp'], currentAppId$: ApplicationStart['currentAppId$'], appList?: ReadonlyMap<string, PublicAppInfo> | undefined, customStorage?: Storage);
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
 |  navigateToApp | <code>ApplicationStart['navigateToApp']</code> |  |
+|  currentAppId$ | <code>ApplicationStart['currentAppId$']</code> |  |
 |  appList | <code>ReadonlyMap&lt;string, PublicAppInfo&gt; &#124; undefined</code> |  |
 |  customStorage | <code>Storage</code> |  |
 
diff --git a/...lugin-plugins-embeddable-public.embeddablestatetransfer.istransferinprogress.md b/...lugin-plugins-embeddable-public.embeddablestatetransfer.istransferinprogress.md
@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [EmbeddableStateTransfer](./kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.md) &gt; [isTransferInProgress](./kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.istransferinprogress.md)
+
+## EmbeddableStateTransfer.isTransferInProgress property
+
+<b>Signature:</b>
+
+```typescript
+isTransferInProgress: boolean;
+```
diff --git a/...dable/public/kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.md b/...dable/public/kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.md
@@ -16,13 +16,14 @@ export declare class EmbeddableStateTransfer
 
 |  Constructor | Modifiers | Description |
 |  --- | --- | --- |
-|  [(constructor)(navigateToApp, appList, customStorage)](./kibana-plugin-plugins-embeddable-public.embeddablestatetransfer._constructor_.md) |  | Constructs a new instance of the <code>EmbeddableStateTransfer</code> class |
+|  [(constructor)(navigateToApp, currentAppId$, appList, customStorage)](./kibana-plugin-plugins-embeddable-public.embeddablestatetransfer._constructor_.md) |  | Constructs a new instance of the <code>EmbeddableStateTransfer</code> class |
 
 ## Properties
 
 |  Property | Modifiers | Type | Description |
 |  --- | --- | --- | --- |
 |  [getAppNameFromId](./kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.getappnamefromid.md) |  | <code>(appId: string) =&gt; string &#124; undefined</code> | Fetches an internationalized app title when given an appId. |
+|  [isTransferInProgress](./kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.istransferinprogress.md) |  | <code>boolean</code> |  |
 
 ## Methods
 

diff --git a/src/plugins/dashboard/public/application/dashboard_app.tsx b/src/plugins/dashboard/public/application/dashboard_app.tsx
@@ -45,6 +45,7 @@ import { removeQueryParam } from '../services/kibana_utils';
 import { IndexPattern } from '../services/data';
 import { EmbeddableRenderer } from '../services/embeddable';
 import { DashboardContainerInput } from '.';
+import { leaveConfirmStrings } from '../dashboard_strings';
 
 export interface DashboardAppProps {
   history: History;
@@ -64,8 +65,9 @@ export function DashboardApp({
     core,
     onAppLeave,
     uiSettings,
-    indexPatterns: indexPatternService,
+    embeddable,
     dashboardCapabilities,
+    indexPatterns: indexPatternService,
   } = useKibana<DashboardAppServices>().services;
 
   const [lastReloadTime, setLastReloadTime] = useState(0);
@@ -196,17 +198,22 @@ export function DashboardApp({
       return;
     }
     onAppLeave((actions) => {
-      if (dashboardStateManager?.getIsDirty()) {
-        // TODO: Finish App leave handler with overrides when redirecting to an editor.
-        // return actions.confirm(leaveConfirmStrings.leaveSubtitle, leaveConfirmStrings.leaveTitle);
+      if (
+        dashboardStateManager?.getIsDirty() &&
+        !embeddable.getStateTransfer().isTransferInProgress
+      ) {
+        return actions.confirm(
+          leaveConfirmStrings.getLeaveSubtitle(),
+          leaveConfirmStrings.getLeaveTitle()
+        );
       }
       return actions.default();
     });
     return () => {
       // reset on app leave handler so leaving from the listing page doesn't trigger a confirmation
       onAppLeave((actions) => actions.default());
     };
-  }, [dashboardStateManager, dashboardContainer, onAppLeave]);
+  }, [dashboardStateManager, dashboardContainer, onAppLeave, embeddable]);
 
   // Refresh the dashboard container when lastReloadTime changes
   useEffect(() => {

diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts
@@ -23,6 +23,7 @@ import { EmbeddableStateTransfer } from '.';
 import { ApplicationStart, PublicAppInfo } from '../../../../../core/public';
 import { EMBEDDABLE_EDITOR_STATE_KEY, EMBEDDABLE_PACKAGE_STATE_KEY } from './types';
 import { EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY } from './embeddable_state_transfer';
+import { Subject } from 'rxjs';
 
 const createStorage = (): Storage => {
   const createMockStore = () => {
@@ -46,16 +47,24 @@ const createStorage = (): Storage => {
 describe('embeddable state transfer', () => {
   let application: jest.Mocked<ApplicationStart>;
   let stateTransfer: EmbeddableStateTransfer;
+  let currentAppId$: Subject<string | undefined>;
   let store: Storage;
 
   const destinationApp = 'superUltraVisualize';
   const originatingApp = 'superUltraTestDashboard';
 
   beforeEach(() => {
+    currentAppId$ = new Subject();
+    currentAppId$.next(originatingApp);
     const core = coreMock.createStart();
     application = core.application;
     store = createStorage();
-    stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, undefined, store);
+    stateTransfer = new EmbeddableStateTransfer(
+      application.navigateToApp,
+      currentAppId$,
+      undefined,
+      store
+    );
   });
 
   it('cannot fetch app name when given no app list', async () => {
@@ -67,7 +76,7 @@ describe('embeddable state transfer', () => {
       ['testId', { title: 'State Transfer Test App Hello' } as PublicAppInfo],
       ['testId2', { title: 'State Transfer Test App Goodbye' } as PublicAppInfo],
     ]);
-    stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, appsList);
+    stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, currentAppId$, appsList);
     expect(stateTransfer.getAppNameFromId('kibanana')).toBeUndefined();
   });
 
@@ -76,7 +85,7 @@ describe('embeddable state transfer', () => {
       ['testId', { title: 'State Transfer Test App Hello' } as PublicAppInfo],
       ['testId2', { title: 'State Transfer Test App Goodbye' } as PublicAppInfo],
     ]);
-    stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, appsList);
+    stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, currentAppId$, appsList);
     expect(stateTransfer.getAppNameFromId('testId')).toBe('State Transfer Test App Hello');
     expect(stateTransfer.getAppNameFromId('testId2')).toBe('State Transfer Test App Goodbye');
   });
@@ -107,6 +116,13 @@ describe('embeddable state transfer', () => {
     });
   });
 
+  it('sets isTransferInProgress to true when sending an outgoing editor state', async () => {
+    await stateTransfer.navigateToEditor(destinationApp, { state: { originatingApp } });
+    expect(stateTransfer.isTransferInProgress).toEqual(true);
+    currentAppId$.next(destinationApp);
+    expect(stateTransfer.isTransferInProgress).toEqual(false);
+  });
+
   it('can send an outgoing embeddable package state', async () => {
     await stateTransfer.navigateToWithEmbeddablePackage(destinationApp, {
       state: { type: 'coolestType', input: { savedObjectId: '150' } },
@@ -135,6 +151,15 @@ describe('embeddable state transfer', () => {
     });
   });
 
+  it('sets isTransferInProgress to true when sending an outgoing embeddable package state', async () => {
+    await stateTransfer.navigateToWithEmbeddablePackage(destinationApp, {
+      state: { type: 'coolestType', input: { savedObjectId: '150' } },
+    });
+    expect(stateTransfer.isTransferInProgress).toEqual(true);
+    currentAppId$.next(destinationApp);
+    expect(stateTransfer.isTransferInProgress).toEqual(false);
+  });
+
   it('can fetch an incoming editor state', async () => {
     store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, {
       [EMBEDDABLE_EDITOR_STATE_KEY]: { originatingApp: 'superUltraTestDashboard' },

diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts
@@ -38,14 +38,20 @@ export const EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY = 'EMBEDDABLE_STATE_TRANSFER'
  * @public
  */
 export class EmbeddableStateTransfer {
+  public isTransferInProgress: boolean;
   private storage: Storage;
 
   constructor(
     private navigateToApp: ApplicationStart['navigateToApp'],
+    currentAppId$: ApplicationStart['currentAppId$'],
     private appList?: ReadonlyMap<string, PublicAppInfo> | undefined,
     customStorage?: Storage
   ) {
     this.storage = customStorage ? customStorage : new Storage(sessionStorage);
+    this.isTransferInProgress = false;
+    currentAppId$.subscribe(() => {
+      this.isTransferInProgress = false;
+    });
   }
 
   /**
@@ -105,6 +111,7 @@ export class EmbeddableStateTransfer {
       state: EmbeddableEditorState;
     }
   ): Promise<void> {
+    this.isTransferInProgress = true;
     await this.navigateToWithState<EmbeddableEditorState>(appId, EMBEDDABLE_EDITOR_STATE_KEY, {
       ...options,
       appendToExistingState: true,
@@ -119,6 +126,7 @@ export class EmbeddableStateTransfer {
     appId: string,
     options?: { path?: string; state: EmbeddablePackageState }
   ): Promise<void> {
+    this.isTransferInProgress = true;
     await this.navigateToWithState<EmbeddablePackageState>(appId, EMBEDDABLE_PACKAGE_STATE_KEY, {
       ...options,
       appendToExistingState: true,

diff --git a/src/plugins/embeddable/public/plugin.tsx b/src/plugins/embeddable/public/plugin.tsx
@@ -161,6 +161,7 @@ export class EmbeddablePublicPlugin implements Plugin<EmbeddableSetup, Embeddabl
 
     this.stateTransferService = new EmbeddableStateTransfer(
       core.application.navigateToApp,
+      core.application.currentAppId$,
       this.appList
     );
     this.isRegistryReady = true;
@@ -206,7 +207,12 @@ export class EmbeddablePublicPlugin implements Plugin<EmbeddableSetup, Embeddabl
         ),
       getStateTransfer: (storage?: Storage) =>
         storage
-          ? new EmbeddableStateTransfer(core.application.navigateToApp, this.appList, storage)
+          ? new EmbeddableStateTransfer(
+              core.application.navigateToApp,
+              core.application.currentAppId$,
+              this.appList,
+              storage
+            )
           : this.stateTransferService,
       EmbeddablePanel: getEmbeddablePanelHoc(),
       telemetry: getTelemetryFunction(commonContract),

diff --git a/src/plugins/embeddable/public/public.api.md b/src/plugins/embeddable/public/public.api.md
@@ -628,12 +628,14 @@ export interface EmbeddableStartDependencies {
 export class EmbeddableStateTransfer {
     // Warning: (ae-forgotten-export) The symbol "ApplicationStart" needs to be exported by the entry point index.d.ts
     // Warning: (ae-forgotten-export) The symbol "PublicAppInfo" needs to be exported by the entry point index.d.ts
-    constructor(navigateToApp: ApplicationStart['navigateToApp'], appList?: ReadonlyMap<string, PublicAppInfo> | undefined, customStorage?: Storage);
+    constructor(navigateToApp: ApplicationStart['navigateToApp'], currentAppId$: ApplicationStart['currentAppId$'], appList?: ReadonlyMap<string, PublicAppInfo> | undefined, customStorage?: Storage);
     // (undocumented)
     clearEditorState(): void;
     getAppNameFromId: (appId: string) => string | undefined;
     getIncomingEditorState(removeAfterFetch?: boolean): EmbeddableEditorState | undefined;
     getIncomingEmbeddablePackage(removeAfterFetch?: boolean): EmbeddablePackageState | undefined;
+    // (undocumented)
+    isTransferInProgress: boolean;
     // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "ApplicationStart"
     navigateToEditor(appId: string, options?: {
         path?: string;

diff --git a/test/common/services/security/test_user.ts b/test/common/services/security/test_user.ts
@@ -87,11 +87,11 @@ export async function createTestUserService(
         });
 
         if (browser && testSubjects && shouldRefreshBrowser) {
-          // accept alert if it pops up
-          const alert = await browser.getAlert();
-          await alert?.accept();
           if (await testSubjects.exists('kibanaChrome', { allowHidden: true })) {
             await browser.refresh();
+            // accept alert if it pops up
+            const alert = await browser.getAlert();
+            await alert?.accept();
             await testSubjects.find('kibanaChrome', config.get('timeouts.find') * 10);
           }
         }

diff --git a/test/functional/apps/dashboard/create_and_add_embeddables.js b/test/functional/apps/dashboard/create_and_add_embeddables.js
@@ -80,7 +80,7 @@ export default function ({ getService, getPageObjects }) {
       });
 
       it('saves the saved visualization url to the app link', async () => {
-        await PageObjects.header.clickVisualize();
+        await PageObjects.header.clickVisualize(true);
         const currentUrl = await browser.getCurrentUrl();
         expect(currentUrl).to.contain(VisualizeConstants.EDIT_PATH);
       });

diff --git a/test/functional/apps/dashboard/dashboard_time_picker.js b/test/functional/apps/dashboard/dashboard_time_picker.js
@@ -38,6 +38,8 @@ export default function ({ getService, getPageObjects }) {
     after(async () => {
       await kibanaServer.uiSettings.replace({});
       await browser.refresh();
+      const alert = await browser.getAlert();
+      await alert?.accept();
     });
 
     it('Visualization updated when time picker changes', async () => {
@@ -86,6 +88,8 @@ export default function ({ getService, getPageObjects }) {
         `)`;
       log.debug('go to url' + `${kibanaBaseUrl}#${urlQuery}`);
       await browser.get(`${kibanaBaseUrl}#${urlQuery}`, true);
+      const alert = await browser.getAlert();
+      await alert?.accept();
       await PageObjects.header.waitUntilLoadingHasFinished();
       const time = await PageObjects.timePicker.getTimeConfig();
       const refresh = await PageObjects.timePicker.getRefreshConfig();
@@ -97,6 +101,8 @@ export default function ({ getService, getPageObjects }) {
     it('Timepicker respects dateFormat from UI settings', async () => {
       await kibanaServer.uiSettings.replace({ dateFormat: 'YYYY-MM-DD HH:mm:ss.SSS' });
       await browser.refresh();
+      const alert = await browser.getAlert();
+      await alert?.accept();
       await PageObjects.dashboard.gotoDashboardLandingPage();
       await PageObjects.dashboard.clickNewDashboard();
       await PageObjects.dashboard.addVisualizations([PIE_CHART_VIS_NAME]);

diff --git a/test/functional/apps/dashboard/panel_context_menu.ts b/test/functional/apps/dashboard/panel_context_menu.ts
@@ -110,7 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       const searchName = 'my search';
 
       before(async () => {
-        await PageObjects.header.clickDiscover();
+        await PageObjects.header.clickDiscover(true);
         await PageObjects.discover.clickNewSearchButton();
         await dashboardVisualizations.createSavedSearch({ name: searchName, fields: ['bytes'] });
         await PageObjects.header.waitUntilLoadingHasFinished();

diff --git a/test/functional/apps/dashboard/panel_replacing.ts b/test/functional/apps/dashboard/panel_replacing.ts
@@ -70,6 +70,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
     it('replaced panel persisted correctly when dashboard is hard refreshed', async () => {
       const currentUrl = await browser.getCurrentUrl();
       await browser.get(currentUrl, true);
+      const alert = await browser.getAlert();
+      await alert?.accept();
       await PageObjects.header.waitUntilLoadingHasFinished();
       await PageObjects.dashboard.waitForRenderComplete();
       const panelTitles = await PageObjects.dashboard.getPanelTitles();

diff --git a/test/functional/page_objects/header_page.ts b/test/functional/page_objects/header_page.ts
@@ -31,14 +31,16 @@ export function HeaderPageProvider({ getService, getPageObjects }: FtrProviderCo
   const defaultFindTimeout = config.get('timeouts.find');
 
   class HeaderPage {
-    public async clickDiscover() {
+    public async clickDiscover(ignoreAppLeaveWarning = false) {
       await appsMenu.clickLink('Discover', { category: 'kibana' });
+      await this.onAppLeaveWarning(ignoreAppLeaveWarning);
       await PageObjects.common.waitForTopNavToBeVisible();
       await this.awaitGlobalLoadingIndicatorHidden();
     }
 
-    public async clickVisualize() {
+    public async clickVisualize(ignoreAppLeaveWarning = false) {
       await appsMenu.clickLink('Visualize', { category: 'kibana' });
+      await this.onAppLeaveWarning(ignoreAppLeaveWarning);
       await this.awaitGlobalLoadingIndicatorHidden();
       await retry.waitFor('first breadcrumb to be "Visualize"', async () => {
         const firstBreadcrumb = await globalNav.getFirstBreadcrumb();
@@ -95,6 +97,17 @@ export function HeaderPageProvider({ getService, getPageObjects }: FtrProviderCo
       log.debug('awaitKibanaChrome');
       await testSubjects.find('kibanaChrome', defaultFindTimeout * 10);
     }
+
+    public async onAppLeaveWarning(ignoreWarning = false) {
+      await retry.try(async () => {
+        const warning = await testSubjects.exists('confirmModalTitleText');
+        if (warning) {
+          await testSubjects.click(
+            ignoreWarning ? 'confirmModalConfirmButton' : 'confirmModalCancelButton'
+          );
+        }
+      });
+    }
   }
 
   return new HeaderPage();

diff --git a/test/functional/services/dashboard/visualizations.ts b/test/functional/services/dashboard/visualizations.ts
@@ -58,8 +58,7 @@ export function DashboardVisualizationProvider({ getService, getPageObjects }: F
       fields?: string[];
     }) {
       log.debug(`createSavedSearch(${name})`);
-      await PageObjects.header.clickDiscover();
-
+      await PageObjects.header.clickDiscover(true);
       await PageObjects.timePicker.setHistoricalDataRange();
 
       if (query) {