Skip to content

Commit

Permalink
[Lens] Implement types for reference-based operations (#83603)
Browse files Browse the repository at this point in the history
* [Lens] Implement types for reference-based operations

* Update from review feedback
  • Loading branch information
Wylie Conlon authored Nov 20, 2020
1 parent d31ee21 commit b50e7ba
Show file tree
Hide file tree
Showing 30 changed files with 1,341 additions and 179 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ describe('Datatable Visualization', () => {

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

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

it('returns undefined if the metric dimension is defined', () => {
Expand All @@ -427,7 +427,7 @@ describe('Datatable Visualization', () => {

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

expect(error).not.toBeDefined();
expect(error).toBeUndefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export const validateDatasourceAndVisualization = (
? currentVisualization?.getErrorMessages(currentVisualizationState, frameAPI)
: undefined;

if (datasourceValidationErrors || visualizationValidationErrors) {
if (datasourceValidationErrors?.length || visualizationValidationErrors?.length) {
return [...(datasourceValidationErrors || []), ...(visualizationValidationErrors || [])];
}
return undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export const InnerVisualizationWrapper = ({
[dispatch]
);

if (localState.configurationValidationError) {
if (localState.configurationValidationError?.length) {
let showExtraErrors = null;
if (localState.configurationValidationError.length > 1) {
if (localState.expandError) {
Expand Down Expand Up @@ -445,7 +445,7 @@ export const InnerVisualizationWrapper = ({
);
}

if (localState.expressionBuildError) {
if (localState.expressionBuildError?.length) {
return (
<EuiFlexGroup style={{ maxWidth: '100%' }} direction="column" alignItems="center">
<EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ export function DimensionEditor(props: DimensionEditorProps) {
function getErrorMessage(
selectedColumn: IndexPatternColumn | undefined,
incompatibleSelectedOperationType: boolean,
input: 'none' | 'field' | undefined,
input: 'none' | 'field' | 'fullReference' | undefined,
fieldInvalid: boolean
) {
if (selectedColumn && incompatibleSelectedOperationType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@ describe('IndexPatternDimensionEditorPanel', () => {
indexPatternId: '1',
columns: {},
columnOrder: [],
incompleteColumns: {},
},
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ type Props = Pick<
'layerId' | 'columnId' | 'state' | 'filterOperations'
>;

// TODO: the support matrix should be available outside of the dimension panel

// TODO: This code has historically been memoized, as a potentially performance
// sensitive task. If we can add memoization without breaking the behavior, we should.
export const getOperationSupportMatrix = (props: Props): OperationSupportMatrix => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,15 @@ import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks';
import { Ast } from '@kbn/interpreter/common';
import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks';
import { getFieldByNameFactory } from './pure_helpers';
import {
operationDefinitionMap,
getErrorMessages,
createMockedReferenceOperation,
} from './operations';

jest.mock('./loader');
jest.mock('../id_generator');
jest.mock('./operations');

const fieldsOne = [
{
Expand Down Expand Up @@ -489,6 +495,56 @@ describe('IndexPattern Data Source', () => {
expect(ast.chain[0].arguments.timeFields).toEqual(['timestamp']);
expect(ast.chain[0].arguments.timeFields).not.toContain('timefield');
});

describe('references', () => {
beforeEach(() => {
// @ts-expect-error we are inserting an invalid type
operationDefinitionMap.testReference = createMockedReferenceOperation();

// @ts-expect-error we are inserting an invalid type
operationDefinitionMap.testReference.toExpression.mockReturnValue(['mock']);
});

afterEach(() => {
delete operationDefinitionMap.testReference;
});

it('should collect expression references and append them', async () => {
const queryBaseState: IndexPatternBaseState = {
currentIndexPatternId: '1',
layers: {
first: {
indexPatternId: '1',
columnOrder: ['col1', 'col2'],
columns: {
col1: {
label: 'Count of records',
dataType: 'date',
isBucketed: false,
sourceField: 'timefield',
operationType: 'cardinality',
},
col2: {
label: 'Reference',
dataType: 'number',
isBucketed: false,
// @ts-expect-error not a valid type
operationType: 'testReference',
references: ['col1'],
},
},
},
},
};

const state = enrichBaseState(queryBaseState);

const ast = indexPatternDatasource.toExpression(state, 'first') as Ast;
// @ts-expect-error we can't isolate just the reference type
expect(operationDefinitionMap.testReference.toExpression).toHaveBeenCalled();
expect(ast.chain[2]).toEqual('mock');
});
});
});

describe('#insertLayer', () => {
Expand Down Expand Up @@ -599,11 +655,33 @@ describe('IndexPattern Data Source', () => {

describe('getTableSpec', () => {
it('should include col1', () => {
expect(publicAPI.getTableSpec()).toEqual([
{
columnId: 'col1',
expect(publicAPI.getTableSpec()).toEqual([{ columnId: 'col1' }]);
});

it('should skip columns that are being referenced', () => {
publicAPI = indexPatternDatasource.getPublicAPI({
state: {
layers: {
first: {
indexPatternId: '1',
columnOrder: ['col1', 'col2'],
columns: {
// @ts-ignore this is too little information for a real column
col1: {
dataType: 'number',
},
col2: {
// @ts-expect-error update once we have a reference operation outside tests
references: ['col1'],
},
},
},
},
},
]);
layerId: 'first',
});

expect(publicAPI.getTableSpec()).toEqual([{ columnId: 'col2' }]);
});
});

Expand Down Expand Up @@ -764,7 +842,7 @@ describe('IndexPattern Data Source', () => {
dataType: 'number',
isBucketed: false,
label: 'Foo',
operationType: 'document',
operationType: 'avg',
sourceField: 'bytes',
},
},
Expand All @@ -774,7 +852,7 @@ describe('IndexPattern Data Source', () => {
};
expect(
indexPatternDatasource.getErrorMessages(state as IndexPatternPrivateState)
).not.toBeDefined();
).toBeUndefined();
});

it('should return no errors with layers with no columns', () => {
Expand All @@ -792,7 +870,31 @@ describe('IndexPattern Data Source', () => {
},
currentIndexPatternId: '1',
};
expect(indexPatternDatasource.getErrorMessages(state)).not.toBeDefined();
expect(indexPatternDatasource.getErrorMessages(state)).toBeUndefined();
});

it('should bubble up invalid configuration from operations', () => {
(getErrorMessages as jest.Mock).mockClear();
(getErrorMessages as jest.Mock).mockReturnValueOnce(['error 1', 'error 2']);
const state: IndexPatternPrivateState = {
indexPatternRefs: [],
existingFields: {},
isFirstExistenceFetch: false,
indexPatterns: expectedIndexPatterns,
layers: {
first: {
indexPatternId: '1',
columnOrder: [],
columns: {},
},
},
currentIndexPatternId: '1',
};
expect(indexPatternDatasource.getErrorMessages(state)).toEqual([
{ shortMessage: 'error 1', longMessage: '' },
{ shortMessage: 'error 2', longMessage: '' },
]);
expect(getErrorMessages).toHaveBeenCalledTimes(1);
});
});
});
Loading

0 comments on commit b50e7ba

Please sign in to comment.