[Lens] Fix multi terms fields validation (#126618)

* 🐛 Fix fields validation for multi terms

* 🐛 Fix unit tests + add some new ones

* 🌐 Removed unused translations

* Revert "🌐 Removed unused translations"

This reverts commit a94ee69.

* 🌐 Removed unused translations

* 🐛 Extends deeper validation to drag and drop as well + more tests

* 🐛 Fix last issue with refactored function

* ✨ Filtered fields with unsupported types when in multi mode

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

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) {

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,15 +32,87 @@ describe('helpers', () => {
           dataType: 'number',
           isBucketed: false,
           label: 'Foo',
-          operationType: 'average', // <= invalid
-          sourceField: 'timestamp',
+          operationType: 'average',
+          sourceField: 'timestamp', // <= invalid type for average
         },
         createMockedIndexPattern()
       );
       expect(messages).toHaveLength(1);
       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(
         {

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;

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts

@@ -336,6 +336,12 @@ interface BaseOperationDefinitionProps<C extends BaseIndexPatternColumn, P = {}>
    * 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 {

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 = '›';

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<OperationSupportMatrix, 'operationByField'>;
   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 (
       <>
         <FieldSelect
-          fieldIsInvalid={false}
+          fieldIsInvalid={Boolean(invalidFields?.[0])}
           currentIndexPattern={indexPattern}
           existingFields={existingFields}
           operationByField={operationSupportMatrix.operationByField}
@@ -129,21 +132,31 @@ export function FieldInputs({
         {localValues.map(({ id, value, isNew }, index) => {
           // 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<OperationSupportMatrix['operationByField']>((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 (
             <EuiDraggable
@@ -170,7 +183,7 @@ export function FieldInputs({
                   </EuiFlexItem>
                   <EuiFlexItem grow={true} style={{ minWidth: 0 }}>
                     <FieldSelect
-                      fieldIsInvalid={false}
+                      fieldIsInvalid={shouldShowError}
                       currentIndexPattern={indexPattern}
                       existingFields={existingFields}
                       operationByField={filteredOperationByField}
@@ -180,7 +193,7 @@ export function FieldInputs({
                       onChoose={(choice) => {
                         onFieldSelectChange(choice, index);
                       }}
-                      isInvalid={shouldShowScriptedFieldError}
+                      isInvalid={shouldShowError}
                       data-test-subj={`indexPattern-dimension-field-${index}`}
                     />
                   </EuiFlexItem>
@@ -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(', '),
+      },
+    });
+  }
+}

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();