cleanups

x-pack/plugins/lens/public/datasources/form_based/form_based.tsx

@@ -761,14 +761,20 @@ export function getFormBasedDatasource({
         layerErrorMessages,
         (layerId, columnId) => {
           const layer = state.layers[layerId];
-          const invalidColumn = isColumnInvalid(
+          const column = layer.columns[columnId];
+          const indexPattern = framePublicAPI.dataViews.indexPatterns[layer.indexPatternId];
+          if (!column || !indexPattern) {
+            // this is a different issue that should be catched earlier
+            return false;
+          }
+          return isColumnInvalid(
             layer,
+            column,
             columnId,
-            framePublicAPI.dataViews.indexPatterns[layer.indexPatternId],
+            indexPattern,
             framePublicAPI.dateRange,
             uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET)
           );
-          return !invalidColumn;
         }
       );
 
@@ -1009,7 +1015,7 @@ function getLayerErrorMessages(
 function getInvalidDimensionErrorMessages(
   state: FormBasedPrivateState,
   currentErrorMessages: UserMessage[],
-  isValidColumn: (layerId: string, columnId: string) => boolean
+  isInvalidColumn: (layerId: string, columnId: string) => boolean
 ) {
   // generate messages for invalid columns
   const columnErrorMessages: UserMessage[] = Object.keys(state.layers)
@@ -1026,7 +1032,7 @@ function getInvalidDimensionErrorMessages(
           continue;
         }
 
-        if (!isValidColumn(layerId, columnId)) {
+        if (isInvalidColumn(layerId, columnId)) {
           messages.push({
             uniqueId: EDITOR_INVALID_DIMENSION,
             severity: 'error',

x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx

@@ -90,9 +90,11 @@ export const timeScaleOperation: OperationDefinition<TimeScaleIndexPatternColumn
       return true;
     },
     getErrorMessage: (layer: FormBasedLayer, columnId: string) => {
-      const errors: FieldBasedOperationErrorMessage[] = [
-        ...getErrorsForDateReference(layer, columnId, NORMALIZE_BY_UNIT_NAME),
-      ];
+      const errors: FieldBasedOperationErrorMessage[] = getErrorsForDateReference(
+        layer,
+        columnId,
+        NORMALIZE_BY_UNIT_NAME
+      );
 
       if (!(layer.columns[columnId] as TimeScaleIndexPatternColumn).params.unit) {
         errors.push({

x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.test.tsx

@@ -1129,7 +1129,7 @@ invalid: "
       ).toHaveLength(0);
     });
 
-    it('returns return empty array if a fullReference operation is passed with the arguments', () => {
+    it('returns empty array if a fullReference operation is passed with the arguments', () => {
       expect(
         formulaOperation.getErrorMessage!(
           getNewLayerWithFormula('moving_average(average(bytes), window=7)'),

x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx

@@ -75,20 +75,13 @@ export const formulaOperation: OperationDefinition<FormulaIndexPatternColumn, 'm
       }
 
       const visibleOperationsMap = filterByVisibleOperation(operationDefinitionMap);
-      const { root, error } = tryToParse(column.params.formula, visibleOperationsMap);
-      if (error || !root) {
-        return error?.message
-          ? [
-              {
-                uniqueId: error.id,
-                message: error.message,
-              },
-            ]
-          : [];
+      const parseResponse = tryToParse(column.params.formula, visibleOperationsMap);
+      if ('error' in parseResponse) {
+        return [{ uniqueId: parseResponse.error.id, message: parseResponse.error.message }];
       }
 
       const errors = runASTValidation(
-        root,
+        parseResponse.root,
         layer,
         indexPattern,
         visibleOperationsMap,
@@ -113,19 +106,17 @@ export const formulaOperation: OperationDefinition<FormulaIndexPatternColumn, 'm
         ...managedColumns
           .flatMap(([id, col]) => {
             const def = visibleOperationsMap[col.operationType];
-            if (def?.getErrorMessage) {
-              // TOOD: it would be nice to have nicer column names here rather than `Part of <formula content>`
-              const messages = def.getErrorMessage(
+            // TOOD: it would be nice to have nicer column names here rather than `Part of <formula content>`
+            return (
+              def?.getErrorMessage?.(
                 layer,
                 id,
                 indexPattern,
                 dateRange,
                 visibleOperationsMap,
                 targetBars
-              );
-              return messages;
-            }
-            return [];
+              ) ?? []
+            );
           })
           // dedup messages with the same content
           .reduce((memo, message) => {

x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation.ts

@@ -204,7 +204,7 @@ function getMessageFromId(
         // this is just a placeholder because the actual message comes from the query validator
         message: i18n.translate('xpack.lens.indexPattern.invalidQuery', {
           defaultMessage: 'Invalid {language} query, please check the syntax and retry',
-          values: { operation: meta.language },
+          values: { language: meta.language },
         }),
       };
     case 'wrongFirstArgument':
@@ -492,7 +492,7 @@ function getMessageFromId(
 export function tryToParse(
   formula: string,
   operations: Record<string, unknown>
-): { root: TinymathAST; error: null } | { root: null; error: ErrorWrapper } {
+): { root: TinymathAST } | { error: ErrorWrapper } {
   let root;
   try {
     root = parse(formula);
@@ -504,28 +504,13 @@ export function tryToParse(
     // TODO: not sure why we just consider the first error here
     const maybeQueryProblems = getRawQueryValidationError(formula, operations);
     if (maybeQueryProblems.length > 0) {
-      return {
-        root: null,
-        error: {
-          ...maybeQueryProblems[0],
-          locations: [],
-        },
-      };
+      return { error: { ...maybeQueryProblems[0], locations: [] } };
     }
     return {
-      root: null,
-      error: getMessageFromId(
-        {
-          id: 'failedParsing',
-          meta: {
-            expression: formula,
-          },
-        },
-        []
-      ),
+      error: getMessageFromId({ id: 'failedParsing', meta: { expression: formula } }, []),
     };
   }
-  return { root, error: null };
+  return { root };
 }
 
 export function runASTValidation(
@@ -768,15 +753,7 @@ function checkTopNodeReturnType(ast: TinymathAST): ErrorWrapper[] {
     (tinymathFunctions[ast.name]?.outputType || DEFAULT_RETURN_TYPE) !== DEFAULT_RETURN_TYPE
   ) {
     return [
-      getMessageFromId(
-        {
-          id: 'wrongReturnedType',
-          meta: {
-            text: ast.text,
-          },
-        },
-        getNodeLocation(ast)
-      ),
+      getMessageFromId({ id: 'wrongReturnedType', meta: { text: ast.text } }, getNodeLocation(ast)),
     ];
   }
   return [];
@@ -878,12 +855,7 @@ function runFullASTValidation(
         if (!canHaveParams(nodeOperation) && namedArguments.length) {
           errors.push(
             getMessageFromId(
-              {
-                id: 'cannotAcceptParameter',
-                meta: {
-                  operation: node.name,
-                },
-              },
+              { id: 'cannotAcceptParameter', meta: { operation: node.name } },
               getNodeLocation(node)
             )
           );
@@ -1151,12 +1123,7 @@ export function validateMathNodes(
     if (node.args.length > positionalArguments.length) {
       errors.push(
         getMessageFromId(
-          {
-            id: 'tooManyArguments',
-            meta: {
-              operation: node.name,
-            },
-          },
+          { id: 'tooManyArguments', meta: { operation: node.name } },
           getNodeLocation(node)
         )
       );
@@ -1172,12 +1139,7 @@ export function validateMathNodes(
     if (hasFieldAsArgument) {
       errors.push(
         getMessageFromId(
-          {
-            id: 'shouldNotHaveField',
-            meta: {
-              operation: node.name,
-            },
-          },
+          { id: 'shouldNotHaveField', meta: { operation: node.name } },
           getNodeLocation(node)
         )
       );

x-pack/plugins/lens/public/datasources/form_based/utils.tsx

@@ -113,14 +113,12 @@ export function getSamplingValue(layer: FormBasedLayer) {
 
 export function isColumnInvalid(
   layer: FormBasedLayer,
+  column: GenericIndexPatternColumn,
   columnId: string,
   indexPattern: IndexPattern,
-  dateRange: DateRange | undefined,
+  dateRange: DateRange,
   targetBars: number
 ): boolean {
-  const column: GenericIndexPatternColumn | undefined = layer.columns[columnId];
-  if (!column || !indexPattern) return false;
-
   // check also references for errors
   const referencesHaveErrors =
     'references' in column &&
@@ -148,7 +146,7 @@ function hasReferencesErrors(
   layer: FormBasedLayer,
   column: ReferenceBasedIndexPatternColumn,
   indexPattern: IndexPattern,
-  dateRange: DateRange | undefined,
+  dateRange: DateRange,
   targetBars: number
 ) {
   return column.references?.some((referenceId: string) => {
@@ -293,6 +291,7 @@ export function getSearchWarningMessages(
                 )
                 .map((col) => col.label)
             ).map((label) => ({
+              // TODO: we probably need to move label as part of the meta data
               uniqueId: `${UNSUPPORTED_DOWNSAMPLED_INDEX_AGG_PREFIX}--${label}`,
               severity: 'warning',
               fixableInEditor: true,

x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx

@@ -7,7 +7,6 @@
 
 import { type ExtraAppendLayerArg, getXyVisualization } from './visualization';
 import { Position } from '@elastic/charts';
-import { EUIAmsterdamColorBlindPalette } from '@kbn/coloring';
 import {
   Operation,
   OperationDescriptor,

x-pack/plugins/lens/public/visualizations/xy/visualization.tsx

@@ -802,31 +802,26 @@ export const getXyVisualization = ({
       });
     });
 
-    // Data error handling below here
-    const hasNoAccessors = ({ accessors }: XYDataLayerConfig) =>
-      accessors == null || accessors.length === 0;
-
-    const hasNoSplitAccessor = ({ splitAccessor, seriesType }: XYDataLayerConfig) =>
-      seriesType.includes('percentage') && splitAccessor == null;
-
     // check if the layers in the state are compatible with this type of chart
     if (state && state.layers.length > 1) {
       // Order is important here: Y Axis is fundamental to exist to make it valid
-      const checks: Array<['Y' | 'Break down', (layer: XYDataLayerConfig) => boolean]> = [
-        ['Y', hasNoAccessors],
-        ['Break down', hasNoSplitAccessor],
-      ];
-
-      for (const [dimension, criteria] of checks) {
-        const layerValidation = validateLayersForDimension(dimension, state.layers, criteria);
-        if (!layerValidation.valid) {
-          errors.push({
-            ...layerValidation.error,
-            severity: 'error',
-            fixableInEditor: true,
-            displayLocations: [{ id: 'visualization' }],
-          });
-        }
+      const yLayerValidation = validateLayersForDimension(
+        'y',
+        state.layers,
+        ({ accessors }) => accessors == null || accessors.length === 0 // has no accessor
+      );
+      if (!yLayerValidation.valid) {
+        errors.push(yLayerValidation.error);
+      }
+
+      const breakDownLayerValidation = validateLayersForDimension(
+        'break_down',
+        state.layers,
+        ({ splitAccessor, seriesType }) =>
+          seriesType.includes('percentage') && splitAccessor == null // check if no split accessor
+      );
+      if (!breakDownLayerValidation.valid) {
+        errors.push(breakDownLayerValidation.error);
       }
     }
     // temporary fix for #87068
@@ -962,6 +957,7 @@ export const getXyVisualization = ({
           const { groupId } = axisGroup;
 
           warnings.push({
+            // TODO: can we push the group into the metadata and use a correct unique ID here?
             uniqueId: `${XY_MIXED_LOG_SCALE}${groupId}`,
             severity: 'warning',
             shortMessage: '',
@@ -991,6 +987,7 @@ export const getXyVisualization = ({
 
           axisGroup.mixedDomainSeries.forEach(({ accessor }) => {
             warnings.push({
+              // TODO: can we push the group into the metadata and use a correct unique ID here?
               uniqueId: `${XY_MIXED_LOG_SCALE_DIMENSION}${accessor}`,
               severity: 'warning',
               shortMessage: '',

x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx

@@ -259,14 +259,17 @@ export const supportedDataLayer = {
 
 // i18n ids cannot be dynamically generated, hence the function below
 function getMessageIdsForDimension(
-  dimension: 'Y' | 'Break down',
+  dimension: 'y' | 'break_down',
   layers: number[],
   isHorizontal: boolean
-): Pick<UserMessage, 'uniqueId' | 'shortMessage' | 'longMessage'> {
+): UserMessage {
   const layersList = layers.map((i: number) => i + 1).join(', ');
   switch (dimension) {
-    case 'Break down':
+    case 'break_down':
       return {
+        severity: 'error',
+        fixableInEditor: true,
+        displayLocations: [{ id: 'visualization' }],
         uniqueId: XY_BREAKDOWN_MISSING_AXIS,
         shortMessage: i18n.translate('xpack.lens.xyVisualization.dataFailureSplitShort', {
           defaultMessage: `Missing {axis}.`,
@@ -277,8 +280,11 @@ function getMessageIdsForDimension(
           values: { layers: layers.length, layersList, axis: 'Break down by axis' },
         }),
       };
-    case 'Y':
+    case 'y':
       return {
+        severity: 'error',
+        fixableInEditor: true,
+        displayLocations: [{ id: 'visualization' }],
         uniqueId: XY_Y_MISSING_AXIS,
         shortMessage: i18n.translate('xpack.lens.xyVisualization.dataFailureYShort', {
           defaultMessage: `Missing {axis}.`,
@@ -371,14 +377,14 @@ export function getLayersByType(state: State, byType?: string) {
 }
 
 export function validateLayersForDimension(
-  dimension: 'Y' | 'Break down',
+  dimension: 'y' | 'break_down',
   allLayers: XYLayerConfig[],
   missingCriteria: (layer: XYDataLayerConfig) => boolean
 ):
   | { valid: true }
   | {
       valid: false;
-      error: Pick<UserMessage, 'uniqueId' | 'shortMessage' | 'longMessage'>;
+      error: UserMessage;
     } {
   const dataLayers = allLayers
     .map((layer, i) => ({ layer, originalIndex: i }))

x-pack/plugins/translations/translations/fr-FR.json

@@ -22860,7 +22860,7 @@
     "xpack.lens.indexPattern.formulaExpressionParseError": "La formule {expression} ne peut pas être analysée",
     "xpack.lens.indexPattern.formulaExpressionWrongType": "Les paramètres de l'opération {operation} dans la formule ont un type incorrect : {params}",
     "xpack.lens.indexPattern.formulaExpressionWrongTypeArgument": "Le type de l'argument {name} pour l'opération {operation} dans la formule est incorrect : {type} au lieu de {expectedType}",
-    "xpack.lens.indexPattern.formulaFieldNotFound": "{variablesLength, plural, one {Champ} other {Champs}} {variablesList} introuvable(s)",
+    "xpack.lens.indexPattern.formulaFieldNotFound": "{missingFieldCount, plural, one {Champ} other {Champs}} {missingFieldList} introuvable(s)",
     "xpack.lens.indexPattern.formulaFieldNotRequired": "L'opération {operation} n'accepte aucun champ comme argument",
     "xpack.lens.indexPattern.formulaMathMissingArgument": "L'opération {operation} dans la formule ne comprend pas les arguments {count} : {params}",
     "xpack.lens.indexPattern.formulaOperationDuplicateParams": "Les paramètres de l'opération {operation} ont été déclarés plusieurs fois : {params}",
@@ -44950,4 +44950,4 @@
     "xpack.serverlessObservability.nav.projectSettings": "Paramètres de projet",
     "xpack.serverlessObservability.nav.synthetics": "Synthetics"
   }
-}
+}