From 972cb7b0dbdbc28d1aba6c9f8fa7ce872642dbb3 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Mon, 27 May 2024 16:19:29 +0200 Subject: [PATCH] cleanups --- .../datasources/form_based/form_based.tsx | 16 ++++-- .../definitions/calculations/time_scale.tsx | 8 ++- .../definitions/formula/formula.test.tsx | 2 +- .../definitions/formula/formula.tsx | 27 +++------ .../definitions/formula/validation.ts | 56 +++---------------- .../public/datasources/form_based/utils.tsx | 9 ++- .../visualizations/xy/visualization.test.tsx | 1 - .../visualizations/xy/visualization.tsx | 41 +++++++------- .../xy/visualization_helpers.tsx | 18 ++++-- .../translations/translations/fr-FR.json | 4 +- .../translations/translations/ja-JP.json | 4 +- .../translations/translations/zh-CN.json | 4 +- 12 files changed, 76 insertions(+), 114 deletions(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx index 691308eab67a9b..1bdd12a5cb49cd 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx +++ b/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', diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx index 1eb5212aa31569..c79d25d5cf17cc 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx @@ -90,9 +90,11 @@ export const timeScaleOperation: OperationDefinition { - 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({ diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.test.tsx index c1423c2ce192e3..85e15d5c4446ad 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.test.tsx +++ b/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)'), diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx index eda28464722406..3f5329a2722d20 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx @@ -75,20 +75,13 @@ export const formulaOperation: OperationDefinition { const def = visibleOperationsMap[col.operationType]; - if (def?.getErrorMessage) { - // TOOD: it would be nice to have nicer column names here rather than `Part of ` - const messages = def.getErrorMessage( + // TOOD: it would be nice to have nicer column names here rather than `Part of ` + return ( + def?.getErrorMessage?.( layer, id, indexPattern, dateRange, visibleOperationsMap, targetBars - ); - return messages; - } - return []; + ) ?? [] + ); }) // dedup messages with the same content .reduce((memo, message) => { diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation.ts index 3188d21cc6be39..86bea70ca19947 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation.ts +++ b/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 -): { 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) ) ); diff --git a/x-pack/plugins/lens/public/datasources/form_based/utils.tsx b/x-pack/plugins/lens/public/datasources/form_based/utils.tsx index 3961c5675f8551..8d74c44c40db62 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/utils.tsx +++ b/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, diff --git a/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx b/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx index 0f4bd99a45fdac..3e1bda9ebce383 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx +++ b/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, diff --git a/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx b/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx index 2d87c9de29babd..487e23e1224b7b 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/visualization.tsx +++ b/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: '', diff --git a/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx b/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx index d01d5844c56554..e627df38833916 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/visualization_helpers.tsx +++ b/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 { 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; + error: UserMessage; } { const dataLayers = allLayers .map((layer, i) => ({ layer, originalIndex: i })) diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index f73cbc46de2227..f3952bdf74ec98 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/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" } -} +} \ No newline at end of file diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 42ef3c2abf1375..372abac0354c0f 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -22834,7 +22834,7 @@ "xpack.lens.indexPattern.formulaExpressionNotHandled": "式{operation}の演算には次のパラメーターがありません：{params}", "xpack.lens.indexPattern.formulaExpressionParseError": "式{expression}を解析できません", "xpack.lens.indexPattern.formulaExpressionWrongType": "式の演算{operation}のパラメーターの型が正しくありません：{params}", - "xpack.lens.indexPattern.formulaFieldNotFound": "{variablesLength, plural, other {フィールド}} {variablesList}が見つかりません", + "xpack.lens.indexPattern.formulaFieldNotFound": "{missingFieldCount, plural, other {フィールド}} {missingFieldList}が見つかりません", "xpack.lens.indexPattern.formulaFieldNotRequired": "演算{operation}ではどのフィールドも引数として使用できません", "xpack.lens.indexPattern.formulaMathMissingArgument": "式{operation}の演算には{count}個の引数がありません：{params}", "xpack.lens.indexPattern.formulaOperationDuplicateParams": "演算{operation}のパラメーターが複数回宣言されました：{params}", @@ -44920,4 +44920,4 @@ "xpack.serverlessObservability.nav.projectSettings": "プロジェクト設定", "xpack.serverlessObservability.nav.synthetics": "Synthetics" } -} +} \ No newline at end of file diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index d35e005f3bb612..53490131727711 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -22868,7 +22868,7 @@ "xpack.lens.indexPattern.formulaExpressionParseError": "公式 {expression} 无法解析", "xpack.lens.indexPattern.formulaExpressionWrongType": "公式中运算 {operation} 的参数的类型不正确：{params}", "xpack.lens.indexPattern.formulaExpressionWrongTypeArgument": "公式中运算 {operation} 的 {name} 参数的类型不正确：{type} 而非 {expectedType}", - "xpack.lens.indexPattern.formulaFieldNotFound": "找不到{variablesLength, plural, other {字段}} {variablesList}", + "xpack.lens.indexPattern.formulaFieldNotFound": "找不到{missingFieldCount, plural, other {字段}} {missingFieldList}", "xpack.lens.indexPattern.formulaFieldNotRequired": "运算 {operation} 不接受任何字段作为参数", "xpack.lens.indexPattern.formulaMathMissingArgument": "公式中的运算 {operation} 缺失 {count} 个参数：{params}", "xpack.lens.indexPattern.formulaOperationDuplicateParams": "运算 {operation} 的参数已声明多次：{params}", @@ -44969,4 +44969,4 @@ "xpack.serverlessObservability.nav.projectSettings": "项目设置", "xpack.serverlessObservability.nav.synthetics": "Synthetics" } -} +} \ No newline at end of file