From f5556f132732f070d10277053913fffaafebdedf Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Wed, 9 Mar 2022 11:34:47 +0100 Subject: [PATCH] [Lens] Fix multi terms fields validation (#126618) * :bug: Fix fields validation for multi terms * :bug: Fix unit tests + add some new ones * :globe_with_meridians: Removed unused translations * Revert ":globe_with_meridians: Removed unused translations" This reverts commit a94ee699c0b7b7e5db2f2b3b6a764a454f18510b. * :globe_with_meridians: Removed unused translations * :bug: Extends deeper validation to drag and drop as well + more tests * :bug: Fix last issue with refactored function * :sparkles: Filtered fields with unsupported types when in multi mode Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../droppable/get_drop_props.ts | 2 +- .../operations/definitions/helpers.test.ts | 83 +++++++++- .../operations/definitions/helpers.tsx | 62 +++++--- .../operations/definitions/index.ts | 6 + .../operations/definitions/terms/constants.ts | 14 ++ .../definitions/terms/field_inputs.tsx | 55 +++++-- .../definitions/terms/helpers.test.ts | 2 +- .../operations/definitions/terms/helpers.ts | 68 +++++++- .../operations/definitions/terms/index.tsx | 112 +++++-------- .../definitions/terms/terms.test.tsx | 150 +++++++++++++++++- .../public/indexpattern_datasource/utils.tsx | 3 +- .../translations/translations/ja-JP.json | 2 - .../translations/translations/zh-CN.json | 2 - 13 files changed, 434 insertions(+), 127 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/constants.ts diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable/get_drop_props.ts b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable/get_drop_props.ts index d85cbd438ffe7..f3c48bace4a5f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable/get_drop_props.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable/get_drop_props.ts @@ -134,7 +134,7 @@ function getDropPropsForField({ const isTheSameIndexPattern = state.layers[layerId].indexPatternId === dragging.indexPatternId; const newOperation = getNewOperation(dragging.field, filterOperations, targetColumn); - if (!!(isTheSameIndexPattern && newOperation)) { + if (isTheSameIndexPattern && newOperation) { const nextLabel = operationLabels[newOperation].displayName; if (!targetColumn) { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts index bf24e31ad4f59..616c1f5719cc6 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts @@ -7,6 +7,7 @@ import { createMockedIndexPattern } from '../../mocks'; import { getInvalidFieldMessage } from './helpers'; +import type { TermsIndexPatternColumn } from './terms'; describe('helpers', () => { describe('getInvalidFieldMessage', () => { @@ -16,13 +17,13 @@ describe('helpers', () => { dataType: 'number', isBucketed: false, label: 'Foo', - operationType: 'count', // <= invalid - sourceField: 'bytes', + operationType: 'count', + sourceField: 'NoBytes', // <= invalid }, createMockedIndexPattern() ); expect(messages).toHaveLength(1); - expect(messages![0]).toEqual('Field bytes was not found'); + expect(messages![0]).toEqual('Field NoBytes was not found'); }); it('returns an error if a field is the wrong type', () => { @@ -31,8 +32,8 @@ describe('helpers', () => { dataType: 'number', isBucketed: false, label: 'Foo', - operationType: 'average', // <= invalid - sourceField: 'timestamp', + operationType: 'average', + sourceField: 'timestamp', // <= invalid type for average }, createMockedIndexPattern() ); @@ -40,6 +41,78 @@ describe('helpers', () => { expect(messages![0]).toEqual('Field timestamp is of the wrong type'); }); + it('returns an error if one field amongst multiples does not exist', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'terms', + sourceField: 'geo.src', + params: { + secondaryFields: ['NoBytes'], // <= field does not exist + }, + } as TermsIndexPatternColumn, + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual('Field NoBytes was not found'); + }); + + it('returns an error if multiple fields do not exist', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'terms', + sourceField: 'NotExisting', + params: { + secondaryFields: ['NoBytes'], // <= field does not exist + }, + } as TermsIndexPatternColumn, + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual('Fields NotExisting, NoBytes were not found'); + }); + + it('returns an error if one field amongst multiples has the wrong type', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'terms', + sourceField: 'geo.src', + params: { + secondaryFields: ['timestamp'], // <= invalid type + }, + } as TermsIndexPatternColumn, + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual('Field timestamp is of the wrong type'); + }); + + it('returns an error if multiple fields are of the wrong type', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'terms', + sourceField: 'start_date', // <= invalid type + params: { + secondaryFields: ['timestamp'], // <= invalid type + }, + } as TermsIndexPatternColumn, + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual('Fields start_date, timestamp are of the wrong type'); + }); + it('returns no message if all fields are matching', () => { const messages = getInvalidFieldMessage( { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx index 73e0e61a68950..c464ce0da027c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx @@ -12,7 +12,8 @@ import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn, } from './column_types'; -import { IndexPattern } from '../../types'; +import { IndexPattern, IndexPatternField } from '../../types'; +import { hasField } from '../../pure_utils'; export function getInvalidFieldMessage( column: FieldBasedIndexPatternColumn, @@ -21,47 +22,66 @@ export function getInvalidFieldMessage( if (!indexPattern) { return; } - const { sourceField, operationType } = column; - const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; - const operationDefinition = operationType && operationDefinitionMap[operationType]; + const { operationType } = column; + const operationDefinition = operationType ? operationDefinitionMap[operationType] : undefined; + const fieldNames = + hasField(column) && operationDefinition + ? operationDefinition?.getCurrentFields?.(column) ?? [column.sourceField] + : []; + const fields = fieldNames.map((fieldName) => indexPattern.getFieldByName(fieldName)); + const filteredFields = fields.filter(Boolean) as IndexPatternField[]; const isInvalid = Boolean( - sourceField && - operationDefinition && + fields.length > filteredFields.length || !( - field && operationDefinition?.input === 'field' && - operationDefinition.getPossibleOperationForField(field) !== undefined + filteredFields.every( + (field) => operationDefinition.getPossibleOperationForField(field) != null + ) ) ); const isWrongType = Boolean( - sourceField && - operationDefinition && - field && - !operationDefinition.isTransferable( + filteredFields.length && + !operationDefinition?.isTransferable( column as GenericIndexPatternColumn, indexPattern, operationDefinitionMap ) ); + if (isInvalid) { + // Missing fields have priority over wrong type + // This has been moved as some transferable checks also perform exist checks internally and fail eventually + // but that would make type mismatch error appear in place of missing fields scenarios + const missingFields = fields.map((field, i) => (field ? null : fieldNames[i])).filter(Boolean); + if (missingFields.length) { + return [ + i18n.translate('xpack.lens.indexPattern.fieldsNotFound', { + defaultMessage: + '{count, plural, one {Field} other {Fields}} {missingFields} {count, plural, one {was} other {were}} not found', + values: { + count: missingFields.length, + missingFields: missingFields.join(', '), + }, + }), + ]; + } if (isWrongType) { + // as fallback show all the fields as invalid? + const wrongTypeFields = + operationDefinition?.getNonTransferableFields?.(column, indexPattern) ?? fieldNames; return [ - i18n.translate('xpack.lens.indexPattern.fieldWrongType', { - defaultMessage: 'Field {invalidField} is of the wrong type', + i18n.translate('xpack.lens.indexPattern.fieldsWrongType', { + defaultMessage: + '{count, plural, one {Field} other {Fields}} {invalidFields} {count, plural, one {is} other {are}} of the wrong type', values: { - invalidField: sourceField, + count: wrongTypeFields.length, + invalidFields: wrongTypeFields.join(', '), }, }), ]; } - return [ - i18n.translate('xpack.lens.indexPattern.fieldNotFound', { - defaultMessage: 'Field {invalidField} was not found', - values: { invalidField: sourceField }, - }), - ]; } return undefined; 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 a048f2b559191..dec70130d1282 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 @@ -336,6 +336,12 @@ interface BaseOperationDefinitionProps * Operation can influence some visual default settings. This function is used to collect default values offered */ getDefaultVisualSettings?: (column: C) => { truncateText?: boolean }; + + /** + * Utility function useful for multi fields operation in order to get fields + * are not pass the transferable checks + */ + getNonTransferableFields?: (column: C, indexPattern: IndexPattern) => string[]; } interface BaseBuildColumnArgs { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/constants.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/constants.ts new file mode 100644 index 0000000000000..64967aab08733 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/constants.ts @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export const DEFAULT_SIZE = 3; +// Elasticsearch limit +export const MAXIMUM_MAX_DOC_COUNT = 100; +export const DEFAULT_MAX_DOC_COUNT = 1; +export const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); + +export const MULTI_KEY_VISUAL_SEPARATOR = '›'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/field_inputs.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/field_inputs.tsx index e187b12f323c6..b21bfdc67fe23 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/field_inputs.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/field_inputs.tsx @@ -21,6 +21,7 @@ import { FieldSelect } from '../../../dimension_panel/field_select'; import type { TermsIndexPatternColumn } from './types'; import type { IndexPattern, IndexPatternPrivateState } from '../../../types'; import type { OperationSupportMatrix } from '../../../dimension_panel'; +import { supportedTypes } from './constants'; const generateId = htmlIdGenerator(); export const MAX_MULTI_FIELDS_SIZE = 3; @@ -29,6 +30,7 @@ export interface FieldInputsProps { column: TermsIndexPatternColumn; indexPattern: IndexPattern; existingFields: IndexPatternPrivateState['existingFields']; + invalidFields?: string[]; operationSupportMatrix: Pick; onChange: (newValues: string[]) => void; } @@ -51,6 +53,7 @@ export function FieldInputs({ indexPattern, existingFields, operationSupportMatrix, + invalidFields, }: FieldInputsProps) { const onChangeWrapped = useCallback( (values: WrappedValue[]) => @@ -90,7 +93,7 @@ export function FieldInputs({ return ( <> { // need to filter the available fields for multiple terms // * a scripted field should be removed - // * if a field has been used, should it be removed? Probably yes? - // * if a scripted field was used in a singular term, should it be marked as invalid for multi-terms? Probably yes? + // * a field of unsupported type should be removed + // * a field that has been used + // * a scripted field was used in a singular term, should be marked as invalid for multi-terms const filteredOperationByField = Object.keys(operationSupportMatrix.operationByField) - .filter( - (key) => - (!rawValuesLookup.has(key) && !indexPattern.getFieldByName(key)?.scripted) || - key === value - ) + .filter((key) => { + if (key === value) { + return true; + } + const field = indexPattern.getFieldByName(key); + return ( + !rawValuesLookup.has(key) && + field && + !field.scripted && + supportedTypes.has(field.type) + ); + }) .reduce((memo, key) => { memo[key] = operationSupportMatrix.operationByField[key]; return memo; }, {}); - const shouldShowScriptedFieldError = Boolean( - value && indexPattern.getFieldByName(value)?.scripted && localValuesFilled.length > 1 + const shouldShowError = Boolean( + value && + ((indexPattern.getFieldByName(value)?.scripted && localValuesFilled.length > 1) || + invalidFields?.includes(value)) ); return ( { onFieldSelectChange(choice, index); }} - isInvalid={shouldShowScriptedFieldError} + isInvalid={shouldShowError} data-test-subj={`indexPattern-dimension-field-${index}`} /> @@ -233,3 +246,21 @@ export function FieldInputs({ ); } + +export function getInputFieldErrorMessage(isScriptedField: boolean, invalidFields: string[]) { + if (isScriptedField) { + return i18n.translate('xpack.lens.indexPattern.terms.scriptedFieldErrorShort', { + defaultMessage: 'Scripted fields are not supported when using multiple fields', + }); + } + if (invalidFields.length) { + return i18n.translate('xpack.lens.indexPattern.terms.invalidFieldsErrorShort', { + defaultMessage: + 'Invalid {invalidFieldsCount, plural, one {field} other {fields}}: {invalidFields}. Check your data view or pick another field.', + values: { + invalidFieldsCount: invalidFields.length, + invalidFields: invalidFields.map((fieldName) => `"${fieldName}"`).join(', '), + }, + }); + } +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.test.ts index 628a5d6a7740a..cdc0f92121f06 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.test.ts @@ -15,9 +15,9 @@ import { getDisallowedTermsMessage, getMultiTermsScriptedFieldErrorMessage, isSortableByColumn, - MULTI_KEY_VISUAL_SEPARATOR, } from './helpers'; import { ReferenceBasedIndexPatternColumn } from '../column_types'; +import { MULTI_KEY_VISUAL_SEPARATOR } from './constants'; const indexPattern = createMockedIndexPattern(); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.ts index 2917abbf848f8..49e612f8bb0d2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/helpers.ts @@ -10,7 +10,7 @@ import { uniq } from 'lodash'; import type { CoreStart } from 'kibana/public'; import { buildEsQuery } from '@kbn/es-query'; import { getEsQueryConfig } from '../../../../../../../../src/plugins/data/public'; -import { operationDefinitionMap } from '../index'; +import { GenericIndexPatternColumn, operationDefinitionMap } from '../index'; import { defaultLabel } from '../filters'; import { isReferenced } from '../../layer_helpers'; @@ -18,9 +18,9 @@ import type { FieldStatsResponse } from '../../../../../common'; import type { FrameDatasourceAPI } from '../../../../types'; import type { FiltersIndexPatternColumn } from '../index'; import type { TermsIndexPatternColumn } from './types'; -import type { IndexPatternLayer, IndexPattern } from '../../../types'; - -export const MULTI_KEY_VISUAL_SEPARATOR = '›'; +import type { IndexPatternLayer, IndexPattern, IndexPatternField } from '../../../types'; +import { MULTI_KEY_VISUAL_SEPARATOR, supportedTypes } from './constants'; +import { isColumnOfType } from '../helpers'; const fullSeparatorString = ` ${MULTI_KEY_VISUAL_SEPARATOR} `; @@ -213,3 +213,63 @@ export function isSortableByColumn(layer: IndexPatternLayer, columnId: string) { !isReferenced(layer, columnId) ); } + +export function isScriptedField(field: IndexPatternField): boolean; +export function isScriptedField(fieldName: string, indexPattern: IndexPattern): boolean; +export function isScriptedField( + fieldName: string | IndexPatternField, + indexPattern?: IndexPattern +) { + if (typeof fieldName === 'string') { + const field = indexPattern?.getFieldByName(fieldName); + return field && field.scripted; + } + return fieldName.scripted; +} + +export function getFieldsByValidationState( + newIndexPattern: IndexPattern, + column?: GenericIndexPatternColumn, + field?: string | IndexPatternField +): { + allFields: Array; + validFields: string[]; + invalidFields: string[]; +} { + const newFieldNames: string[] = []; + if (column && 'sourceField' in column) { + if (column.sourceField) { + newFieldNames.push(column.sourceField); + } + if (isColumnOfType('terms', column)) { + newFieldNames.push(...(column.params?.secondaryFields ?? [])); + } + } + if (field) { + newFieldNames.push(typeof field === 'string' ? field : field.name || field.displayName); + } + const newFields = newFieldNames.map((fieldName) => newIndexPattern.getFieldByName(fieldName)); + // lodash groupby does not provide the index arg, so had to write it manually :( + const validFields: string[] = []; + const invalidFields: string[] = []; + // mind to check whether a column was passed, in such case single term with scripted field is ok + const canAcceptScripted = Boolean(column && newFields.length === 1); + newFieldNames.forEach((fieldName, i) => { + const newField = newFields[i]; + const isValid = + newField && + supportedTypes.has(newField.type) && + newField.aggregatable && + (!newField.aggregationRestrictions || newField.aggregationRestrictions.terms) && + (canAcceptScripted || !isScriptedField(newField)); + + const arrayToPush = isValid ? validFields : invalidFields; + arrayToPush.push(fieldName); + }); + + return { + allFields: newFields, + validFields, + invalidFields, + }; +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx index e30b3bbe8c0b5..68df9ff444fc5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx @@ -26,19 +26,26 @@ import type { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { ValuesInput } from './values_input'; -import { getInvalidFieldMessage, isColumnOfType } from '../helpers'; -import { FieldInputs, MAX_MULTI_FIELDS_SIZE } from './field_inputs'; +import { getInvalidFieldMessage } from '../helpers'; +import { FieldInputs, getInputFieldErrorMessage, MAX_MULTI_FIELDS_SIZE } from './field_inputs'; import { FieldInput as FieldInputBase, getErrorMessage, } from '../../../dimension_panel/field_input'; import type { TermsIndexPatternColumn } from './types'; -import type { IndexPattern, IndexPatternField } from '../../../types'; +import type { IndexPatternField } from '../../../types'; import { getDisallowedTermsMessage, getMultiTermsScriptedFieldErrorMessage, + getFieldsByValidationState, isSortableByColumn, } from './helpers'; +import { + DEFAULT_MAX_DOC_COUNT, + DEFAULT_SIZE, + MAXIMUM_MAX_DOC_COUNT, + supportedTypes, +} from './constants'; export function supportsRarityRanking(field?: IndexPatternField) { // these es field types can't be sorted by rarity @@ -79,27 +86,12 @@ function ofName(name?: string, count: number = 0, rare: boolean = false) { }); } -function isScriptedField(field: IndexPatternField): boolean; -function isScriptedField(fieldName: string, indexPattern: IndexPattern): boolean; -function isScriptedField(fieldName: string | IndexPatternField, indexPattern?: IndexPattern) { - if (typeof fieldName === 'string') { - const field = indexPattern?.getFieldByName(fieldName); - return field && field.scripted; - } - return fieldName.scripted; -} - // It is not always possible to know if there's a numeric field, so just ignore it for now function getParentFormatter(params: Partial) { return { id: params.secondaryFields?.length ? 'multi_terms' : 'terms' }; } const idPrefix = htmlIdGenerator()(); -const DEFAULT_SIZE = 3; -// Elasticsearch limit -const MAXIMUM_MAX_DOC_COUNT = 100; -export const DEFAULT_MAX_DOC_COUNT = 1; -const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); export const termsOperation: OperationDefinition = { type: 'terms', @@ -112,30 +104,18 @@ export const termsOperation: OperationDefinition { - const secondaryFields = new Set(); - if (targetColumn.params?.secondaryFields?.length) { - targetColumn.params.secondaryFields.forEach((fieldName) => { - if (!isScriptedField(fieldName, indexPattern)) { - secondaryFields.add(fieldName); - } - }); - } - if (sourceColumn && 'sourceField' in sourceColumn && sourceColumn?.sourceField) { - if (!isScriptedField(sourceColumn.sourceField, indexPattern)) { - secondaryFields.add(sourceColumn.sourceField); - } - } - if (sourceColumn && isColumnOfType('terms', sourceColumn)) { - if (sourceColumn?.params?.secondaryFields?.length) { - sourceColumn.params.secondaryFields.forEach((fieldName) => { - if (!isScriptedField(fieldName, indexPattern)) { - secondaryFields.add(fieldName); - } - }); - } - } - if (field && !isScriptedField(field)) { - secondaryFields.add(field.name); + const secondaryFields = new Set( + getFieldsByValidationState(indexPattern, targetColumn).validFields + ); + + const validFieldsToAdd = getFieldsByValidationState( + indexPattern, + sourceColumn, + field + ).validFields; + + for (const validField of validFieldsToAdd) { + secondaryFields.add(validField); } // remove the sourceField secondaryFields.delete(targetColumn.sourceField); @@ -155,27 +135,12 @@ export const termsOperation: OperationDefinition('terms', sourceColumn)) { - counter += - sourceColumn.params.secondaryFields?.filter((f) => { - return !isScriptedField(f, indexPattern) && !originalTerms.has(f); - }).length ?? 0; - } - } - } + const { validFields } = getFieldsByValidationState(indexPattern, sourceColumn, field); + const counter = validFields.filter((fieldName) => !originalTerms.has(fieldName)).length; // reject when there are no new fields to add if (!counter) { return false; @@ -209,14 +174,15 @@ export const termsOperation: OperationDefinition { + return getFieldsByValidationState(newIndexPattern, column).invalidFields; + }, isTransferable: (column, newIndexPattern) => { - const newField = newIndexPattern.getFieldByName(column.sourceField); + const { allFields, invalidFields } = getFieldsByValidationState(newIndexPattern, column); return Boolean( - newField && - supportedTypes.has(newField.type) && - newField.aggregatable && - (!newField.aggregationRestrictions || newField.aggregationRestrictions.terms) && + allFields.length && + invalidFields.length === 0 && (!column.params.otherBucket || !newIndexPattern.hasRestrictions) ); }, @@ -446,6 +412,7 @@ export const termsOperation: OperationDefinition ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index 7a42560751273..b2d202763afe1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -59,7 +59,8 @@ const defaultProps = { data: dataPluginMock.createStartContract(), http: {} as HttpSetup, indexPattern: createMockedIndexPattern(), - operationDefinitionMap: {}, + // need to provide the terms operation as some helpers use operation specific features + operationDefinitionMap: { terms: termsOperation as unknown as GenericOperationDefinition }, isFullscreen: false, toggleFullscreen: jest.fn(), setIsCloseable: jest.fn(), @@ -1016,7 +1017,7 @@ describe('terms', () => { ).toBeTruthy(); }); - it('should show an error message when field is invalid', () => { + it('should show an error message when first field is invalid', () => { const updateLayerSpy = jest.fn(); const existingFields = getExistingFields(); const operationSupportMatrix = getDefaultOperationSupportMatrix('col1', existingFields); @@ -1049,7 +1050,7 @@ describe('terms', () => { ).toBe('Invalid field. Check your data view or pick another field.'); }); - it('should show an error message when field is not supported', () => { + it('should show an error message when first field is not supported', () => { const updateLayerSpy = jest.fn(); const existingFields = getExistingFields(); const operationSupportMatrix = getDefaultOperationSupportMatrix('col1', existingFields); @@ -1083,6 +1084,74 @@ describe('terms', () => { ).toBe('This field does not work with the selected function.'); }); + it('should show an error message when any field but the first is invalid', () => { + const updateLayerSpy = jest.fn(); + const existingFields = getExistingFields(); + const operationSupportMatrix = getDefaultOperationSupportMatrix('col1', existingFields); + + layer.columns.col1 = { + label: 'Top value of geo.src + 1 other', + dataType: 'string', + isBucketed: true, + operationType: 'terms', + params: { + orderBy: { type: 'alphabetical' }, + size: 3, + orderDirection: 'asc', + secondaryFields: ['unsupported'], + }, + sourceField: 'geo.src', + } as TermsIndexPatternColumn; + const instance = mount( + + ); + expect( + instance.find('[data-test-subj="indexPattern-field-selection-row"]').first().prop('error') + ).toBe('Invalid field: "unsupported". Check your data view or pick another field.'); + }); + + it('should show an error message when any field but the first is not supported', () => { + const updateLayerSpy = jest.fn(); + const existingFields = getExistingFields(); + const operationSupportMatrix = getDefaultOperationSupportMatrix('col1', existingFields); + + layer.columns.col1 = { + label: 'Top value of geo.src + 1 other', + dataType: 'date', + isBucketed: true, + operationType: 'terms', + params: { + orderBy: { type: 'alphabetical' }, + size: 3, + orderDirection: 'asc', + secondaryFields: ['timestamp'], + }, + sourceField: 'geo.src', + } as TermsIndexPatternColumn; + const instance = mount( + + ); + expect( + instance.find('[data-test-subj="indexPattern-field-selection-row"]').first().prop('error') + ).toBe('Invalid field: "timestamp". Check your data view or pick another field.'); + }); + it('should render the an add button for single layer, but no other hints', () => { const updateLayerSpy = jest.fn(); const existingFields = getExistingFields(); @@ -1370,6 +1439,38 @@ describe('terms', () => { ); }); + it('should filter fields with unsupported types when in multi terms mode', () => { + const updateLayerSpy = jest.fn(); + const existingFields = getExistingFields(); + const operationSupportMatrix = getDefaultOperationSupportMatrix('col1', existingFields); + + (layer.columns.col1 as TermsIndexPatternColumn).params.secondaryFields = ['memory']; + const instance = mount( + + ); + + // get inner instance + expect( + instance.find('[data-test-subj="indexPattern-dimension-field-0"]').at(1).prop('options') + ).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + options: expect.arrayContaining([ + expect.not.objectContaining({ 'data-test-subj': 'lns-fieldOption-timestamp' }), + ]), + }), + ]) + ); + }); + it('should limit the number of multiple fields', () => { const updateLayerSpy = jest.fn(); const existingFields = getExistingFields(); @@ -2350,4 +2451,47 @@ describe('terms', () => { }); }); }); + + describe('getNonTransferableFields', () => { + it('should return empty array if all fields are transferable', () => { + expect( + termsOperation.getNonTransferableFields?.( + createMultiTermsColumn(['source']), + defaultProps.indexPattern + ) + ).toEqual([]); + expect( + termsOperation.getNonTransferableFields?.( + createMultiTermsColumn(['source', 'bytes']), + defaultProps.indexPattern + ) + ).toEqual([]); + expect( + termsOperation.getNonTransferableFields?.( + createMultiTermsColumn([]), + defaultProps.indexPattern + ) + ).toEqual([]); + expect( + termsOperation.getNonTransferableFields?.( + createMultiTermsColumn(['source', 'geo.src']), + defaultProps.indexPattern + ) + ).toEqual([]); + }); + it('should return only non transferable fields (invalid or not existence)', () => { + expect( + termsOperation.getNonTransferableFields?.( + createMultiTermsColumn(['source', 'timestamp']), + defaultProps.indexPattern + ) + ).toEqual(['timestamp']); + expect( + termsOperation.getNonTransferableFields?.( + createMultiTermsColumn(['source', 'unsupported']), + defaultProps.indexPattern + ) + ).toEqual(['unsupported']); + }); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx index 161d8b63bd12d..b5f82653565c8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx @@ -33,7 +33,8 @@ import { FiltersIndexPatternColumn, isQueryValid } from './operations/definition import { checkColumnForPrecisionError, Query } from '../../../../../src/plugins/data/common'; import { hasField } from './pure_utils'; import { mergeLayer } from './state_helpers'; -import { DEFAULT_MAX_DOC_COUNT, supportsRarityRanking } from './operations/definitions/terms'; +import { supportsRarityRanking } from './operations/definitions/terms'; +import { DEFAULT_MAX_DOC_COUNT } from './operations/definitions/terms/constants'; import { getOriginalId } from '../../common/expressions'; export function isColumnInvalid( diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 412b3706f278c..6ab97b08c93d1 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -468,7 +468,6 @@ "xpack.lens.indexPattern.fieldItem.visualizeGeoFieldLinkText": "Mapsで可視化", "xpack.lens.indexPattern.fieldItemTooltip": "可視化するには、ドラッグアンドドロップします。", "xpack.lens.indexPattern.fieldNoOperation": "フィールド{field}は演算なしで使用できません", - "xpack.lens.indexPattern.fieldNotFound": "フィールド {invalidField} が見つかりませんでした", "xpack.lens.indexPattern.fieldPanelEmptyStringValue": "空の文字列", "xpack.lens.indexPattern.fieldPlaceholder": "フィールド", "xpack.lens.indexPattern.fieldStatsButtonAriaLabel": "プレビュー{fieldName}：{fieldType}", @@ -480,7 +479,6 @@ "xpack.lens.indexPattern.fieldStatsNoData": "このフィールドは空です。500件のサンプリングされたドキュメントに存在しません。このフィールドを構成に追加すると、空白のグラフが作成される場合があります。", "xpack.lens.indexPattern.fieldTimeDistributionLabel": "時間分布", "xpack.lens.indexPattern.fieldTopValuesLabel": "トップの値", - "xpack.lens.indexPattern.fieldWrongType": "フィールド{invalidField}の型が正しくありません", "xpack.lens.indexPattern.filterBy.clickToEdit": "クリックして編集", "xpack.lens.indexPattern.filterBy.emptyFilterQuery": "（空）", "xpack.lens.indexPattern.filterBy.label": "フィルタリング条件", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 0b97869ad0a4a..93878dc3ba43b 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -473,7 +473,6 @@ "xpack.lens.indexPattern.fieldItem.visualizeGeoFieldLinkText": "在 Maps 中可视化", "xpack.lens.indexPattern.fieldItemTooltip": "拖放以可视化。", "xpack.lens.indexPattern.fieldNoOperation": "没有运算，无法使用字段 {field}", - "xpack.lens.indexPattern.fieldNotFound": "未找到字段 {invalidField}", "xpack.lens.indexPattern.fieldPanelEmptyStringValue": "空字符串", "xpack.lens.indexPattern.fieldPlaceholder": "字段", "xpack.lens.indexPattern.fieldStatsButtonAriaLabel": "预览 {fieldName}：{fieldType}", @@ -485,7 +484,6 @@ "xpack.lens.indexPattern.fieldStatsNoData": "此字段为空，因为它不存在于 500 个采样文档中。将此字段添加到配置可能会导致空图表。", "xpack.lens.indexPattern.fieldTimeDistributionLabel": "时间分布", "xpack.lens.indexPattern.fieldTopValuesLabel": "排名最前值", - "xpack.lens.indexPattern.fieldWrongType": "字段 {invalidField} 的类型不正确", "xpack.lens.indexPattern.filterBy.clickToEdit": "单击以编辑", "xpack.lens.indexPattern.filterBy.emptyFilterQuery": "（空）", "xpack.lens.indexPattern.filterBy.label": "筛选依据",