From 0f1dd47e9c6f40f51a26aa04d54125efd6026287 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 21 Jun 2019 13:01:09 -0400 Subject: [PATCH 1/7] Add split series to lens xy chart --- .../editor_frame/editor_frame.test.tsx | 1 - .../editor_frame/suggestion_helpers.test.ts | 15 ++++++-- .../editor_frame/suggestion_helpers.ts | 4 ++- .../editor_frame/suggestion_panel.tsx | 3 ++ .../editor_frame/workspace_panel.tsx | 3 ++ x-pack/legacy/plugins/lens/public/types.ts | 1 + .../xy_config_panel.tsx | 17 ++++++++++ .../xy_suggestions.test.ts | 34 +++++++++++++++---- .../xy_visualization_plugin/xy_suggestions.ts | 23 +++++++++---- .../xy_visualization.test.ts | 4 ++- .../xy_visualization.tsx | 2 +- 11 files changed, 89 insertions(+), 18 deletions(-) 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 8d67c632ea5c9..1074a4583df8b 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 @@ -446,7 +446,6 @@ Object { setDatasourceState(updatedState); }); - expect(mockDatasource.getPublicAPI).toHaveBeenCalledTimes(2); expect(mockDatasource.getPublicAPI).toHaveBeenLastCalledWith( updatedState, expect.any(Function) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts index 850cdfc2b3c0f..27b4a63f16fac 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts @@ -5,19 +5,25 @@ */ import { getSuggestions } from './suggestion_helpers'; -import { createMockVisualization } from '../mocks'; -import { TableSuggestion } from '../../types'; +import { createMockVisualization, createMockDatasource } from '../mocks'; +import { TableSuggestion, DatasourcePublicAPI } from '../../types'; const generateSuggestion = (datasourceSuggestionId: number = 1, state = {}) => ({ state, table: { datasourceSuggestionId, columns: [], isMultiRow: false }, }); +const mockDatasource: DatasourcePublicAPI = { + ...createMockDatasource().getPublicAPI({}, jest.fn()), + generateColumnId: () => 'splitcolid', +}; + describe('suggestion helpers', () => { it('should return suggestions array', () => { const mockVisualization = createMockVisualization(); const suggestedState = {}; const suggestions = getSuggestions( + mockDatasource, [generateSuggestion()], { vis1: { @@ -38,6 +44,7 @@ describe('suggestion helpers', () => { const mockVisualization1 = createMockVisualization(); const mockVisualization2 = createMockVisualization(); const suggestions = getSuggestions( + mockDatasource, [generateSuggestion()], { vis1: { @@ -64,6 +71,7 @@ describe('suggestion helpers', () => { const mockVisualization1 = createMockVisualization(); const mockVisualization2 = createMockVisualization(); const suggestions = getSuggestions( + mockDatasource, [generateSuggestion()], { vis1: { @@ -94,6 +102,7 @@ describe('suggestion helpers', () => { const table1: TableSuggestion = { datasourceSuggestionId: 0, columns: [], isMultiRow: true }; const table2: TableSuggestion = { datasourceSuggestionId: 1, columns: [], isMultiRow: true }; getSuggestions( + mockDatasource, [{ state: {}, table: table1 }, { state: {}, table: table2 }], { vis1: mockVisualization1, @@ -114,6 +123,7 @@ describe('suggestion helpers', () => { const tableState1 = {}; const tableState2 = {}; const suggestions = getSuggestions( + mockDatasource, [generateSuggestion(1, tableState1), generateSuggestion(1, tableState2)], { vis1: { @@ -143,6 +153,7 @@ describe('suggestion helpers', () => { const mockVisualization2 = createMockVisualization(); const currentState = {}; getSuggestions( + mockDatasource, [generateSuggestion(1), generateSuggestion(2)], { vis1: mockVisualization1, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts index 459f5d89fb9c3..d3f37ab1d4e3f 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Visualization, DatasourceSuggestion } from '../../types'; +import { Visualization, DatasourceSuggestion, DatasourcePublicAPI } from '../../types'; import { Action } from './state_management'; export interface Suggestion { @@ -24,6 +24,7 @@ export interface Suggestion { * action with `toSwitchAction` and dispatching it */ export function getSuggestions( + datasource: DatasourcePublicAPI, datasourceTableSuggestions: DatasourceSuggestion[], visualizationMap: Record, activeVisualizationId: string | null, @@ -36,6 +37,7 @@ export function getSuggestions( .map(([visualizationId, visualization]) => { return visualization .getSuggestions({ + datasource, tables: datasourceTables, state: visualizationId === activeVisualizationId ? visualizationState : undefined, }) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx index 9d9730db37651..d82275e58ea89 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx @@ -33,6 +33,9 @@ export function SuggestionPanel({ ); const suggestions = getSuggestions( + activeDatasource.getPublicAPI(datasourceState, newState => + dispatch({ type: 'UPDATE_DATASOURCE_STATE', newState }) + ), datasourceSuggestions, visualizationMap, activeVisualizationId, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx index dc9b5ffce6b49..eea64a12de960 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx @@ -44,6 +44,9 @@ export function WorkspacePanel({ ); const suggestions = getSuggestions( + activeDatasource.getPublicAPI(datasourceState, newState => + dispatch({ type: 'UPDATE_DATASOURCE_STATE', newState }) + ), datasourceSuggestions, visualizationMap, activeVisualizationId, diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 367d1bdd99c79..1e0916d3ac643 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -145,6 +145,7 @@ export interface VisualizationProps { export interface SuggestionRequest { // It is up to the Visualization to rank these tables + datasource: DatasourcePublicAPI; tables: TableSuggestion[]; state?: T; // State is only passed if the visualization is active } diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 86883b4e629e3..99a719b2eccf3 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -161,6 +161,23 @@ export function XYConfigPanel(props: VisualizationProps) { )} + + op.isBucketed === true, + suggestedPriority: 0, + }} + /> + + { + const datasource: DatasourcePublicAPI = { + ...createMockDatasource().getPublicAPI({}, jest.fn()), + generateColumnId: jest.fn(() => 'testcol'), + }; + function numCol(columnId: string): TableColumn { return { columnId, @@ -68,6 +74,7 @@ describe('xy_suggestions', () => { expect( getSuggestions({ + datasource, tables: [ { datasourceSuggestionId: 0, isMultiRow: true, columns: [dateCol('a')] }, { datasourceSuggestionId: 1, isMultiRow: true, columns: [strCol('foo'), strCol('bar')] }, @@ -80,6 +87,7 @@ describe('xy_suggestions', () => { test('suggests a basic x y chart with date on x', () => { const [suggestion, ...rest] = getSuggestions({ + datasource, tables: [ { datasourceSuggestionId: 0, @@ -93,7 +101,9 @@ describe('xy_suggestions', () => { expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` Object { "seriesType": "line", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "testcol", + ], "stackAccessors": Array [], "x": "date", "y": Array [ @@ -105,6 +115,7 @@ Object { test('suggests a split x y chart with date on x', () => { const [suggestion, ...rest] = getSuggestions({ + datasource, tables: [ { datasourceSuggestionId: 1, @@ -133,6 +144,7 @@ Object { test('supports multiple suggestions', () => { const [s1, s2, ...rest] = getSuggestions({ + datasource, tables: [ { datasourceSuggestionId: 0, @@ -152,7 +164,9 @@ Object { Array [ Object { "seriesType": "line", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "testcol", + ], "stackAccessors": Array [], "x": "date", "y": Array [ @@ -161,7 +175,9 @@ Array [ }, Object { "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "testcol", + ], "stackAccessors": Array [], "x": "country", "y": Array [ @@ -174,6 +190,7 @@ Array [ test('handles two numeric values', () => { const [suggestion] = getSuggestions({ + datasource, tables: [ { datasourceSuggestionId: 1, @@ -186,7 +203,9 @@ Array [ expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` Object { "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "testcol", + ], "stackAccessors": Array [], "x": "quantity", "y": Array [ @@ -198,6 +217,7 @@ Object { test('handles unbucketed suggestions', () => { const [suggestion] = getSuggestions({ + datasource, tables: [ { datasourceSuggestionId: 1, @@ -221,7 +241,9 @@ Object { expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` Object { "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "testcol", + ], "stackAccessors": Array [], "x": "mybool", "y": Array [ diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts index 28ef677e49644..6233087234316 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts @@ -6,7 +6,13 @@ import { partition } from 'lodash'; import { Position } from '@elastic/charts'; -import { SuggestionRequest, VisualizationSuggestion, TableColumn, TableSuggestion } from '../types'; +import { + SuggestionRequest, + VisualizationSuggestion, + TableColumn, + TableSuggestion, + DatasourcePublicAPI, +} from '../types'; import { State } from './types'; const columnSortOrder = { @@ -35,10 +41,13 @@ export function getSuggestions( columns.some(col => col.operation.dataType === 'number') && !columns.some(col => !columnSortOrder.hasOwnProperty(col.operation.dataType)) ) - .map(table => getSuggestionForColumns(table)); + .map(table => getSuggestionForColumns(opts.datasource, table)); } -function getSuggestionForColumns(table: TableSuggestion): VisualizationSuggestion { +function getSuggestionForColumns( + datasource: DatasourcePublicAPI, + table: TableSuggestion +): VisualizationSuggestion { const [buckets, values] = partition( prioritizeColumns(table.columns), col => col.operation.isBucketed @@ -46,10 +55,10 @@ function getSuggestionForColumns(table: TableSuggestion): VisualizationSuggestio if (buckets.length >= 1) { const [x, splitBy] = buckets; - return getSuggestion(table.datasourceSuggestionId, x, values, splitBy); + return getSuggestion(datasource, table.datasourceSuggestionId, x, values, splitBy); } else { const [x, ...yValues] = values; - return getSuggestion(table.datasourceSuggestionId, x, yValues); + return getSuggestion(datasource, table.datasourceSuggestionId, x, yValues); } } @@ -63,6 +72,7 @@ function prioritizeColumns(columns: TableColumn[]) { } function getSuggestion( + datasource: DatasourcePublicAPI, datasourceSuggestionId: number, xValue: TableColumn, yValues: TableColumn[], @@ -83,7 +93,8 @@ function getSuggestion( title, legend: { isVisible: true, position: Position.Right }, seriesType: isDate ? 'line' : 'bar', - splitSeriesAccessors: splitBy && isDate ? [splitBy.columnId] : [], + splitSeriesAccessors: + splitBy && isDate ? [splitBy.columnId] : [datasource.generateColumnId()], stackAccessors: splitBy && !isDate ? [splitBy.columnId] : [], x: { accessor: xValue.columnId, diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts index b4ddc5617eaec..56fe1ce184852 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts @@ -53,7 +53,9 @@ Object { "position": "right", }, "seriesType": "line", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "test-id1", + ], "stackAccessors": Array [], "title": "Empty XY Chart", "x": Object { diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx index 494b653c6d944..347b620baeb15 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx @@ -34,7 +34,7 @@ export const xyVisualization: Visualization = { showGridlines: false, title: 'Y', }, - splitSeriesAccessors: [], + splitSeriesAccessors: [datasource.generateColumnId()], stackAccessors: [], } ); From c163eee8476fd6d1caaeb4b030f75a83c9dc58b3 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 21 Jun 2019 13:41:15 -0400 Subject: [PATCH 2/7] Test for multiple split support in lens xy chart --- .../xy_config_panel.test.tsx | 20 ++++++++++ .../xy_config_panel.tsx | 37 ++++++++++++++----- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 95c4543f32547..31edba79f892d 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -376,6 +376,26 @@ describe('XYConfigPanel', () => { }); }); + test('allows adding split dimensions', () => { + const setState = jest.fn(); + const state = testState(); + const component = mount( + 'zed' }} + setState={setState} + state={{ ...state, splitSeriesAccessors: ['a', 'b', 'c'] }} + /> + ); + + (testSubj(component, 'lnsXY_splitSeriesDimensionPanel_add').onClick as Function)(); + + expect(setState).toHaveBeenCalledTimes(1); + expect(setState.mock.calls[0][0]).toMatchObject({ + splitSeriesAccessors: ['a', 'b', 'c', 'zed'], + }); + }); + test('allows toggling the y axis gridlines', () => { const toggleYGridlines = (showGridlines: boolean) => { const setState = jest.fn(); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 99a719b2eccf3..8e136f159b33e 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -166,16 +166,33 @@ export function XYConfigPanel(props: VisualizationProps) { defaultMessage: 'Split series', })} > - op.isBucketed === true, - suggestedPriority: 0, - }} - /> + <> + {state.splitSeriesAccessors.map(columnId => ( + op.isBucketed === true, + suggestedPriority: 0, + }} + /> + ))} + + setState({ + ...state, + splitSeriesAccessors: [ + ...state.splitSeriesAccessors, + datasource.generateColumnId(), + ], + }) + } + iconType="plusInCircle" + /> + Date: Thu, 27 Jun 2019 16:02:13 -0400 Subject: [PATCH 3/7] Remove id generation from datasource --- .../visualization.test.tsx | 9 ++++-- .../visualization.tsx | 5 ++-- .../editor_frame/suggestion_helpers.test.ts | 5 +--- .../lens/public/editor_frame_plugin/mocks.tsx | 1 - .../public/id_generator/id_generator.test.ts | 13 ++++++++ .../lens/public/id_generator/id_generator.ts | 11 +++++++ .../plugins/lens/public/id_generator/index.ts | 7 +++++ .../plugins/lens/public/id_generator/mock.ts | 10 +++++++ .../indexpattern_plugin/indexpattern.tsx | 5 ---- x-pack/legacy/plugins/lens/public/types.ts | 1 - .../xy_config_panel.test.tsx | 13 +++++--- .../xy_config_panel.tsx | 8 ++--- .../xy_suggestions.test.ts | 30 +++++++++++++------ .../xy_visualization_plugin/xy_suggestions.ts | 3 +- .../xy_visualization.test.ts | 14 +++++---- .../xy_visualization.tsx | 7 +++-- 16 files changed, 98 insertions(+), 44 deletions(-) create mode 100644 x-pack/legacy/plugins/lens/public/id_generator/id_generator.test.ts create mode 100644 x-pack/legacy/plugins/lens/public/id_generator/id_generator.ts create mode 100644 x-pack/legacy/plugins/lens/public/id_generator/index.ts create mode 100644 x-pack/legacy/plugins/lens/public/id_generator/mock.ts diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx index 2cde89fe2d5d8..b1a892ce6d829 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx @@ -14,12 +14,16 @@ import { import { mount } from 'enzyme'; import { act } from 'react-dom/test-utils'; import { Operation, DataType } from '../types'; +import * as generator from '../id_generator'; +import { mockGeneratedIds } from '../id_generator/mock'; + +jest.mock('../id_generator'); describe('Datatable Visualization', () => { describe('#initialize', () => { it('should initialize from the empty state', () => { const datasource = createMockDatasource(); - datasource.publicAPIMock.generateColumnId.mockReturnValueOnce('id'); + mockGeneratedIds(generator, 'id'); expect(datatableVisualization.initialize(datasource.publicAPIMock)).toEqual({ columns: [{ id: 'id', label: '' }], }); @@ -30,7 +34,6 @@ describe('Datatable Visualization', () => { const expectedState: DatatableVisualizationState = { columns: [{ id: 'saved', label: 'label' }], }; - expect(datasource.publicAPIMock.generateColumnId).not.toHaveBeenCalled(); expect(datatableVisualization.initialize(datasource.publicAPIMock, expectedState)).toEqual( expectedState ); @@ -138,7 +141,7 @@ describe('Datatable Visualization', () => { /> ); - datasource.publicAPIMock.generateColumnId.mockReturnValueOnce('newId'); + mockGeneratedIds(generator, 'newId'); act(() => { wrapper diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx index d4d9476240e68..7bd1dbedd1ac0 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.tsx @@ -24,6 +24,7 @@ import { VisualizationSuggestion, } from '../types'; import { NativeRenderer } from '../native_renderer'; +import { generateId } from '../id_generator'; export interface DatatableVisualizationState { columns: Array<{ @@ -117,7 +118,7 @@ export function DatatableConfigPanel(props: VisualizationProps { const newColumns = [...state.columns]; newColumns.push({ - id: datasource.generateColumnId(), + id: generateId(), label: '', }); setState({ @@ -141,7 +142,7 @@ export const datatableVisualization: Visualization< state || { columns: [ { - id: datasource.generateColumnId(), + id: generateId(), label: '', }, ], diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts index 1cba1d22b9f76..41054c07f7af8 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts @@ -13,10 +13,7 @@ const generateSuggestion = (datasourceSuggestionId: number = 1, state = {}) => ( table: { datasourceSuggestionId, columns: [], isMultiRow: false }, }); -const mockDatasource: DatasourcePublicAPI = { - ...createMockDatasource().getPublicAPI({}, jest.fn()), - generateColumnId: () => 'splitcolid', -}; +const mockDatasource: DatasourcePublicAPI = createMockDatasource().getPublicAPI({}, jest.fn()); describe('suggestion helpers', () => { it('should return suggestions array', () => { diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx index d6e9e2f530fc0..cb75fab6e457f 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/mocks.tsx @@ -27,7 +27,6 @@ export function createMockDatasource(): DatasourceMock { const publicAPIMock: jest.Mocked = { getTableSpec: jest.fn(() => []), getOperationForColumnId: jest.fn(), - generateColumnId: jest.fn(), renderDimensionPanel: jest.fn(), removeColumnInTableSpec: jest.fn(), moveColumnTo: jest.fn(), diff --git a/x-pack/legacy/plugins/lens/public/id_generator/id_generator.test.ts b/x-pack/legacy/plugins/lens/public/id_generator/id_generator.test.ts new file mode 100644 index 0000000000000..29aae6117e442 --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/id_generator/id_generator.test.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { generateId } from './id_generator'; + +describe('XYConfigPanel', () => { + it('generates different ids', () => { + expect(generateId()).not.toEqual(generateId()); + }); +}); diff --git a/x-pack/legacy/plugins/lens/public/id_generator/id_generator.ts b/x-pack/legacy/plugins/lens/public/id_generator/id_generator.ts new file mode 100644 index 0000000000000..82579769925eb --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/id_generator/id_generator.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import uuid from 'uuid/v4'; + +export function generateId() { + return uuid(); +} diff --git a/x-pack/legacy/plugins/lens/public/id_generator/index.ts b/x-pack/legacy/plugins/lens/public/id_generator/index.ts new file mode 100644 index 0000000000000..541e7e6aa4a70 --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/id_generator/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export * from './id_generator'; diff --git a/x-pack/legacy/plugins/lens/public/id_generator/mock.ts b/x-pack/legacy/plugins/lens/public/id_generator/mock.ts new file mode 100644 index 0000000000000..691ec119ca645 --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/id_generator/mock.ts @@ -0,0 +1,10 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export function mockGeneratedIds(generator: { generateId: () => string }, ...ids: string[]) { + const mock = jest.spyOn(generator, 'generateId'); + ids.forEach(id => mock.mockReturnValueOnce(id)); +} diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index a1b3d9b6efe3d..f348ae6b88a69 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -262,11 +262,6 @@ export function getIndexPatternDatasource(chrome: Chrome, toastNotifications: To } return columnToOperation(state.columns[columnId]); }, - generateColumnId: () => { - // TODO: Come up with a more compact form of generating unique column ids - return uuid.v4(); - }, - renderDimensionPanel: (domElement: Element, props: DatasourceDimensionPanelProps) => { render( { export interface DatasourcePublicAPI { getTableSpec: () => TableSpec; getOperationForColumnId: (columnId: string) => Operation | null; - generateColumnId: () => string; // Render can be called many times renderDimensionPanel: (domElement: Element, props: DatasourceDimensionPanelProps) => void; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 31edba79f892d..a1f8868d0b1c4 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -12,6 +12,10 @@ import { DatasourcePublicAPI, DatasourceDimensionPanelProps, Operation } from '. import { State, SeriesType } from './types'; import { Position } from '@elastic/charts'; import { NativeRendererProps } from '../native_renderer'; +import * as generator from '../id_generator'; +import { mockGeneratedIds } from '../id_generator/mock'; + +jest.mock('../id_generator'); describe('XYConfigPanel', () => { const dragDropContext = { dragging: undefined, setDragging: jest.fn() }; @@ -20,7 +24,6 @@ describe('XYConfigPanel', () => { return { duplicateColumn: () => [], getOperationForColumnId: () => null, - generateColumnId: () => 'TESTID', getTableSpec: () => [], moveColumnTo: () => {}, removeColumnInTableSpec: () => [], @@ -357,12 +360,13 @@ describe('XYConfigPanel', () => { }); test('allows adding y dimensions', () => { + mockGeneratedIds(generator, 'zed'); const setState = jest.fn(); const state = testState(); const component = mount( 'zed' }} + datasource={mockDatasource()} setState={setState} state={{ ...state, y: { ...state.y, accessors: ['a', 'b', 'c'] } }} /> @@ -377,12 +381,13 @@ describe('XYConfigPanel', () => { }); test('allows adding split dimensions', () => { + mockGeneratedIds(generator, 'foo'); const setState = jest.fn(); const state = testState(); const component = mount( 'zed' }} + datasource={mockDatasource()} setState={setState} state={{ ...state, splitSeriesAccessors: ['a', 'b', 'c'] }} /> @@ -392,7 +397,7 @@ describe('XYConfigPanel', () => { expect(setState).toHaveBeenCalledTimes(1); expect(setState.mock.calls[0][0]).toMatchObject({ - splitSeriesAccessors: ['a', 'b', 'c', 'zed'], + splitSeriesAccessors: ['a', 'b', 'c', 'foo'], }); }); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 8e136f159b33e..56a07f3c2e6a5 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -20,6 +20,7 @@ import { import { State, SeriesType } from './types'; import { VisualizationProps, Operation } from '../types'; import { NativeRenderer } from '../native_renderer'; +import { generateId } from '../id_generator'; const chartTypeIcons: Array<{ id: SeriesType; label: string; iconType: IconType }> = [ { @@ -184,10 +185,7 @@ export function XYConfigPanel(props: VisualizationProps) { onClick={() => setState({ ...state, - splitSeriesAccessors: [ - ...state.splitSeriesAccessors, - datasource.generateColumnId(), - ], + splitSeriesAccessors: [...state.splitSeriesAccessors, generateId()], }) } iconType="plusInCircle" @@ -321,7 +319,7 @@ export function XYConfigPanel(props: VisualizationProps) { ...state, y: { ...state.y, - accessors: [...state.y.accessors, datasource.generateColumnId()], + accessors: [...state.y.accessors, generateId()], }, }) } diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts index e50f4f0580129..8ad3af57cd8ac 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts @@ -9,12 +9,13 @@ import { TableColumn, VisualizationSuggestion, DatasourcePublicAPI } from '../ty import { State } from './types'; import { Ast } from '@kbn/interpreter/target/common'; import { createMockDatasource } from '../editor_frame_plugin/mocks'; +import * as generator from '../id_generator'; +import { mockGeneratedIds } from '../id_generator/mock'; + +jest.mock('../id_generator'); describe('xy_suggestions', () => { - const datasource: DatasourcePublicAPI = { - ...createMockDatasource().getPublicAPI({}, jest.fn()), - generateColumnId: jest.fn(() => 'testcol'), - }; + const datasource: DatasourcePublicAPI = createMockDatasource().getPublicAPI({}, jest.fn()); function numCol(columnId: string): TableColumn { return { @@ -76,6 +77,7 @@ describe('xy_suggestions', () => { expect( getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 0, isMultiRow: true, columns: [dateCol('a')] }, { @@ -97,8 +99,10 @@ describe('xy_suggestions', () => { }); test('suggests a basic x y chart with date on x', () => { + mockGeneratedIds(generator, 'aaa'); const [suggestion, ...rest] = getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 0, @@ -115,7 +119,7 @@ describe('xy_suggestions', () => { Object { "seriesType": "line", "splitSeriesAccessors": Array [ - "testcol", + "aaa", ], "stackAccessors": Array [], "x": "date", @@ -129,6 +133,7 @@ Object { test('suggests a split x y chart with date on x', () => { const [suggestion, ...rest] = getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 1, @@ -158,8 +163,10 @@ Object { }); test('supports multiple suggestions', () => { + mockGeneratedIds(generator, 'bbb', 'ccc'); const [s1, s2, ...rest] = getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 0, @@ -182,7 +189,7 @@ Array [ Object { "seriesType": "line", "splitSeriesAccessors": Array [ - "testcol", + "bbb", ], "stackAccessors": Array [], "x": "date", @@ -193,7 +200,7 @@ Array [ Object { "seriesType": "bar", "splitSeriesAccessors": Array [ - "testcol", + "ccc", ], "stackAccessors": Array [], "x": "country", @@ -206,8 +213,10 @@ Array [ }); test('handles two numeric values', () => { + mockGeneratedIds(generator, 'ddd'); const [suggestion] = getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 1, @@ -223,7 +232,7 @@ Array [ Object { "seriesType": "bar", "splitSeriesAccessors": Array [ - "testcol", + "ddd", ], "stackAccessors": Array [], "x": "quantity", @@ -235,8 +244,10 @@ Object { }); test('handles unbucketed suggestions', () => { + mockGeneratedIds(generator, 'eee'); const [suggestion] = getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 1, @@ -263,7 +274,7 @@ Object { Object { "seriesType": "bar", "splitSeriesAccessors": Array [ - "testcol", + "eee", ], "stackAccessors": Array [], "x": "mybool", @@ -277,6 +288,7 @@ Object { test('adds a preview expression with disabled axes and legend', () => { const [suggestion] = getSuggestions( { + datasource, tables: [ { datasourceSuggestionId: 0, diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts index a9207d409eb84..42e544f129609 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.ts @@ -15,6 +15,7 @@ import { } from '../types'; import { State } from './types'; import { toExpression } from './to_expression'; +import { generateId } from '../id_generator'; const columnSortOrder = { date: 0, @@ -91,7 +92,7 @@ function getSuggestion( title, legend: { isVisible: true, position: Position.Right }, seriesType: isDate ? 'line' : 'bar', - splitSeriesAccessors: splitBy && isDate ? [splitBy.columnId] : [], + splitSeriesAccessors: splitBy && isDate ? [splitBy.columnId] : [generateId()], stackAccessors: splitBy && !isDate ? [splitBy.columnId] : [], x: { accessor: xValue.columnId, diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts index 56fe1ce184852..ae126f0bcd8c8 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts @@ -10,6 +10,10 @@ import { Ast } from '@kbn/interpreter/target/common'; import { Operation } from '../types'; import { State } from './types'; import { createMockDatasource } from '../editor_frame_plugin/mocks'; +import { mockGeneratedIds } from '../id_generator/mock'; +import * as generator from '../id_generator'; + +jest.mock('../id_generator'); function exampleState(): State { return { @@ -36,10 +40,8 @@ function exampleState(): State { describe('xy_visualization', () => { describe('#initialize', () => { it('loads default state', () => { + mockGeneratedIds(generator, 'test-id1', 'test-id2', 'test-id3'); const mockDatasource = createMockDatasource(); - mockDatasource.publicAPIMock.generateColumnId - .mockReturnValue('test-id1') - .mockReturnValueOnce('test-id2'); const initialState = xyVisualization.initialize(mockDatasource.publicAPIMock); expect(initialState.x.accessor).toBeDefined(); @@ -54,19 +56,19 @@ Object { }, "seriesType": "line", "splitSeriesAccessors": Array [ - "test-id1", + "test-id3", ], "stackAccessors": Array [], "title": "Empty XY Chart", "x": Object { - "accessor": "test-id2", + "accessor": "test-id1", "position": "bottom", "showGridlines": false, "title": "X", }, "y": Object { "accessors": Array [ - "test-id1", + "test-id2", ], "position": "left", "showGridlines": false, diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx index 1d5cbe3eeeae3..6baf4305656e0 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.tsx @@ -13,6 +13,7 @@ import { XYConfigPanel } from './xy_config_panel'; import { Visualization } from '../types'; import { State, PersistableState } from './types'; import { toExpression } from './to_expression'; +import { generateId } from '../id_generator'; export const xyVisualization: Visualization = { getSuggestions, @@ -24,18 +25,18 @@ export const xyVisualization: Visualization = { title: 'Empty XY Chart', legend: { isVisible: true, position: Position.Right }, x: { - accessor: datasource.generateColumnId(), + accessor: generateId(), position: Position.Bottom, showGridlines: false, title: 'X', }, y: { - accessors: [datasource.generateColumnId()], + accessors: [generateId()], position: Position.Left, showGridlines: false, title: 'Y', }, - splitSeriesAccessors: [datasource.generateColumnId()], + splitSeriesAccessors: [generateId()], stackAccessors: [], } ); From a26225538f414afbfcfbccb4e7a58082b1db70c5 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Fri, 28 Jun 2019 09:41:14 -0400 Subject: [PATCH 4/7] Fix typescript errors --- .../editor_frame/suggestion_helpers.test.ts | 12 ++---------- .../editor_frame/suggestion_helpers.ts | 1 - .../editor_frame/suggestion_panel.tsx | 4 ---- .../editor_frame/workspace_panel.tsx | 3 --- .../lens/public/indexpattern_plugin/indexpattern.tsx | 1 - x-pack/legacy/plugins/lens/public/types.ts | 1 - .../xy_visualization_plugin/xy_config_panel.tsx | 2 +- 7 files changed, 3 insertions(+), 21 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts index 68726581a43c9..ac30bc2adae3e 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.test.ts @@ -5,22 +5,19 @@ */ import { getSuggestions } from './suggestion_helpers'; -import { createMockVisualization, createMockDatasource } from '../mocks'; -import { TableSuggestion, DatasourcePublicAPI } from '../../types'; +import { createMockVisualization } from '../mocks'; +import { TableSuggestion } from '../../types'; const generateSuggestion = (datasourceSuggestionId: number = 1, state = {}) => ({ state, table: { datasourceSuggestionId, columns: [], isMultiRow: false }, }); -const mockDatasource: DatasourcePublicAPI = createMockDatasource().getPublicAPI({}, jest.fn()); - describe('suggestion helpers', () => { it('should return suggestions array', () => { const mockVisualization = createMockVisualization(); const suggestedState = {}; const suggestions = getSuggestions( - mockDatasource, [generateSuggestion()], { vis1: { @@ -47,7 +44,6 @@ describe('suggestion helpers', () => { const mockVisualization1 = createMockVisualization(); const mockVisualization2 = createMockVisualization(); const suggestions = getSuggestions( - mockDatasource, [generateSuggestion()], { vis1: { @@ -92,7 +88,6 @@ describe('suggestion helpers', () => { const mockVisualization1 = createMockVisualization(); const mockVisualization2 = createMockVisualization(); const suggestions = getSuggestions( - mockDatasource, [generateSuggestion()], { vis1: { @@ -141,7 +136,6 @@ describe('suggestion helpers', () => { const table1: TableSuggestion = { datasourceSuggestionId: 0, columns: [], isMultiRow: true }; const table2: TableSuggestion = { datasourceSuggestionId: 1, columns: [], isMultiRow: true }; getSuggestions( - mockDatasource, [{ state: {}, table: table1 }, { state: {}, table: table2 }], { vis1: mockVisualization1, @@ -162,7 +156,6 @@ describe('suggestion helpers', () => { const tableState1 = {}; const tableState2 = {}; const suggestions = getSuggestions( - mockDatasource, [generateSuggestion(1, tableState1), generateSuggestion(1, tableState2)], { vis1: { @@ -210,7 +203,6 @@ describe('suggestion helpers', () => { const mockVisualization2 = createMockVisualization(); const currentState = {}; getSuggestions( - mockDatasource, [generateSuggestion(1), generateSuggestion(2)], { vis1: mockVisualization1, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts index e0708e341096e..a9886387c6c74 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_helpers.ts @@ -27,7 +27,6 @@ export interface Suggestion { * action with `toSwitchAction` and dispatching it */ export function getSuggestions( - datasource: DatasourcePublicAPI, datasourceTableSuggestions: DatasourceSuggestion[], visualizationMap: Record, activeVisualizationId: string | null, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx index 5e071f1cf6493..5425ce24bcd06 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/suggestion_panel.tsx @@ -6,7 +6,6 @@ import React, { useState, useEffect } from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; - import { EuiIcon, EuiTitle, EuiPanel, EuiIconTip } from '@elastic/eui'; import { toExpression } from '@kbn/interpreter/common'; import { i18n } from '@kbn/i18n'; @@ -104,9 +103,6 @@ export function SuggestionPanel({ ); const suggestions = getSuggestions( - activeDatasource.getPublicAPI(datasourceState, newState => - dispatch({ type: 'UPDATE_DATASOURCE_STATE', newState }) - ), datasourceSuggestions, visualizationMap, activeVisualizationId, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx index e75f7236ee419..7a0017d83ad3a 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.tsx @@ -53,9 +53,6 @@ export function WorkspacePanel({ ); const suggestions = getSuggestions( - activeDatasource.getPublicAPI(datasourceState, newState => - dispatch({ type: 'UPDATE_DATASOURCE_STATE', newState }) - ), datasourceSuggestions, visualizationMap, activeVisualizationId, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index f348ae6b88a69..3873a077edab2 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -10,7 +10,6 @@ import { render } from 'react-dom'; import { Chrome } from 'ui/chrome'; import { ToastNotifications } from 'ui/notify/toasts/toast_notifications'; import { EuiComboBox } from '@elastic/eui'; -import uuid from 'uuid'; import { Datasource, DataType } from '..'; import { DatasourceDimensionPanelProps, diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 0a86082105b29..54e7cc1e6a3a0 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -144,7 +144,6 @@ export interface VisualizationProps { export interface SuggestionRequest { // It is up to the Visualization to rank these tables - datasource: DatasourcePublicAPI; tables: TableSuggestion[]; state?: T; // State is only passed if the visualization is active } diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 56a07f3c2e6a5..8d756d83e8aad 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -175,7 +175,7 @@ export function XYConfigPanel(props: VisualizationProps) { nativeProps={{ columnId, dragDropContext: props.dragDropContext, - filterOperations: op => op.isBucketed === true, + filterOperations: (op: Operation) => op.isBucketed === true, suggestedPriority: 0, }} /> From 70df6318afda993faf5c2f1513cabc03992d9b3a Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Tue, 9 Jul 2019 14:50:38 -0400 Subject: [PATCH 5/7] Fix removal of xy columns Tidy up id generator mocks --- .../visualization.test.tsx | 7 +- .../plugins/lens/public/id_generator/mock.ts | 10 -- .../indexpattern_plugin/indexpattern.test.tsx | 47 +++++++ .../indexpattern_plugin/indexpattern.tsx | 18 ++- x-pack/legacy/plugins/lens/public/types.ts | 2 +- .../multi_column_editor.tsx | 72 +++++++++++ .../xy_config_panel.test.tsx | 7 +- .../xy_config_panel.tsx | 120 +++++++----------- .../xy_suggestions.test.ts | 11 +- .../xy_visualization.test.ts | 8 +- 10 files changed, 195 insertions(+), 107 deletions(-) delete mode 100644 x-pack/legacy/plugins/lens/public/id_generator/mock.ts create mode 100644 x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx index b1a892ce6d829..d43008f68c330 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/visualization.test.tsx @@ -14,8 +14,7 @@ import { import { mount } from 'enzyme'; import { act } from 'react-dom/test-utils'; import { Operation, DataType } from '../types'; -import * as generator from '../id_generator'; -import { mockGeneratedIds } from '../id_generator/mock'; +import { generateId } from '../id_generator'; jest.mock('../id_generator'); @@ -23,7 +22,7 @@ describe('Datatable Visualization', () => { describe('#initialize', () => { it('should initialize from the empty state', () => { const datasource = createMockDatasource(); - mockGeneratedIds(generator, 'id'); + (generateId as jest.Mock).mockReturnValueOnce('id'); expect(datatableVisualization.initialize(datasource.publicAPIMock)).toEqual({ columns: [{ id: 'id', label: '' }], }); @@ -141,7 +140,7 @@ describe('Datatable Visualization', () => { /> ); - mockGeneratedIds(generator, 'newId'); + (generateId as jest.Mock).mockReturnValueOnce('newId'); act(() => { wrapper diff --git a/x-pack/legacy/plugins/lens/public/id_generator/mock.ts b/x-pack/legacy/plugins/lens/public/id_generator/mock.ts deleted file mode 100644 index 691ec119ca645..0000000000000 --- a/x-pack/legacy/plugins/lens/public/id_generator/mock.ts +++ /dev/null @@ -1,10 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -export function mockGeneratedIds(generator: { generateId: () => string }, ...ids: string[]) { - const mock = jest.spyOn(generator, 'generateId'); - ids.forEach(id => mock.mockReturnValueOnce(id)); -} diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx index c3610ab2cf95d..b39f00af1e1fd 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx @@ -12,6 +12,7 @@ import { IndexPatternPersistedState, IndexPatternPrivateState, IndexPatternDataPanel, + IndexPatternColumn, } from './indexpattern'; import { DatasourcePublicAPI, Operation, Datasource } from '../types'; import { createMockedDragDropContext } from './mocks'; @@ -508,6 +509,52 @@ describe('IndexPattern Data Source', () => { }); }); + describe('removeColumnInTableSpec', () => { + it('should remove the specified column', async () => { + const initialState = await indexPatternDatasource.initialize(persistedState); + const setState = jest.fn(); + const sampleColumn: IndexPatternColumn = { + dataType: 'number', + isBucketed: false, + label: 'foo', + operationId: 'bar', + operationType: 'max', + sourceField: 'baz', + suggestedOrder: 0, + }; + const columns: Record = { + a: { + ...sampleColumn, + suggestedOrder: 0, + }, + b: { + ...sampleColumn, + suggestedOrder: 1, + }, + c: { + ...sampleColumn, + suggestedOrder: 2, + }, + }; + const api = indexPatternDatasource.getPublicAPI( + { + ...initialState, + columnOrder: ['a', 'b', 'c'], + columns, + }, + setState + ); + + api.removeColumnInTableSpec('b'); + + expect(setState.mock.calls[0][0].columnOrder).toEqual(['a', 'c']); + expect(setState.mock.calls[0][0].columns).toEqual({ + a: columns.a, + c: columns.c, + }); + }); + }); + describe('getOperationForColumnId', () => { it('should get an operation for col1', () => { expect(publicAPI.getOperationForColumnId('col1')).toEqual({ diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx index 2849d88478f74..69774e24c1388 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.tsx @@ -10,12 +10,12 @@ import { render } from 'react-dom'; import { Chrome } from 'ui/chrome'; import { ToastNotifications } from 'ui/notify/toasts/toast_notifications'; import { EuiComboBox } from '@elastic/eui'; -import uuid from 'uuid'; import { DatasourceDimensionPanelProps, DatasourceDataPanelProps, DimensionPriority, DatasourceSuggestion, + Operation, } from '../types'; import { getIndexPatterns } from './loader'; import { ChildDragDropProvider, DragDrop } from '../drag_drop'; @@ -165,7 +165,7 @@ export function IndexPatternDataPanel(props: DatasourceDataPanelProps(prop: string, object: Record): Record { + const result = { ...object }; + delete result[prop]; + return result; +} + export function getIndexPatternDatasource(chrome: Chrome, toastNotifications: ToastNotifications) { // Not stateful. State is persisted to the frame const indexPatternDatasource: Datasource = { @@ -273,7 +279,13 @@ export function getIndexPatternDatasource(chrome: Chrome, toastNotifications: To ); }, - removeColumnInTableSpec: (columnId: string) => [], + removeColumnInTableSpec: (columnId: string) => { + setState({ + ...state, + columnOrder: state.columnOrder.filter(id => id !== columnId), + columns: removeProperty(columnId, state.columns), + }); + }, moveColumnTo: (columnId: string, targetIndex: number) => {}, duplicateColumn: (columnId: string) => [], }; diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index eca8e36648f08..9812db7440891 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -75,7 +75,7 @@ export interface DatasourcePublicAPI { // Render can be called many times renderDimensionPanel: (domElement: Element, props: DatasourceDimensionPanelProps) => void; - removeColumnInTableSpec: (columnId: string) => TableSpec; + removeColumnInTableSpec: (columnId: string) => void; moveColumnTo: (columnId: string, targetIndex: number) => void; duplicateColumn: (columnId: string) => TableSpec; } diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx new file mode 100644 index 0000000000000..4746ec1e53728 --- /dev/null +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { EuiButtonIcon, EuiButton } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { NativeRenderer } from '../native_renderer'; +import { generateId } from '../id_generator'; +import { DatasourcePublicAPI, Operation } from '../types'; +import { DragContextState } from '../drag_drop'; + +interface Props { + accessors: string[]; + datasource: DatasourcePublicAPI; + dragDropContext: DragContextState; + onRemove: (accessor: string) => void; + onAdd: (accessor: string) => void; + filterOperations: (op: Operation) => boolean; + suggestedPriority?: 0 | 1 | 2 | undefined; + testSubj: string; +} + +export function MultiColumnEditor({ + accessors, + datasource, + dragDropContext, + onRemove, + onAdd, + filterOperations, + suggestedPriority, + testSubj, +}: Props) { + return ( + <> + {accessors.map(accessor => ( +
+ + { + datasource.removeColumnInTableSpec(accessor); + onRemove(accessor); + }} + aria-label={i18n.translate('xpack.lens.xyChart.removeAriaLabel', { + defaultMessage: 'Remove', + })} + /> +
+ ))} + onAdd(generateId())} + iconType="plusInCircle" + /> + + ); +} diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx index 1c08db4b12c14..1ee799b7b59ab 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.test.tsx @@ -12,8 +12,7 @@ import { DatasourcePublicAPI, DatasourceDimensionPanelProps, Operation } from '. import { State, SeriesType } from './types'; import { Position } from '@elastic/charts'; import { NativeRendererProps } from '../native_renderer'; -import * as generator from '../id_generator'; -import { mockGeneratedIds } from '../id_generator/mock'; +import { generateId } from '../id_generator'; jest.mock('../id_generator'); @@ -333,7 +332,7 @@ describe('XYConfigPanel', () => { }); test('allows adding y dimensions', () => { - mockGeneratedIds(generator, 'zed'); + (generateId as jest.Mock).mockReturnValueOnce('zed'); const setState = jest.fn(); const state = testState(); const component = mount( @@ -354,7 +353,7 @@ describe('XYConfigPanel', () => { }); test('allows adding split dimensions', () => { - mockGeneratedIds(generator, 'foo'); + (generateId as jest.Mock).mockReturnValueOnce('foo'); const setState = jest.fn(); const state = testState(); const component = mount( diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx index 7087426e559ab..78c5fc7af147d 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_config_panel.tsx @@ -13,14 +13,12 @@ import { EuiForm, EuiFormRow, EuiSwitch, - EuiButtonIcon, - EuiButton, IconType, } from '@elastic/eui'; import { State, SeriesType } from './types'; -import { VisualizationProps, Operation } from '../types'; +import { VisualizationProps } from '../types'; import { NativeRenderer } from '../native_renderer'; -import { generateId } from '../id_generator'; +import { MultiColumnEditor } from './multi_column_editor'; const chartTypeIcons: Array<{ id: SeriesType; label: string; iconType: IconType }> = [ { @@ -149,30 +147,23 @@ export function XYConfigPanel(props: VisualizationProps) { defaultMessage: 'Split series', })} > - <> - {state.splitSeriesAccessors.map(columnId => ( - op.isBucketed === true, - suggestedPriority: 0, - }} - /> - ))} - - setState({ - ...state, - splitSeriesAccessors: [...state.splitSeriesAccessors, generateId()], - }) - } - iconType="plusInCircle" - /> - + + setState({ ...state, splitSeriesAccessors: [...state.splitSeriesAccessors, accessor] }) + } + onRemove={accessor => + setState({ + ...state, + splitSeriesAccessors: state.splitSeriesAccessors.filter(col => col !== accessor), + }) + } + filterOperations={op => op.isBucketed} + suggestedPriority={0} + testSubj="splitSeriesDimensionPanel" + /> ) { defaultMessage: 'Value', })} > - <> - {state.y.accessors.map(accessor => ( -
- - !op.isBucketed && op.dataType === 'number', - }} - /> - { - datasource.removeColumnInTableSpec(accessor); - setState({ - ...state, - y: { - ...state.y, - accessors: state.y.accessors.filter(col => col !== accessor), - }, - }); - }} - aria-label={i18n.translate('xpack.lens.xyChart.yRemoveAriaLabel', { - defaultMessage: 'Remove', - })} - /> -
- ))} - - setState({ - ...state, - y: { - ...state.y, - accessors: [...state.y.accessors, generateId()], - }, - }) - } - iconType="plusInCircle" - /> - + + setState({ + ...state, + y: { + ...state.y, + accessors: [...state.y.accessors, accessor], + }, + }) + } + onRemove={accessor => + setState({ + ...state, + y: { + ...state.y, + accessors: state.y.accessors.filter(col => col !== accessor), + }, + }) + } + filterOperations={op => !op.isBucketed && op.dataType === 'number'} + testSubj="yDimensionPanel" + /> diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts index fd719b68a3ab0..1523d6afdd4d5 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts @@ -8,8 +8,7 @@ import { getSuggestions } from './xy_suggestions'; import { TableColumn, VisualizationSuggestion } from '../types'; import { State } from './types'; import { Ast } from '@kbn/interpreter/target/common'; -import * as generator from '../id_generator'; -import { mockGeneratedIds } from '../id_generator/mock'; +import { generateId } from '../id_generator'; jest.mock('../id_generator'); @@ -92,7 +91,7 @@ describe('xy_suggestions', () => { }); test('suggests a basic x y chart with date on x', () => { - mockGeneratedIds(generator, 'aaa'); + (generateId as jest.Mock).mockReturnValueOnce('aaa'); const [suggestion, ...rest] = getSuggestions({ tables: [ { @@ -148,7 +147,7 @@ Object { }); test('supports multiple suggestions', () => { - mockGeneratedIds(generator, 'bbb', 'ccc'); + (generateId as jest.Mock).mockReturnValueOnce('bbb').mockReturnValueOnce('ccc'); const [s1, s2, ...rest] = getSuggestions({ tables: [ { @@ -194,7 +193,7 @@ Array [ }); test('handles two numeric values', () => { - mockGeneratedIds(generator, 'ddd'); + (generateId as jest.Mock).mockReturnValueOnce('ddd'); const [suggestion] = getSuggestions({ tables: [ { @@ -221,7 +220,7 @@ Object { }); test('handles unbucketed suggestions', () => { - mockGeneratedIds(generator, 'eee'); + (generateId as jest.Mock).mockReturnValueOnce('eee'); const [suggestion] = getSuggestions({ tables: [ { diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts index a160fdeebebb0..4e09b0da8482b 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts @@ -10,8 +10,7 @@ import { Ast } from '@kbn/interpreter/target/common'; import { Operation } from '../types'; import { State } from './types'; import { createMockDatasource } from '../editor_frame_plugin/mocks'; -import { mockGeneratedIds } from '../id_generator/mock'; -import * as generator from '../id_generator'; +import { generateId } from '../id_generator'; jest.mock('../id_generator'); @@ -39,7 +38,10 @@ function exampleState(): State { describe('xy_visualization', () => { describe('#initialize', () => { it('loads default state', () => { - mockGeneratedIds(generator, 'test-id1', 'test-id2', 'test-id3'); + (generateId as jest.Mock) + .mockReturnValueOnce('test-id1') + .mockReturnValueOnce('test-id2') + .mockReturnValue('test-id3'); const mockDatasource = createMockDatasource(); const initialState = xyVisualization.initialize(mockDatasource.publicAPIMock); From dd1524aefff00fdb4deca3ee048cb11ca38a9c17 Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Wed, 10 Jul 2019 14:48:30 -0400 Subject: [PATCH 6/7] Disallow deletion of the last accessor in a multi-accessor panel. Disallow splitting a series by date --- .../multi_column_editor.tsx | 30 ++++++++++--------- .../xy_config_panel.tsx | 2 +- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx index 4746ec1e53728..f0f804eacf864 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/multi_column_editor.tsx @@ -35,7 +35,7 @@ export function MultiColumnEditor({ }: Props) { return ( <> - {accessors.map(accessor => ( + {accessors.map((accessor, i) => (
- { - datasource.removeColumnInTableSpec(accessor); - onRemove(accessor); - }} - aria-label={i18n.translate('xpack.lens.xyChart.removeAriaLabel', { - defaultMessage: 'Remove', - })} - /> + {i === accessors.length - 1 ? null : ( + { + datasource.removeColumnInTableSpec(accessor); + onRemove(accessor); + }} + aria-label={i18n.translate('xpack.lens.xyChart.removeAriaLabel', { + defaultMessage: 'Remove', + })} + /> + )}
))} ) { splitSeriesAccessors: state.splitSeriesAccessors.filter(col => col !== accessor), }) } - filterOperations={op => op.isBucketed} + filterOperations={op => op.isBucketed && op.dataType !== 'date'} suggestedPriority={0} testSubj="splitSeriesDimensionPanel" /> From 44831d8f48a61102f30862419048a0d382c0bd1e Mon Sep 17 00:00:00 2001 From: Christopher Davies Date: Wed, 10 Jul 2019 16:45:16 -0400 Subject: [PATCH 7/7] Update snapshots --- .../xy_suggestions.test.ts | 76 +++++++++++-------- .../xy_visualization.test.ts | 8 +- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts index 287a99fe7d877..9ea0345f2367a 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_suggestions.test.ts @@ -105,7 +105,9 @@ describe('xy_suggestions', () => { expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` Object { "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "aaa", + ], "x": "date", "y": Array [ "bytes", @@ -127,18 +129,18 @@ describe('xy_suggestions', () => { expect(rest).toHaveLength(0); expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` - Object { - "seriesType": "line", - "splitSeriesAccessors": Array [ - "product", - ], - "x": "date", - "y": Array [ - "price", - "quantity", - ], - } - `); + Object { + "seriesType": "line", + "splitSeriesAccessors": Array [ + "product", + ], + "x": "date", + "y": Array [ + "price", + "quantity", + ], + } + `); }); test('supports multiple suggestions', () => { @@ -163,7 +165,9 @@ describe('xy_suggestions', () => { Array [ Object { "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "bbb", + ], "x": "date", "y": Array [ "price", @@ -171,7 +175,9 @@ describe('xy_suggestions', () => { }, Object { "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "ccc", + ], "x": "country", "y": Array [ "count", @@ -194,15 +200,17 @@ describe('xy_suggestions', () => { }); expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` - Object { - "seriesType": "bar", - "splitSeriesAccessors": Array [], - "x": "quantity", - "y": Array [ - "price", - ], - } - `); + Object { + "seriesType": "bar", + "splitSeriesAccessors": Array [ + "ddd", + ], + "x": "quantity", + "y": Array [ + "price", + ], + } + `); }); test('handles unbucketed suggestions', () => { @@ -229,15 +237,17 @@ describe('xy_suggestions', () => { }); expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` - Object { - "seriesType": "bar", - "splitSeriesAccessors": Array [], - "x": "mybool", - "y": Array [ - "num votes", - ], - } - `); + Object { + "seriesType": "bar", + "splitSeriesAccessors": Array [ + "eee", + ], + "x": "mybool", + "y": Array [ + "num votes", + ], + } + `); }); test('adds a preview expression with disabled axes and legend', () => { diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts index 374e1ab19b1c8..a2eeffaab4a7b 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_visualization.test.ts @@ -55,17 +55,19 @@ describe('xy_visualization', () => { "position": "right", }, "seriesType": "bar", - "splitSeriesAccessors": Array [], + "splitSeriesAccessors": Array [ + "test-id3", + ], "title": "Empty XY Chart", "x": Object { - "accessor": "test-id2", + "accessor": "test-id1", "position": "bottom", "showGridlines": false, "title": "X", }, "y": Object { "accessors": Array [ - "test-id1", + "test-id2", ], "position": "left", "showGridlines": false,