restore error severity for wrong field type

elastic · Dec 8, 2022 · 331853a · 331853a
1 parent f96d7c9
commit 331853a
Show file tree

Hide file tree

Showing 11 changed files with 156 additions and 82 deletions.
diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/cardinality.tsx
@@ -16,11 +16,12 @@ import { FieldBasedIndexPatternColumn, ValueFormatConfig } from './column_types'
 
 import {
   getFormatFromPreviousColumn,
-  getInvalidFieldMessage,
   getSafeName,
   getFilter,
   combineErrorMessages,
   isColumnOfType,
+  getWrongFieldTypeMessage,
+  getMissingFieldMessage,
 } from './helpers';
 import { adjustTimeScaleLabelSuffix } from '../time_scale_utils';
 import { getDisallowedPreviousShiftMessage } from '../../time_shift_utils';
@@ -92,11 +93,15 @@ export const cardinalityOperation: OperationDefinition<
   },
   getErrorMessage: (layer, columnId, indexPattern) =>
     combineErrorMessages([
+      getWrongFieldTypeMessage(
+        layer.columns[columnId] as FieldBasedIndexPatternColumn,
+        indexPattern
+      ),
       getDisallowedPreviousShiftMessage(layer, columnId),
       getColumnReducedTimeRangeError(layer, columnId, indexPattern),
     ]),
   getWarningMessages: (layer, columnId, indexPattern) =>
-    getInvalidFieldMessage(
+    getMissingFieldMessage(
       layer.columns[columnId] as FieldBasedIndexPatternColumn,
       indexPattern
     )?.map((msg) => <div>{msg}</div>),

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/count.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/count.tsx
@@ -16,11 +16,12 @@ import { OperationDefinition, ParamEditorProps } from '.';
 import { FieldBasedIndexPatternColumn, ValueFormatConfig } from './column_types';
 import type { IndexPatternField } from '../../../../types';
 import {
-  getInvalidFieldMessage,
   getFilter,
   combineErrorMessages,
   getFormatFromPreviousColumn,
   isColumnOfType,
+  getWrongFieldTypeMessage,
+  getMissingFieldMessage,
 } from './helpers';
 import { adjustTimeScaleLabelSuffix } from '../time_scale_utils';
 import { getDisallowedPreviousShiftMessage } from '../../time_shift_utils';
@@ -91,11 +92,15 @@ export const countOperation: OperationDefinition<CountIndexPatternColumn, 'field
   input: 'field',
   getErrorMessage: (layer, columnId, indexPattern) =>
     combineErrorMessages([
+      getWrongFieldTypeMessage(
+        layer.columns[columnId] as FieldBasedIndexPatternColumn,
+        indexPattern
+      ),
       getDisallowedPreviousShiftMessage(layer, columnId),
       getColumnReducedTimeRangeError(layer, columnId, indexPattern),
     ]),
   getWarningMessages: (layer, columnId, indexPattern) =>
-    getInvalidFieldMessage(
+    getMissingFieldMessage(
       layer.columns[columnId] as FieldBasedIndexPatternColumn,
       indexPattern
     )?.map((msg) => <div>{msg}</div>),

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/date_histogram.tsx
@@ -32,7 +32,7 @@ import { buildExpressionFunction } from '@kbn/expressions-plugin/public';
 import { updateColumnParam } from '../layer_helpers';
 import { OperationDefinition, ParamEditorProps } from '.';
 import { FieldBasedIndexPatternColumn } from './column_types';
-import { getInvalidFieldMessage, getSafeName } from './helpers';
+import { getMissingFieldMessage, getSafeName, getWrongFieldTypeMessage } from './helpers';
 import { FormBasedLayer } from '../../types';
 import { TooltipWrapper } from '../../../../shared_components';
 
@@ -85,9 +85,15 @@ export const dateHistogramOperation: OperationDefinition<
   priority: 5, // Highest priority level used
   operationParams: [{ name: 'interval', type: 'string', required: false }],
   getErrorMessage: (layer, columnId, indexPattern) =>
-    [getMultipleDateHistogramsErrorMessage(layer, columnId) || ''].filter(Boolean),
+    [
+      getWrongFieldTypeMessage(
+        layer.columns[columnId] as FieldBasedIndexPatternColumn,
+        indexPattern
+      ),
+      getMultipleDateHistogramsErrorMessage(layer, columnId) || '',
+    ].filter(Boolean),
   getWarningMessages: (layer, columnId, indexPattern) =>
-    getInvalidFieldMessage(
+    getMissingFieldMessage(
       layer.columns[columnId] as FieldBasedIndexPatternColumn,
       indexPattern
     )?.map((msg) => <div>{msg}</div>),

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/helpers.tsx
@@ -16,20 +16,20 @@ import {
 import type { FormBasedLayer } from '../../types';
 import { hasField } from '../../pure_utils';
 
-export function getInvalidFieldMessage(
+export function getMissingFieldMessage(
   column: FieldBasedIndexPatternColumn,
   indexPattern?: IndexPattern
 ): string[] | undefined {
-  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: 1,
-        missingFields: 'lolz',
-      },
-    }),
-  ];
+  // 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: 1,
+  //       missingFields: 'lolz',
+  //     },
+  //   }),
+  // ];
   if (!indexPattern) {
     return;
   }
@@ -40,59 +40,74 @@ export function getInvalidFieldMessage(
       ? operationDefinition?.getCurrentFields?.(column) ?? [column.sourceField]
       : [];
   const fields = fieldNames.map((fieldName) => indexPattern.getFieldByName(fieldName));
-  const filteredFields = fields.filter(Boolean) as IndexPatternField[];
-
-  const isInvalid = Boolean(
-    fields.length > filteredFields.length ||
-      !(
-        operationDefinition?.input === 'field' &&
-        filteredFields.every(
-          (field) => operationDefinition.getPossibleOperationForField(field) != null
-        )
-      )
-  );
+
+  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(', '),
+        },
+      }),
+    ];
+  }
+
+  return undefined;
+}
+
+export function getWrongFieldTypeMessage(
+  column: FieldBasedIndexPatternColumn,
+  indexPattern?: IndexPattern
+): string[] | undefined {
+  // const wrongTypeFields = ['wrong!'];
+  // return [
+  //   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: {
+  //       count: wrongTypeFields.length,
+  //       invalidFields: wrongTypeFields.join(', '),
+  //     },
+  //   }),
+  // ];
+  if (!indexPattern) {
+    return;
+  }
+  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))
+    .filter(Boolean) as IndexPatternField[];
 
   const isWrongType = Boolean(
-    filteredFields.length &&
+    fields.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.fieldsWrongType', {
-          defaultMessage:
-            '{count, plural, one {Field} other {Fields}} {invalidFields} {count, plural, one {is} other {are}} of the wrong type',
-          values: {
-            count: wrongTypeFields.length,
-            invalidFields: wrongTypeFields.join(', '),
-          },
-        }),
-      ];
-    }
+  if (isWrongType) {
+    const wrongTypeFields =
+      operationDefinition?.getNonTransferableFields?.(column, indexPattern) ?? fieldNames;
+    return [
+      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: {
+          count: wrongTypeFields.length,
+          invalidFields: wrongTypeFields.join(', '),
+        },
+      }),
+    ];
   }
 
   return undefined;

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.tsx
@@ -24,9 +24,10 @@ import type { IndexPatternField, IndexPattern } from '../../../../types';
 import { DataType } from '../../../../types';
 import {
   getFormatFromPreviousColumn,
-  getInvalidFieldMessage,
   getSafeName,
   getFilter,
+  getMissingFieldMessage,
+  getWrongFieldTypeMessage,
 } from './helpers';
 import { adjustTimeScaleLabelSuffix } from '../time_scale_utils';
 import { getDisallowedPreviousShiftMessage } from '../../time_shift_utils';
@@ -63,7 +64,7 @@ const supportedTypes = new Set([
   'date_range',
 ]);
 
-export function getInvalidSortFieldMessage(
+export function getMissingSortFieldMessage(
   sortField: string,
   indexPattern?: IndexPattern
 ): undefined | string {
@@ -77,7 +78,17 @@ export function getInvalidSortFieldMessage(
       values: { invalidField: sortField },
     });
   }
-  if (field.type !== 'date') {
+}
+
+export function getWrongSortFieldTypeMessage(
+  sortField: string,
+  indexPattern?: IndexPattern
+): undefined | string {
+  if (!indexPattern) {
+    return;
+  }
+  const field = indexPattern.getFieldByName(sortField);
+  if (field?.type !== 'date') {
     return i18n.translate('xpack.lens.indexPattern.lastValue.invalidTypeSortField', {
       defaultMessage: 'Field {invalidField} is not a date field and cannot be used for sorting',
       values: { invalidField: sortField },
@@ -194,21 +205,32 @@ export const lastValueOperation: OperationDefinition<
   },
   getErrorMessage(layer, columnId, indexPattern) {
     const errorMessages: string[] = [];
+    const column = layer.columns[columnId] as LastValueIndexPatternColumn;
+
+    const wrongFieldTypeMessage = getWrongFieldTypeMessage(column, indexPattern);
+    const wrongSortFieldTypeMessage = getWrongSortFieldTypeMessage(
+      column.params.sortField,
+      indexPattern
+    );
+
+    if (wrongFieldTypeMessage) errorMessages.push(...wrongFieldTypeMessage);
+    if (wrongSortFieldTypeMessage) errorMessages.push(wrongSortFieldTypeMessage);
+
     errorMessages.push(...(getDisallowedPreviousShiftMessage(layer, columnId) || []));
     errorMessages.push(...(getColumnReducedTimeRangeError(layer, columnId, indexPattern) || []));
     return errorMessages.length ? errorMessages : undefined;
   },
   getWarningMessages(layer, columnId, indexPattern) {
     const warningMessages = [];
     const column = layer.columns[columnId] as LastValueIndexPatternColumn;
-    const invalidFieldMessage = getInvalidFieldMessage(column, indexPattern);
-    const invalidSortFieldMessage = getInvalidSortFieldMessage(
+    const missingFieldMessage = getMissingFieldMessage(column, indexPattern);
+    const wrongSortFieldTypeMessage = getMissingSortFieldMessage(
       column.params.sortField,
       indexPattern
     );
 
-    if (invalidFieldMessage) warningMessages.push(...invalidFieldMessage);
-    if (invalidSortFieldMessage) warningMessages.push(invalidSortFieldMessage);
+    if (missingFieldMessage) warningMessages.push(...missingFieldMessage);
+    if (wrongSortFieldTypeMessage) warningMessages.push(wrongSortFieldTypeMessage);
 
     return warningMessages.map((msg) => <div>{msg}</div>);
   },
@@ -310,7 +332,7 @@ export const lastValueOperation: OperationDefinition<
       });
 
     const dateFields = getDateFields(indexPattern);
-    const isSortFieldInvalid = !!getInvalidSortFieldMessage(
+    const isSortFieldInvalid = !!getWrongSortFieldTypeMessage(
       currentColumn.params.sortField,
       indexPattern
     );

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/metrics.tsx
@@ -13,11 +13,12 @@ import { buildExpressionFunction } from '@kbn/expressions-plugin/public';
 import { OperationDefinition, ParamEditorProps } from '.';
 import {
   getFormatFromPreviousColumn,
-  getInvalidFieldMessage,
   getSafeName,
   getFilter,
   combineErrorMessages,
   isColumnOfType,
+  getWrongFieldTypeMessage,
+  getMissingFieldMessage,
 } from './helpers';
 import {
   FieldBasedIndexPatternColumn,
@@ -211,11 +212,15 @@ function buildMetricOperation<T extends MetricColumn<string>>({
 
     getErrorMessage: (layer, columnId, indexPattern) =>
       combineErrorMessages([
+        getWrongFieldTypeMessage(
+          layer.columns[columnId] as FieldBasedIndexPatternColumn,
+          indexPattern
+        ),
         getDisallowedPreviousShiftMessage(layer, columnId),
         getColumnReducedTimeRangeError(layer, columnId, indexPattern),
       ]),
     getWarningMessages: (layer, columnId, indexPattern) =>
-      getInvalidFieldMessage(
+      getMissingFieldMessage(
         layer.columns[columnId] as FieldBasedIndexPatternColumn,
         indexPattern
       )?.map((msg) => <div>{msg}</div>),

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile.tsx
@@ -18,12 +18,13 @@ import {
 import { OperationDefinition } from '.';
 import {
   getFormatFromPreviousColumn,
-  getInvalidFieldMessage,
   getSafeName,
   isValidNumber,
   getFilter,
   isColumnOfType,
   combineErrorMessages,
+  getWrongFieldTypeMessage,
+  getMissingFieldMessage,
 } from './helpers';
 import { FieldBasedIndexPatternColumn } from './column_types';
 import { adjustTimeScaleLabelSuffix } from '../time_scale_utils';
@@ -289,11 +290,15 @@ export const percentileOperation: OperationDefinition<
   },
   getErrorMessage: (layer, columnId, indexPattern) =>
     combineErrorMessages([
+      getWrongFieldTypeMessage(
+        layer.columns[columnId] as FieldBasedIndexPatternColumn,
+        indexPattern
+      ),
       getDisallowedPreviousShiftMessage(layer, columnId),
       getColumnReducedTimeRangeError(layer, columnId, indexPattern),
     ]),
   getWarningMessages: (layer, columnId, indexPattern) =>
-    getInvalidFieldMessage(
+    getMissingFieldMessage(
       layer.columns[columnId] as FieldBasedIndexPatternColumn,
       indexPattern
     )?.map((msg) => <div>{msg}</div>),