From 834195a751138450de8ae5e64df48708caa35c77 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Tue, 24 Dec 2019 08:21:38 +0000 Subject: [PATCH] [Lens] Disable saving visualization until there are no changes to the document (#52982) Adding unit test for new functionality Fixing type error Removing unnecessary act statements Removing unnecessary assertion Co-authored-by: Elastic Machine --- .../lens/public/app_plugin/app.test.tsx | 119 +++++++++++++++--- .../plugins/lens/public/app_plugin/app.tsx | 5 +- 2 files changed, 106 insertions(+), 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 index b8176cdb14c12..1cdae05833b98 100644 --- 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); + }); + function getButton(instance: ReactWrapper): TopNavMenuData { return (instance .find('[data-test-subj="lnsApp_topNav"]') @@ -322,12 +349,13 @@ 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' }] }, @@ -335,6 +363,7 @@ describe('Lens App', () => { }); (args.docStorage.save as jest.Mock).mockImplementation(async ({ id }) => ({ id: id || 'aaa', + expression: 'kibana 2', })); const instance = mount(); @@ -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,15 +408,40 @@ 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); + args.editorFrame = frame; + + const instance = mount(); 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(); @@ -396,8 +449,10 @@ 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(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(); 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); + }); + it('uses the default time and query language settings', () => { - const args = makeDefaultArgs(); + const args = defaultArgs; args.editorFrame = frame; mount(); @@ -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(); @@ -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(); @@ -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(); diff --git a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx index d3f2ac624a692..443bcf99a4f09 100644 --- 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 }) =>