From 95bb309b57f8833f838ef498cc7d3490c09c651b Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 6 Apr 2020 16:46:27 -0400 Subject: [PATCH] [Lens] Fix empty dimensions after upgrading from pre-7.7 --- .../editor_frame/editor_frame.test.tsx | 44 ++++++++++----- .../editor_frame/editor_frame.tsx | 23 +++++--- x-pack/legacy/plugins/lens/public/types.ts | 11 +++- .../xy_visualization/xy_visualization.test.ts | 25 +++++++-- .../xy_visualization/xy_visualization.tsx | 54 +++++++++++++------ 5 files changed, 117 insertions(+), 40 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx index 8d8d38944e18a..f129a36392567 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx @@ -116,7 +116,7 @@ describe('editor_frame', () => { expect(mockDatasource.initialize).toHaveBeenCalled(); }); - it('should not initialize datasource and visualization if no initial one is specificed', () => { + it('should not initialize datasource and visualization if no initial one is specified', () => { act(() => { mount( { expect(mockVisualization.initialize).toHaveBeenCalled(); }); - it('should pass the public frame api into visualization initialize', async () => { + it('should use initialize the visualization and update with private state', async () => { + const initializeMock = { + ...mockVisualization, + initialize: jest.fn().mockReturnValue('initialized'), + }; await act(async () => { mount( { dateRange={{ fromDate: 'now-7d', toDate: 'now' }} /> ); - expect(mockVisualization.initialize).not.toHaveBeenCalled(); + expect(initializeMock.initialize).not.toHaveBeenCalled(); }); - expect(mockVisualization.initialize).toHaveBeenCalledWith({ - datasourceLayers: {}, - addNewLayer: expect.any(Function), - removeLayers: expect.any(Function), - query: { query: '', language: 'lucene' }, - filters: [], - dateRange: { fromDate: 'now-7d', toDate: 'now' }, - }); + expect(initializeMock.initialize).toHaveBeenCalledTimes(2); + expect(initializeMock.initialize).toHaveBeenCalledWith( + { + datasourceLayers: {}, + addNewLayer: expect.any(Function), + removeLayers: expect.any(Function), + query: { query: '', language: 'lucene' }, + filters: [], + dateRange: { fromDate: 'now-7d', toDate: 'now' }, + }, + null + ); + + expect(initializeMock.initialize).toHaveBeenCalledWith( + { + datasourceLayers: {}, + addNewLayer: expect.any(Function), + removeLayers: expect.any(Function), + query: { query: '', language: 'lucene' }, + filters: [], + dateRange: { fromDate: 'now-7d', toDate: 'now' }, + }, + 'initialized' + ); }); it('should add new layer on active datasource on frame api call', async () => { diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx index 082519d9a8feb..54aee1bc3b217 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { isEqual } from 'lodash'; import React, { useEffect, useReducer } from 'react'; import { CoreSetup, CoreStart } from 'src/core/public'; import { ReactExpressionRendererType } from '../../../../../../../src/plugins/expressions/public'; @@ -163,13 +164,21 @@ export function EditorFrame(props: EditorFrameProps) { // Initialize visualization as soon as all datasources are ready useEffect(() => { - if (allLoaded && state.visualization.state === null && activeVisualization) { - const initialVisualizationState = activeVisualization.initialize(framePublicAPI); - dispatch({ - type: 'UPDATE_VISUALIZATION_STATE', - visualizationId: activeVisualization.id, - newState: initialVisualizationState, - }); + if (allLoaded && activeVisualization) { + // Initialize from either empty state or saved doc + const initializedState = activeVisualization.initialize( + framePublicAPI, + state.visualization.state ?? null + ); + + // Visualizations can have private state that is not saved + if (!isEqual(state.visualization.state, initializedState)) { + dispatch({ + type: 'UPDATE_VISUALIZATION_STATE', + visualizationId: activeVisualization.id, + newState: initializedState, + }); + } } }, [allLoaded, activeVisualization, state.visualization.state]); diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 3d67b7b2da460..1b5c4324f41fa 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -398,7 +398,16 @@ export interface Visualization { switchVisualizationType?: (visualizationTypeId: string, state: T) => T; - // For initializing from saved object + /** + * Initialize is called when opening the visualization, such as in the following cases: + * + * - Loading Lens in an empty state + * - Loading a saved Lens visualization which uses this visualization + * - When switching to this visualization in the chart switcher + * + * The response of the initialize call will be used to update the editor state, such as + * if the visualization tracks UI state that is not saved + */ initialize: (frame: FramePublicAPI, state?: P) => T; getPersistableState: (state: T) => P; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.test.ts index beccf0dc46eb4..f5ca2616c8b13 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.test.ts @@ -111,10 +111,27 @@ describe('xy_visualization', () => { `); }); - it('loads from persisted state', () => { - expect(xyVisualization.initialize(createMockFramePublicAPI(), exampleState())).toEqual( - exampleState() - ); + it('removes dimensions from saved state if they are not on datasource', () => { + const state = exampleState(); + expect(xyVisualization.initialize(createMockFramePublicAPI(), state)).toEqual({ + ...state, + layers: [ + { + layerId: 'first', + seriesType: 'area', + splitAccessor: undefined, + xAccessor: undefined, + accessors: [], + }, + ], + }); + }); + + it('keeps dimensions from saved state if they are in datasource', () => { + const state = exampleState(); + const frame = createMockFramePublicAPI(); + (frame.datasourceLayers.first.getOperationForColumnId as jest.Mock).mockReturnValue(true); + expect(xyVisualization.initialize(frame, state)).toEqual(state); }); }); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.tsx index c72fa0fec24d7..1b19bcb8f3834 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization/xy_visualization.tsx @@ -126,28 +126,50 @@ export const xyVisualization: Visualization = { getSuggestions, initialize(frame, state) { - return ( - state || { - title: 'Empty XY chart', - legend: { isVisible: true, position: Position.Right }, - preferredSeriesType: defaultSeriesType, - layers: [ - { - layerId: frame.addNewLayer(), - accessors: [], - position: Position.Top, - seriesType: defaultSeriesType, - showGridlines: false, - }, - ], - } - ); + if (state) { + // Before 7.7, the XY visualization would generate IDs for missing dimensions + // Instead of migrating, we use the initialization step to hide these + return { + ...state, + layers: state.layers.map(layer => { + const datasource = frame.datasourceLayers[layer.layerId]; + const hasXAccessor = + layer.xAccessor && datasource.getOperationForColumnId(layer.xAccessor); + const hasSplitAccessor = + layer.splitAccessor && datasource.getOperationForColumnId(layer.splitAccessor); + const yAccessors = layer.accessors.filter(accessor => + datasource.getOperationForColumnId(accessor) + ); + return { + ...layer, + accessors: yAccessors, + xAccessor: hasXAccessor ? layer.xAccessor : undefined, + splitAccessor: hasSplitAccessor ? layer.splitAccessor : undefined, + }; + }), + }; + } + return { + title: 'Empty XY chart', + legend: { isVisible: true, position: Position.Right }, + preferredSeriesType: defaultSeriesType, + layers: [ + { + layerId: frame.addNewLayer(), + accessors: [], + position: Position.Top, + seriesType: defaultSeriesType, + showGridlines: false, + }, + ], + }; }, getPersistableState: state => state, getConfiguration(props) { const layer = props.state.layers.find(l => l.layerId === props.layerId)!; + return { groups: [ {