From 381b61ae6a0724afe47bc26fd5f5b501dcd776d8 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 21 Sep 2020 16:33:02 -0400 Subject: [PATCH 1/7] [Lens] Fieldless operations --- .../dimension_panel/dimension_editor.tsx | 210 ++++++++++-------- .../dimension_panel/dimension_panel.tsx | 27 ++- .../operations/definitions/cardinality.tsx | 7 +- .../operations/definitions/column_types.ts | 2 +- .../operations/definitions/count.tsx | 1 + .../operations/definitions/date_histogram.tsx | 1 + .../definitions/filters/filters.test.tsx | 25 +-- .../definitions/filters/filters.tsx | 28 ++- .../operations/definitions/index.ts | 99 ++++++--- .../operations/definitions/metrics.tsx | 10 +- .../operations/definitions/terms.tsx | 1 + .../operations/operations.test.ts | 27 ++- .../operations/operations.ts | 67 +++--- .../test/functional/apps/lens/smokescreen.ts | 1 + 14 files changed, 297 insertions(+), 209 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 153757ac37da1..d84e9cc4d78fe 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -104,8 +104,10 @@ export function DimensionEditor(props: DimensionEditorProps) { setInvalidOperationType, ] = useState(null); - const ParamEditor = - selectedColumn && operationDefinitionMap[selectedColumn.operationType].paramEditor; + const selectedOperationDefinition = + selectedColumn && operationDefinitionMap[selectedColumn.operationType]; + + const ParamEditor = selectedOperationDefinition?.paramEditor; const fieldMap: Record = useMemo(() => { const fields: Record = {}; @@ -129,6 +131,10 @@ export function DimensionEditor(props: DimensionEditorProps) { [ ...asOperationOptions(validOperationTypes, true), ...asOperationOptions(possibleOperationTypes, false), + ...asOperationOptions( + operationFieldSupportMatrix.operationWithoutField, + !selectedColumn || !hasField(selectedColumn) + ), ], 'operationType' ); @@ -166,12 +172,37 @@ export function DimensionEditor(props: DimensionEditorProps) { compatibleWithCurrentField ? '' : ' incompatible' }`, onClick() { + if (selectedColumn?.operationType === operationType) { + return; + } + // todo: when moving from terms agg to filters, we want to create a filter `$field.name : *` // it probably has to be re-thought when removing the field name. const isTermsToFilters = selectedColumn?.operationType === 'terms' && operationType === 'filters'; if (!selectedColumn || !compatibleWithCurrentField) { + if (operationDefinitionMap[operationType].input === 'none') { + setInvalidOperationType(null); + setState( + changeColumn({ + state, + layerId, + columnId, + newColumn: buildColumn({ + columns: props.state.layers[props.layerId].columns, + suggestedPriority: props.suggestedPriority, + layerId: props.layerId, + op: operationType, + indexPattern: currentIndexPattern, + previousColumn: selectedColumn, + }), + }) + ); + trackUiEvent(`indexpattern_dimension_operation_${operationType}`); + return; + } + const possibleFields = fieldByOperation[operationType] || []; if (possibleFields.length === 1) { @@ -200,16 +231,14 @@ export function DimensionEditor(props: DimensionEditorProps) { if (incompatibleSelectedOperationType && !isTermsToFilters) { setInvalidOperationType(null); } - if (selectedColumn.operationType === operationType) { - return; - } + const newColumn: IndexPatternColumn = buildColumn({ columns: props.state.layers[props.layerId].columns, suggestedPriority: props.suggestedPriority, layerId: props.layerId, op: operationType, indexPattern: currentIndexPattern, - field: fieldMap[selectedColumn.sourceField], + field: hasField(selectedColumn) ? fieldMap[selectedColumn.sourceField] : undefined, previousColumn: selectedColumn, }); @@ -244,91 +273,96 @@ export function DimensionEditor(props: DimensionEditorProps) {
- - { - setState( - deleteColumn({ - state, - layerId, - columnId, - }) - ); - }} - onChoose={(choice) => { - let column: IndexPatternColumn; - if ( - !incompatibleSelectedOperationType && - selectedColumn && - 'field' in choice && - choice.operationType === selectedColumn.operationType - ) { - // If we just changed the field are not in an error state and the operation didn't change, - // we use the operations onFieldChange method to calculate the new column. - column = changeField(selectedColumn, currentIndexPattern, fieldMap[choice.field]); - } else { - // Otherwise we'll use the buildColumn method to calculate a new column - const compatibleOperations = - ('field' in choice && - operationFieldSupportMatrix.operationByField[choice.field]) || - []; - let operation; - if (compatibleOperations.length > 0) { - operation = - incompatibleSelectedOperationType && - compatibleOperations.includes(incompatibleSelectedOperationType) - ? incompatibleSelectedOperationType - : compatibleOperations[0]; - } else if ('field' in choice) { - operation = choice.operationType; - } - column = buildColumn({ - columns: props.state.layers[props.layerId].columns, - field: fieldMap[choice.field], - indexPattern: currentIndexPattern, - layerId: props.layerId, - suggestedPriority: props.suggestedPriority, - op: operation as OperationType, - previousColumn: selectedColumn, - }); + > + { + setState( + deleteColumn({ + state, + layerId, + columnId, + }) + ); + }} + onChoose={(choice) => { + let column: IndexPatternColumn; + if ( + !incompatibleSelectedOperationType && + selectedColumn && + 'field' in choice && + choice.operationType === selectedColumn.operationType + ) { + // If we just changed the field are not in an error state and the operation didn't change, + // we use the operations onFieldChange method to calculate the new column. + column = changeField(selectedColumn, currentIndexPattern, fieldMap[choice.field]); + } else { + // Otherwise we'll use the buildColumn method to calculate a new column + const compatibleOperations = + ('field' in choice && + operationFieldSupportMatrix.operationByField[choice.field]) || + []; + let operation; + if (compatibleOperations.length > 0) { + operation = + incompatibleSelectedOperationType && + compatibleOperations.includes(incompatibleSelectedOperationType) + ? incompatibleSelectedOperationType + : compatibleOperations[0]; + } else if ('field' in choice) { + operation = choice.operationType; + } + column = buildColumn({ + columns: props.state.layers[props.layerId].columns, + field: fieldMap[choice.field], + indexPattern: currentIndexPattern, + layerId: props.layerId, + suggestedPriority: props.suggestedPriority, + op: operation as OperationType, + previousColumn: selectedColumn, + }); + } - setState( - changeColumn({ - state, - layerId, - columnId, - newColumn: column, - keepParams: false, - }) - ); - setInvalidOperationType(null); - }} - /> - + setState( + changeColumn({ + state, + layerId, + columnId, + newColumn: column, + keepParams: false, + }) + ); + setInvalidOperationType(null); + }} + /> + + ) : null} {!incompatibleSelectedOperationType && ParamEditor && ( <> diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx index 923f7145d1c64..9689e76b4adf4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx @@ -48,6 +48,7 @@ export type IndexPatternDimensionEditorProps = DatasourceDimensionEditorProps< export interface OperationFieldSupportMatrix { operationByField: Partial>; + operationWithoutField: OperationType[]; fieldByOperation: Partial>; } @@ -67,25 +68,31 @@ const getOperationFieldSupportMatrix = (props: Props): OperationFieldSupportMatr ).filter((operation) => props.filterOperations(operation.operationMetaData)); const supportedOperationsByField: Partial> = {}; + const supportedOperationsWithoutField: OperationType[] = []; const supportedFieldsByOperation: Partial> = {}; filteredOperationsByMetadata.forEach(({ operations }) => { operations.forEach((operation) => { - if (supportedOperationsByField[operation.field]) { - supportedOperationsByField[operation.field]!.push(operation.operationType); - } else { - supportedOperationsByField[operation.field] = [operation.operationType]; - } - - if (supportedFieldsByOperation[operation.operationType]) { - supportedFieldsByOperation[operation.operationType]!.push(operation.field); - } else { - supportedFieldsByOperation[operation.operationType] = [operation.field]; + if (operation.type === 'field') { + if (supportedOperationsByField[operation.field]) { + supportedOperationsByField[operation.field]!.push(operation.operationType); + } else { + supportedOperationsByField[operation.field] = [operation.operationType]; + } + + if (supportedFieldsByOperation[operation.operationType]) { + supportedFieldsByOperation[operation.operationType]!.push(operation.field); + } else { + supportedFieldsByOperation[operation.operationType] = [operation.field]; + } + } else if (operation.type === 'none') { + supportedOperationsWithoutField.push(operation.operationType); } }); }); return { operationByField: _.mapValues(supportedOperationsByField, _.uniq), + operationWithoutField: _.uniq(supportedOperationsWithoutField), fieldByOperation: _.mapValues(supportedFieldsByOperation, _.uniq), }; }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx index b0777c7febd7d..6cf0a7fb1fe15 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx @@ -6,7 +6,7 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; -import { FormattedIndexPatternColumn } from './column_types'; +import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; const supportedTypes = new Set(['string', 'boolean', 'number', 'ip', 'date']); @@ -21,7 +21,9 @@ function ofName(name: string) { }); } -export interface CardinalityIndexPatternColumn extends FormattedIndexPatternColumn { +export interface CardinalityIndexPatternColumn + extends FormattedIndexPatternColumn, + FieldBasedIndexPatternColumn { operationType: 'cardinality'; } @@ -30,6 +32,7 @@ export const cardinalityOperation: OperationDefinition { if ( supportedTypes.has(type) && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts index 3244eeb94d1e2..4a9f521567582 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts @@ -14,7 +14,6 @@ import { Operation, DimensionPriority } from '../../../types'; export interface BaseIndexPatternColumn extends Operation { // Private operationType: string; - sourceField: string; suggestedPriority?: DimensionPriority; customLabel?: boolean; } @@ -49,5 +48,6 @@ export type ParameterlessIndexPatternColumn< > = TBase & { operationType: TOperationType }; export interface FieldBasedIndexPatternColumn extends BaseIndexPatternColumn { + sourceField: string; suggestedPriority?: DimensionPriority; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index bb1aef856de78..44c14f803a4c4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -23,6 +23,7 @@ export const countOperation: OperationDefinition = { displayName: i18n.translate('xpack.lens.indexPattern.count', { defaultMessage: 'Count', }), + input: 'field', onFieldChange: (oldColumn, indexPattern, field) => { return { ...oldColumn, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index 83ae8e47b39ca..00575a2c1cb25 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -43,6 +43,7 @@ export const dateHistogramOperation: OperationDefinition { if ( type === 'date' && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx index 2d79c5faf74fe..3ac01886537dc 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx @@ -59,7 +59,6 @@ describe('filters', () => { operationType: 'filters', scale: 'ordinal', isBucketed: true, - sourceField: 'Records', params: { filters: [ { @@ -112,34 +111,14 @@ describe('filters', () => { }); }); - describe('getPossibleOperationForField', () => { + describe('getPossibleOperation', () => { it('should return operation with the right type for document', () => { - expect( - filtersOperation.getPossibleOperationForField({ - aggregatable: true, - searchable: true, - name: 'test', - displayName: 'test', - type: 'document', - }) - ).toEqual({ + expect(filtersOperation.getPossibleOperation()).toEqual({ dataType: 'string', isBucketed: true, scale: 'ordinal', }); }); - - it('should not return operation if field type is not document', () => { - expect( - filtersOperation.getPossibleOperationForField({ - aggregatable: false, - searchable: true, - name: 'test', - displayName: 'test', - type: 'string', - }) - ).toEqual(undefined); - }); }); describe('popover param editor', () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx index cc1e23cb82a49..4cdbbfbaa0e68 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx @@ -11,7 +11,7 @@ import { i18n } from '@kbn/i18n'; import { EuiFormRow, EuiLink, htmlIdGenerator } from '@elastic/eui'; import { updateColumnParam } from '../../../state_helpers'; import { OperationDefinition } from '../index'; -import { FieldBasedIndexPatternColumn } from '../column_types'; +import { BaseIndexPatternColumn } from '../column_types'; import { FilterPopover } from './filter_popover'; import { IndexPattern } from '../../../types'; import { Query, esKuery, esQuery } from '../../../../../../../../src/plugins/data/public'; @@ -61,7 +61,7 @@ export const isQueryValid = (input: Query, indexPattern: IndexPattern) => { } }; -export interface FiltersIndexPatternColumn extends FieldBasedIndexPatternColumn { +export interface FiltersIndexPatternColumn extends BaseIndexPatternColumn { operationType: 'filters'; params: { filters: Filter[]; @@ -72,20 +72,11 @@ export const filtersOperation: OperationDefinition = type: 'filters', displayName: filtersLabel, priority: 3, // Higher than any metric - getPossibleOperationForField: ({ type }) => { - if (type === 'document') { - return { - dataType: 'string', - isBucketed: true, - scale: 'ordinal', - }; - } - }, - isTransferable: () => false, - onFieldChange: (oldColumn, indexPattern, field) => oldColumn, + input: 'none', + isTransferable: () => true, - buildColumn({ suggestedPriority, field, previousColumn }) { + buildColumn({ suggestedPriority, previousColumn }) { let params = { filters: [defaultFilter] }; if (previousColumn?.operationType === 'terms') { params = { @@ -108,11 +99,18 @@ export const filtersOperation: OperationDefinition = scale: 'ordinal', suggestedPriority, isBucketed: true, - sourceField: field.name, params, }; }, + getPossibleOperation() { + return { + dataType: 'string', + isBucketed: true, + scale: 'ordinal', + }; + }, + toEsAggsConfig: (column, columnId, indexPattern) => { const validFilters = column.params.filters?.filter((f: Filter) => isQueryValid(f.input, indexPattern) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 3dd8b659fe0a5..53538ac092275 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -139,40 +139,71 @@ interface BaseBuildColumnArgs { * indicates a field based column, `getPossibleOperationForField` has to be * specified, otherwise `getPossibleOperationForDocument` has to be defined. */ -export interface OperationDefinition - extends BaseOperationDefinitionProps { - /** - * Returns the meta data of the operation if applied to the given field. Undefined - * if the field is not applicable to the operation. - */ - getPossibleOperationForField: (field: IndexPatternField) => OperationMetadata | undefined; - /** - * Builds the column object for the given parameters. Should include default p - */ - buildColumn: ( - arg: BaseBuildColumnArgs & { - field: IndexPatternField; - previousColumn?: IndexPatternColumn; - } - ) => C; - /** - * This method will be called if the user changes the field of an operation. - * You must implement it and return the new column after the field change. - * The most simple implementation will just change the field on the column, and keep - * the rest the same. Some implementations might want to change labels, or their parameters - * when changing the field. - * - * This will only be called for switching the field, not for initially selecting a field. - * - * See {@link OperationDefinition#transfer} for controlling column building when switching an - * index pattern not just a field. - * - * @param oldColumn The column before the user changed the field. - * @param indexPattern The index pattern that field is on. - * @param field The field that the user changed to. - */ - onFieldChange: (oldColumn: C, indexPattern: IndexPattern, field: IndexPatternField) => C; -} + +// interface SharedOperationDefinition +// extends BaseOperationDefinitionProps { +// /** +// * Builds the column object for the given parameters. Should include default p +// */ +// buildColumn: ( +// arg: BaseBuildColumnArgs & { +// field?: IndexPatternField; +// previousColumn?: IndexPatternColumn; +// } +// ) => C; +// } + +export type OperationDefinition = + | (BaseOperationDefinitionProps & { + input: 'none'; + /** + * Builds the column object for the given parameters. Should include default p + */ + buildColumn: ( + arg: BaseBuildColumnArgs & { + previousColumn?: IndexPatternColumn; + } + ) => C; + /** + * Returns the meta data of the operation if applied. Undefined + * if the field is not applicable. + */ + getPossibleOperation: () => OperationMetadata | undefined; + }) + | (BaseOperationDefinitionProps & { + input: 'field'; + /** + * Returns the meta data of the operation if applied to the given field. Undefined + * if the field is not applicable to the operation. + */ + getPossibleOperationForField: (field: IndexPatternField) => OperationMetadata | undefined; + /** + * Builds the column object for the given parameters. Should include default p + */ + buildColumn: ( + arg: BaseBuildColumnArgs & { + field: IndexPatternField; + previousColumn?: IndexPatternColumn; + } + ) => C; + /** + * This method will be called if the user changes the field of an operation. + * You must implement it and return the new column after the field change. + * The most simple implementation will just change the field on the column, and keep + * the rest the same. Some implementations might want to change labels, or their parameters + * when changing the field. + * + * This will only be called for switching the field, not for initially selecting a field. + * + * See {@link OperationDefinition#transfer} for controlling column building when switching an + * index pattern not just a field. + * + * @param oldColumn The column before the user changed the field. + * @param indexPattern The index pattern that field is on. + * @param field The field that the user changed to. + */ + onFieldChange: (oldColumn: C, indexPattern: IndexPattern, field: IndexPatternField) => C; + }); /** * A union type of all available operation types. The operation type is a unique id of an operation. diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx index 4c37d95f6b050..d454d70051cc5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -6,11 +6,12 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; -import { FormattedIndexPatternColumn } from './column_types'; +import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; -type MetricColumn = FormattedIndexPatternColumn & { - operationType: T; -}; +type MetricColumn = FormattedIndexPatternColumn & + FieldBasedIndexPatternColumn & { + operationType: T; + }; function buildMetricOperation>({ type, @@ -27,6 +28,7 @@ function buildMetricOperation>({ type, priority, displayName, + input: 'field', getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => { if ( fieldType === 'number' && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx index 20c421008a746..d32efa15fd0a3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx @@ -54,6 +54,7 @@ export const termsOperation: OperationDefinition = { defaultMessage: 'Top values', }), priority: 3, // Higher than any metric + input: 'field', getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( supportedTypes.has(type) && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts index 4ac3fc89500f9..7ee7dfcdc6de0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts @@ -182,7 +182,7 @@ describe('getOperationTypesForField', () => { }, }; - it('should build a column for the given operation type if it is passed in', () => { + it('should build a column for the given field-based operation type if it is passed in', () => { const column = buildColumn({ layerId: 'first', indexPattern: expectedIndexPatterns[1], @@ -194,6 +194,17 @@ describe('getOperationTypesForField', () => { expect(column.operationType).toEqual('count'); }); + it('should build a column for the given no-input operation type if it is passed in', () => { + const column = buildColumn({ + layerId: 'first', + indexPattern: expectedIndexPatterns[1], + columns: state.layers.first.columns, + suggestedPriority: 0, + op: 'filters', + }); + expect(column.operationType).toEqual('filters'); + }); + it('should build a column for the given operation type and field if it is passed in', () => { const field = expectedIndexPatterns[1].fields[1]; const column = buildColumn({ @@ -222,18 +233,22 @@ describe('getOperationTypesForField', () => { ); }); - it('should list out all field-operation tuples for different operation meta data', () => { + it('should list out all operation tuples', () => { expect(getAvailableOperationsByMetadata(expectedIndexPatterns[1])).toMatchInlineSnapshot(` Array [ Object { "operationMetaData": Object { - "dataType": "number", + "dataType": "string", "isBucketed": true, "scale": "ordinal", }, "operations": Array [ Object { - "field": "bytes", + "operationType": "filters", + "type": "none", + }, + Object { + "field": "source", "operationType": "terms", "type": "field", }, @@ -241,13 +256,13 @@ describe('getOperationTypesForField', () => { }, Object { "operationMetaData": Object { - "dataType": "string", + "dataType": "number", "isBucketed": true, "scale": "ordinal", }, "operations": Array [ Object { - "field": "source", + "field": "bytes", "operationType": "terms", "type": "field", }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts index 9e5a0f496357d..570fb17a53706 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts @@ -80,11 +80,16 @@ export function isDocumentOperation(type: string) { return documentOperations.has(type); } -interface OperationFieldTuple { - type: 'field'; - operationType: OperationType; - field: string; -} +type OperationFieldTuple = + | { + type: 'field'; + operationType: OperationType; + field: string; + } + | { + type: 'none'; + operationType: OperationType; + }; /** * Returns all possible operations (matches between operations and fields of the index @@ -100,11 +105,18 @@ interface OperationFieldTuple { * [ * { * operationMetaData: { dataType: 'string', isBucketed: true }, - * operations: ['terms'] + * operations: [{ + * type: 'field', + * operationType: ['terms'], + * field: 'keyword' + * }] * }, * { - * operationMetaData: { dataType: 'number', isBucketed: false }, - * operations: ['avg', 'min', 'max'] + * operationMetaData: { dataType: 'string', isBucketed: true }, + * operations: [{ + * type: 'none', + * operationType: ['filters'], + * }] * }, * ] * ``` @@ -133,30 +145,31 @@ export function getAvailableOperationsByMetadata(indexPattern: IndexPattern) { }; operationDefinitions.sort(getSortScoreByPriority).forEach((operationDefinition) => { - indexPattern.fields.forEach((field) => { + if (operationDefinition.input === 'field') { + indexPattern.fields.forEach((field) => { + addToMap( + { + type: 'field', + operationType: operationDefinition.type, + field: field.name, + }, + operationDefinition.getPossibleOperationForField(field) + ); + }); + } else if (operationDefinition.input === 'none') { addToMap( { - type: 'field', + type: 'none', operationType: operationDefinition.type, - field: field.name, }, - getPossibleOperationForField(operationDefinition, field) + operationDefinition.getPossibleOperation() ); - }); + } }); return Object.values(operationByMetadata); } -function getPossibleOperationForField( - operationDefinition: GenericOperationDefinition, - field: IndexPatternField -): OperationMetadata | undefined { - return 'getPossibleOperationForField' in operationDefinition - ? operationDefinition.getPossibleOperationForField(field) - : undefined; -} - /** * Changes the field of the passed in colum. To do so, this method uses the `onFieldChange` function of * the operation definition of the column. Returns a new column object with the field changed. @@ -203,7 +216,7 @@ export function buildColumn({ suggestedPriority: DimensionPriority | undefined; layerId: string; indexPattern: IndexPattern; - field: IndexPatternField; + field?: IndexPatternField; previousColumn?: IndexPatternColumn; }): IndexPatternColumn { const operationDefinition = operationDefinitionMap[op]; @@ -220,16 +233,18 @@ export function buildColumn({ previousColumn, }; + if (operationDefinition.input === 'none') { + return operationDefinition.buildColumn(baseOptions); + } + if (!field) { throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); } - const newColumn = operationDefinition.buildColumn({ + return operationDefinition.buildColumn({ ...baseOptions, field, }); - - return newColumn; } export { operationDefinitionMap } from './definitions'; diff --git a/x-pack/test/functional/apps/lens/smokescreen.ts b/x-pack/test/functional/apps/lens/smokescreen.ts index 42807a23cb13a..fc91d13c4b253 100644 --- a/x-pack/test/functional/apps/lens/smokescreen.ts +++ b/x-pack/test/functional/apps/lens/smokescreen.ts @@ -66,6 +66,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // legend item(s), so we're using a class selector here. expect(await find.allByCssSelector('.echLegendItem')).to.have.length(3); }); + it('should create an xy visualization with filters aggregation', async () => { await PageObjects.visualize.gotoVisualizationLandingPage(); await listingTable.searchForItemWithName('lnsXYvis'); From 6e41b8b0d763da489ff58fb622aa9c77428fb23d Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 21 Sep 2020 19:08:33 -0400 Subject: [PATCH 2/7] Overhaul types --- .../dimension_panel/dimension_editor.tsx | 2 +- .../indexpattern_suggestions.ts | 12 +- .../operations/definitions/cardinality.tsx | 9 +- .../operations/definitions/column_types.ts | 18 -- .../operations/definitions/count.tsx | 13 +- .../operations/definitions/date_histogram.tsx | 43 ++-- .../definitions/filters/filters.tsx | 6 +- .../operations/definitions/index.ts | 194 ++++++++++-------- .../operations/definitions/metrics.tsx | 7 +- .../operations/definitions/terms.tsx | 61 +++--- .../operations/index.ts | 2 +- .../operations/operations.ts | 8 +- .../indexpattern_datasource/state_helpers.ts | 3 +- 13 files changed, 195 insertions(+), 183 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index d84e9cc4d78fe..998ed810b0434 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -364,7 +364,7 @@ export function DimensionEditor(props: DimensionEditorProps) { ) : null} - {!incompatibleSelectedOperationType && ParamEditor && ( + {!incompatibleSelectedOperationType && selectedColumn && ParamEditor && ( <> field.name === layer.columns[firstBucket].sourceField) - ?.displayName || ''; + currentFields.find((field) => { + const column = layer.columns[firstBucket]; + return hasField(column) && column.sourceField === field.name; + })?.displayName || ''; const secondBucketLabel = - currentFields.find((field) => field.name === layer.columns[secondBucket].sourceField) - ?.displayName || ''; + currentFields.find((field) => { + const column = layer.columns[secondBucket]; + return hasField(column) && column.sourceField === field.name; + })?.displayName || ''; return buildSuggestion({ state, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx index 6cf0a7fb1fe15..5a490cdd2c38e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx @@ -27,7 +27,7 @@ export interface CardinalityIndexPatternColumn operationType: 'cardinality'; } -export const cardinalityOperation: OperationDefinition = { +export const cardinalityOperation: OperationDefinition = { type: OPERATION_TYPE, displayName: i18n.translate('xpack.lens.indexPattern.cardinality', { defaultMessage: 'Unique count', @@ -43,7 +43,8 @@ export const cardinalityOperation: OperationDefinition { - const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); + const c = column as CardinalityIndexPatternColumn; + const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); return Boolean( newField && @@ -75,13 +76,13 @@ export const cardinalityOperation: OperationDefinition { return { - ...oldColumn, + ...(oldColumn as CardinalityIndexPatternColumn), label: ofName(field.displayName), sourceField: field.name, }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts index 4a9f521567582..2e95e3fd4250f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts @@ -30,24 +30,6 @@ export interface FormattedIndexPatternColumn extends BaseIndexPatternColumn { }; } -/** - * Base type for a column that doesn't have additional parameter. - * - * * `TOperationType` should be a string type containing just the type - * of the operation (e.g. `"sum"`). - * * `TBase` is the base column interface the operation type is set for - - * by default this is `FieldBasedIndexPatternColumn`, so - * `ParameterlessIndexPatternColumn<'foo'>` will give you a column type - * for an operation named foo that operates on a field. - * By passing in another `TBase` (e.g. just `BaseIndexPatternColumn`), - * you can also create other column types. - */ -export type ParameterlessIndexPatternColumn< - TOperationType extends string, - TBase extends BaseIndexPatternColumn = FieldBasedIndexPatternColumn -> = TBase & { operationType: TOperationType }; - export interface FieldBasedIndexPatternColumn extends BaseIndexPatternColumn { sourceField: string; - suggestedPriority?: DimensionPriority; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index 44c14f803a4c4..de5b50f5be908 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -6,18 +6,19 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; -import { FormattedIndexPatternColumn } from './column_types'; +import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField } from '../../types'; const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { defaultMessage: 'Count of records', }); -export type CountIndexPatternColumn = FormattedIndexPatternColumn & { - operationType: 'count'; -}; +export type CountIndexPatternColumn = FormattedIndexPatternColumn & + FieldBasedIndexPatternColumn & { + operationType: 'count'; + }; -export const countOperation: OperationDefinition = { +export const countOperation: OperationDefinition = { type: 'count', priority: 2, displayName: i18n.translate('xpack.lens.indexPattern.count', { @@ -26,7 +27,7 @@ export const countOperation: OperationDefinition = { input: 'field', onFieldChange: (oldColumn, indexPattern, field) => { return { - ...oldColumn, + ...(oldColumn as CountIndexPatternColumn), label: field.displayName, sourceField: field.name, }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index 00575a2c1cb25..f8d7890aaf0e4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -37,7 +37,10 @@ export interface DateHistogramIndexPatternColumn extends FieldBasedIndexPatternC }; } -export const dateHistogramOperation: OperationDefinition = { +export const dateHistogramOperation: OperationDefinition< + DateHistogramIndexPatternColumn, + 'field' +> = { type: 'date_histogram', displayName: i18n.translate('xpack.lens.indexPattern.dateHistogram', { defaultMessage: 'Date histogram', @@ -79,7 +82,8 @@ export const dateHistogramOperation: OperationDefinition { - const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); + const c = column as DateHistogramIndexPatternColumn; + const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); return Boolean( newField && @@ -89,7 +93,8 @@ export const dateHistogramOperation: OperationDefinition { - const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); + const col = column as DateHistogramIndexPatternColumn; + const newField = newIndexPattern.fields.find((field) => field.name === col.sourceField); if ( newField && newField.aggregationRestrictions && @@ -98,9 +103,9 @@ export const dateHistogramOperation: OperationDefinition { return { - ...oldColumn, + ...(oldColumn as DateHistogramIndexPatternColumn), label: field.displayName, sourceField: field.name, }; }, toEsAggsConfig: (column, columnId, indexPattern) => { - const usedField = indexPattern.fields.find((field) => field.name === column.sourceField); + const c = column as DateHistogramIndexPatternColumn; + const usedField = indexPattern.fields.find((field) => field.name === c.sourceField); return { id: columnId, enabled: true, type: 'date_histogram', schema: 'segment', params: { - field: column.sourceField, - time_zone: column.params.timeZone, + field: c.sourceField, + time_zone: c.params.timeZone, useNormalizedEsInterval: !usedField || !usedField.aggregationRestrictions?.date_histogram, - interval: column.params.interval, + interval: c.params.interval, drop_partials: false, min_doc_count: 0, extended_bounds: {}, }, }; }, - paramEditor: ({ state, setState, currentColumn: currentColumn, layerId, dateRange, data }) => { + paramEditor: ({ state, setState, currentColumn, layerId, dateRange, data }) => { + const column = currentColumn as DateHistogramIndexPatternColumn; const field = - currentColumn && + column && state.indexPatterns[state.layers[layerId].indexPatternId].fields.find( - (currentField) => currentField.name === currentColumn.sourceField + (currentField) => currentField.name === column.sourceField ); const intervalIsRestricted = field!.aggregationRestrictions && field!.aggregationRestrictions.date_histogram; - const interval = parseInterval(currentColumn.params.interval); + const interval = parseInterval(column.params.interval); // We force the interval value to 1 if it's empty, since that is the ES behavior, // and the isValidInterval function doesn't handle the empty case properly. Fixing @@ -188,13 +195,13 @@ export const dateHistogramOperation: OperationDefinition )} - {currentColumn.params.interval !== autoInterval && ( + {column.params.interval !== autoInterval && ( ) : ( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx index 4cdbbfbaa0e68..18fe41511c86e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx @@ -68,7 +68,7 @@ export interface FiltersIndexPatternColumn extends BaseIndexPatternColumn { }; } -export const filtersOperation: OperationDefinition = { +export const filtersOperation: OperationDefinition = { type: 'filters', displayName: filtersLabel, priority: 3, // Higher than any metric @@ -112,7 +112,7 @@ export const filtersOperation: OperationDefinition = }, toEsAggsConfig: (column, columnId, indexPattern) => { - const validFilters = column.params.filters?.filter((f: Filter) => + const validFilters = (column as FiltersIndexPatternColumn).params.filters?.filter((f: Filter) => isQueryValid(f.input, indexPattern) ); return { @@ -128,7 +128,7 @@ export const filtersOperation: OperationDefinition = paramEditor: ({ state, setState, currentColumn, layerId, data }) => { const indexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; - const filters = currentColumn.params.filters; + const filters = (currentColumn as FiltersIndexPatternColumn).params.filters; const setFilters = (newFilters: Filter[]) => setState( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 53538ac092275..2eb73ac52b0ab 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -27,21 +27,6 @@ import { IndexPatternPrivateState, IndexPattern, IndexPatternField } from '../.. import { DateRange } from '../../../../common'; import { DataPublicPluginStart } from '../../../../../../../src/plugins/data/public'; -// List of all operation definitions registered to this data source. -// If you want to implement a new operation, add the definition to this array and -// the column type to the `IndexPatternColumn` union type below. -const internalOperationDefinitions = [ - filtersOperation, - termsOperation, - dateHistogramOperation, - minOperation, - maxOperation, - averageOperation, - cardinalityOperation, - sumOperation, - countOperation, -]; - /** * A union type of all available column types. If a column is of an unknown type somewhere * withing the indexpattern data source it should be typed as `IndexPatternColumn` to make @@ -58,6 +43,23 @@ export type IndexPatternColumn = | SumIndexPatternColumn | CountIndexPatternColumn; +export type FieldBasedIndexPatternColumn = Extract; + +// List of all operation definitions registered to this data source. +// If you want to implement a new operation, add the definition to this array and +// the column type to the `IndexPatternColumn` union type below. +const internalOperationDefinitions = [ + filtersOperation, + termsOperation, + dateHistogramOperation, + minOperation, + maxOperation, + averageOperation, + cardinalityOperation, + sumOperation, + countOperation, +]; + export { termsOperation } from './terms'; export { filtersOperation } from './filters'; export { dateHistogramOperation } from './date_histogram'; @@ -67,8 +69,8 @@ export { countOperation } from './count'; /** * Properties passed to the operation-specific part of the popover editor */ -export interface ParamEditorProps { - currentColumn: C; +export interface ParamEditorProps { + currentColumn: IndexPatternColumn; state: IndexPatternPrivateState; setState: StateSetter; columnId: string; @@ -101,30 +103,34 @@ interface BaseOperationDefinitionProps { * return an updated column. If not implemented, the `id` function is used instead. */ onOtherColumnChanged?: ( - currentColumn: C, + currentColumn: IndexPatternColumn, columns: Partial> ) => C; /** * React component for operation specific settings shown in the popover editor */ - paramEditor?: React.ComponentType>; + paramEditor?: React.ComponentType; /** * Function turning a column into an agg config passed to the `esaggs` function * together with the agg configs returned from other columns. */ - toEsAggsConfig: (column: C, columnId: string, indexPattern: IndexPattern) => unknown; + toEsAggsConfig: ( + column: IndexPatternColumn, + columnId: string, + indexPattern: IndexPattern + ) => unknown; /** * Returns true if the `column` can also be used on `newIndexPattern`. * If this function returns false, the column is removed when switching index pattern * for a layer */ - isTransferable: (column: C, newIndexPattern: IndexPattern) => boolean; + isTransferable: (column: IndexPatternColumn, newIndexPattern: IndexPattern) => boolean; /** * Transfering a column to another index pattern. This can be used to * adjust operation specific settings such as reacting to aggregation restrictions * present on the new index pattern. */ - transfer?: (column: C, newIndexPattern: IndexPattern) => C; + transfer?: (column: IndexPatternColumn, newIndexPattern: IndexPattern) => C; } interface BaseBuildColumnArgs { @@ -134,76 +140,76 @@ interface BaseBuildColumnArgs { indexPattern: IndexPattern; } +interface FieldlessOperationDefinition { + input: 'none'; + /** + * Builds the column object for the given parameters. Should include default p + */ + buildColumn: ( + arg: BaseBuildColumnArgs & { + previousColumn?: IndexPatternColumn; + } + ) => C; + /** + * Returns the meta data of the operation if applied. Undefined + * if the field is not applicable. + */ + getPossibleOperation: () => OperationMetadata | undefined; +} + +interface FieldBasedOperationDefinition { + input: 'field'; + /** + * Returns the meta data of the operation if applied to the given field. Undefined + * if the field is not applicable to the operation. + */ + getPossibleOperationForField: (field: IndexPatternField) => OperationMetadata | undefined; + /** + * Builds the column object for the given parameters. Should include default p + */ + buildColumn: ( + arg: BaseBuildColumnArgs & { + field: IndexPatternField; + previousColumn?: IndexPatternColumn; + } + ) => C; + /** + * This method will be called if the user changes the field of an operation. + * You must implement it and return the new column after the field change. + * The most simple implementation will just change the field on the column, and keep + * the rest the same. Some implementations might want to change labels, or their parameters + * when changing the field. + * + * This will only be called for switching the field, not for initially selecting a field. + * + * See {@link OperationDefinition#transfer} for controlling column building when switching an + * index pattern not just a field. + * + * @param oldColumn The column before the user changed the field. + * @param indexPattern The index pattern that field is on. + * @param field The field that the user changed to. + */ + onFieldChange: ( + oldColumn: FieldBasedIndexPatternColumn, + indexPattern: IndexPattern, + field: IndexPatternField + ) => C; +} + +interface OperationDefinitionMap { + field: FieldBasedOperationDefinition; + none: FieldlessOperationDefinition; +} + /** * Shape of an operation definition. If the type parameter of the definition * indicates a field based column, `getPossibleOperationForField` has to be - * specified, otherwise `getPossibleOperationForDocument` has to be defined. + * specified, otherwise `getPossibleOperation` has to be defined. */ - -// interface SharedOperationDefinition -// extends BaseOperationDefinitionProps { -// /** -// * Builds the column object for the given parameters. Should include default p -// */ -// buildColumn: ( -// arg: BaseBuildColumnArgs & { -// field?: IndexPatternField; -// previousColumn?: IndexPatternColumn; -// } -// ) => C; -// } - -export type OperationDefinition = - | (BaseOperationDefinitionProps & { - input: 'none'; - /** - * Builds the column object for the given parameters. Should include default p - */ - buildColumn: ( - arg: BaseBuildColumnArgs & { - previousColumn?: IndexPatternColumn; - } - ) => C; - /** - * Returns the meta data of the operation if applied. Undefined - * if the field is not applicable. - */ - getPossibleOperation: () => OperationMetadata | undefined; - }) - | (BaseOperationDefinitionProps & { - input: 'field'; - /** - * Returns the meta data of the operation if applied to the given field. Undefined - * if the field is not applicable to the operation. - */ - getPossibleOperationForField: (field: IndexPatternField) => OperationMetadata | undefined; - /** - * Builds the column object for the given parameters. Should include default p - */ - buildColumn: ( - arg: BaseBuildColumnArgs & { - field: IndexPatternField; - previousColumn?: IndexPatternColumn; - } - ) => C; - /** - * This method will be called if the user changes the field of an operation. - * You must implement it and return the new column after the field change. - * The most simple implementation will just change the field on the column, and keep - * the rest the same. Some implementations might want to change labels, or their parameters - * when changing the field. - * - * This will only be called for switching the field, not for initially selecting a field. - * - * See {@link OperationDefinition#transfer} for controlling column building when switching an - * index pattern not just a field. - * - * @param oldColumn The column before the user changed the field. - * @param indexPattern The index pattern that field is on. - * @param field The field that the user changed to. - */ - onFieldChange: (oldColumn: C, indexPattern: IndexPattern, field: IndexPatternField) => C; - }); +export type OperationDefinition< + C extends BaseIndexPatternColumn, + Input extends keyof OperationDefinitionMap +> = BaseOperationDefinitionProps & OperationDefinitionMap[Input]; /** * A union type of all available operation types. The operation type is a unique id of an operation. @@ -215,7 +221,13 @@ export type OperationType = typeof internalOperationDefinitions[number]['type']; * This is an operation definition of an unspecified column out of all possible * column types. */ -export type GenericOperationDefinition = OperationDefinition; +export type GenericOperationDefinition = + | OperationDefinition + | OperationDefinition; + +export type OperationTypeFromDefinition< + GenericOperationDefinition +> = GenericOperationDefinition extends BaseOperationDefinitionProps ? P : never; /** * List of all available operation definitions @@ -235,5 +247,5 @@ export const operationDefinitions = internalOperationDefinitions as GenericOpera */ export const operationDefinitionMap = internalOperationDefinitions.reduce( (definitionMap, definition) => ({ ...definitionMap, [definition.type]: definition }), - {} -) as Record; + {} as Record +); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx index d454d70051cc5..b9bffd1fb9406 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -43,7 +43,8 @@ function buildMetricOperation>({ } }, isTransferable: (column, newIndexPattern) => { - const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); + const c = column as T; + const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); return Boolean( newField && @@ -76,11 +77,11 @@ function buildMetricOperation>({ type: column.operationType, schema: 'metric', params: { - field: column.sourceField, + field: (column as T).sourceField, missing: 0, }, }), - } as OperationDefinition; + } as OperationDefinition; } export type SumIndexPatternColumn = MetricColumn<'sum'>; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx index d32efa15fd0a3..e8ebf1143f9f1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx @@ -48,7 +48,7 @@ export interface TermsIndexPatternColumn extends FieldBasedIndexPatternColumn { }; } -export const termsOperation: OperationDefinition = { +export const termsOperation: OperationDefinition = { type: 'terms', displayName: i18n.translate('xpack.lens.indexPattern.terms', { defaultMessage: 'Top values', @@ -65,7 +65,8 @@ export const termsOperation: OperationDefinition = { } }, isTransferable: (column, newIndexPattern) => { - const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); + const c = column as TermsIndexPatternColumn; + const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); return Boolean( newField && @@ -96,48 +97,52 @@ export const termsOperation: OperationDefinition = { }, }; }, - toEsAggsConfig: (column, columnId, _indexPattern) => ({ - id: columnId, - enabled: true, - type: 'terms', - schema: 'segment', - params: { - field: column.sourceField, - orderBy: - column.params.orderBy.type === 'alphabetical' ? '_key' : column.params.orderBy.columnId, - order: column.params.orderDirection, - size: column.params.size, - otherBucket: false, - otherBucketLabel: 'Other', - missingBucket: false, - missingBucketLabel: 'Missing', - }, - }), + toEsAggsConfig: (column, columnId, _indexPattern) => { + const c = column as TermsIndexPatternColumn; + return { + id: columnId, + enabled: true, + type: 'terms', + schema: 'segment', + params: { + field: c.sourceField, + orderBy: c.params.orderBy.type === 'alphabetical' ? '_key' : c.params.orderBy.columnId, + order: c.params.orderDirection, + size: c.params.size, + otherBucket: false, + otherBucketLabel: 'Other', + missingBucket: false, + missingBucketLabel: 'Missing', + }, + }; + }, onFieldChange: (oldColumn, indexPattern, field) => { return { - ...oldColumn, + ...(oldColumn as TermsIndexPatternColumn), label: ofName(field.displayName), sourceField: field.name, }; }, onOtherColumnChanged: (currentColumn, columns) => { - if (currentColumn.params.orderBy.type === 'column') { + const column = currentColumn as TermsIndexPatternColumn; + if (column.params.orderBy.type === 'column') { // check whether the column is still there and still a metric - const columnSortedBy = columns[currentColumn.params.orderBy.columnId]; + const columnSortedBy = columns[column.params.orderBy.columnId]; if (!columnSortedBy || !isSortableByColumn(columnSortedBy)) { return { - ...currentColumn, + ...column, params: { - ...currentColumn.params, + ...column.params, orderBy: { type: 'alphabetical' }, orderDirection: 'asc', }, }; } } - return currentColumn; + return column; }, paramEditor: ({ state, setState, currentColumn, layerId }) => { + const col = currentColumn as TermsIndexPatternColumn; const SEPARATOR = '$$$'; function toValue(orderBy: TermsIndexPatternColumn['params']['orderBy']) { if (orderBy.type === 'alphabetical') { @@ -184,7 +189,7 @@ export const termsOperation: OperationDefinition = { min={1} max={20} step={1} - value={currentColumn.params.size} + value={col.params.size} showInput showLabels compressed @@ -217,7 +222,7 @@ export const termsOperation: OperationDefinition = { compressed data-test-subj="indexPattern-terms-orderBy" options={orderOptions} - value={toValue(currentColumn.params.orderBy)} + value={toValue(col.params.orderBy)} onChange={(e: React.ChangeEvent) => setState( updateColumnParam({ @@ -258,7 +263,7 @@ export const termsOperation: OperationDefinition = { }), }, ]} - value={currentColumn.params.orderDirection} + value={col.params.orderDirection} onChange={(e: React.ChangeEvent) => setState( updateColumnParam({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts index 1e2bc5dcb6b62..31a36c59274da 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts @@ -5,4 +5,4 @@ */ export * from './operations'; -export { OperationType, IndexPatternColumn } from './definitions'; +export { OperationType, IndexPatternColumn, FieldBasedIndexPatternColumn } from './definitions'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts index 570fb17a53706..46dd73ba849a2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts @@ -63,7 +63,7 @@ export function getOperationTypesForField(field: IndexPatternField): OperationTy return operationDefinitions .filter( (operationDefinition) => - 'getPossibleOperationForField' in operationDefinition && + operationDefinition.input === 'field' && operationDefinition.getPossibleOperationForField(field) ) .sort(getSortScoreByPriority) @@ -184,13 +184,13 @@ export function changeField( ) { const operationDefinition = operationDefinitionMap[column.operationType]; - if (!('onFieldChange' in operationDefinition)) { + if (operationDefinition.input === 'field' && 'sourceField' in column) { + return operationDefinition.onFieldChange(column, indexPattern, newField); + } else { throw new Error( "Invariant error: Cannot change field if operation isn't a field based operaiton" ); } - - return operationDefinition.onFieldChange(column, indexPattern, newField); } /** diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts index 51691ae18a99a..c977a7e0fa370 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts @@ -5,8 +5,7 @@ */ import _ from 'lodash'; -import { isColumnTransferable } from './operations'; -import { operationDefinitionMap, IndexPatternColumn } from './operations'; +import { isColumnTransferable, operationDefinitionMap, IndexPatternColumn } from './operations'; import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from './types'; export function updateColumnParam({ From a1a05eb78d155843782e2a9661dbb64cc60b83dc Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Tue, 22 Sep 2020 17:20:47 -0400 Subject: [PATCH 3/7] Fix invalid state and add tests --- .../dimension_panel/dimension_editor.tsx | 73 ++- .../dimension_panel/dimension_panel.test.tsx | 446 ++++++++---------- .../dimension_panel/dimension_panel.tsx | 19 +- .../dimension_panel/field_select.tsx | 8 +- .../test/functional/apps/lens/smokescreen.ts | 2 + 5 files changed, 234 insertions(+), 314 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 998ed810b0434..ff56e8d6eaf87 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -17,7 +17,7 @@ import { } from '@elastic/eui'; import { EuiFormLabel } from '@elastic/eui'; import { IndexPatternColumn, OperationType } from '../indexpattern'; -import { IndexPatternDimensionEditorProps, OperationFieldSupportMatrix } from './dimension_panel'; +import { IndexPatternDimensionEditorProps, OperationSupportMatrix } from './dimension_panel'; import { operationDefinitionMap, getOperationDisplay, @@ -36,7 +36,7 @@ const operationPanels = getOperationDisplay(); export interface DimensionEditorProps extends IndexPatternDimensionEditorProps { selectedColumn?: IndexPatternColumn; - operationFieldSupportMatrix: OperationFieldSupportMatrix; + operationSupportMatrix: OperationSupportMatrix; currentIndexPattern: IndexPattern; } @@ -90,7 +90,7 @@ const LabelInput = ({ value, onChange }: { value: string; onChange: (value: stri export function DimensionEditor(props: DimensionEditorProps) { const { selectedColumn, - operationFieldSupportMatrix, + operationSupportMatrix, state, columnId, setState, @@ -98,7 +98,7 @@ export function DimensionEditor(props: DimensionEditorProps) { currentIndexPattern, hideGrouping, } = props; - const { operationByField, fieldByOperation } = operationFieldSupportMatrix; + const { operationByField, fieldByOperation } = operationSupportMatrix; const [ incompatibleSelectedOperationType, setInvalidOperationType, @@ -132,7 +132,7 @@ export function DimensionEditor(props: DimensionEditorProps) { ...asOperationOptions(validOperationTypes, true), ...asOperationOptions(possibleOperationTypes, false), ...asOperationOptions( - operationFieldSupportMatrix.operationWithoutField, + operationSupportMatrix.operationWithoutField, !selectedColumn || !hasField(selectedColumn) ), ], @@ -172,37 +172,30 @@ export function DimensionEditor(props: DimensionEditorProps) { compatibleWithCurrentField ? '' : ' incompatible' }`, onClick() { - if (selectedColumn?.operationType === operationType) { - return; - } - - // todo: when moving from terms agg to filters, we want to create a filter `$field.name : *` - // it probably has to be re-thought when removing the field name. - const isTermsToFilters = - selectedColumn?.operationType === 'terms' && operationType === 'filters'; - - if (!selectedColumn || !compatibleWithCurrentField) { - if (operationDefinitionMap[operationType].input === 'none') { - setInvalidOperationType(null); - setState( - changeColumn({ - state, - layerId, - columnId, - newColumn: buildColumn({ - columns: props.state.layers[props.layerId].columns, - suggestedPriority: props.suggestedPriority, - layerId: props.layerId, - op: operationType, - indexPattern: currentIndexPattern, - previousColumn: selectedColumn, - }), - }) - ); - trackUiEvent(`indexpattern_dimension_operation_${operationType}`); + if (operationDefinitionMap[operationType].input === 'none') { + // Clear invalid state because we are creating a valid column + setInvalidOperationType(null); + if (selectedColumn?.operationType === operationType) { return; } - + setState( + changeColumn({ + state, + layerId, + columnId, + newColumn: buildColumn({ + columns: props.state.layers[props.layerId].columns, + suggestedPriority: props.suggestedPriority, + layerId: props.layerId, + op: operationType, + indexPattern: currentIndexPattern, + previousColumn: selectedColumn, + }), + }) + ); + trackUiEvent(`indexpattern_dimension_operation_${operationType}`); + return; + } else if (!selectedColumn || !compatibleWithCurrentField) { const possibleFields = fieldByOperation[operationType] || []; if (possibleFields.length === 1) { @@ -228,8 +221,11 @@ export function DimensionEditor(props: DimensionEditorProps) { trackUiEvent(`indexpattern_dimension_operation_${operationType}`); return; } - if (incompatibleSelectedOperationType && !isTermsToFilters) { - setInvalidOperationType(null); + + setInvalidOperationType(null); + + if (selectedColumn?.operationType === operationType) { + return; } const newColumn: IndexPatternColumn = buildColumn({ @@ -296,7 +292,7 @@ export function DimensionEditor(props: DimensionEditorProps) { currentIndexPattern={currentIndexPattern} existingFields={state.existingFields} fieldMap={fieldMap} - operationFieldSupportMatrix={operationFieldSupportMatrix} + operationSupportMatrix={operationSupportMatrix} selectedColumnOperationType={selectedColumn && selectedColumn.operationType} selectedColumnSourceField={ selectedColumn && hasField(selectedColumn) ? selectedColumn.sourceField : undefined @@ -325,8 +321,7 @@ export function DimensionEditor(props: DimensionEditorProps) { } else { // Otherwise we'll use the buildColumn method to calculate a new column const compatibleOperations = - ('field' in choice && - operationFieldSupportMatrix.operationByField[choice.field]) || + ('field' in choice && operationSupportMatrix.operationByField[choice.field]) || []; let operation; if (compatibleOperations.length > 0) { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx index dd7611ca7f41f..2d7539a9e3ee8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx @@ -22,6 +22,7 @@ import { mountWithIntl as mount, shallowWithIntl as shallow } from 'test_utils/e import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup, CoreSetup } from 'kibana/public'; import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { IndexPatternPrivateState } from '../types'; +import { IndexPatternColumn } from '../operations'; import { documentField } from '../document_field'; import { OperationMetadata } from '../../types'; @@ -81,12 +82,39 @@ const expectedIndexPatterns = { }, }; +const bytesColumn: IndexPatternColumn = { + label: 'Max of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'max', + sourceField: 'bytes', + params: { format: { id: 'bytes' } }, +}; + +/** + * The datasource exposes four main pieces of code which are tested at + * an integration test level. The main reason for this fairly high level + * of testing is that there is a lot of UI logic that isn't easily + * unit tested, such as the transient invalid state. + * + * - Dimension trigger: Not tested here + * - Dimension editor component: First half of the tests + * + * - canHandleDrop: Tests for dropping of fields or other dimensions + * - onDrop: Correct application of drop logic + */ describe('IndexPatternDimensionEditorPanel', () => { let state: IndexPatternPrivateState; let setState: jest.Mock; let defaultProps: IndexPatternDimensionEditorProps; let dragDropContext: DragContextState; + function getStateWithColumns(columns: Record) { + return { ...state, layers: { first: { ...state.layers.first, columns } } }; + } + beforeEach(() => { state = { indexPatternRefs: [], @@ -179,7 +207,7 @@ describe('IndexPatternDimensionEditorPanel', () => { expect(filterOperations).toBeCalled(); }); - it('should show field select combo box on click', () => { + it('should show field select', () => { wrapper = mount(); expect( @@ -187,6 +215,29 @@ describe('IndexPatternDimensionEditorPanel', () => { ).toHaveLength(1); }); + it('should not show field select on fieldless operation', () => { + wrapper = mount( + + ); + + expect( + wrapper.find(EuiComboBox).filter('[data-test-subj="indexPattern-dimension-field"]') + ).toHaveLength(0); + }); + it('should not show any choices if the filter returns false', () => { wrapper = mount( { wrapper = mount( ); @@ -292,26 +324,7 @@ describe('IndexPatternDimensionEditorPanel', () => { wrapper = mount( ); @@ -324,30 +337,15 @@ describe('IndexPatternDimensionEditorPanel', () => { expect(items.find(({ label }) => label === 'Date histogram')!['data-test-subj']).toContain( 'incompatible' ); + + // Fieldless operation is compatible with field + expect(items.find(({ label }) => label === 'Filters')!['data-test-subj']).toContain( + 'compatible' + ); }); it('should keep the operation when switching to another field compatible with this operation', () => { - const initialState: IndexPatternPrivateState = { - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - label: 'Max of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'max', - sourceField: 'bytes', - params: { format: { id: 'bytes' } }, - }, - }, - }, - }, - }; + const initialState: IndexPatternPrivateState = getStateWithColumns({ col1: bytesColumn }); wrapper = mount( @@ -415,27 +413,7 @@ describe('IndexPatternDimensionEditorPanel', () => { wrapper = mount( ); @@ -505,27 +483,7 @@ describe('IndexPatternDimensionEditorPanel', () => { wrapper = mount( ); @@ -553,28 +511,13 @@ describe('IndexPatternDimensionEditorPanel', () => { wrapper = mount( ); @@ -640,6 +583,62 @@ describe('IndexPatternDimensionEditorPanel', () => { expect(wrapper.find('[data-test-subj="indexPattern-invalid-operation"]')).toHaveLength(0); }); + it('should leave error state if the original operation is re-selected', () => { + wrapper = mount(); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-terms incompatible"]') + .simulate('click'); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-date_histogram"]') + .simulate('click'); + + expect(wrapper.find('[data-test-subj="indexPattern-invalid-operation"]')).toHaveLength(0); + }); + + it('should leave error state when switching from incomplete state to fieldless operation', () => { + wrapper = mount(); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-terms incompatible"]') + .simulate('click'); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-filters incompatible"]') + .simulate('click'); + + expect(wrapper.find('[data-test-subj="indexPattern-invalid-operation"]')).toHaveLength(0); + }); + + it('should leave error state when re-selecting the original fieldless function', () => { + wrapper = mount( + + ); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-terms incompatible"]') + .simulate('click'); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-filters"]') + .simulate('click'); + + expect(wrapper.find('[data-test-subj="indexPattern-invalid-operation"]')).toHaveLength(0); + }); + it('should indicate fields compatible with selected operation', () => { wrapper = mount(); @@ -701,28 +700,18 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should select the Records field when count is selected', () => { - const initialState: IndexPatternPrivateState = { - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col2: { - dataType: 'number', - isBucketed: false, - label: '', - operationType: 'avg', - sourceField: 'bytes', - }, - }, - }, - }, - }; wrapper = mount( ); @@ -737,28 +726,18 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should indicate document and field compatibility with selected document operation', () => { - const initialState: IndexPatternPrivateState = { - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col2: { - dataType: 'number', - isBucketed: false, - label: '', - operationType: 'count', - sourceField: 'Records', - }, - }, - }, - }, - }; wrapper = mount( ); @@ -942,28 +921,18 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should indicate document compatibility when document operation is selected', () => { - const initialState: IndexPatternPrivateState = { - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col2: { - dataType: 'number', - isBucketed: false, - label: '', - operationType: 'count', - sourceField: 'Records', - }, - }, - }, - }, - }; wrapper = mount( ); @@ -1031,26 +1000,9 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should use helper function when changing the function', () => { - const initialState: IndexPatternPrivateState = { - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - label: 'Max of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'max', - sourceField: 'bytes', - }, - }, - }, - }, - }; + const initialState: IndexPatternPrivateState = getStateWithColumns({ + col1: bytesColumn, + }); wrapper = mount( ); @@ -1095,25 +1047,16 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('allows custom format', () => { - const stateWithNumberCol: IndexPatternPrivateState = { - ...state, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Average of memory', - dataType: 'number', - isBucketed: false, - // Private - operationType: 'avg', - sourceField: 'memory', - }, - }, - }, + const stateWithNumberCol: IndexPatternPrivateState = getStateWithColumns({ + col1: { + label: 'Average of memory', + dataType: 'number', + isBucketed: false, + // Private + operationType: 'avg', + sourceField: 'memory', }, - }; + }); wrapper = mount( @@ -1145,29 +1088,19 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('keeps decimal places while switching', () => { - const stateWithNumberCol: IndexPatternPrivateState = { - ...state, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Average of memory', - dataType: 'number', - isBucketed: false, - // Private - operationType: 'avg', - sourceField: 'memory', - params: { - format: { id: 'bytes', params: { decimals: 0 } }, - }, - }, - }, + const stateWithNumberCol: IndexPatternPrivateState = getStateWithColumns({ + col1: { + label: 'Average of memory', + dataType: 'number', + isBucketed: false, + // Private + operationType: 'avg', + sourceField: 'memory', + params: { + format: { id: 'bytes', params: { decimals: 0 } }, }, }, - }; - + }); wrapper = mount( ); @@ -1195,28 +1128,19 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('allows custom format with number of decimal places', () => { - const stateWithNumberCol: IndexPatternPrivateState = { - ...state, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Average of memory', - dataType: 'number', - isBucketed: false, - // Private - operationType: 'avg', - sourceField: 'memory', - params: { - format: { id: 'bytes', params: { decimals: 2 } }, - }, - }, - }, + const stateWithNumberCol: IndexPatternPrivateState = getStateWithColumns({ + col1: { + label: 'Average of memory', + dataType: 'number', + isBucketed: false, + // Private + operationType: 'avg', + sourceField: 'memory', + params: { + format: { id: 'bytes', params: { decimals: 2 } }, }, }, - }; + }); wrapper = mount( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx index 9689e76b4adf4..c4d8300722f83 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx @@ -46,7 +46,7 @@ export type IndexPatternDimensionEditorProps = DatasourceDimensionEditorProps< dateRange: DateRange; }; -export interface OperationFieldSupportMatrix { +export interface OperationSupportMatrix { operationByField: Partial>; operationWithoutField: OperationType[]; fieldByOperation: Partial>; @@ -59,7 +59,7 @@ type Props = Pick< // TODO: This code has historically been memoized, as a potentially performance // sensitive task. If we can add memoization without breaking the behavior, we should. -const getOperationFieldSupportMatrix = (props: Props): OperationFieldSupportMatrix => { +const getOperationSupportMatrix = (props: Props): OperationSupportMatrix => { const layerId = props.layerId; const currentIndexPattern = props.state.indexPatterns[props.state.layers[layerId].indexPatternId]; @@ -98,13 +98,13 @@ const getOperationFieldSupportMatrix = (props: Props): OperationFieldSupportMatr }; export function canHandleDrop(props: DatasourceDimensionDropProps) { - const operationFieldSupportMatrix = getOperationFieldSupportMatrix(props); + const operationSupportMatrix = getOperationSupportMatrix(props); const { dragging } = props.dragDropContext; const layerIndexPatternId = props.state.layers[props.layerId].indexPatternId; function hasOperationForField(field: IndexPatternField) { - return Boolean(operationFieldSupportMatrix.operationByField[field.name]); + return Boolean(operationSupportMatrix.operationByField[field.name]); } if (isDraggedField(dragging)) { @@ -126,11 +126,11 @@ export function canHandleDrop(props: DatasourceDimensionDropProps) { - const operationFieldSupportMatrix = getOperationFieldSupportMatrix(props); + const operationSupportMatrix = getOperationSupportMatrix(props); const droppedItem = props.droppedItem; function hasOperationForField(field: IndexPatternField) { - return Boolean(operationFieldSupportMatrix.operationByField[field.name]); + return Boolean(operationSupportMatrix.operationByField[field.name]); } if (isDraggedOperation(droppedItem) && droppedItem.layerId === props.layerId) { @@ -174,8 +174,7 @@ export function onDrop(props: DatasourceDimensionDropHandlerProps ); }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx index 60f60d7cb80c1..4b549f537da30 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx @@ -20,7 +20,7 @@ import { EuiHighlight } from '@elastic/eui'; import { OperationType } from '../indexpattern'; import { LensFieldIcon } from '../lens_field_icon'; import { DataType } from '../../types'; -import { OperationFieldSupportMatrix } from './dimension_panel'; +import { OperationSupportMatrix } from './dimension_panel'; import { IndexPattern, IndexPatternField, IndexPatternPrivateState } from '../types'; import { trackUiEvent } from '../../lens_ui_telemetry'; import { fieldExists } from '../pure_helpers'; @@ -37,7 +37,7 @@ export interface FieldSelectProps extends EuiComboBoxProps<{}> { incompatibleSelectedOperationType: OperationType | null; selectedColumnOperationType?: OperationType; selectedColumnSourceField?: string; - operationFieldSupportMatrix: OperationFieldSupportMatrix; + operationSupportMatrix: OperationSupportMatrix; onChoose: (choice: FieldChoice) => void; onDeleteColumn: () => void; existingFields: IndexPatternPrivateState['existingFields']; @@ -49,13 +49,13 @@ export function FieldSelect({ incompatibleSelectedOperationType, selectedColumnOperationType, selectedColumnSourceField, - operationFieldSupportMatrix, + operationSupportMatrix, onChoose, onDeleteColumn, existingFields, ...rest }: FieldSelectProps) { - const { operationByField } = operationFieldSupportMatrix; + const { operationByField } = operationSupportMatrix; const memoizedFieldOptions = useMemo(() => { const fields = Object.keys(operationByField).sort(); diff --git a/x-pack/test/functional/apps/lens/smokescreen.ts b/x-pack/test/functional/apps/lens/smokescreen.ts index fc91d13c4b253..6ee54bce1c94e 100644 --- a/x-pack/test/functional/apps/lens/smokescreen.ts +++ b/x-pack/test/functional/apps/lens/smokescreen.ts @@ -72,6 +72,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await listingTable.searchForItemWithName('lnsXYvis'); await PageObjects.lens.clickVisualizeListItemTitle('lnsXYvis'); await PageObjects.lens.goToTimeRange(); + // Change the IP field to filters await PageObjects.lens.configureDimension({ dimension: 'lnsXY_splitDimensionPanel > lns-dimensionTrigger', operation: 'filters', @@ -79,6 +80,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); await PageObjects.lens.addFilterToAgg(`geo.src : CN`); + // Verify that the field was persisted from the transition expect(await PageObjects.lens.getFiltersAggLabels()).to.eql([`ip : *`, `geo.src : CN`]); expect(await find.allByCssSelector('.echLegendItem')).to.have.length(2); }); From 90f26d471f796528c9ab087f1de874bdca80dc7b Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Wed, 23 Sep 2020 12:23:01 -0400 Subject: [PATCH 4/7] Fix types --- .../operations/definitions/cardinality.tsx | 7 ++-- .../operations/definitions/count.tsx | 2 +- .../operations/definitions/date_histogram.tsx | 36 +++++++++---------- .../definitions/filters/filters.tsx | 4 +-- .../operations/definitions/index.ts | 36 +++++++++---------- .../operations/definitions/metrics.tsx | 5 ++- .../operations/definitions/ranges/ranges.tsx | 3 +- .../operations/definitions/terms.tsx | 33 ++++++++--------- 8 files changed, 58 insertions(+), 68 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx index 5a490cdd2c38e..65119d3978ee6 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx @@ -43,8 +43,7 @@ export const cardinalityOperation: OperationDefinition { - const c = column as CardinalityIndexPatternColumn; - const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); + const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); return Boolean( newField && @@ -76,13 +75,13 @@ export const cardinalityOperation: OperationDefinition { return { - ...(oldColumn as CardinalityIndexPatternColumn), + ...oldColumn, label: ofName(field.displayName), sourceField: field.name, }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index de5b50f5be908..cdf1a6b760493 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -27,7 +27,7 @@ export const countOperation: OperationDefinition { return { - ...(oldColumn as CountIndexPatternColumn), + ...oldColumn, label: field.displayName, sourceField: field.name, }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index 911c40e84edd9..185f44405bb4b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -81,8 +81,7 @@ export const dateHistogramOperation: OperationDefinition< }; }, isTransferable: (column, newIndexPattern) => { - const c = column as DateHistogramIndexPatternColumn; - const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); + const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); return Boolean( newField && @@ -92,8 +91,7 @@ export const dateHistogramOperation: OperationDefinition< ); }, transfer: (column, newIndexPattern) => { - const col = column as DateHistogramIndexPatternColumn; - const newField = newIndexPattern.fields.find((field) => field.name === col.sourceField); + const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); if ( newField && newField.aggregationRestrictions && @@ -102,9 +100,9 @@ export const dateHistogramOperation: OperationDefinition< const restrictions = newField.aggregationRestrictions.date_histogram; return { - ...col, + ...column, params: { - ...col.params, + ...column.params, timeZone: restrictions.time_zone, // TODO this rewrite logic is simplified - if the current interval is a multiple of // the restricted interval, we could carry it over directly. However as the current @@ -115,28 +113,27 @@ export const dateHistogramOperation: OperationDefinition< }; } - return col; + return column; }, onFieldChange: (oldColumn, indexPattern, field) => { return { - ...(oldColumn as DateHistogramIndexPatternColumn), + ...oldColumn, label: field.displayName, sourceField: field.name, }; }, toEsAggsConfig: (column, columnId, indexPattern) => { - const c = column as DateHistogramIndexPatternColumn; - const usedField = indexPattern.fields.find((field) => field.name === c.sourceField); + const usedField = indexPattern.fields.find((field) => field.name === column.sourceField); return { id: columnId, enabled: true, type: 'date_histogram', schema: 'segment', params: { - field: c.sourceField, - time_zone: c.params.timeZone, + field: column.sourceField, + time_zone: column.params.timeZone, useNormalizedEsInterval: !usedField || !usedField.aggregationRestrictions?.date_histogram, - interval: c.params.interval, + interval: column.params.interval, drop_partials: false, min_doc_count: 0, extended_bounds: {}, @@ -144,16 +141,15 @@ export const dateHistogramOperation: OperationDefinition< }; }, paramEditor: ({ state, setState, currentColumn, layerId, dateRange, data }) => { - const column = currentColumn as DateHistogramIndexPatternColumn; const field = - column && + currentColumn && state.indexPatterns[state.layers[layerId].indexPatternId].fields.find( - (currentField) => currentField.name === column.sourceField + (currentField) => currentField.name === currentColumn.sourceField ); const intervalIsRestricted = field!.aggregationRestrictions && field!.aggregationRestrictions.date_histogram; - const interval = parseInterval(column.params.interval); + const interval = parseInterval(currentColumn.params.interval); // We force the interval value to 1 if it's empty, since that is the ES behavior, // and the isValidInterval function doesn't handle the empty case properly. Fixing @@ -194,13 +190,13 @@ export const dateHistogramOperation: OperationDefinition< label={i18n.translate('xpack.lens.indexPattern.dateHistogram.autoInterval', { defaultMessage: 'Customize time interval', })} - checked={column.params.interval !== autoInterval} + checked={currentColumn.params.interval !== autoInterval} onChange={onChangeAutoInterval} compressed /> )} - {column.params.interval !== autoInterval && ( + {currentColumn.params.interval !== autoInterval && ( ) : ( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx index 8dfa548cd62a7..ad0b9f2dbb0ab 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx @@ -112,7 +112,7 @@ export const filtersOperation: OperationDefinition { - const validFilters = (column as FiltersIndexPatternColumn).params.filters?.filter((f: Filter) => + const validFilters = column.params.filters?.filter((f: Filter) => isQueryValid(f.input, indexPattern) ); return { @@ -128,7 +128,7 @@ export const filtersOperation: OperationDefinition { const indexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; - const filters = (currentColumn as FiltersIndexPatternColumn).params.filters; + const filters = currentColumn.params.filters; const setFilters = (newFilters: Filter[]) => setState( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 75d22bf6c40ea..57b439c5268a0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -73,8 +73,8 @@ export { countOperation } from './count'; /** * Properties passed to the operation-specific part of the popover editor */ -export interface ParamEditorProps { - currentColumn: IndexPatternColumn; +export interface ParamEditorProps { + currentColumn: C; state: IndexPatternPrivateState; setState: StateSetter; columnId: string; @@ -107,34 +107,30 @@ interface BaseOperationDefinitionProps { * return an updated column. If not implemented, the `id` function is used instead. */ onOtherColumnChanged?: ( - currentColumn: IndexPatternColumn, + currentColumn: C, columns: Partial> ) => C; /** * React component for operation specific settings shown in the popover editor */ - paramEditor?: React.ComponentType; + paramEditor?: React.ComponentType>; /** * Function turning a column into an agg config passed to the `esaggs` function * together with the agg configs returned from other columns. */ - toEsAggsConfig: ( - column: IndexPatternColumn, - columnId: string, - indexPattern: IndexPattern - ) => unknown; + toEsAggsConfig: (column: C, columnId: string, indexPattern: IndexPattern) => unknown; /** * Returns true if the `column` can also be used on `newIndexPattern`. * If this function returns false, the column is removed when switching index pattern * for a layer */ - isTransferable: (column: IndexPatternColumn, newIndexPattern: IndexPattern) => boolean; + isTransferable: (column: C, newIndexPattern: IndexPattern) => boolean; /** * Transfering a column to another index pattern. This can be used to * adjust operation specific settings such as reacting to aggregation restrictions * present on the new index pattern. */ - transfer?: (column: IndexPatternColumn, newIndexPattern: IndexPattern) => C; + transfer?: (column: C, newIndexPattern: IndexPattern) => C; } interface BaseBuildColumnArgs { @@ -174,7 +170,8 @@ interface FieldBasedOperationDefinition { buildColumn: ( arg: BaseBuildColumnArgs & { field: IndexPatternField; - previousColumn?: IndexPatternColumn; + // previousColumn?: IndexPatternColumn; + previousColumn?: C; } ) => C; /** @@ -194,7 +191,8 @@ interface FieldBasedOperationDefinition { * @param field The field that the user changed to. */ onFieldChange: ( - oldColumn: FieldBasedIndexPatternColumn, + // oldColumn: FieldBasedIndexPatternColumn, + oldColumn: C, indexPattern: IndexPattern, field: IndexPatternField ) => C; @@ -229,10 +227,6 @@ export type GenericOperationDefinition = | OperationDefinition | OperationDefinition; -export type OperationTypeFromDefinition< - GenericOperationDefinition -> = GenericOperationDefinition extends BaseOperationDefinitionProps ? P : never; - /** * List of all available operation definitions */ @@ -249,7 +243,11 @@ export const operationDefinitions = internalOperationDefinitions as GenericOpera * (e.g. `import { termsOperation } from './operations/definitions'`). This map is * intended to be used in situations where the operation type is not known during compile time. */ -export const operationDefinitionMap = internalOperationDefinitions.reduce( +export const operationDefinitionMap: Record< + string, + GenericOperationDefinition +> = internalOperationDefinitions.reduce( (definitionMap, definition) => ({ ...definitionMap, [definition.type]: definition }), - {} as Record + // {} as Record + {} ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx index b9bffd1fb9406..c02f7bcb7d2cd 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -43,8 +43,7 @@ function buildMetricOperation>({ } }, isTransferable: (column, newIndexPattern) => { - const c = column as T; - const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); + const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); return Boolean( newField && @@ -77,7 +76,7 @@ function buildMetricOperation>({ type: column.operationType, schema: 'metric', params: { - field: (column as T).sourceField, + field: column.sourceField, missing: 0, }, }), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx index 530c2e962759b..1971fb2875bed 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx @@ -76,12 +76,13 @@ function getEsAggsParams({ sourceField, params }: RangeIndexPatternColumn) { }; } -export const rangeOperation: OperationDefinition = { +export const rangeOperation: OperationDefinition = { type: 'range', displayName: i18n.translate('xpack.lens.indexPattern.ranges', { defaultMessage: 'Ranges', }), priority: 4, // Higher than terms, so numbers get histogram + input: 'field', getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( type === 'number' && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx index 4971019d1973f..c147029bbd3c7 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms.tsx @@ -65,8 +65,7 @@ export const termsOperation: OperationDefinition { - const c = column as TermsIndexPatternColumn; - const newField = newIndexPattern.fields.find((field) => field.name === c.sourceField); + const newField = newIndexPattern.fields.find((field) => field.name === column.sourceField); return Boolean( newField && @@ -98,17 +97,17 @@ export const termsOperation: OperationDefinition { - const c = column as TermsIndexPatternColumn; return { id: columnId, enabled: true, type: 'terms', schema: 'segment', params: { - field: c.sourceField, - orderBy: c.params.orderBy.type === 'alphabetical' ? '_key' : c.params.orderBy.columnId, - order: c.params.orderDirection, - size: c.params.size, + field: column.sourceField, + orderBy: + column.params.orderBy.type === 'alphabetical' ? '_key' : column.params.orderBy.columnId, + order: column.params.orderDirection, + size: column.params.size, otherBucket: false, otherBucketLabel: 'Other', missingBucket: false, @@ -118,31 +117,29 @@ export const termsOperation: OperationDefinition { return { - ...(oldColumn as TermsIndexPatternColumn), + ...oldColumn, label: ofName(field.displayName), sourceField: field.name, }; }, onOtherColumnChanged: (currentColumn, columns) => { - const column = currentColumn as TermsIndexPatternColumn; - if (column.params.orderBy.type === 'column') { + if (currentColumn.params.orderBy.type === 'column') { // check whether the column is still there and still a metric - const columnSortedBy = columns[column.params.orderBy.columnId]; + const columnSortedBy = columns[currentColumn.params.orderBy.columnId]; if (!columnSortedBy || !isSortableByColumn(columnSortedBy)) { return { - ...column, + ...currentColumn, params: { - ...column.params, + ...currentColumn.params, orderBy: { type: 'alphabetical' }, orderDirection: 'asc', }, }; } } - return column; + return currentColumn; }, paramEditor: ({ state, setState, currentColumn, layerId }) => { - const col = currentColumn as TermsIndexPatternColumn; const SEPARATOR = '$$$'; function toValue(orderBy: TermsIndexPatternColumn['params']['orderBy']) { if (orderBy.type === 'alphabetical') { @@ -189,7 +186,7 @@ export const termsOperation: OperationDefinition) => setState( updateColumnParam({ @@ -263,7 +260,7 @@ export const termsOperation: OperationDefinition) => setState( updateColumnParam({ From cb02b8c6ed385428b90a4cb88c864307defbaf7b Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Wed, 23 Sep 2020 12:51:52 -0400 Subject: [PATCH 5/7] Small cleanup --- .../operations/definitions/index.ts | 1 - .../operations/operations.test.ts | 32 +++++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 57b439c5268a0..38aec866ca5cb 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -248,6 +248,5 @@ export const operationDefinitionMap: Record< GenericOperationDefinition > = internalOperationDefinitions.reduce( (definitionMap, definition) => ({ ...definitionMap, [definition.type]: definition }), - // {} as Record {} ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts index 612dd74a11c45..c1bd4b84099b7 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts @@ -238,18 +238,14 @@ describe('getOperationTypesForField', () => { Array [ Object { "operationMetaData": Object { - "dataType": "string", + "dataType": "date", "isBucketed": true, "scale": "interval", }, "operations": Array [ Object { - "operationType": "filters", - "type": "none", - }, - Object { - "field": "source", - "operationType": "terms", + "field": "timestamp", + "operationType": "date_histogram", "type": "field", }, ], @@ -258,12 +254,12 @@ describe('getOperationTypesForField', () => { "operationMetaData": Object { "dataType": "number", "isBucketed": true, - "scale": "ordinal", + "scale": "interval", }, "operations": Array [ Object { "field": "bytes", - "operationType": "terms", + "operationType": "range", "type": "field", }, ], @@ -275,6 +271,10 @@ describe('getOperationTypesForField', () => { "scale": "ordinal", }, "operations": Array [ + Object { + "operationType": "filters", + "type": "none", + }, Object { "field": "source", "operationType": "terms", @@ -282,6 +282,20 @@ describe('getOperationTypesForField', () => { }, ], }, + Object { + "operationMetaData": Object { + "dataType": "number", + "isBucketed": true, + "scale": "ordinal", + }, + "operations": Array [ + Object { + "field": "bytes", + "operationType": "terms", + "type": "field", + }, + ], + }, Object { "operationMetaData": Object { "dataType": "number", From 454afc21676e828ded36ab6acc7e7f5365a7e798 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Thu, 24 Sep 2020 16:56:09 -0400 Subject: [PATCH 6/7] Add additional error message --- .../dimension_panel/dimension_editor.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index db7c89c48987d..2572f732aa1b3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -281,10 +281,14 @@ export function DimensionEditor(props: DimensionEditorProps) { fullWidth isInvalid={Boolean(incompatibleSelectedOperationType)} error={ - selectedColumn - ? i18n.translate('xpack.lens.indexPattern.invalidOperationLabel', { - defaultMessage: 'To use this function, select a different field.', - }) + selectedColumn && incompatibleSelectedOperationType + ? selectedOperationDefinition?.input === 'field' + ? i18n.translate('xpack.lens.indexPattern.invalidOperationLabel', { + defaultMessage: 'To use this function, select a different field.', + }) + : i18n.translate('xpack.lens.indexPattern.chooseFieldLabel', { + defaultMessage: 'To use this function, select a field.', + }) : undefined } > From 0d819059712bbace97b4bf3d26bc089bf153a68b Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Thu, 24 Sep 2020 17:17:08 -0400 Subject: [PATCH 7/7] Reset field selector to empty state when invalid --- .../dimension_panel/field_select.tsx | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx index dafe0dbe3d192..de472cb09cdfe 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx @@ -173,15 +173,13 @@ export function FieldSelect({ options={(memoizedFieldOptions as unknown) as EuiComboBoxOptionOption[]} isInvalid={Boolean(incompatibleSelectedOperationType)} selectedOptions={ - ((selectedColumnOperationType - ? selectedColumnSourceField - ? [ - { - label: fieldMap[selectedColumnSourceField].displayName, - value: { type: 'field', field: selectedColumnSourceField }, - }, - ] - : [memoizedFieldOptions[0]] + ((selectedColumnOperationType && selectedColumnSourceField + ? [ + { + label: fieldMap[selectedColumnSourceField].displayName, + value: { type: 'field', field: selectedColumnSourceField }, + }, + ] : []) as unknown) as EuiComboBoxOptionOption[] } singleSelection={{ asPlainText: true }}