From 62d106eee7cd4a5391a1cbddea564d43017b1809 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 10 Feb 2020 14:54:09 +0100 Subject: [PATCH 1/6] feat: filter out pinned filters --- x-pack/legacy/plugins/lens/public/app_plugin/app.tsx | 7 +++++++ 1 file changed, 7 insertions(+) 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 c901d4c0c1497..693e9a3026f51 100644 --- a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx @@ -320,8 +320,15 @@ export function App({ {lastKnownDoc && state.isSaveModalVisible && ( { + const appFilters = lastKnownDoc.state.filters.filter( + f => !esFilters.isFilterPinned(f) + ); const doc = { ...lastKnownDoc, + state: { + ...lastKnownDoc.state, + filters: appFilters, + }, id: props.newCopyOnSave ? undefined : lastKnownDoc.id, title: props.newTitle, }; From 65a9a7b5ab6b76a599631c79ce8c22255b82b878 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 11 Feb 2020 19:43:31 +0100 Subject: [PATCH 2/6] test: added test --- .../lens/public/app_plugin/app.test.tsx | 143 +++++++++++++----- .../plugins/lens/public/app_plugin/app.tsx | 23 ++- 2 files changed, 121 insertions(+), 45 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 99926c646da22..a09f1899740bd 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 @@ -38,7 +38,10 @@ jest const { TopNavMenu } = npStart.plugins.navigation.ui; -const waitForPromises = () => new Promise(resolve => setTimeout(resolve)); +const waitForPromises = async () => + act(async () => { + await new Promise(resolve => setTimeout(resolve)); + }); function createMockFrame(): jest.Mocked { return { @@ -220,6 +223,7 @@ describe('Lens App', () => { }); instance.setProps({ docId: '1234' }); + await waitForPromises(); expect(defaultArgs.core.chrome.setBreadcrumbs).toHaveBeenCalledWith([ @@ -322,6 +326,7 @@ describe('Lens App', () => { interface SaveProps { newCopyOnSave: boolean; newTitle: string; + newState?: object; } let defaultArgs: jest.Mocked<{ @@ -373,8 +378,10 @@ describe('Lens App', () => { async function save({ initialDocId, addToDashboardMode, + lastKnownDoc = { expression: 'kibana 3' }, ...saveProps }: SaveProps & { + lastKnownDoc?: object; initialDocId?: string; addToDashboardMode?: boolean; }) { @@ -392,6 +399,7 @@ describe('Lens App', () => { state: { query: 'fake query', datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] }, + filters: [], }, }); (args.docStorage.save as jest.Mock).mockImplementation(async ({ id }) => ({ @@ -403,6 +411,17 @@ describe('Lens App', () => { await waitForPromises(); + const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern; + const field = ({ name: 'myfield' } as unknown) as IFieldType; + const pinnedField = ({ name: 'pinnedField' } as unknown) as IFieldType; + + const unpinned = esFilters.buildExistsFilter(field, indexPattern); + const pinned = esFilters.buildExistsFilter(pinnedField, indexPattern); + FilterManager.setFiltersStore([pinned], esFilters.FilterStateStore.GLOBAL_STATE); + + act(() => args.data.query.filterManager.setFilters([pinned, unpinned])); + await waitForPromises(); + if (initialDocId) { expect(args.docStorage.load).toHaveBeenCalledTimes(1); } else { @@ -410,10 +429,12 @@ describe('Lens App', () => { } const onChange = frame.mount.mock.calls[0][1].onChange; - onChange({ - filterableIndexPatterns: [], - doc: ({ id: initialDocId, expression: 'kibana 3' } as unknown) as Document, - }); + act(() => + onChange({ + filterableIndexPatterns: [], + doc: { id: initialDocId, ...lastKnownDoc } as Document, + }) + ); instance.update(); @@ -441,10 +462,12 @@ describe('Lens App', () => { expect(getButton(instance).disableButton).toEqual(true); const onChange = frame.mount.mock.calls[0][1].onChange; - onChange({ - filterableIndexPatterns: [], - doc: ({ id: 'will save this', expression: 'valid expression' } as unknown) as Document, - }); + act(() => + onChange({ + filterableIndexPatterns: [], + doc: ({ id: 'will save this', expression: 'valid expression' } as unknown) as Document, + }) + ); instance.update(); expect(getButton(instance).disableButton).toEqual(true); }); @@ -482,10 +505,12 @@ describe('Lens App', () => { expect(getButton(instance).disableButton).toEqual(true); const onChange = frame.mount.mock.calls[0][1].onChange; - onChange({ - filterableIndexPatterns: [], - doc: ({ id: 'will save this', expression: 'valid expression' } as unknown) as Document, - }); + act(() => + onChange({ + filterableIndexPatterns: [], + doc: ({ id: 'will save this', expression: 'valid expression' } as unknown) as Document, + }) + ); instance.update(); expect(getButton(instance).disableButton).toEqual(false); @@ -559,10 +584,12 @@ describe('Lens App', () => { const instance = mount(); const onChange = frame.mount.mock.calls[0][1].onChange; - onChange({ - filterableIndexPatterns: [], - doc: ({ id: undefined, expression: 'new expression' } as unknown) as Document, - }); + act(() => + onChange({ + filterableIndexPatterns: [], + doc: ({ id: undefined, expression: 'new expression' } as unknown) as Document, + }) + ); instance.update(); @@ -593,6 +620,38 @@ describe('Lens App', () => { expect(args.redirectTo).toHaveBeenCalledWith('aaa'); }); + + it('saves app filters and does not save pinned filters', async () => { + const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern; + const field = ({ name: 'myfield' } as unknown) as IFieldType; + const pinnedField = ({ name: 'pinnedField' } as unknown) as IFieldType; + + const unpinned = esFilters.buildExistsFilter(field, indexPattern); + const pinned = esFilters.buildExistsFilter(pinnedField, indexPattern); + FilterManager.setFiltersStore([pinned], esFilters.FilterStateStore.GLOBAL_STATE); + await waitForPromises(); + + const { args } = await save({ + initialDocId: '1234', + newCopyOnSave: false, + newTitle: 'hello there2', + lastKnownDoc: { + expression: 'kibana 3', + state: { + filters: [pinned, unpinned], + }, + }, + }); + + expect(args.docStorage.save).toHaveBeenCalledWith({ + id: '1234', + title: 'hello there2', + expression: 'kibana 3', + state: { + filters: [unpinned], + }, + }); + }); }); }); @@ -658,10 +717,12 @@ describe('Lens App', () => { ); const onChange = frame.mount.mock.calls[0][1].onChange; - onChange({ - filterableIndexPatterns: [{ id: '1', title: 'newIndex' }], - doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document, - }); + act(() => + onChange({ + filterableIndexPatterns: [{ id: '1', title: 'newIndex' }], + doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document, + }) + ); await waitForPromises(); instance.update(); @@ -674,12 +735,15 @@ 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, expression: 'valid expression' } as unknown) as Document, - }); + act(() => + onChange({ + filterableIndexPatterns: [{ id: '2', title: 'second index' }], + doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document, + }) + ); await waitForPromises(); + instance.update(); expect(TopNavMenu).toHaveBeenLastCalledWith( @@ -689,17 +753,18 @@ describe('Lens App', () => { {} ); }); - it('updates the editor frame when the user changes query or time in the search bar', () => { const args = defaultArgs; args.editorFrame = frame; const instance = mount(); - instance.find(TopNavMenu).prop('onQuerySubmit')!({ - dateRange: { from: 'now-14d', to: 'now-7d' }, - query: { query: 'new', language: 'lucene' }, - }); + act(() => + instance.find(TopNavMenu).prop('onQuerySubmit')!({ + dateRange: { from: 'now-14d', to: 'now-7d' }, + query: { query: 'new', language: 'lucene' }, + }) + ); instance.update(); @@ -728,7 +793,9 @@ describe('Lens App', () => { const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern; const field = ({ name: 'myfield' } as unknown) as IFieldType; - args.data.query.filterManager.setFilters([esFilters.buildExistsFilter(field, indexPattern)]); + act(() => + args.data.query.filterManager.setFilters([esFilters.buildExistsFilter(field, indexPattern)]) + ); instance.update(); @@ -852,10 +919,12 @@ describe('Lens App', () => { const instance = mount(); - instance.find(TopNavMenu).prop('onQuerySubmit')!({ - dateRange: { from: 'now-14d', to: 'now-7d' }, - query: { query: 'new', language: 'lucene' }, - }); + act(() => + instance.find(TopNavMenu).prop('onQuerySubmit')!({ + dateRange: { from: 'now-14d', to: 'now-7d' }, + query: { query: 'new', language: 'lucene' }, + }) + ); const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern; const field = ({ name: 'myfield' } as unknown) as IFieldType; @@ -865,10 +934,10 @@ describe('Lens App', () => { const pinned = esFilters.buildExistsFilter(pinnedField, indexPattern); FilterManager.setFiltersStore([pinned], esFilters.FilterStateStore.GLOBAL_STATE); - args.data.query.filterManager.setFilters([pinned, unpinned]); + act(() => args.data.query.filterManager.setFilters([pinned, unpinned])); instance.update(); - instance.find(TopNavMenu).prop('onClearSavedQuery')!(); + act(() => instance.find(TopNavMenu).prop('onClearSavedQuery')!()); instance.update(); expect(frame.mount).toHaveBeenLastCalledWith( 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 693e9a3026f51..b062f238769cc 100644 --- a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx @@ -320,15 +320,22 @@ export function App({ {lastKnownDoc && state.isSaveModalVisible && ( { - const appFilters = lastKnownDoc.state.filters.filter( - f => !esFilters.isFilterPinned(f) - ); + const pinnedFilters = lastKnownDoc.state?.filters.filter(esFilters.isFilterPinned); + + const lastDocWithoutPinned = pinnedFilters + ? { + ...lastKnownDoc, + state: { + ...lastKnownDoc.state, + filters: lastKnownDoc.state?.filters.filter( + f => !esFilters.isFilterPinned(f) + ), + }, + } + : lastKnownDoc; + const doc = { - ...lastKnownDoc, - state: { - ...lastKnownDoc.state, - filters: appFilters, - }, + ...lastDocWithoutPinned, id: props.newCopyOnSave ? undefined : lastKnownDoc.id, title: props.newTitle, }; From b0787d7df0d7ce68ddc99a34036704fd8d037334 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 11 Feb 2020 19:46:31 +0100 Subject: [PATCH 3/6] fix: removing trash --- .../plugins/lens/public/app_plugin/app.test.tsx | 11 ----------- 1 file changed, 11 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 a09f1899740bd..735f8e275e7c5 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 @@ -411,17 +411,6 @@ describe('Lens App', () => { await waitForPromises(); - const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern; - const field = ({ name: 'myfield' } as unknown) as IFieldType; - const pinnedField = ({ name: 'pinnedField' } as unknown) as IFieldType; - - const unpinned = esFilters.buildExistsFilter(field, indexPattern); - const pinned = esFilters.buildExistsFilter(pinnedField, indexPattern); - FilterManager.setFiltersStore([pinned], esFilters.FilterStateStore.GLOBAL_STATE); - - act(() => args.data.query.filterManager.setFilters([pinned, unpinned])); - await waitForPromises(); - if (initialDocId) { expect(args.docStorage.load).toHaveBeenCalledTimes(1); } else { From e165eb227faba00ef2e055f8c6bd5e0c7a8549f6 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 11 Feb 2020 21:37:50 +0100 Subject: [PATCH 4/6] Update app.test.tsx fix: removing trash --- x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx | 1 - 1 file changed, 1 deletion(-) 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 735f8e275e7c5..374e3270b3d45 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 @@ -326,7 +326,6 @@ describe('Lens App', () => { interface SaveProps { newCopyOnSave: boolean; newTitle: string; - newState?: object; } let defaultArgs: jest.Mocked<{ From e883885cd300e1340362667fef9135b0745f6bd6 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 12 Feb 2020 22:13:21 +0100 Subject: [PATCH 5/6] fix: conflict solving --- x-pack/legacy/plugins/lens/public/app_plugin/app.tsx | 1 + 1 file changed, 1 insertion(+) 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 b062f238769cc..4be5b26865686 100644 --- a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx @@ -20,6 +20,7 @@ import { EditorFrameInstance } from '../types'; import { NativeRenderer } from '../native_renderer'; import { trackUiEvent } from '../lens_ui_telemetry'; import { + esFilters, Filter, IndexPattern as IndexPatternInstance, IndexPatternsContract, From 86f440a032d2282ce01ddab00b5aa5a1a9aed735 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 13 Feb 2020 11:10:43 +0100 Subject: [PATCH 6/6] refactor: to partition --- x-pack/legacy/plugins/lens/public/app_plugin/app.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 4be5b26865686..a212cb0a1a879 100644 --- a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx @@ -321,16 +321,16 @@ export function App({ {lastKnownDoc && state.isSaveModalVisible && ( { - const pinnedFilters = lastKnownDoc.state?.filters.filter(esFilters.isFilterPinned); - - const lastDocWithoutPinned = pinnedFilters + const [pinnedFilters, appFilters] = _.partition( + lastKnownDoc.state?.filters, + esFilters.isFilterPinned + ); + const lastDocWithoutPinned = pinnedFilters?.length ? { ...lastKnownDoc, state: { ...lastKnownDoc.state, - filters: lastKnownDoc.state?.filters.filter( - f => !esFilters.isFilterPinned(f) - ), + filters: appFilters, }, } : lastKnownDoc;