Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Visualization validation and better error messages #81439

Merged
merged 54 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
1bed7c0
:sparkles: First pass with visualization validation + error messages
dej611 Oct 20, 2020
bb93701
:fire: Remove indexpattern error handling for now
dej611 Oct 21, 2020
0aea6c5
:label: Fix type issues
dej611 Oct 21, 2020
d389d85
:white_check_mark: Add getErrorMessage test for data table
dej611 Oct 21, 2020
4c48b23
:white_check_mark: Add tests for pie and metric error messages
dej611 Oct 21, 2020
91a700a
:globe_with_meridians: Fix i18n checks issues
dej611 Oct 22, 2020
b1d71dd
:bug: Fix last issue
dej611 Oct 22, 2020
be2312d
:white_check_mark: Add more tests for the XY visualization validation…
dej611 Oct 22, 2020
c64603a
:ok_hand: Included all feedback from first review
dej611 Oct 27, 2020
8e77401
:pencil2: Off by one message
dej611 Oct 27, 2020
64a22ef
Merge remote-tracking branch 'upstream/master' into feature/lens/disp…
dej611 Oct 27, 2020
7089b2f
:globe_with_meridians: Fix i18n duplicate id
dej611 Oct 27, 2020
86cc058
:globe_with_meridians: Fix last i18n issue
dej611 Oct 28, 2020
4e0c179
Merge remote-tracking branch 'upstream/master' into feature/lens/disp…
dej611 Oct 28, 2020
6672302
:bug: Fixed a hook reflow issue
dej611 Oct 28, 2020
ee80a6b
:recycle:+:white_check_mark: Reworked validation flow + tests
dej611 Oct 28, 2020
f1127f4
:label: Fix type issue
dej611 Oct 28, 2020
95f9daf
:bug: Improved XY corner cases validation logic
dej611 Oct 29, 2020
ce7f857
:bug: Fix empty datatable scenario
dej611 Oct 29, 2020
d88d55d
:sparkles: + :white_check_mark: Improved error messages for invalid d…
dej611 Oct 29, 2020
694757b
:globe_with_meridians: Add missing i18n translation
dej611 Oct 29, 2020
0277aa5
:label: Fix type issues
dej611 Oct 29, 2020
822a23e
:globe_with_meridians: Fix i18n issues
dej611 Oct 29, 2020
957bd98
:sparkles: Filter out suggestions which fail to build
dej611 Oct 29, 2020
7bfabd1
Merge branch 'master' into feature/lens/display-error-workspace
kibanamachine Oct 30, 2020
2fd7a90
:truck: Migrate datatable validation logic to the building phase, han…
dej611 Oct 30, 2020
4464285
:label: Fix type issue
dej611 Oct 30, 2020
e039c4f
:pencil2: Add comment for future enhancements
dej611 Oct 30, 2020
a84fad2
:pencil2: Updated comment
dej611 Oct 30, 2020
d06f099
Merge branch 'master' into feature/lens/display-error-workspace
kibanamachine Oct 30, 2020
135c40d
:world_with_meridians: Refactor axis labels
dej611 Oct 30, 2020
b12a320
:globe_with_meridians: Reworked few validation messages
dej611 Oct 30, 2020
9f5d951
:bug: Fix break down validation + percentage charts
dej611 Oct 30, 2020
f984d5d
:white_check_mark: Align tests with new validation logic
dej611 Oct 30, 2020
5e265ae
Merge branch 'feature/lens/display-error-workspace' of github.com:dej…
dej611 Oct 30, 2020
4b3c36a
Merge branch 'master' into feature/lens/display-error-workspace
kibanamachine Nov 2, 2020
c7fdd61
:recycle: Fix suggestion panel validation to match main panel
dej611 Nov 2, 2020
d0bce6b
:globe_with_meridians: Fix i18n issues
dej611 Nov 2, 2020
cb0214e
:wrench: Fix some refs for validation checks in suggestions
dej611 Nov 2, 2020
d3fabc2
Merge branch 'feature/lens/display-error-workspace' of github.com:dej…
dej611 Nov 2, 2020
6c27723
:bug: Fix missing key prop in multiple errors scenario
dej611 Nov 2, 2020
9437162
Merge branch 'master' into feature/lens/display-error-workspace
kibanamachine Nov 2, 2020
4ac3bd8
Merge branch 'master' into feature/lens/display-error-workspace
kibanamachine Nov 2, 2020
6798112
Merge remote-tracking branch 'upstream/master' into feature/lens/disp…
dej611 Nov 2, 2020
8d31435
:bug: Fix swtich issue from XY to partition
dej611 Nov 2, 2020
97dc1ae
:globe_with_meridians: Fix i18n messages and aligned tests
dej611 Nov 2, 2020
33b71fa
Merge branch 'master' into feature/lens/display-error-workspace
kibanamachine Nov 3, 2020
0a0f5d3
:bug: Fix suggestions switching bug
dej611 Nov 3, 2020
37dca5a
:refactor: Add more validation + refactored validation logic in a sin…
dej611 Nov 3, 2020
fa32b19
:pencil2: Add note about lint hooks disable rule
dej611 Nov 3, 2020
1a440a9
Merge remote-tracking branch 'upstream/master' into feature/lens/disp…
dej611 Nov 4, 2020
cdb7054
:rotating_light: Fix linting issue
dej611 Nov 4, 2020
11caf4a
:white_check_mark: Align tests with new API
dej611 Nov 4, 2020
8ccd1d4
:ok_hand: Early exists added
dej611 Nov 4, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -357,20 +357,77 @@ describe('Datatable Visualization', () => {
datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
dataType: 'string',
isBucketed: true,
isBucketed: false, // <= make them metrics
label: 'label',
});

const expression = datatableVisualization.toExpression(
{ layers: [layer] },
frame.datasourceLayers
) as Ast;

const tableArgs = buildExpression(expression).findFunction('lens_datatable_columns');

expect(tableArgs).toHaveLength(1);
expect(tableArgs[0].arguments).toEqual({
columnIds: ['c', 'b'],
});
});

it('returns no expression if the metric dimension is not defined', () => {
const datasource = createMockDatasource('test');
const layer = { layerId: 'a', columns: ['b', 'c'] };
const frame = mockFrame();
frame.datasourceLayers = { a: datasource.publicAPIMock };
datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
dataType: 'string',
isBucketed: true, // move it from the metric to the break down by side
label: 'label',
});

const expression = datatableVisualization.toExpression(
{ layers: [layer] },
frame.datasourceLayers
);

expect(expression).toEqual(null);
});
});

describe('#getErrorMessages', () => {
it('returns undefined if the datasource is missing a metric dimension', () => {
const datasource = createMockDatasource('test');
const layer = { layerId: 'a', columns: ['b', 'c'] };
const frame = mockFrame();
frame.datasourceLayers = { a: datasource.publicAPIMock };
datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
dataType: 'string',
isBucketed: true, // move it from the metric to the break down by side
label: 'label',
});

const error = datatableVisualization.getErrorMessages({ layers: [layer] }, frame);

expect(error).not.toBeDefined();
});

it('returns undefined if the metric dimension is defined', () => {
const datasource = createMockDatasource('test');
const layer = { layerId: 'a', columns: ['b', 'c'] };
const frame = mockFrame();
frame.datasourceLayers = { a: datasource.publicAPIMock };
datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
dataType: 'string',
isBucketed: false, // keep it a metric
label: 'label',
});

const error = datatableVisualization.getErrorMessages({ layers: [layer] }, frame);

expect(error).not.toBeDefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@

import { Ast } from '@kbn/interpreter/common';
import { i18n } from '@kbn/i18n';
import { SuggestionRequest, Visualization, VisualizationSuggestion, Operation } from '../types';
import {
SuggestionRequest,
Visualization,
VisualizationSuggestion,
Operation,
DatasourcePublicAPI,
} from '../types';
import { LensIconChartDatatable } from '../assets/chart_datatable';

export interface LayerState {
Expand Down Expand Up @@ -128,16 +134,13 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
},

getConfiguration({ state, frame, layerId }) {
const layer = state.layers.find((l) => l.layerId === layerId);
if (!layer) {
const { sortedColumns, datasource } =
getDataSourceAndSortedColumns(state, frame.datasourceLayers, layerId) || {};

if (!sortedColumns) {
return { groups: [] };
}

const datasource = frame.datasourceLayers[layer.layerId];
const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
// When we add a column it could be empty, and therefore have no order
const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));

return {
groups: [
{
Expand All @@ -146,7 +149,9 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
defaultMessage: 'Break down by',
}),
layerId: state.layers[0].layerId,
accessors: sortedColumns.filter((c) => datasource.getOperationForColumnId(c)?.isBucketed),
accessors: sortedColumns.filter(
(c) => datasource!.getOperationForColumnId(c)?.isBucketed
),
supportsMoreColumns: true,
filterOperations: (op) => op.isBucketed,
dataTestSubj: 'lnsDatatable_column',
Expand All @@ -158,7 +163,7 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
}),
layerId: state.layers[0].layerId,
accessors: sortedColumns.filter(
(c) => !datasource.getOperationForColumnId(c)?.isBucketed
(c) => !datasource!.getOperationForColumnId(c)?.isBucketed
),
supportsMoreColumns: true,
filterOperations: (op) => !op.isBucketed,
Expand Down Expand Up @@ -194,14 +199,19 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
};
},

toExpression(state, datasourceLayers, { title, description } = {}): Ast {
const layer = state.layers[0];
const datasource = datasourceLayers[layer.layerId];
const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
// When we add a column it could be empty, and therefore have no order
const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));
const operations = sortedColumns
.map((columnId) => ({ columnId, operation: datasource.getOperationForColumnId(columnId) }))
toExpression(state, datasourceLayers, { title, description } = {}): Ast | null {
const { sortedColumns, datasource } =
getDataSourceAndSortedColumns(state, datasourceLayers, state.layers[0].layerId) || {};

if (
sortedColumns?.length &&
sortedColumns.filter((c) => !datasource!.getOperationForColumnId(c)?.isBucketed).length === 0
) {
return null;
}

const operations = sortedColumns!
.map((columnId) => ({ columnId, operation: datasource!.getOperationForColumnId(columnId) }))
.filter((o): o is { columnId: string; operation: Operation } => !!o.operation);

return {
Expand Down Expand Up @@ -232,4 +242,24 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
],
};
},

getErrorMessages(state, frame) {
return undefined;
},
};

function getDataSourceAndSortedColumns(
state: DatatableVisualizationState,
datasourceLayers: Record<string, DatasourcePublicAPI>,
layerId: string
) {
const layer = state.layers.find((l: LayerState) => l.layerId === layerId);
if (!layer) {
return undefined;
}
const datasource = datasourceLayers[layer.layerId];
const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
// When we add a column it could be empty, and therefore have no order
const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));
return { datasource, sortedColumns };
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { SavedObjectReference } from 'kibana/public';
import { Ast } from '@kbn/interpreter/common';
import { Datasource, DatasourcePublicAPI, Visualization } from '../../types';
import { Datasource, DatasourcePublicAPI, FramePublicAPI, Visualization } from '../../types';
import { buildExpression } from './expression_helpers';
import { Document } from '../../persistence/saved_object_store';
import { VisualizeFieldContext } from '../../../../../../src/plugins/ui_actions/public';
Expand Down Expand Up @@ -91,3 +91,29 @@ export async function persistedStateToExpression(
datasourceLayers,
});
}

export const validateDatasourceAndVisualization = (
currentDataSource: Datasource | null,
currentDatasourceState: unknown | null,
currentVisualization: Visualization | null,
currentVisualizationState: unknown | undefined,
frameAPI: FramePublicAPI
):
| Array<{
shortMessage: string;
longMessage: string;
}>
| undefined => {
const datasourceValidationErrors = currentDatasourceState
? currentDataSource?.getErrorMessages(currentDatasourceState)
: undefined;

const visualizationValidationErrors = currentVisualizationState
? currentVisualization?.getErrorMessages(currentVisualizationState, frameAPI)
: undefined;

if (datasourceValidationErrors || visualizationValidationErrors) {
return [...(datasourceValidationErrors || []), ...(visualizationValidationErrors || [])];
}
return undefined;
};
Loading