Skip to content

Commit

Permalink
[Lens] Suggestion improvements (#43688)
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 authored Sep 2, 2019
1 parent 65008c9 commit 3948678
Show file tree
Hide file tree
Showing 47 changed files with 1,407 additions and 464 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ describe('Datatable Visualization', () => {
const baseOperation: Operation = {
dataType: 'string',
isBucketed: true,
isMetric: false,
label: '',
};
expect(filterOperations({ ...baseOperation })).toEqual(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,42 +127,57 @@ export const datatableVisualization: Visualization<

getSuggestions({
tables,
state,
}: SuggestionRequest<DatatableVisualizationState>): Array<
VisualizationSuggestion<DatatableVisualizationState>
> {
const maxColumnCount = Math.max.apply(undefined, tables.map(table => table.columns.length));
return tables.map(table => {
const title = i18n.translate('xpack.lens.datatable.visualizationOf', {
defaultMessage: 'Table: {operations}',
values: {
operations: 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, less 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),
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),
},
],
},
],
},
previewIcon: 'visTable',
};
});
previewIcon: 'visTable',
// dont show suggestions for reduced versions or single-line tables
hide: table.changeType === 'reduced' || !table.isMultiRow,
};
})
);
},

renderConfigPanel: (domElement, props) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ describe('chart_switch', () => {
datasourceSuggestionId: 0,
isMultiRow: true,
layerId: 'a',
changeType: 'unchanged',
},
},
]);
Expand Down Expand Up @@ -205,7 +206,6 @@ describe('chart_switch', () => {
label: '',
dataType: 'string',
isBucketed: true,
isMetric: false,
},
},
{
Expand All @@ -214,13 +214,13 @@ describe('chart_switch', () => {
label: '',
dataType: 'number',
isBucketed: false,
isMetric: true,
},
},
],
datasourceSuggestionId: 0,
layerId: 'first',
isMultiRow: true,
changeType: 'unchanged',
},
},
]);
Expand Down Expand Up @@ -435,7 +435,6 @@ describe('chart_switch', () => {
label: '',
dataType: 'string',
isBucketed: true,
isMetric: false,
},
},
{
Expand All @@ -444,13 +443,13 @@ describe('chart_switch', () => {
label: '',
dataType: 'number',
isBucketed: false,
isMetric: true,
},
},
],
datasourceSuggestionId: 0,
layerId: 'a',
isMultiRow: true,
changeType: 'unchanged',
},
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { flatten } from 'lodash';
import { i18n } from '@kbn/i18n';
import { Visualization, FramePublicAPI, Datasource } from '../../types';
import { Action } from './state_management';
import { getSuggestions, switchToSuggestion } from './suggestion_helpers';
import { getSuggestions, switchToSuggestion, Suggestion } from './suggestion_helpers';

interface VisualizationSelection {
visualizationId: string;
Expand Down Expand Up @@ -106,13 +106,7 @@ export function ChartSwitch(props: Props) {
([_layerId, datasource]) => datasource.getTableSpec().length > 0
);

const topSuggestion = getSuggestions({
datasourceMap: props.datasourceMap,
datasourceStates: props.datasourceStates,
visualizationMap: { [visualizationId]: newVisualization },
activeVisualizationId: props.visualizationId,
visualizationState: props.visualizationState,
})[0];
const topSuggestion = getTopSuggestion(props, visualizationId, newVisualization);

let dataLoss: VisualizationSelection['dataLoss'];

Expand Down Expand Up @@ -242,3 +236,20 @@ export function ChartSwitch(props: Props) {
</div>
);
}
function getTopSuggestion(
props: Props,
visualizationId: string,
newVisualization: Visualization<unknown, unknown>
): Suggestion | undefined {
return getSuggestions({
datasourceMap: props.datasourceMap,
datasourceStates: props.datasourceStates,
visualizationMap: { [visualizationId]: newVisualization },
activeVisualizationId: props.visualizationId,
visualizationState: props.visualizationState,
}).filter(suggestion => {
// don't use extended versions of current data table on switching between visualizations
// to avoid confusing the user.
return suggestion.changeType !== 'extended';
})[0];
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function generateSuggestion(datasourceSuggestionId = 1, state = {}): DatasourceS
datasourceSuggestionId,
isMultiRow: true,
layerId: 'first',
changeType: 'unchanged',
},
};
}
Expand Down Expand Up @@ -907,6 +908,7 @@ describe('editor_frame', () => {
datasourceSuggestionId: 0,
isMultiRow: true,
layerId: 'first',
changeType: 'unchanged',
},
},
]);
Expand Down Expand Up @@ -1068,18 +1070,32 @@ describe('editor_frame', () => {
expect(mockVisualization2.getSuggestions).toHaveBeenCalled();
});

it('should display top 3 suggestions in descending order', async () => {
it('should display top 5 suggestions in descending order', async () => {
const instance = mount(
<EditorFrame
{...getDefaultProps()}
visualizationMap={{
testVis: {
...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',
},
Expand All @@ -1099,14 +1115,14 @@ describe('editor_frame', () => {
datasourceSuggestionId: 0,
score: 0.4,
state: {},
title: 'Suggestion4',
title: 'Suggestion5',
previewIcon: 'empty',
},
{
datasourceSuggestionId: 0,
score: 0.45,
state: {},
title: 'Suggestion3',
title: 'Suggestion4',
previewIcon: 'empty',
},
],
Expand All @@ -1133,7 +1149,7 @@ describe('editor_frame', () => {
.find('[data-test-subj="lnsSuggestion"]')
.find(EuiPanel)
.map(el => el.parents(EuiToolTip).prop('content'))
).toEqual(['Suggestion1', 'Suggestion2', 'Suggestion3']);
).toEqual(['Suggestion1', 'Suggestion2', 'Suggestion3', 'Suggestion4', 'Suggestion5']);
});

it('should switch to suggested visualization', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export function prependDatasourceExpression(
}

export function prependKibanaContext(
expression: Ast | string | null,
expression: Ast | string,
{
timeRange,
query,
Expand All @@ -73,8 +73,7 @@ export function prependKibanaContext(
query?: Query;
filters?: Filter[];
}
): Ast | null {
if (!expression) return null;
): Ast {
const parsedExpression = typeof expression === 'string' ? fromExpression(expression) : expression;

return {
Expand Down Expand Up @@ -131,8 +130,15 @@ export function buildExpression({
},
};

return prependKibanaContext(
prependDatasourceExpression(visualizationExpression, datasourceMap, datasourceStates),
expressionContext
const completeExpression = prependDatasourceExpression(
visualizationExpression,
datasourceMap,
datasourceStates
);

if (completeExpression) {
return prependKibanaContext(completeExpression, expressionContext);
} else {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ const generateSuggestion = (
layerId: string = 'first'
): DatasourceSuggestion => ({
state,
table: { datasourceSuggestionId, columns: [], isMultiRow: false, layerId },
table: {
datasourceSuggestionId,
columns: [],
isMultiRow: false,
layerId,
changeType: 'unchanged',
},
});

let datasourceMap: Record<string, DatasourceMock>;
Expand Down Expand Up @@ -233,12 +239,14 @@ describe('suggestion helpers', () => {
columns: [],
isMultiRow: true,
layerId: 'first',
changeType: 'unchanged',
};
const table2: TableSuggestion = {
datasourceSuggestionId: 1,
columns: [],
isMultiRow: true,
layerId: 'first',
changeType: 'unchanged',
};
datasourceMap.mock.getDatasourceSuggestionsFromCurrentState.mockReturnValue([
{ state: {}, table: table1 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import _ from 'lodash';
import { Ast } from '@kbn/interpreter/common';
import { Visualization, Datasource, FramePublicAPI } from '../../types';
import { Visualization, Datasource, FramePublicAPI, TableChangeType } from '../../types';
import { Action } from './state_management';

export interface Suggestion {
Expand All @@ -20,6 +20,8 @@ export interface Suggestion {
visualizationState: unknown;
previewExpression?: Ast | string;
previewIcon: string;
hide?: boolean;
changeType: TableChangeType;
}

/**
Expand Down Expand Up @@ -100,6 +102,7 @@ export function getSuggestions({
datasourceState: datasourceSuggestion.state,
datasourceId: datasourceSuggestion.datasourceId,
columns: datasourceSuggestion.table.columns.length,
changeType: datasourceSuggestion.table.changeType,
};
});
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,23 @@ describe('suggestion_panel', () => {
expect(passedExpression).toMatchInlineSnapshot(`
Object {
"chain": Array [
Object {
"arguments": Object {},
"function": "kibana",
"type": "function",
},
Object {
"arguments": Object {
"query": Array [
"{\\"query\\":\\"\\",\\"language\\":\\"lucene\\"}",
],
"timeRange": Array [
"{\\"from\\":\\"now-7d\\",\\"to\\":\\"now\\"}",
],
},
"function": "kibana_context",
"type": "function",
},
Object {
"arguments": Object {
"layerIds": Array [
Expand Down
Loading

0 comments on commit 3948678

Please sign in to comment.