[Lens] Warn if leaving with unsaved visualization (#67689)

* [Lens] Warn if leaving with unsaved visualization * Made confirmation logic more robust and add title Co-authored-by: Elastic Machine <[email protected]>
elastic · Jun 1, 2020 · b061d85 · b061d85
1 parent add5b11
commit b061d85
Show file tree

Hide file tree

Showing 3 changed files with 208 additions and 66 deletions.
diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx
@@ -9,6 +9,7 @@ import { ReactWrapper } from 'enzyme';
 import { act } from 'react-dom/test-utils';
 import { App } from './app';
 import { EditorFrameInstance } from '../types';
+import { AppMountParameters } from 'kibana/public';
 import { Storage } from '../../../../../src/plugins/kibana_utils/public';
 import { Document, SavedObjectStore } from '../persistence';
 import { mount } from 'enzyme';
@@ -111,6 +112,7 @@ describe('Lens App', () => {
       newlyCreated?: boolean
     ) => void;
     originatingApp: string | undefined;
+    onAppLeave: AppMountParameters['onAppLeave'];
   }> {
     return ({
       navigation: navigationStartMock,
@@ -153,6 +155,7 @@ describe('Lens App', () => {
           newlyCreated?: boolean
         ) => {}
       ),
+      onAppLeave: jest.fn(),
     } as unknown) as jest.Mocked<{
       navigation: typeof navigationStartMock;
       editorFrame: EditorFrameInstance;
@@ -168,6 +171,7 @@ describe('Lens App', () => {
         newlyCreated?: boolean
       ) => void;
       originatingApp: string | undefined;
+      onAppLeave: AppMountParameters['onAppLeave'];
     }>;
   }
 
@@ -357,22 +361,7 @@ describe('Lens App', () => {
         newTitle: string;
       }
 
-      let defaultArgs: jest.Mocked<{
-        editorFrame: EditorFrameInstance;
-        navigation: typeof navigationStartMock;
-        data: typeof dataStartMock;
-        core: typeof core;
-        storage: Storage;
-        docId?: string;
-        docStorage: SavedObjectStore;
-        redirectTo: (
-          id?: string,
-          returnToOrigin?: boolean,
-          originatingApp?: string | undefined,
-          newlyCreated?: boolean
-        ) => void;
-        originatingApp: string | undefined;
-      }>;
+      let defaultArgs: ReturnType<typeof makeDefaultArgs>;
 
       beforeEach(() => {
         defaultArgs = makeDefaultArgs();
@@ -486,30 +475,6 @@ describe('Lens App', () => {
         expect(getButton(instance).disableButton).toEqual(true);
       });
 
-      it('shows a disabled save button when there are no changes to the document', async () => {
-        const args = defaultArgs;
-        (args.docStorage.load as jest.Mock).mockResolvedValue({
-          id: '1234',
-          title: 'My cool doc',
-          expression: '',
-        } as jest.ResolvedValue<Document>);
-        args.editorFrame = frame;
-
-        instance = mount(<App {...args} />);
-        expect(getButton(instance).disableButton).toEqual(true);
-
-        const onChange = frame.mount.mock.calls[0][1].onChange;
-
-        act(() => {
-          onChange({
-            filterableIndexPatterns: [],
-            doc: ({ id: '1234', expression: 'valid expression' } as unknown) as Document,
-          });
-        });
-        instance.update();
-        expect(getButton(instance).disableButton).toEqual(false);
-      });
-
       it('shows a save button that is enabled when the frame has provided its state', async () => {
         const args = defaultArgs;
         args.editorFrame = frame;
@@ -691,21 +656,7 @@ describe('Lens App', () => {
   });
 
   describe('query bar state management', () => {
-    let defaultArgs: jest.Mocked<{
-      editorFrame: EditorFrameInstance;
-      data: typeof dataStartMock;
-      navigation: typeof navigationStartMock;
-      core: typeof core;
-      storage: Storage;
-      docId?: string;
-      docStorage: SavedObjectStore;
-      redirectTo: (
-        id?: string,
-        returnToOrigin?: boolean,
-        originatingApp?: string | undefined,
-        newlyCreated?: boolean
-      ) => void;
-    }>;
+    let defaultArgs: ReturnType<typeof makeDefaultArgs>;
 
     beforeEach(() => {
       defaultArgs = makeDefaultArgs();
@@ -1001,4 +952,159 @@ describe('Lens App', () => {
 
     expect(args.core.notifications.toasts.addDanger).toHaveBeenCalled();
   });
+
+  describe('showing a confirm message when leaving', () => {
+    let defaultArgs: ReturnType<typeof makeDefaultArgs>;
+    let defaultLeave: jest.Mock;
+    let confirmLeave: jest.Mock;
+
+    beforeEach(() => {
+      defaultArgs = makeDefaultArgs();
+      defaultLeave = jest.fn();
+      confirmLeave = jest.fn();
+      (defaultArgs.docStorage.load as jest.Mock).mockResolvedValue({
+        id: '1234',
+        title: 'My cool doc',
+        expression: 'valid expression',
+        state: {
+          query: 'kuery',
+          datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
+        },
+      } as jest.ResolvedValue<Document>);
+    });
+
+    it('should not show a confirm message if there is no expression to save', () => {
+      instance = mount(<App {...defaultArgs} />);
+
+      const lastCall =
+        defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
+      lastCall({ default: defaultLeave, confirm: confirmLeave });
+
+      expect(defaultLeave).toHaveBeenCalled();
+      expect(confirmLeave).not.toHaveBeenCalled();
+    });
+
+    it('does not confirm if the user is missing save permissions', () => {
+      const args = defaultArgs;
+      args.core.application = {
+        ...args.core.application,
+        capabilities: {
+          ...args.core.application.capabilities,
+          visualize: { save: false, saveQuery: false, show: true },
+        },
+      };
+      args.editorFrame = frame;
+
+      instance = mount(<App {...args} />);
+
+      const onChange = frame.mount.mock.calls[0][1].onChange;
+      act(() =>
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document,
+        })
+      );
+      instance.update();
+
+      const lastCall =
+        defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
+      lastCall({ default: defaultLeave, confirm: confirmLeave });
+
+      expect(defaultLeave).toHaveBeenCalled();
+      expect(confirmLeave).not.toHaveBeenCalled();
+    });
+
+    it('should confirm when leaving with an unsaved doc', () => {
+      defaultArgs.editorFrame = frame;
+      instance = mount(<App {...defaultArgs} />);
+
+      const onChange = frame.mount.mock.calls[0][1].onChange;
+      act(() =>
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document,
+        })
+      );
+      instance.update();
+
+      const lastCall =
+        defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
+      lastCall({ default: defaultLeave, confirm: confirmLeave });
+
+      expect(confirmLeave).toHaveBeenCalled();
+      expect(defaultLeave).not.toHaveBeenCalled();
+    });
+
+    it('should confirm when leaving with unsaved changes to an existing doc', async () => {
+      defaultArgs.editorFrame = frame;
+      instance = mount(<App {...defaultArgs} />);
+      await act(async () => {
+        instance.setProps({ docId: '1234' });
+      });
+
+      const onChange = frame.mount.mock.calls[0][1].onChange;
+      act(() =>
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: '1234', expression: 'different expression' } as unknown) as Document,
+        })
+      );
+      instance.update();
+
+      const lastCall =
+        defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
+      lastCall({ default: defaultLeave, confirm: confirmLeave });
+
+      expect(confirmLeave).toHaveBeenCalled();
+      expect(defaultLeave).not.toHaveBeenCalled();
+    });
+
+    it('should not confirm when changes are saved', async () => {
+      defaultArgs.editorFrame = frame;
+      instance = mount(<App {...defaultArgs} />);
+      await act(async () => {
+        instance.setProps({ docId: '1234' });
+      });
+
+      const onChange = frame.mount.mock.calls[0][1].onChange;
+      act(() =>
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: '1234', expression: 'valid expression' } as unknown) as Document,
+        })
+      );
+      instance.update();
+
+      const lastCall =
+        defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
+      lastCall({ default: defaultLeave, confirm: confirmLeave });
+
+      expect(defaultLeave).toHaveBeenCalled();
+      expect(confirmLeave).not.toHaveBeenCalled();
+    });
+
+    it('should confirm when the latest doc is invalid', async () => {
+      defaultArgs.editorFrame = frame;
+      instance = mount(<App {...defaultArgs} />);
+      await act(async () => {
+        instance.setProps({ docId: '1234' });
+      });
+
+      const onChange = frame.mount.mock.calls[0][1].onChange;
+      act(() =>
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: '1234', expression: null } as unknown) as Document,
+        })
+      );
+      instance.update();
+
+      const lastCall =
+        defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
+      lastCall({ default: defaultLeave, confirm: confirmLeave });
+
+      expect(confirmLeave).toHaveBeenCalled();
+      expect(defaultLeave).not.toHaveBeenCalled();
+    });
+  });
 });
diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx
@@ -10,7 +10,7 @@ import { I18nProvider } from '@kbn/i18n/react';
 import { i18n } from '@kbn/i18n';
 import { Query, DataPublicPluginStart } from 'src/plugins/data/public';
 import { NavigationPublicPluginStart } from 'src/plugins/navigation/public';
-import { AppMountContext, NotificationsStart } from 'kibana/public';
+import { AppMountContext, AppMountParameters, NotificationsStart } from 'kibana/public';
 import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
 import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public';
 import {
@@ -57,6 +57,7 @@ export function App({
   redirectTo,
   originatingAppFromUrl,
   navigation,
+  onAppLeave,
 }: {
   editorFrame: EditorFrameInstance;
   data: DataPublicPluginStart;
@@ -72,6 +73,7 @@ export function App({
     newlyCreated?: boolean
   ) => void;
   originatingAppFromUrl?: string | undefined;
+  onAppLeave: AppMountParameters['onAppLeave'];
 }) {
   const language =
     storage.get('kibana.userQueryLanguage') || core.uiSettings.get('search:queryLanguage');
@@ -94,6 +96,12 @@ export function App({
 
   const { lastKnownDoc } = state;
 
+  const isSaveable =
+    lastKnownDoc &&
+    lastKnownDoc.expression &&
+    lastKnownDoc.expression.length > 0 &&
+    core.application.capabilities.visualize.save;
+
   useEffect(() => {
     // Clear app-specific filters when navigating to Lens. Necessary because Lens
     // can be loaded without a full page refresh
@@ -123,7 +131,31 @@ export function App({
       filterSubscription.unsubscribe();
       timeSubscription.unsubscribe();
     };
-  }, []);
+  }, [data.query.filterManager, data.query.timefilter.timefilter]);
+
+  useEffect(() => {
+    onAppLeave((actions) => {
+      // Confirm when the user has made any changes to an existing doc
+      // or when the user has configured something without saving
+      if (
+        core.application.capabilities.visualize.save &&
+        (state.persistedDoc?.expression
+          ? !_.isEqual(lastKnownDoc?.expression, state.persistedDoc.expression)
+          : lastKnownDoc?.expression)
+      ) {
+        return actions.confirm(
+          i18n.translate('xpack.lens.app.unsavedWorkMessage', {
+            defaultMessage: 'Leave Lens with unsaved work?',
+          }),
+          i18n.translate('xpack.lens.app.unsavedWorkTitle', {
+            defaultMessage: 'Unsaved changes',
+          })
+        );
+      } else {
+        return actions.default();
+      }
+    });
+  }, [lastKnownDoc, onAppLeave, state.persistedDoc, core.application.capabilities.visualize.save]);
 
   // Sync Kibana breadcrumbs any time the saved document's title changes
   useEffect(() => {
@@ -144,7 +176,7 @@ export function App({
           : i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }),
       },
     ]);
-  }, [state.persistedDoc && state.persistedDoc.title]);
+  }, [core.application, core.chrome, core.http.basePath, state.persistedDoc]);
 
   useEffect(() => {
     if (docId && (!state.persistedDoc || state.persistedDoc.id !== docId)) {
@@ -187,13 +219,16 @@ export function App({
           redirectTo();
         });
     }
-  }, [docId]);
-
-  const isSaveable =
-    lastKnownDoc &&
-    lastKnownDoc.expression &&
-    lastKnownDoc.expression.length > 0 &&
-    core.application.capabilities.visualize.save;
+  }, [
+    core.notifications,
+    data.indexPatterns,
+    data.query.filterManager,
+    docId,
+    // TODO: These dependencies are changing too often
+    // docStorage,
+    // redirectTo,
+    // state.persistedDoc,
+  ]);
 
   const runSave = (
     saveProps: Omit<OnSaveProps, 'onTitleDuplicate' | 'newDescription'> & {
@@ -257,7 +292,7 @@ export function App({
       core.notifications.toasts.addDanger({
         title: e.message,
       }),
-    []
+    [core.notifications.toasts]
   );
 
   const { TopNavMenu } = navigation.ui;

diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx
@@ -92,6 +92,7 @@ export async function mountApp(
           redirectTo(routeProps, id, returnToOrigin, originatingApp, newlyCreated)
         }
         originatingAppFromUrl={originatingAppFromUrl}
+        onAppLeave={params.onAppLeave}
       />
     );
   };