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 8f9e736499069..b3f3a389c4490 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 @@ -126,58 +126,54 @@ export const datatableVisualization: Visualization< getPersistableState: state => state, getSuggestions({ - tables, + table, state, }: SuggestionRequest): Array< VisualizationSuggestion > { - const maxColumnCount = Math.max.apply(undefined, tables.map(table => table.columns.length)); - return ( - tables - // don't suggest current table if visualization is active - .filter(({ changeType }) => !state || changeType !== 'unchanged') - .map(table => { - const title = - table.changeType === 'unchanged' - ? i18n.translate('xpack.lens.datatable.suggestionLabel', { - defaultMessage: 'As table', - }) - : i18n.translate('xpack.lens.datatable.visualizationOf', { - defaultMessage: 'Table {operations}', - values: { - operations: - table.label || - table.columns - .map(col => col.operation.label) - .join( - i18n.translate('xpack.lens.datatable.conjunctionSign', { - defaultMessage: ' & ', - description: - 'A character that can be used for conjunction of multiple enumarated items. Make sure to include spaces around it if needed.', - }) - ), - }, - }); - - return { - title, - // largest possible table will have a score of 0.2, fewer columns reduce score - score: (table.columns.length / maxColumnCount) * 0.2, - datasourceSuggestionId: table.datasourceSuggestionId, - state: { - layers: [ - { - layerId: table.layerId, - columns: table.columns.map(col => col.columnId), - }, - ], + if (state && table.changeType === 'unchanged') { + return []; + } + const title = + table.changeType === 'unchanged' + ? i18n.translate('xpack.lens.datatable.suggestionLabel', { + defaultMessage: 'As table', + }) + : i18n.translate('xpack.lens.datatable.visualizationOf', { + defaultMessage: 'Table {operations}', + values: { + operations: + table.label || + table.columns + .map(col => col.operation.label) + .join( + i18n.translate('xpack.lens.datatable.conjunctionSign', { + defaultMessage: ' & ', + description: + 'A character that can be used for conjunction of multiple enumarated items. Make sure to include spaces around it if needed.', + }) + ), }, - previewIcon: 'visTable', - // dont show suggestions for reduced versions or single-line tables - hide: table.changeType === 'reduced' || !table.isMultiRow, - }; - }) - ); + }); + + return [ + { + title, + // table with >= 10 columns will have a score of 0.6, fewer columns reduce score + score: (Math.min(table.columns.length, 10) / 10) * 0.6, + state: { + layers: [ + { + layerId: table.layerId, + columns: table.columns.map(col => col.columnId), + }, + ], + }, + previewIcon: 'visTable', + // dont show suggestions for reduced versions or single-line tables + hide: table.changeType === 'reduced' || !table.isMultiRow, + }, + ]; }, renderConfigPanel: (domElement, props) => diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx index 5624553d92ddf..298b25b5090c4 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/chart_switch.test.tsx @@ -34,7 +34,6 @@ describe('chart_switch', () => { score: 1, title: '', state: `suggestion ${id}`, - datasourceSuggestionId: options.tables[0].datasourceSuggestionId, previewIcon: 'empty', }, ]; @@ -91,7 +90,6 @@ describe('chart_switch', () => { state: {}, table: { columns: [], - datasourceSuggestionId: 0, isMultiRow: true, layerId: 'a', changeType: 'unchanged', @@ -217,7 +215,6 @@ describe('chart_switch', () => { }, }, ], - datasourceSuggestionId: 0, layerId: 'first', isMultiRow: true, changeType: 'unchanged', @@ -446,7 +443,6 @@ describe('chart_switch', () => { }, }, ], - datasourceSuggestionId: 0, layerId: 'a', isMultiRow: true, changeType: 'unchanged', 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 c7d1eb543d10d..2faa2ed902b5b 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 @@ -24,12 +24,11 @@ import { EuiPanel, EuiToolTip } from '@elastic/eui'; // datasources to be processed by its callers. const waitForPromises = () => new Promise(resolve => setTimeout(resolve)); -function generateSuggestion(datasourceSuggestionId = 1, state = {}): DatasourceSuggestion { +function generateSuggestion(state = {}): DatasourceSuggestion { return { state, table: { columns: [], - datasourceSuggestionId, isMultiRow: true, layerId: 'first', changeType: 'unchanged', @@ -905,7 +904,6 @@ describe('editor_frame', () => { state: {}, table: { columns: [], - datasourceSuggestionId: 0, isMultiRow: true, layerId: 'first', changeType: 'unchanged', @@ -989,7 +987,6 @@ describe('editor_frame', () => { { title: 'Suggested vis', score: 1, - datasourceSuggestionId: 0, state: initialState, previewIcon: 'empty', }, @@ -1047,6 +1044,17 @@ describe('editor_frame', () => { }); it('should fetch suggestions of all visualizations', async () => { + mockDatasource.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ + { + state: {}, + table: { + changeType: 'unchanged', + columns: [], + isMultiRow: true, + layerId: 'first', + }, + }, + ]); mount( { ...mockVisualization, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.1, state: {}, title: 'Suggestion6', previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.5, state: {}, title: 'Suggestion3', previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.7, state: {}, title: 'Suggestion2', previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.8, state: {}, title: 'Suggestion1', @@ -1112,14 +1116,12 @@ describe('editor_frame', () => { ...mockVisualization, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.4, state: {}, title: 'Suggestion5', previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.45, state: {}, title: 'Suggestion4', @@ -1163,7 +1165,6 @@ describe('editor_frame', () => { ...mockVisualization, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.8, state: suggestionVisState, title: 'Suggestion1', @@ -1222,14 +1223,12 @@ describe('editor_frame', () => { ...mockVisualization, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.2, state: {}, title: 'Suggestion1', previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.8, state: suggestionVisState, title: 'Suggestion2', @@ -1279,14 +1278,12 @@ describe('editor_frame', () => { ...mockVisualization, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.2, state: {}, title: 'Suggestion1', previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.6, state: {}, title: 'Suggestion2', @@ -1298,7 +1295,6 @@ describe('editor_frame', () => { ...mockVisualization2, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.8, state: suggestionVisState, title: 'Suggestion3', 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 cb4a5bf79e817..7b3e4454a5e39 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 @@ -8,14 +8,9 @@ import { getSuggestions } from './suggestion_helpers'; import { createMockVisualization, createMockDatasource, DatasourceMock } from '../mocks'; import { TableSuggestion, DatasourceSuggestion } from '../../types'; -const generateSuggestion = ( - datasourceSuggestionId: number = 0, - state = {}, - layerId: string = 'first' -): DatasourceSuggestion => ({ +const generateSuggestion = (state = {}, layerId: string = 'first'): DatasourceSuggestion => ({ state, table: { - datasourceSuggestionId, columns: [], isMultiRow: false, layerId, @@ -58,7 +53,6 @@ describe('suggestion helpers', () => { ...mockVisualization, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.5, title: 'Test', state: suggestedState, @@ -88,14 +82,12 @@ describe('suggestion helpers', () => { ...mockVisualization1, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.5, title: 'Test', state: {}, previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.5, title: 'Test2', state: {}, @@ -107,7 +99,6 @@ describe('suggestion helpers', () => { ...mockVisualization2, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.5, title: 'Test3', state: {}, @@ -193,14 +184,12 @@ describe('suggestion helpers', () => { ...mockVisualization1, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.2, title: 'Test', state: {}, previewIcon: 'empty', }, { - datasourceSuggestionId: 0, score: 0.8, title: 'Test2', state: {}, @@ -212,7 +201,6 @@ describe('suggestion helpers', () => { ...mockVisualization2, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.6, title: 'Test3', state: {}, @@ -235,14 +223,12 @@ describe('suggestion helpers', () => { const mockVisualization1 = createMockVisualization(); const mockVisualization2 = createMockVisualization(); const table1: TableSuggestion = { - datasourceSuggestionId: 0, columns: [], isMultiRow: true, layerId: 'first', changeType: 'unchanged', }; const table2: TableSuggestion = { - datasourceSuggestionId: 1, columns: [], isMultiRow: true, layerId: 'first', @@ -262,10 +248,10 @@ describe('suggestion helpers', () => { datasourceMap, datasourceStates, }); - expect(mockVisualization1.getSuggestions.mock.calls[0][0].tables[0]).toEqual(table1); - expect(mockVisualization1.getSuggestions.mock.calls[0][0].tables[1]).toEqual(table2); - expect(mockVisualization2.getSuggestions.mock.calls[0][0].tables[0]).toEqual(table1); - expect(mockVisualization2.getSuggestions.mock.calls[0][0].tables[1]).toEqual(table2); + expect(mockVisualization1.getSuggestions.mock.calls[0][0].table).toEqual(table1); + expect(mockVisualization1.getSuggestions.mock.calls[1][0].table).toEqual(table2); + expect(mockVisualization2.getSuggestions.mock.calls[0][0].table).toEqual(table1); + expect(mockVisualization2.getSuggestions.mock.calls[1][0].table).toEqual(table2); }); it('should map the suggestion ids back to the correct datasource ids and states', () => { @@ -274,41 +260,45 @@ describe('suggestion helpers', () => { const tableState1 = {}; const tableState2 = {}; datasourceMap.mock.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ - generateSuggestion(0, tableState1), - generateSuggestion(1, tableState2), + generateSuggestion(tableState1), + generateSuggestion(tableState2), + ]); + const vis1Suggestions = jest.fn(); + vis1Suggestions.mockReturnValueOnce([ + { + score: 0.3, + title: 'Test', + state: {}, + previewIcon: 'empty', + }, + ]); + vis1Suggestions.mockReturnValueOnce([ + { + score: 0.2, + title: 'Test2', + state: {}, + previewIcon: 'empty', + }, + ]); + const vis2Suggestions = jest.fn(); + vis2Suggestions.mockReturnValueOnce([]); + vis2Suggestions.mockReturnValueOnce([ + { + score: 0.1, + title: 'Test3', + state: {}, + previewIcon: 'empty', + }, ]); const suggestions = getSuggestions({ visualizationMap: { vis1: { ...mockVisualization1, - getSuggestions: () => [ - { - datasourceSuggestionId: 0, - score: 0.3, - title: 'Test', - state: {}, - previewIcon: 'empty', - }, - { - datasourceSuggestionId: 1, - score: 0.2, - title: 'Test2', - state: {}, - previewIcon: 'empty', - }, - ], + getSuggestions: vis1Suggestions, }, vis2: { ...mockVisualization2, - getSuggestions: () => [ - { - datasourceSuggestionId: 1, - score: 0.1, - title: 'Test3', - state: {}, - previewIcon: 'empty', - }, - ], + getSuggestions: vis2Suggestions, }, }, activeVisualizationId: 'vis1', @@ -367,7 +357,6 @@ describe('suggestion helpers', () => { ...mockVisualization1, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.8, title: 'Test2', state: {}, @@ -379,7 +368,6 @@ describe('suggestion helpers', () => { ...mockVisualization2, getSuggestions: () => [ { - datasourceSuggestionId: 0, score: 0.6, title: 'Test3', state: {}, 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 b3aac25fd1fcf..270c279375088 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 @@ -6,7 +6,13 @@ import _ from 'lodash'; import { Ast } from '@kbn/interpreter/common'; -import { Visualization, Datasource, FramePublicAPI, TableChangeType } from '../../types'; +import { + Visualization, + Datasource, + FramePublicAPI, + TableChangeType, + TableSuggestion, +} from '../../types'; import { Action } from './state_management'; export interface Suggestion { @@ -63,52 +69,75 @@ export function getSuggestions({ ) ); + // Collect all table suggestions from available datasources const datasourceTableSuggestions = _.flatten( datasources.map(([datasourceId, datasource]) => { const datasourceState = datasourceStates[datasourceId].state; - return ( - (field - ? datasource.getDatasourceSuggestionsForField(datasourceState, field) - : datasource.getDatasourceSuggestionsFromCurrentState(datasourceState) - ) - // TODO have the datasource in there by default - .map(suggestion => ({ ...suggestion, datasourceId })) - ); + return (field + ? datasource.getDatasourceSuggestionsForField(datasourceState, field) + : datasource.getDatasourceSuggestionsFromCurrentState(datasourceState) + ).map(suggestion => ({ ...suggestion, datasourceId })); }) - ).map((suggestion, index) => ({ - ...suggestion, - table: { ...suggestion.table, datasourceSuggestionId: index }, - })); - - const datasourceTables = datasourceTableSuggestions.map(({ table }) => table); + ); + // Pass all table suggestions to all visualization extensions to get visualization suggestions + // and rank them by score return _.flatten( - Object.entries(visualizationMap).map(([visualizationId, visualization]) => { - return visualization - .getSuggestions({ - tables: datasourceTables, - state: visualizationId === activeVisualizationId ? visualizationState : undefined, - }) - .map(({ datasourceSuggestionId, state, ...suggestion }) => { - const datasourceSuggestion = datasourceTableSuggestions[datasourceSuggestionId]; - return { - ...suggestion, + Object.entries(visualizationMap).map(([visualizationId, visualization]) => + _.flatten( + datasourceTableSuggestions.map(datasourceSuggestion => { + const table = datasourceSuggestion.table; + const currentVisualizationState = + visualizationId === activeVisualizationId ? visualizationState : undefined; + const keptLayerIds = + visualizationId !== activeVisualizationId + ? [datasourceSuggestion.table.layerId] + : allLayerIds; + return getVisualizationSuggestions( + visualization, + table, visualizationId, - visualizationState: state, - keptLayerIds: - visualizationId !== activeVisualizationId - ? [datasourceSuggestion.table.layerId] - : allLayerIds, - datasourceState: datasourceSuggestion.state, - datasourceId: datasourceSuggestion.datasourceId, - columns: datasourceSuggestion.table.columns.length, - changeType: datasourceSuggestion.table.changeType, - }; - }); - }) + datasourceSuggestion, + currentVisualizationState, + keptLayerIds + ); + }) + ) + ) ).sort((a, b) => b.score - a.score); } +/** + * Queries a single visualization extensions for a single datasource suggestion and + * creates an array of complete suggestions containing both the target datasource + * state and target visualization state along with suggestion meta data like score, + * title and preview expression. + */ +function getVisualizationSuggestions( + visualization: Visualization, + table: TableSuggestion, + visualizationId: string, + datasourceSuggestion: { datasourceId: string; state: unknown; table: TableSuggestion }, + currentVisualizationState: unknown, + keptLayerIds: string[] +) { + return visualization + .getSuggestions({ + table, + state: currentVisualizationState, + }) + .map(({ state, ...visualizationSuggestion }) => ({ + ...visualizationSuggestion, + visualizationId, + visualizationState: state, + keptLayerIds, + datasourceState: datasourceSuggestion.state, + datasourceId: datasourceSuggestion.datasourceId, + columns: table.columns.length, + changeType: table.changeType, + })); +} + export function switchToSuggestion( frame: FramePublicAPI, dispatch: (action: Action) => void, diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.test.tsx b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.test.tsx index 04f173c75eab2..1e50e58ec993e 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.test.tsx +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/editor_frame/workspace_panel.test.tsx @@ -549,7 +549,6 @@ describe('workspace_panel', () => { it('should immediately transition if exactly one suggestion is returned', () => { const expectedTable: TableSuggestion = { - datasourceSuggestionId: 0, isMultiRow: true, layerId: '1', columns: [], @@ -566,7 +565,6 @@ describe('workspace_panel', () => { score: 0.5, title: 'my title', state: {}, - datasourceSuggestionId: 0, previewIcon: 'empty', }, ]); @@ -577,7 +575,7 @@ describe('workspace_panel', () => { expect(mockDatasource.getDatasourceSuggestionsForField).toHaveBeenCalledTimes(1); expect(mockVisualization.getSuggestions).toHaveBeenCalledWith( expect.objectContaining({ - tables: [expectedTable], + table: expectedTable, }) ); expect(mockDispatch).toHaveBeenCalledWith({ @@ -594,7 +592,6 @@ describe('workspace_panel', () => { { state: {}, table: { - datasourceSuggestionId: 0, isMultiRow: true, layerId: '1', columns: [], @@ -607,7 +604,6 @@ describe('workspace_panel', () => { score: 0.5, title: 'my title', state: {}, - datasourceSuggestionId: 0, previewIcon: 'empty', }, ]); @@ -622,7 +618,6 @@ describe('workspace_panel', () => { { state: {}, table: { - datasourceSuggestionId: 0, isMultiRow: true, layerId: '1', columns: [], @@ -635,7 +630,6 @@ describe('workspace_panel', () => { score: 0.5, title: 'my title', state: {}, - datasourceSuggestionId: 0, previewIcon: 'empty', }, ]); @@ -650,7 +644,6 @@ describe('workspace_panel', () => { { state: {}, table: { - datasourceSuggestionId: 0, isMultiRow: true, layerId: '1', columns: [], @@ -663,7 +656,6 @@ describe('workspace_panel', () => { score: 0.5, title: 'my title', state: {}, - datasourceSuggestionId: 0, previewIcon: 'empty', }, ]); @@ -681,7 +673,6 @@ describe('workspace_panel', () => { { state: {}, table: { - datasourceSuggestionId: 0, isMultiRow: true, columns: [], layerId: '1', @@ -691,7 +682,6 @@ describe('workspace_panel', () => { { state: {}, table: { - datasourceSuggestionId: 1, isMultiRow: true, columns: [], layerId: '1', @@ -699,6 +689,14 @@ describe('workspace_panel', () => { }, }, ]); + mockVisualization.getSuggestions.mockReturnValueOnce([ + { + score: 0.5, + title: 'second suggestion', + state: {}, + previewIcon: 'empty', + }, + ]); mockVisualization.getSuggestions.mockReturnValueOnce([ { score: 0.8, @@ -706,14 +704,6 @@ describe('workspace_panel', () => { state: { isFirst: true, }, - datasourceSuggestionId: 1, - previewIcon: 'empty', - }, - { - score: 0.5, - title: 'second suggestion', - state: {}, - datasourceSuggestionId: 0, previewIcon: 'empty', }, ]); diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.test.tsx index ae26633a848ea..7201cd12fce2a 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.test.tsx @@ -211,7 +211,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -253,7 +252,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -296,7 +294,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -393,7 +390,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -435,7 +431,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -478,7 +473,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -654,7 +648,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'extended', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -825,7 +818,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(suggestions[0].table).toEqual({ changeType: 'initial', label: undefined, - datasourceSuggestionId: 0, isMultiRow: true, columns: [ expect.objectContaining({ @@ -928,7 +920,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(indexPatternDatasource.getDatasourceSuggestionsFromCurrentState(state)).toEqual([ expect.objectContaining({ table: { - datasourceSuggestionId: 0, isMultiRow: true, changeType: 'unchanged', label: undefined, @@ -948,7 +939,6 @@ describe('IndexPattern Data Source suggestions', () => { }), expect.objectContaining({ table: { - datasourceSuggestionId: 1, isMultiRow: true, changeType: 'unchanged', label: undefined, @@ -994,7 +984,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(indexPatternDatasource.getDatasourceSuggestionsFromCurrentState(state)[0]).toEqual( expect.objectContaining({ table: { - datasourceSuggestionId: 0, isMultiRow: true, changeType: 'extended', label: 'Over time', @@ -1062,7 +1051,6 @@ describe('IndexPattern Data Source suggestions', () => { expect(indexPatternDatasource.getDatasourceSuggestionsFromCurrentState(state)[2]).toEqual( expect.objectContaining({ table: { - datasourceSuggestionId: 2, isMultiRow: true, changeType: 'extended', label: 'Over time', diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.ts index 5ed236ee73933..8185d18070d2d 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern_suggestions.ts @@ -22,7 +22,6 @@ function buildSuggestion({ state, updatedLayer, layerId, - datasourceSuggestionId, label, changeType, }: { @@ -30,7 +29,6 @@ function buildSuggestion({ layerId: string; changeType: TableChangeType; updatedLayer?: IndexPatternLayer; - datasourceSuggestionId?: number; label?: string; }): DatasourceSuggestion { const columnOrder = (updatedLayer || state.layers[layerId]).columnOrder; @@ -55,7 +53,6 @@ function buildSuggestion({ operation: columnToOperation(columnMap[columnId]), })), isMultiRow, - datasourceSuggestionId: datasourceSuggestionId || 0, layerId, changeType, label, @@ -331,7 +328,6 @@ export function getDatasourceSuggestionsFromCurrentState( buildSuggestion({ state, layerId, - datasourceSuggestionId: index, changeType: 'unchanged', }) ); @@ -346,7 +342,6 @@ export function getDatasourceSuggestionsFromCurrentState( buildSuggestion({ state, layerId, - datasourceSuggestionId: index, changeType: 'unchanged', }) ); @@ -366,11 +361,6 @@ export function getDatasourceSuggestionsFromCurrentState( return suggestions; }) - ).map( - (suggestion, index): DatasourceSuggestion => ({ - ...suggestion, - table: { ...suggestion.table, datasourceSuggestionId: index }, - }) ); } diff --git a/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.test.ts b/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.test.ts index 73767375465b5..8baa78987b756 100644 --- a/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.test.ts +++ b/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.test.ts @@ -5,7 +5,7 @@ */ import { getSuggestions } from './metric_suggestions'; -import { TableSuggestionColumn } from '..'; +import { TableSuggestionColumn, TableSuggestion } from '..'; describe('metric_suggestions', () => { function numCol(columnId: string): TableSuggestionColumn { @@ -49,65 +49,54 @@ describe('metric_suggestions', () => { }; expect( - getSuggestions({ - tables: [ - { - columns: [dateCol('a')], - datasourceSuggestionId: 0, - isMultiRow: true, - layerId: 'l1', - changeType: 'unchanged', - }, - { - columns: [strCol('foo'), strCol('bar')], - datasourceSuggestionId: 1, - isMultiRow: true, - layerId: 'l1', - changeType: 'unchanged', - }, - { - layerId: 'l1', - datasourceSuggestionId: 2, - isMultiRow: true, - columns: [numCol('bar')], - changeType: 'unchanged', - }, - { - columns: [unknownCol(), numCol('bar')], - datasourceSuggestionId: 3, - isMultiRow: true, - layerId: 'l1', - changeType: 'unchanged', - }, - { - columns: [numCol('bar'), numCol('baz')], - datasourceSuggestionId: 4, - isMultiRow: false, - layerId: 'l1', - changeType: 'unchanged', - }, - ], - }) - ).toEqual([]); - }); - - test('suggests a basic metric chart', () => { - const [suggestion, ...rest] = getSuggestions({ - tables: [ + ([ { - columns: [numCol('bytes')], - datasourceSuggestionId: 0, + columns: [dateCol('a')], + isMultiRow: true, + layerId: 'l1', + changeType: 'unchanged', + }, + { + columns: [strCol('foo'), strCol('bar')], + isMultiRow: true, + layerId: 'l1', + changeType: 'unchanged', + }, + { + layerId: 'l1', + isMultiRow: true, + columns: [numCol('bar')], + changeType: 'unchanged', + }, + { + columns: [unknownCol(), numCol('bar')], + isMultiRow: true, + layerId: 'l1', + changeType: 'unchanged', + }, + { + columns: [numCol('bar'), numCol('baz')], isMultiRow: false, layerId: 'l1', changeType: 'unchanged', }, - ], + ] as TableSuggestion[]).map(table => expect(getSuggestions({ table })).toEqual([])) + ); + }); + + test('suggests a basic metric chart', () => { + const [suggestion, ...rest] = getSuggestions({ + table: { + columns: [numCol('bytes')], + isMultiRow: false, + layerId: 'l1', + changeType: 'unchanged', + }, }); expect(rest).toHaveLength(0); expect(suggestion).toMatchInlineSnapshot(` Object { - "datasourceSuggestionId": 0, "previewExpression": Object { "chain": Array [ Object { diff --git a/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.ts b/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.ts index c078f4ba57752..9cf5133527183 100644 --- a/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.ts +++ b/x-pack/legacy/plugins/lens/public/metric_visualization_plugin/metric_suggestions.ts @@ -12,20 +12,25 @@ import { State } from './types'; * * @param opts */ -export function getSuggestions( - opts: SuggestionRequest -): Array> { - return ( - opts.tables - .filter( - ({ isMultiRow, columns }) => - // We only render metric charts for single-row queries. We require a single, numeric column. - !isMultiRow && columns.length === 1 && columns[0].operation.dataType === 'number' - ) - // don't suggest current table if visualization is active - .filter(({ changeType }) => !opts.state || changeType !== 'unchanged') - .map(table => getSuggestion(table)) - ); +export function getSuggestions({ + table, + state, +}: SuggestionRequest): Array> { + // We only render metric charts for single-row queries. We require a single, numeric column. + if ( + table.isMultiRow || + table.columns.length > 1 || + table.columns[0].operation.dataType !== 'number' + ) { + return []; + } + + // don't suggest current table if visualization is active + if (state && table.changeType === 'unchanged') { + return []; + } + + return [getSuggestion(table)]; } function getSuggestion(table: TableSuggestion): VisualizationSuggestion { @@ -35,7 +40,6 @@ function getSuggestion(table: TableSuggestion): VisualizationSuggestion { return { title, score: 0.5, - datasourceSuggestionId: table.datasourceSuggestionId, previewIcon: 'visMetric', previewExpression: { type: 'expression', diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index affe077909947..14fa018fa6e5d 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -53,11 +53,6 @@ export interface TableSuggestionColumn { * is possible, the visualization returns a `VisualizationSuggestion` object */ export interface TableSuggestion { - /** - * The id of this table. This id has to be included in the `VisualizationSuggestion` to map - * the visualization to the right table as there can be multiple tables in a single `SuggestionRequest`. - */ - datasourceSuggestionId: number; /** * Flag indicating whether the table will include more than one column. * This is not the case for example for a single metric aggregation @@ -222,10 +217,23 @@ export interface VisualizationProps { setState: (newState: T) => void; } +/** + * Object passed to `getSuggestions` of a visualization. + * It contains a possible table the current datasource could + * provide and the state of the visualization if it is currently active. + * + * If the current datasource suggests multiple tables, `getSuggestions` + * is called multiple times with separate `SuggestionRequest` objects. + */ export interface SuggestionRequest { - // It is up to the Visualization to rank these tables - tables: TableSuggestion[]; - state?: T; // State is only passed if the visualization is active + /** + * A table configuration the datasource could provide. + */ + table: TableSuggestion; + /** + * State is only passed if the visualization is active. + */ + state?: T; } /** @@ -256,11 +264,6 @@ export interface VisualizationSuggestion { * The new state of the visualization if this suggestion is applied. */ state: T; - /** - * The id of the `TableSuggestion` object this visualization suggestion is based on. - * This is used to switch the datasource configuration to the right table. - */ - datasourceSuggestionId: number; /** * The expression of the preview of the chart rendered if the suggestion is advertised to the user. * If there is no expression provided, the preview icon is used. @@ -316,5 +319,5 @@ export interface Visualization { // The frame will call this function on all visualizations when the table changes, or when // rendering additional ways of using the data - getSuggestions: (options: SuggestionRequest) => Array>; + getSuggestions: (context: SuggestionRequest) => Array>; } 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 ceab87e9d9516..ececea6a1d99f 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 @@ -5,7 +5,12 @@ */ import { getSuggestions } from './xy_suggestions'; -import { TableSuggestionColumn, VisualizationSuggestion, DataType } from '../types'; +import { + TableSuggestionColumn, + VisualizationSuggestion, + DataType, + TableSuggestion, +} from '../types'; import { State, XYState } from './types'; import { generateId } from '../id_generator'; import { Ast } from '@kbn/interpreter/target/common'; @@ -67,53 +72,44 @@ describe('xy_suggestions', () => { }; expect( - getSuggestions({ - tables: [ - { - datasourceSuggestionId: 0, - isMultiRow: true, - columns: [dateCol('a')], - layerId: 'first', - changeType: 'unchanged', - }, - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [strCol('foo'), strCol('bar')], - layerId: 'first', - changeType: 'unchanged', - }, - { - datasourceSuggestionId: 2, - isMultiRow: false, - columns: [strCol('foo'), numCol('bar')], - layerId: 'first', - changeType: 'unchanged', - }, - { - datasourceSuggestionId: 3, - isMultiRow: true, - columns: [unknownCol(), numCol('bar')], - layerId: 'first', - changeType: 'unchanged', - }, - ], - }) - ).toEqual([]); - }); - - test('suggests a basic x y chart with date on x', () => { - (generateId as jest.Mock).mockReturnValueOnce('aaa'); - const [suggestion, ...rest] = getSuggestions({ - tables: [ + ([ { - datasourceSuggestionId: 0, isMultiRow: true, - columns: [numCol('bytes'), dateCol('date')], + columns: [dateCol('a')], layerId: 'first', changeType: 'unchanged', }, - ], + { + isMultiRow: true, + columns: [strCol('foo'), strCol('bar')], + layerId: 'first', + changeType: 'unchanged', + }, + { + isMultiRow: false, + columns: [strCol('foo'), numCol('bar')], + layerId: 'first', + changeType: 'unchanged', + }, + { + isMultiRow: true, + columns: [unknownCol(), numCol('bar')], + layerId: 'first', + changeType: 'unchanged', + }, + ] as TableSuggestion[]).map(table => expect(getSuggestions({ table })).toEqual([])) + ); + }); + + test('suggests a basic x y chart with date on x', () => { + (generateId as jest.Mock).mockReturnValueOnce('aaa'); + const [suggestion, ...rest] = getSuggestions({ + table: { + isMultiRow: true, + columns: [numCol('bytes'), dateCol('date')], + layerId: 'first', + changeType: 'unchanged', + }, }); expect(rest).toHaveLength(0); @@ -133,21 +129,18 @@ describe('xy_suggestions', () => { test('does not suggest multiple splits', () => { const suggestions = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [ - numCol('price'), - numCol('quantity'), - dateCol('date'), - strCol('product'), - strCol('city'), - ], - layerId: 'first', - changeType: 'unchanged', - }, - ], + table: { + isMultiRow: true, + columns: [ + numCol('price'), + numCol('quantity'), + dateCol('date'), + strCol('product'), + strCol('city'), + ], + layerId: 'first', + changeType: 'unchanged', + }, }); expect(suggestions).toHaveLength(0); @@ -155,15 +148,12 @@ describe('xy_suggestions', () => { test('suggests a split x y chart with date on x', () => { const [suggestion, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], - layerId: 'first', - changeType: 'unchanged', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], + layerId: 'first', + changeType: 'unchanged', + }, }); expect(rest).toHaveLength(0); @@ -184,16 +174,13 @@ describe('xy_suggestions', () => { test('uses datasource provided title if available', () => { const [suggestion, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], - layerId: 'first', - changeType: 'unchanged', - label: 'Datasource title', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], + layerId: 'first', + changeType: 'unchanged', + label: 'Datasource title', + }, }); expect(rest).toHaveLength(0); @@ -202,15 +189,12 @@ describe('xy_suggestions', () => { test('hides reduced suggestions if there is a current state', () => { const [suggestion, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], - layerId: 'first', - changeType: 'reduced', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], + layerId: 'first', + changeType: 'reduced', + }, state: { isHorizontal: false, legend: { isVisible: true, position: 'bottom' }, @@ -233,15 +217,12 @@ describe('xy_suggestions', () => { test('does not hide reduced suggestions if xy visualization is not active', () => { const [suggestion, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], - layerId: 'first', - changeType: 'reduced', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], + layerId: 'first', + changeType: 'reduced', + }, }); expect(rest).toHaveLength(0); @@ -264,15 +245,12 @@ describe('xy_suggestions', () => { ], }; const [suggestion, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], - layerId: 'first', - changeType: 'unchanged', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('price'), numCol('quantity'), dateCol('date'), strCol('product')], + layerId: 'first', + changeType: 'unchanged', + }, state: currentState, }); @@ -303,15 +281,12 @@ describe('xy_suggestions', () => { ], }; const [suggestion, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('price'), numCol('quantity'), strCol('product')], - layerId: 'first', - changeType: 'unchanged', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('price'), numCol('quantity'), strCol('product')], + layerId: 'first', + changeType: 'unchanged', + }, state: currentState, }); @@ -323,66 +298,15 @@ describe('xy_suggestions', () => { expect(suggestion.title).toEqual('Flip'); }); - test('supports multiple suggestions', () => { - (generateId as jest.Mock).mockReturnValueOnce('bbb').mockReturnValueOnce('ccc'); - const [s1, s2, ...rest] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 0, - isMultiRow: true, - columns: [numCol('price'), dateCol('date')], - layerId: 'first', - changeType: 'unchanged', - }, - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('count'), strCol('country')], - layerId: 'first', - changeType: 'unchanged', - }, - ], - }); - - expect(rest).toHaveLength(0); - expect([suggestionSubset(s1), suggestionSubset(s2)]).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - "seriesType": "area", - "splitAccessor": "bbb", - "x": "date", - "y": Array [ - "price", - ], - }, - ], - Array [ - Object { - "seriesType": "bar", - "splitAccessor": "ccc", - "x": "country", - "y": Array [ - "count", - ], - }, - ], - ] - `); - }); - test('handles two numeric values', () => { (generateId as jest.Mock).mockReturnValueOnce('ddd'); const [suggestion] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [numCol('quantity'), numCol('price')], - layerId: 'first', - changeType: 'unchanged', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('quantity'), numCol('price')], + layerId: 'first', + changeType: 'unchanged', + }, }); expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` @@ -402,25 +326,22 @@ describe('xy_suggestions', () => { test('handles unbucketed suggestions', () => { (generateId as jest.Mock).mockReturnValueOnce('eee'); const [suggestion] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 1, - isMultiRow: true, - columns: [ - numCol('num votes'), - { - columnId: 'mybool', - operation: { - dataType: 'boolean', - isBucketed: false, - label: 'Yes / No', - }, + table: { + isMultiRow: true, + columns: [ + numCol('num votes'), + { + columnId: 'mybool', + operation: { + dataType: 'boolean', + isBucketed: false, + label: 'Yes / No', }, - ], - layerId: 'first', - changeType: 'unchanged', - }, - ], + }, + ], + layerId: 'first', + changeType: 'unchanged', + }, }); expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(` @@ -439,15 +360,12 @@ describe('xy_suggestions', () => { test('adds a preview expression with disabled axes and legend', () => { const [suggestion] = getSuggestions({ - tables: [ - { - datasourceSuggestionId: 0, - isMultiRow: true, - columns: [numCol('bytes'), dateCol('date')], - layerId: 'first', - changeType: 'unchanged', - }, - ], + table: { + isMultiRow: true, + columns: [numCol('bytes'), dateCol('date')], + layerId: 'first', + changeType: 'unchanged', + }, }); const expression = suggestion.previewExpression! as Ast; 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 648a411a13413..3ffd15067e73c 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 @@ -47,22 +47,29 @@ function getIconForSeries(type: SeriesType): EuiIconType { * * @param opts */ -export function getSuggestions( - opts: SuggestionRequest -): Array> { - return opts.tables - .filter( - ({ isMultiRow, columns }) => - // We only render line charts for multi-row queries. We require at least - // two columns: one for x and at least one for y, and y columns must be numeric. - // We reject any datasource suggestions which have a column of an unknown type. - isMultiRow && - columns.length > 1 && - columns.some(col => col.operation.dataType === 'number') && - !columns.some(col => !columnSortOrder.hasOwnProperty(col.operation.dataType)) - ) - .map(table => getSuggestionForColumns(table, opts.state)) - .filter((suggestion): suggestion is VisualizationSuggestion => suggestion !== undefined); +export function getSuggestions({ + table, + state, +}: SuggestionRequest): Array> { + if ( + // We only render line charts for multi-row queries. We require at least + // two columns: one for x and at least one for y, and y columns must be numeric. + // We reject any datasource suggestions which have a column of an unknown type. + !table.isMultiRow || + table.columns.length <= 1 || + table.columns.every(col => col.operation.dataType !== 'number') || + table.columns.some(col => !columnSortOrder.hasOwnProperty(col.operation.dataType)) + ) { + return []; + } + + const suggestion = getSuggestionForColumns(table, state); + + if (suggestion) { + return [suggestion]; + } + + return []; } function getSuggestionForColumns( @@ -77,7 +84,6 @@ function getSuggestionForColumns( if (buckets.length === 1 || buckets.length === 2) { const [x, splitBy] = buckets; return getSuggestion( - table.datasourceSuggestionId, table.layerId, table.changeType, x, @@ -89,7 +95,6 @@ function getSuggestionForColumns( } else if (buckets.length === 0) { const [x, ...yValues] = values; return getSuggestion( - table.datasourceSuggestionId, table.layerId, table.changeType, x, @@ -111,7 +116,6 @@ function prioritizeColumns(columns: TableSuggestionColumn[]) { } function getSuggestion( - datasourceSuggestionId: number, layerId: string, changeType: TableChangeType, xValue: TableSuggestionColumn, @@ -133,7 +137,6 @@ function getSuggestion( yValues, splitBy, changeType, - datasourceSuggestionId, xValue, }; @@ -245,7 +248,6 @@ function buildSuggestion({ yValues, splitBy, changeType, - datasourceSuggestionId, xValue, }: { currentState: XYState | undefined; @@ -257,7 +259,6 @@ function buildSuggestion({ splitBy: TableSuggestionColumn | undefined; layerId: string; changeType: string; - datasourceSuggestionId: number; }) { const newLayer = { ...(getExistingLayer(currentState, layerId) || {}), @@ -284,7 +285,6 @@ function buildSuggestion({ score: ((yValues.length > 1 ? 2 : 1) + (splitBy ? 1 : 0)) / 3, // don't advertise chart of same type but with less data hide: currentState && changeType === 'reduced', - datasourceSuggestionId, state, previewIcon: getIconForSeries(seriesType), previewExpression: buildPreviewExpression(state, layerId, xValue, yValues, splitBy),