From cd75f0155338c6362289af0030e1421f06dd7d92 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 9 Aug 2019 14:44:12 -0400 Subject: [PATCH 1/4] Change Lens document to no longer store JSON blob --- x-pack/legacy/plugins/lens/mappings.json | 3 +- .../persistence/saved_object_store.test.ts | 35 +++---------------- .../public/persistence/saved_object_store.ts | 16 ++++----- 3 files changed, 13 insertions(+), 41 deletions(-) diff --git a/x-pack/legacy/plugins/lens/mappings.json b/x-pack/legacy/plugins/lens/mappings.json index 9136447531be8..673197a8b1247 100644 --- a/x-pack/legacy/plugins/lens/mappings.json +++ b/x-pack/legacy/plugins/lens/mappings.json @@ -11,7 +11,8 @@ "type": "keyword" }, "state": { - "type": "text" + "enabled": false, + "type": "object" }, "expression": { "type": "text" diff --git a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts index 12dff938d9be1..6952d1254ee00 100644 --- a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts +++ b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts @@ -67,7 +67,7 @@ describe('LensStore', () => { expression: '', activeDatasourceId: 'indexpattern', - state: JSON.stringify({ + state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: '.kibana_test' }, @@ -75,7 +75,7 @@ describe('LensStore', () => { visualization: { x: 'foo', y: 'baz' }, query: { query: '', language: 'lucene' }, filters: [], - }), + }, }); }); @@ -117,45 +117,18 @@ describe('LensStore', () => { visualizationType: 'line', expression: '', activeDatasourceId: 'indexpattern', - state: JSON.stringify({ + state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: 'lotr' } }, visualization: { gear: ['staff', 'pointy hat'] }, query: { query: '', language: 'lucene' }, filters: [], - }), + }, }); }); }); describe('load', () => { - test('parses the visState', async () => { - const { client, store } = testStore(); - client.get = jest.fn(async () => ({ - id: 'Paul', - type: 'lens', - attributes: { - title: 'Hope clouds observation.', - visualizationType: 'dune', - state: '{ "datasource": { "giantWorms": true } }', - }, - })); - const doc = await store.load('Paul'); - - expect(doc).toEqual({ - id: 'Paul', - type: 'lens', - title: 'Hope clouds observation.', - visualizationType: 'dune', - state: { - datasource: { giantWorms: true }, - }, - }); - - expect(client.get).toHaveBeenCalledTimes(1); - expect(client.get).toHaveBeenCalledWith('lens', 'Paul'); - }); - test('throws if an error is returned', async () => { const { client, store } = testStore(); client.get = jest.fn(async () => ({ diff --git a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts index 5846565dd05a7..32a2677e25ebe 100644 --- a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts @@ -61,18 +61,17 @@ export class SavedObjectIndexStore implements SavedObjectStore { async save(vis: Document) { const { id, type, ...rest } = vis; - const attributes = { - ...rest, - state: JSON.stringify(rest.state), - }; + + // Any is the most straighforward way out here, since the saved + // object client doesn't allow unknowns, and we have them in + // our object. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const attributes: any = rest; const result = await (id ? this.client.update(DOC_TYPE, id, attributes) : this.client.create(DOC_TYPE, attributes)); - return { - ...vis, - id: result.id, - }; + return { ...vis, id: result.id }; } async load(id: string): Promise { @@ -86,7 +85,6 @@ export class SavedObjectIndexStore implements SavedObjectStore { ...attributes, id, type, - state: JSON.parse(((attributes as unknown) as { state: string }).state as string), } as Document; } } From e07c5ed0369e6ba9c45586eff11f070acf317c73 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Wed, 14 Aug 2019 12:12:47 -0400 Subject: [PATCH 2/4] Tweak mappings, remove activeDatasourceId, remove unecessary indexing of expression --- x-pack/legacy/plugins/lens/mappings.json | 6 ++---- .../editor_frame/editor_frame.test.tsx | 7 ------- .../editor_frame/editor_frame.tsx | 1 - .../editor_frame/save.test.ts | 2 -- .../editor_frame_plugin/editor_frame/save.ts | 3 --- .../editor_frame/state_management.ts | 19 +++++++++++++++++-- .../public/persistence/saved_object_store.ts | 1 - 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/x-pack/legacy/plugins/lens/mappings.json b/x-pack/legacy/plugins/lens/mappings.json index 673197a8b1247..8eccf22eb2235 100644 --- a/x-pack/legacy/plugins/lens/mappings.json +++ b/x-pack/legacy/plugins/lens/mappings.json @@ -7,15 +7,13 @@ "visualizationType": { "type": "keyword" }, - "activeDatasourceId": { - "type": "keyword" - }, "state": { "enabled": false, "type": "object" }, "expression": { - "type": "text" + "index": false, + "type": "keyword" } } } diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx index 1b83ed4f07e70..3904c2c114543 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.test.tsx @@ -154,7 +154,6 @@ describe('editor_frame', () => { initialVisualizationId="testVis" ExpressionRenderer={expressionRendererMock} doc={{ - activeDatasourceId: 'testDatasource', visualizationType: 'testVis', title: '', expression: '', @@ -482,7 +481,6 @@ describe('editor_frame', () => { initialVisualizationId="testVis" ExpressionRenderer={expressionRendererMock} doc={{ - activeDatasourceId: 'testDatasource', visualizationType: 'testVis', title: '', expression: '', @@ -725,7 +723,6 @@ describe('editor_frame', () => { initialVisualizationId="testVis" ExpressionRenderer={expressionRendererMock} doc={{ - activeDatasourceId: 'testDatasource', visualizationType: 'testVis', title: '', expression: '', @@ -779,7 +776,6 @@ describe('editor_frame', () => { initialVisualizationId="testVis" ExpressionRenderer={expressionRendererMock} doc={{ - activeDatasourceId: 'testDatasource', visualizationType: 'testVis', title: '', expression: '', @@ -1379,7 +1375,6 @@ describe('editor_frame', () => { expect(onChange).toHaveBeenNthCalledWith(1, { indexPatternTitles: ['resolved'], doc: { - activeDatasourceId: 'testDatasource', expression: '', id: undefined, state: { @@ -1397,7 +1392,6 @@ describe('editor_frame', () => { expect(onChange).toHaveBeenLastCalledWith({ indexPatternTitles: ['resolved'], doc: { - activeDatasourceId: 'testDatasource', expression: '', id: undefined, state: { @@ -1457,7 +1451,6 @@ describe('editor_frame', () => { expect(onChange).toHaveBeenNthCalledWith(3, { indexPatternTitles: [], doc: { - activeDatasourceId: 'testDatasource', expression: expect.stringContaining('vis "expression"'), id: undefined, state: { diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx index ff393eabf8ca5..823852eed6133 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/editor_frame.tsx @@ -198,7 +198,6 @@ export function EditorFrame(props: EditorFrameProps) { ), visualization, state, - activeDatasourceId: state.activeDatasourceId!, framePublicAPI, }); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts index 86c41868ff3dd..6bfe8f70d93c4 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.test.ts @@ -28,7 +28,6 @@ describe('save editor frame state', () => { activeDatasourceId: 'indexpattern', visualization: { activeId: '2', state: {} }, }, - activeDatasourceId: 'indexpattern', framePublicAPI: { addNewLayer: jest.fn(), removeLayers: jest.fn(), @@ -71,7 +70,6 @@ describe('save editor frame state', () => { }); expect(doc).toEqual({ - activeDatasourceId: 'indexpattern', id: undefined, expression: '', state: { diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.ts index 829955d910e25..6c414d9866033 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/save.ts @@ -16,7 +16,6 @@ export interface Props { state: EditorFrameState; visualization: Visualization; framePublicAPI: FramePublicAPI; - activeDatasourceId: string; } export function getSavedObjectFormat({ @@ -24,7 +23,6 @@ export function getSavedObjectFormat({ state, visualization, framePublicAPI, - activeDatasourceId, }: Props): Document { const expression = buildExpression({ visualization, @@ -53,7 +51,6 @@ export function getSavedObjectFormat({ type: 'lens', visualizationType: state.visualization.activeId, expression: expression ? toExpression(expression) : '', - activeDatasourceId, state: { datasourceStates, datasourceMetaData: { diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index 6f76cc15dafed..c8dbe68884edf 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -64,6 +64,21 @@ export type Action = newDatasourceId: string; }; +function getActiveDatasourceIdFromDoc(doc?: Document) { + if (!doc) { + return null; + } + + const [initialDatasourceId] = Object.keys(doc.state.datasourceStates); + return initialDatasourceId || null; +} + +function getInitialDatasourceId(props: EditorFrameProps) { + return props.initialDatasourceId + ? props.initialDatasourceId + : getActiveDatasourceIdFromDoc(props.doc); +} + export const getInitialState = (props: EditorFrameProps): EditorFrameState => { const datasourceStates: EditorFrameState['datasourceStates'] = {}; @@ -81,7 +96,7 @@ export const getInitialState = (props: EditorFrameProps): EditorFrameState => { return { title: i18n.translate('xpack.lens.chartTitle', { defaultMessage: 'New visualization' }), datasourceStates, - activeDatasourceId: props.initialDatasourceId ? props.initialDatasourceId : null, + activeDatasourceId: getInitialDatasourceId(props), visualization: { state: null, activeId: props.initialVisualizationId, @@ -124,7 +139,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta }), {} ), - activeDatasourceId: action.doc.activeDatasourceId, + activeDatasourceId: getActiveDatasourceIdFromDoc(action.doc), visualization: { ...state.visualization, activeId: action.doc.visualizationType, diff --git a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts index e047e8e57c4dd..5e307f78581c8 100644 --- a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts @@ -14,7 +14,6 @@ export interface Document { type?: string; visualizationType: string | null; title: string; - activeDatasourceId: string; expression: string; state: { datasourceMetaData: { From 92486e2a16b0bb89a4e9c705fa1f8af39259eb6d Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Wed, 14 Aug 2019 14:05:04 -0400 Subject: [PATCH 3/4] Fix typings and tests --- .../editor_frame/state_management.test.ts | 2 -- .../editor_frame_plugin/editor_frame/state_management.ts | 2 +- .../editor_frame_plugin/embeddable/embeddable.test.tsx | 1 - .../plugins/lens/public/editor_frame_plugin/plugin.tsx | 3 ++- .../lens/public/persistence/saved_object_store.test.ts | 7 ------- 5 files changed, 3 insertions(+), 12 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts index 22ff323507487..8320a79195442 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.test.ts @@ -45,7 +45,6 @@ describe('editor_frame state management', () => { const initialState = getInitialState({ ...props, doc: { - activeDatasourceId: 'testDatasource', expression: '', state: { datasourceStates: { @@ -370,7 +369,6 @@ describe('editor_frame state management', () => { doc: { id: 'b', expression: '', - activeDatasourceId: 'a', state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { a: { foo: 'c' } }, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts index c8dbe68884edf..60e053bf19133 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/state_management.ts @@ -64,7 +64,7 @@ export type Action = newDatasourceId: string; }; -function getActiveDatasourceIdFromDoc(doc?: Document) { +export function getActiveDatasourceIdFromDoc(doc?: Document) { if (!doc) { return null; } diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/embeddable/embeddable.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/embeddable/embeddable.test.tsx index 813c15b360952..2009eb232562b 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/embeddable/embeddable.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/embeddable/embeddable.test.tsx @@ -17,7 +17,6 @@ jest.mock('../../../../../../../src/legacy/ui/public/inspector', () => ({ })); const savedVis: Document = { - activeDatasourceId: '', expression: 'my | expression', state: { visualization: {}, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx index 08a760671555e..62339a4cc3afc 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/plugin.tsx @@ -19,6 +19,7 @@ import { Datasource, Visualization, EditorFrameSetup, EditorFrameInstance } from import { EditorFrame } from './editor_frame'; import { mergeTables } from './merge_tables'; import { EmbeddableFactory } from './embeddable/embeddable_factory'; +import { getActiveDatasourceIdFromDoc } from './editor_frame/state_management'; export interface EditorFrameSetupPlugins { data: typeof data; @@ -67,7 +68,7 @@ export class EditorFramePlugin { onError={onError} datasourceMap={this.datasources} visualizationMap={this.visualizations} - initialDatasourceId={(doc && doc.activeDatasourceId) || firstDatasourceId || null} + initialDatasourceId={getActiveDatasourceIdFromDoc(doc) || firstDatasourceId || null} initialVisualizationId={ (doc && doc.visualizationType) || firstVisualizationId || null } diff --git a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts index 6952d1254ee00..515d008d82586 100644 --- a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts +++ b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.test.ts @@ -27,7 +27,6 @@ describe('LensStore', () => { title: 'Hello', visualizationType: 'bar', expression: '', - activeDatasourceId: 'indexpattern', state: { datasourceMetaData: { filterableIndexPatterns: [], @@ -46,7 +45,6 @@ describe('LensStore', () => { title: 'Hello', visualizationType: 'bar', expression: '', - activeDatasourceId: 'indexpattern', state: { datasourceMetaData: { filterableIndexPatterns: [], @@ -64,9 +62,7 @@ describe('LensStore', () => { expect(client.create).toHaveBeenCalledWith('lens', { title: 'Hello', visualizationType: 'bar', - expression: '', - activeDatasourceId: 'indexpattern', state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { @@ -86,7 +82,6 @@ describe('LensStore', () => { title: 'Even the very wise cannot see all ends.', visualizationType: 'line', expression: '', - activeDatasourceId: 'indexpattern', state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: 'lotr' } }, @@ -101,7 +96,6 @@ describe('LensStore', () => { title: 'Even the very wise cannot see all ends.', visualizationType: 'line', expression: '', - activeDatasourceId: 'indexpattern', state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: 'lotr' } }, @@ -116,7 +110,6 @@ describe('LensStore', () => { title: 'Even the very wise cannot see all ends.', visualizationType: 'line', expression: '', - activeDatasourceId: 'indexpattern', state: { datasourceMetaData: { filterableIndexPatterns: [] }, datasourceStates: { indexpattern: { type: 'index_pattern', indexPattern: 'lotr' } }, From 2d47bc1cc9b5f71cb31e3c0eea1c8579aa634eed Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Thu, 15 Aug 2019 15:19:10 -0400 Subject: [PATCH 4/4] Remove uneccessary use of any --- .../lens/public/persistence/saved_object_store.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts index 5e307f78581c8..5fa7e3f0aca4a 100644 --- a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts @@ -61,12 +61,9 @@ export class SavedObjectIndexStore implements SavedObjectStore { async save(vis: Document) { const { id, type, ...rest } = vis; - - // Any is the most straighforward way out here, since the saved - // object client doesn't allow unknowns, and we have them in - // our object. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const attributes: any = rest; + // TODO: SavedObjectAttributes should support this kind of object, + // remove this workaround when SavedObjectAttributes is updated. + const attributes = (rest as unknown) as SavedObjectAttributes; const result = await (id ? this.client.update(DOC_TYPE, id, attributes) : this.client.create(DOC_TYPE, attributes));