[Lens] Disable saving visualization until there are no changes to the…

… document (elastic#52982) Adding unit test for new functionality Fixing type error Removing unnecessary act statements Removing unnecessary assertion Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jkelastic · Jan 3, 2020 · 834195a · 834195a
1 parent 53257b3
commit 834195a
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 18 deletions.
diff --git a/x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx
@@ -190,6 +190,7 @@ describe('Lens App', () => {
     (defaultArgs.docStorage.load as jest.Mock).mockResolvedValue({
       id: '1234',
       title: 'Daaaaaaadaumching!',
+      expression: 'valid expression',
       state: {
         query: 'fake query',
         datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
@@ -219,6 +220,7 @@ describe('Lens App', () => {
       args.editorFrame = frame;
       (args.docStorage.load as jest.Mock).mockResolvedValue({
         id: '1234',
+        expression: 'valid expression',
         state: {
           query: 'fake query',
           datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
@@ -244,6 +246,7 @@ describe('Lens App', () => {
         expect.objectContaining({
           doc: {
             id: '1234',
+            expression: 'valid expression',
             state: {
               query: 'fake query',
               datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
@@ -294,6 +297,30 @@ describe('Lens App', () => {
         newTitle: string;
       }
 
+      let defaultArgs: jest.Mocked<{
+        editorFrame: EditorFrameInstance;
+        data: typeof dataStartMock;
+        core: typeof core;
+        dataShim: DataStart;
+        storage: Storage;
+        docId?: string;
+        docStorage: SavedObjectStore;
+        redirectTo: (id?: string) => void;
+      }>;
+
+      beforeEach(() => {
+        defaultArgs = makeDefaultArgs();
+        (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>);
+      });
+
       function getButton(instance: ReactWrapper): TopNavMenuData {
         return (instance
           .find('[data-test-subj="lnsApp_topNav"]')
@@ -322,19 +349,21 @@ describe('Lens App', () => {
         initialDocId?: string;
       }) {
         const args = {
-          ...makeDefaultArgs(),
+          ...defaultArgs,
           docId: initialDocId,
         };
         args.editorFrame = frame;
         (args.docStorage.load as jest.Mock).mockResolvedValue({
           id: '1234',
+          expression: 'kibana',
           state: {
             query: 'fake query',
             datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
           },
         });
         (args.docStorage.save as jest.Mock).mockImplementation(async ({ id }) => ({
           id: id || 'aaa',
+          expression: 'kibana 2',
         }));
 
         const instance = mount(<App {...args} />);
@@ -350,13 +379,12 @@ describe('Lens App', () => {
         const onChange = frame.mount.mock.calls[0][1].onChange;
         onChange({
           filterableIndexPatterns: [],
-          doc: ({ id: initialDocId } as unknown) as Document,
+          doc: ({ id: initialDocId, expression: 'kibana 3' } as unknown) as Document,
         });
 
         instance.update();
 
         expect(getButton(instance).disableButton).toEqual(false);
-
         testSave(instance, saveProps);
 
         await waitForPromises();
@@ -365,7 +393,7 @@ describe('Lens App', () => {
       }
 
       it('shows a disabled save button when the user does not have permissions', async () => {
-        const args = makeDefaultArgs();
+        const args = defaultArgs;
         args.core.application = {
           ...args.core.application,
           capabilities: {
@@ -380,24 +408,51 @@ describe('Lens App', () => {
         expect(getButton(instance).disableButton).toEqual(true);
 
         const onChange = frame.mount.mock.calls[0][1].onChange;
-        onChange({ filterableIndexPatterns: [], doc: ('will save this' as unknown) as Document });
-
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: 'will save this', expression: 'valid expression' } as unknown) as Document,
+        });
         instance.update();
+        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;
+
+        const 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 = makeDefaultArgs();
+        const args = defaultArgs;
         args.editorFrame = frame;
 
         const instance = mount(<App {...args} />);
 
         expect(getButton(instance).disableButton).toEqual(true);
 
         const onChange = frame.mount.mock.calls[0][1].onChange;
-        onChange({ filterableIndexPatterns: [], doc: ('will save this' as unknown) as Document });
-
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: 'will save this', expression: 'valid expression' } as unknown) as Document,
+        });
         instance.update();
 
         expect(getButton(instance).disableButton).toEqual(false);
@@ -413,6 +468,7 @@ describe('Lens App', () => {
         expect(args.docStorage.save).toHaveBeenCalledWith({
           id: undefined,
           title: 'hello there',
+          expression: 'kibana 3',
         });
 
         expect(args.redirectTo).toHaveBeenCalledWith('aaa');
@@ -432,6 +488,7 @@ describe('Lens App', () => {
         expect(args.docStorage.save).toHaveBeenCalledWith({
           id: undefined,
           title: 'hello there',
+          expression: 'kibana 3',
         });
 
         expect(args.redirectTo).toHaveBeenCalledWith('aaa');
@@ -451,6 +508,7 @@ describe('Lens App', () => {
         expect(args.docStorage.save).toHaveBeenCalledWith({
           id: '1234',
           title: 'hello there',
+          expression: 'kibana 3',
         });
 
         expect(args.redirectTo).not.toHaveBeenCalled();
@@ -461,14 +519,17 @@ describe('Lens App', () => {
       });
 
       it('handles save failure by showing a warning, but still allows another save', async () => {
-        const args = makeDefaultArgs();
+        const args = defaultArgs;
         args.editorFrame = frame;
         (args.docStorage.save as jest.Mock).mockRejectedValue({ message: 'failed' });
 
         const instance = mount(<App {...args} />);
 
         const onChange = frame.mount.mock.calls[0][1].onChange;
-        onChange({ filterableIndexPatterns: [], doc: ({ id: undefined } as unknown) as Document });
+        onChange({
+          filterableIndexPatterns: [],
+          doc: ({ id: undefined, expression: 'new expression' } as unknown) as Document,
+        });
 
         instance.update();
 
@@ -486,8 +547,32 @@ describe('Lens App', () => {
   });
 
   describe('query bar state management', () => {
+    let defaultArgs: jest.Mocked<{
+      editorFrame: EditorFrameInstance;
+      data: typeof dataStartMock;
+      core: typeof core;
+      dataShim: DataStart;
+      storage: Storage;
+      docId?: string;
+      docStorage: SavedObjectStore;
+      redirectTo: (id?: string) => void;
+    }>;
+
+    beforeEach(() => {
+      defaultArgs = makeDefaultArgs();
+      (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('uses the default time and query language settings', () => {
-      const args = makeDefaultArgs();
+      const args = defaultArgs;
       args.editorFrame = frame;
 
       mount(<App {...args} />);
@@ -510,7 +595,7 @@ describe('Lens App', () => {
     });
 
     it('updates the index patterns when the editor frame is changed', async () => {
-      const args = makeDefaultArgs();
+      const args = defaultArgs;
       args.editorFrame = frame;
 
       const instance = mount(<App {...args} />);
@@ -525,7 +610,7 @@ describe('Lens App', () => {
       const onChange = frame.mount.mock.calls[0][1].onChange;
       onChange({
         filterableIndexPatterns: [{ id: '1', title: 'newIndex' }],
-        doc: ({ id: undefined } as unknown) as Document,
+        doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document,
       });
 
       await waitForPromises();
@@ -541,7 +626,7 @@ describe('Lens App', () => {
       // Do it again to verify that the dirty checking is done right
       onChange({
         filterableIndexPatterns: [{ id: '2', title: 'second index' }],
-        doc: ({ id: undefined } as unknown) as Document,
+        doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document,
       });
 
       await waitForPromises();
@@ -556,7 +641,7 @@ describe('Lens App', () => {
     });
 
     it('updates the editor frame when the user changes query or time in the search bar', () => {
-      const args = makeDefaultArgs();
+      const args = defaultArgs;
       args.editorFrame = frame;
 
       const instance = mount(<App {...args} />);
@@ -586,7 +671,7 @@ describe('Lens App', () => {
     });
 
     it('updates the filters when the user changes them', () => {
-      const args = makeDefaultArgs();
+      const args = defaultArgs;
       args.editorFrame = frame;
 
       const instance = mount(<App {...args} />);

diff --git a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx
@@ -150,7 +150,10 @@ export function App({
     }
   }, [docId]);
 
-  const isSaveable = lastKnownDoc && core.application.capabilities.visualize.save;
+  const isSaveable =
+    lastKnownDoc &&
+    lastKnownDoc.expression.length > 0 &&
+    core.application.capabilities.visualize.save;
 
   const onError = useCallback(
     (e: { message: string }) =>