From e12c7e37d41788d65b49e152e925636725d16e5e Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Wed, 5 Jun 2024 14:30:14 +0200 Subject: [PATCH] [Lens] Improve user messagging (#184233) This PR cleaned up a bit our user messages. In particular: - marks as required the `uniqueId` for a `UserMessage` making it uniquely identifiable across various message renderers (moves toward this objective: https://github.com/elastic/kibana/issues/151526) - adds a unique ID for each UserMessage in Lens - partially unifies the Error structure between UserMessages and form-based validation. - Opens the door to provide metadata about the errors, that are currently buried within the text/react node (moves toward this objective: https://github.com/elastic/kibana/issues/151526) Subsequent possible steps, outside this PR: - add metadata where required for https://github.com/elastic/kibana/issues/151526 - merge even more the type of `UserMessage`, `FieldBasedOperationErrorMessage` and `ValidationErrors` - centralize the messages id and translations - add a Type for the `uniqueId` to guide future developments --- ...et_application_user_messages.test.tsx.snap | 2 + .../get_application_user_messages.test.tsx | 15 + .../get_application_user_messages.tsx | 12 + .../__snapshots__/utils.test.tsx.snap | 3 + .../form_based/dimension_panel/time_shift.tsx | 2 +- .../datasources/form_based/form_based.test.ts | 6 + .../datasources/form_based/form_based.tsx | 71 +- .../definitions/calculations/counter_rate.tsx | 4 +- .../calculations/cumulative_sum.tsx | 4 +- .../definitions/calculations/differences.tsx | 4 +- .../calculations/moving_average.tsx | 4 +- .../calculations/overall_metric.tsx | 4 +- .../definitions/calculations/time_scale.tsx | 49 +- .../definitions/calculations/utils.test.ts | 18 +- .../definitions/calculations/utils.ts | 119 +- .../operations/definitions/cardinality.tsx | 10 +- .../operations/definitions/count.tsx | 11 +- .../operations/definitions/date_histogram.tsx | 38 +- .../__snapshots__/formula.test.tsx.snap | 6 + .../formula/context_variables.test.ts | 52 +- .../definitions/formula/context_variables.tsx | 72 +- .../formula/editor/formula_editor.tsx | 14 +- .../definitions/formula/formula.test.tsx | 114 +- .../definitions/formula/formula.tsx | 39 +- .../operations/definitions/formula/parse.ts | 8 +- .../definitions/formula/validation.ts | 1064 +++++++++-------- .../definitions/formula/validation_errors.ts | 183 +++ .../operations/definitions/helpers.test.ts | 21 +- .../operations/definitions/helpers.tsx | 34 +- .../operations/definitions/index.ts | 33 +- .../definitions/last_value.test.tsx | 16 +- .../operations/definitions/last_value.tsx | 80 +- .../operations/definitions/metrics.tsx | 10 +- .../operations/definitions/percentile.tsx | 10 +- .../definitions/percentile_ranks.tsx | 10 +- .../definitions/static_value.test.tsx | 8 +- .../operations/definitions/static_value.tsx | 14 +- .../definitions/terms/helpers.test.ts | 52 +- .../operations/definitions/terms/helpers.ts | 235 ++-- .../operations/definitions/terms/index.tsx | 16 +- .../definitions/terms/terms.test.tsx | 19 +- .../form_based/reduced_time_range_utils.tsx | 28 +- .../form_based/time_shift_utils.tsx | 76 +- .../public/datasources/form_based/utils.tsx | 124 +- .../text_based/text_based_languages.test.ts | 2 + .../text_based/text_based_languages.tsx | 2 + .../workspace_panel/workspace_panel.test.tsx | 2 + .../workspace_panel/workspace_panel.tsx | 5 +- .../editor_frame_service/error_helper.tsx | 4 +- .../public/embeddable/embeddable.test.tsx | 1 + .../lens/public/embeddable/embeddable.tsx | 5 +- .../embeddable_info_badges.test.tsx | 78 +- .../embeddable/embeddable_info_badges.tsx | 50 +- x-pack/plugins/lens/public/types.ts | 10 +- .../plugins/lens/public/user_messages_ids.ts | 97 ++ .../gauge/visualization.test.ts | 1 + .../visualizations/gauge/visualization.tsx | 14 + .../heatmap/visualization.test.ts | 1 + .../visualizations/heatmap/visualization.tsx | 3 + .../visualizations/metric/visualization.tsx | 2 + .../partition/visualization.tsx | 8 + .../{state_helpers.ts => state_helpers.tsx} | 111 +- .../visualizations/xy/visualization.test.tsx | 8 +- .../visualizations/xy/visualization.tsx | 103 +- .../xy/visualization_helpers.tsx | 27 +- .../translations/translations/fr-FR.json | 4 +- .../translations/translations/ja-JP.json | 4 +- .../translations/translations/zh-CN.json | 4 +- 68 files changed, 1880 insertions(+), 1380 deletions(-) create mode 100644 x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation_errors.ts create mode 100644 x-pack/plugins/lens/public/user_messages_ids.ts rename x-pack/plugins/lens/public/visualizations/xy/{state_helpers.ts => state_helpers.tsx} (64%) diff --git a/x-pack/plugins/lens/public/app_plugin/__snapshots__/get_application_user_messages.test.tsx.snap b/x-pack/plugins/lens/public/app_plugin/__snapshots__/get_application_user_messages.test.tsx.snap index 6626ac8b51c542..e53df700f6732e 100644 --- a/x-pack/plugins/lens/public/app_plugin/__snapshots__/get_application_user_messages.test.tsx.snap +++ b/x-pack/plugins/lens/public/app_plugin/__snapshots__/get_application_user_messages.test.tsx.snap @@ -88,6 +88,7 @@ Array [ , "severity": "error", "shortMessage": "", + "uniqueId": "editor_missing_dataview", }, Object { "displayLocations": Array [ @@ -99,6 +100,7 @@ Array [ "longMessage": "Could not find the data view: missing_pattern", "severity": "error", "shortMessage": "", + "uniqueId": "editor_missing_expression_dataview", }, ] `; diff --git a/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.test.tsx b/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.test.tsx index f511e7e59112ac..4c5a17373af8d1 100644 --- a/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.test.tsx @@ -44,6 +44,7 @@ describe('application-level user messages', () => { "longMessage": "Visualization type not found.", "severity": "error", "shortMessage": "", + "uniqueId": "editor_missing_vis_type", }, ] `); @@ -73,6 +74,7 @@ describe('application-level user messages', () => { "longMessage": "The visualization type id_for_type_that_doesnt_exist could not be resolved.", "severity": "error", "shortMessage": "Unknown visualization type", + "uniqueId": "editor_unknown_vis_type", }, ] `); @@ -102,6 +104,7 @@ describe('application-level user messages', () => { "longMessage": "Could not find datasource for the visualization", "severity": "error", "shortMessage": "Unknown datasource type", + "uniqueId": "editor_unknown_datasource_type", }, ] `); @@ -210,6 +213,7 @@ describe('filtering user messages', () => { const userMessages: UserMessage[] = [ { + uniqueId: 'unique_id_1', severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'dimensionButton', dimensionId: dimensionId1 }], @@ -217,6 +221,7 @@ describe('filtering user messages', () => { longMessage: '', }, { + uniqueId: 'unique_id_2', severity: 'warning', fixableInEditor: true, displayLocations: [{ id: 'dimensionButton', dimensionId: dimensionId2 }], @@ -224,6 +229,7 @@ describe('filtering user messages', () => { longMessage: '', }, { + uniqueId: 'unique_id_3', severity: 'warning', fixableInEditor: true, displayLocations: [{ id: 'banner' }], @@ -231,6 +237,7 @@ describe('filtering user messages', () => { longMessage: '', }, { + uniqueId: 'unique_id_4', severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'visualization' }], @@ -238,6 +245,7 @@ describe('filtering user messages', () => { longMessage: '', }, { + uniqueId: 'unique_id_5', severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'visualizationInEditor' }], @@ -245,6 +253,7 @@ describe('filtering user messages', () => { longMessage: '', }, { + uniqueId: 'unique_id_6', severity: 'warning', fixableInEditor: true, displayLocations: [{ id: 'visualizationOnEmbeddable' }], @@ -266,6 +275,7 @@ describe('filtering user messages', () => { "longMessage": "", "severity": "warning", "shortMessage": "Deprecation notice!", + "uniqueId": "unique_id_3", }, ] `); @@ -286,6 +296,7 @@ describe('filtering user messages', () => { "longMessage": "", "severity": "error", "shortMessage": "Warning on dimension 1!", + "uniqueId": "unique_id_1", }, ] `); @@ -306,6 +317,7 @@ describe('filtering user messages', () => { "longMessage": "", "severity": "warning", "shortMessage": "Warning on dimension 2!", + "uniqueId": "unique_id_2", }, ] `); @@ -322,6 +334,7 @@ describe('filtering user messages', () => { "longMessage": "", "severity": "error", "shortMessage": "Visualization error!", + "uniqueId": "unique_id_4", }, Object { "displayLocations": Array [ @@ -333,6 +346,7 @@ describe('filtering user messages', () => { "longMessage": "", "severity": "error", "shortMessage": "Visualization editor error!", + "uniqueId": "unique_id_5", }, ] `); @@ -364,6 +378,7 @@ describe('filtering user messages', () => { "longMessage": "", "severity": "warning", "shortMessage": "Visualization embeddable warning!", + "uniqueId": "unique_id_6", }, ] `); diff --git a/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.tsx b/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.tsx index 4041fce3bbd0a4..810acd8d0350fd 100644 --- a/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.tsx +++ b/x-pack/plugins/lens/public/app_plugin/get_application_user_messages.tsx @@ -28,6 +28,13 @@ import type { Visualization, } from '../types'; import { getMissingIndexPattern } from '../editor_frame_service/editor_frame/state_helpers'; +import { + EDITOR_MISSING_DATAVIEW, + EDITOR_MISSING_EXPRESSION_DATAVIEW, + EDITOR_MISSING_VIS_TYPE, + EDITOR_UNKNOWN_DATASOURCE_TYPE, + EDITOR_UNKNOWN_VIS_TYPE, +} from '../user_messages_ids'; /** * Provides a place to register general user messages that don't belong in the datasource or visualization objects @@ -78,6 +85,7 @@ export const getApplicationUserMessages = ({ function getMissingVisTypeError(): UserMessage { return { + uniqueId: EDITOR_MISSING_VIS_TYPE, severity: 'error', displayLocations: [{ id: 'visualizationOnEmbeddable' }], fixableInEditor: true, @@ -90,6 +98,7 @@ function getMissingVisTypeError(): UserMessage { function getUnknownVisualizationTypeError(visType: string): UserMessage { return { + uniqueId: EDITOR_UNKNOWN_VIS_TYPE, severity: 'error', fixableInEditor: false, displayLocations: [{ id: 'visualization' }], @@ -107,6 +116,7 @@ function getUnknownVisualizationTypeError(visType: string): UserMessage { function getUnknownDatasourceTypeError(): UserMessage { return { + uniqueId: EDITOR_UNKNOWN_DATASOURCE_TYPE, severity: 'error', fixableInEditor: false, displayLocations: [{ id: 'visualization' }], @@ -130,6 +140,7 @@ function getMissingIndexPatternsErrors( const canFix = isManagementEnabled && isIndexPatternManagementEnabled; return [ { + uniqueId: EDITOR_MISSING_DATAVIEW, severity: 'error', fixableInEditor: canFix, displayLocations: [{ id: 'visualizationInEditor' }], @@ -176,6 +187,7 @@ function getMissingIndexPatternsErrors( ), }, { + uniqueId: EDITOR_MISSING_EXPRESSION_DATAVIEW, severity: 'error', fixableInEditor: canFix, displayLocations: [{ id: 'visualizationOnEmbeddable' }], diff --git a/x-pack/plugins/lens/public/datasources/form_based/__snapshots__/utils.test.tsx.snap b/x-pack/plugins/lens/public/datasources/form_based/__snapshots__/utils.test.tsx.snap index fe59faa80be9f1..55cf1e8637292b 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/__snapshots__/utils.test.tsx.snap +++ b/x-pack/plugins/lens/public/datasources/form_based/__snapshots__/utils.test.tsx.snap @@ -68,6 +68,7 @@ Object { "longMessage": "", "severity": "warning", "shortMessage": "This may be approximate depending on how the data is indexed. For more precise results, sort by rarity.", + "uniqueId": "precision_error_asc_count_precision", } `; @@ -86,6 +87,7 @@ Object { "longMessage": "", "severity": "warning", "shortMessage": "This might be an approximation. For more precise results, use Filters or increase the number of Top Values.", + "uniqueId": "layer_settings_accuracy_mode_enabled", } `; @@ -104,5 +106,6 @@ Object { "longMessage": "", "severity": "warning", "shortMessage": "This might be an approximation. For more precise results, you can enable accuracy mode, but it increases the load on the Elasticsearch cluster.", + "uniqueId": "layer_settings_accuracy_mode_disabled", } `; diff --git a/x-pack/plugins/lens/public/datasources/form_based/dimension_panel/time_shift.tsx b/x-pack/plugins/lens/public/datasources/form_based/dimension_panel/time_shift.tsx index fe69c9d76d45b1..82da59b38cc277 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/dimension_panel/time_shift.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/dimension_panel/time_shift.tsx @@ -130,7 +130,7 @@ export function TimeShift({ defaultMessage: 'Enter the time shift number and unit', })} error={ - warnings[0] || + warnings[0] ?? (isLocalValueInvalid && i18n.translate('xpack.lens.indexPattern.timeShift.genericInvalidHelp', { defaultMessage: 'Time shift value is not valid.', diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts b/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts index bd01a77f80b9ad..94a85962e28b73 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts @@ -3095,6 +3095,7 @@ describe('IndexPattern Data Source', () => { "longMessage": "error 1", "severity": "error", "shortMessage": "", + "uniqueId": "error 1", }, Object { "displayLocations": Array [ @@ -3106,6 +3107,7 @@ describe('IndexPattern Data Source', () => { "longMessage": "error 2", "severity": "error", "shortMessage": "", + "uniqueId": "error 2", }, ] `); @@ -3160,6 +3162,7 @@ describe('IndexPattern Data Source', () => { />, "severity": "error", "shortMessage": "Layer 1 error: ", + "uniqueId": "error 1", }, Object { "displayLocations": Array [ @@ -3182,6 +3185,7 @@ describe('IndexPattern Data Source', () => { />, "severity": "error", "shortMessage": "Layer 1 error: ", + "uniqueId": "error 2", }, ] `); @@ -3248,6 +3252,7 @@ describe('IndexPattern Data Source', () => {

, "severity": "error", "shortMessage": "", + "uniqueId": "editor_invalid_dimension", }, ] `); @@ -3286,6 +3291,7 @@ describe('IndexPattern Data Source', () => { , "severity": "error", "shortMessage": "", + "uniqueId": undefined, }, ] `); 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 8099a787437baa..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 @@ -100,6 +100,7 @@ import { isColumnOfType } from './operations/definitions/helpers'; import { LayerSettingsPanel } from './layer_settings'; import { FormBasedLayer, LastValueIndexPatternColumn } from '../..'; import { filterAndSortUserMessages } from '../../app_plugin/get_application_user_messages'; +import { EDITOR_INVALID_DIMENSION } from '../../user_messages_ids'; export type { OperationType, GenericIndexPatternColumn } from './operations'; export { deleteColumn } from './operations'; @@ -760,48 +761,56 @@ export function getFormBasedDatasource({ layerErrorMessages, (layerId, columnId) => { const layer = state.layers[layerId]; - return !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) ); } ); - const warningMessages = [ - ...[ - ...(getStateTimeShiftWarningMessages(data.datatableUtilities, state, framePublicAPI) || - []), - ].map((longMessage) => { - const message: UserMessage = { - severity: 'warning', - fixableInEditor: true, - displayLocations: [{ id: 'toolbar' }], - shortMessage: '', - longMessage, - }; + const timeShiftWarningMessages = getStateTimeShiftWarningMessages( + data.datatableUtilities, + state, + framePublicAPI + ); - return message; - }), - ...getPrecisionErrorWarningMessages( - data.datatableUtilities, - state, - framePublicAPI, - core.docLinks, - setState - ), - ...getUnsupportedOperationsWarningMessage(state, framePublicAPI, core.docLinks), - ]; + const precisionErrorWarningMsg = getPrecisionErrorWarningMessages( + data.datatableUtilities, + state, + framePublicAPI, + core.docLinks, + setState + ); + + const unsupportedOpsWarningMsg = getUnsupportedOperationsWarningMessage( + state, + framePublicAPI, + core.docLinks + ); const infoMessages = getNotifiableFeatures(state, framePublicAPI, visualizationInfo); - return layerErrorMessages.concat(dimensionErrorMessages, warningMessages, infoMessages); + return layerErrorMessages.concat( + dimensionErrorMessages, + timeShiftWarningMessages, + precisionErrorWarningMsg, + unsupportedOpsWarningMsg, + infoMessages + ); }, getSearchWarningMessages: (state, warning, request, response) => { - return [...getSearchWarningMessages(state, warning, request, response, core.theme)]; + return getSearchWarningMessages(state, warning, request, response, core.theme); }, checkIntegrity: (state, indexPatterns) => { @@ -916,7 +925,7 @@ function getLayerErrorMessages( setState: StateSetter, core: CoreStart, data: DataPublicPluginStart -) { +): UserMessage[] { const indexPatterns = framePublicAPI.dataViews.indexPatterns; const layerErrors: UserMessage[][] = Object.entries(state.layers) @@ -927,6 +936,7 @@ function getLayerErrorMessages( [] ).map((error) => { const message: UserMessage = { + uniqueId: typeof error === 'string' ? error : error.uniqueId, severity: 'error', fixableInEditor: true, displayLocations: @@ -1005,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) @@ -1022,8 +1032,9 @@ function getInvalidDimensionErrorMessages( continue; } - if (!isValidColumn(layerId, columnId)) { + if (isInvalidColumn(layerId, columnId)) { messages.push({ + uniqueId: EDITOR_INVALID_DIMENSION, severity: 'error', displayLocations: [{ id: 'dimensionButton', dimensionId: columnId }], fixableInEditor: true, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/counter_rate.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/counter_rate.tsx index 93ab93de22f664..af563ccd4bef74 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/counter_rate.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/counter_rate.tsx @@ -129,7 +129,9 @@ export const counterRateOperation: OperationDefinition< } } - return checkForDateHistogram(layer, opName)?.join(', '); + return checkForDateHistogram(layer, opName) + .map((e) => e.message) + .join(', '); }, timeScalingMode: 'mandatory', filterable: true, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/cumulative_sum.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/cumulative_sum.tsx index d7c13d43ba163e..50bcd865b855d5 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/cumulative_sum.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/cumulative_sum.tsx @@ -123,7 +123,9 @@ export const cumulativeSumOperation: OperationDefinition< return dataLayerErrors.join(', '); } } - return checkForDateHistogram(layer, opName)?.join(', '); + return checkForDateHistogram(layer, opName) + .map((e) => e.message) + .join(', '); }, filterable: true, quickFunctionDocumentation: i18n.translate( diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/differences.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/differences.tsx index 59943a23ece5ea..f91636471190e4 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/differences.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/differences.tsx @@ -107,7 +107,9 @@ export const derivativeOperation: OperationDefinition< return dataLayerErrors.join(', '); } } - return checkForDateHistogram(layer, opName)?.join(', '); + return checkForDateHistogram(layer, opName) + .map((e) => e.message) + .join(', '); }, timeScalingMode: 'optional', filterable: true, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/moving_average.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/moving_average.tsx index a2689e7d2209c5..4d0912fd964258 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/moving_average.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/moving_average.tsx @@ -137,7 +137,9 @@ export const movingAverageOperation: OperationDefinition< return dataLayerErrors.join(', '); } } - return checkForDateHistogram(layer, opName)?.join(', '); + return checkForDateHistogram(layer, opName) + .map((e) => e.message) + .join(', '); }, timeScalingMode: 'optional', filterable: true, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/overall_metric.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/overall_metric.tsx index 68c7f18483d12f..df0e55ff40dfb4 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/overall_metric.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/overall_metric.tsx @@ -24,7 +24,7 @@ import type { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn, } from '../column_types'; -import { optionallHistogramBasedOperationToExpression } from './utils'; +import { optionalHistogramBasedOperationToExpression } from './utils'; import type { OperationDefinition } from '..'; import { getFormatFromPreviousColumn } from '../helpers'; @@ -79,7 +79,7 @@ function buildOverallMetricOperation { - return optionallHistogramBasedOperationToExpression(layer, columnId, 'overall_metric', { + return optionalHistogramBasedOperationToExpression(layer, columnId, 'overall_metric', { metric: [metric], }); }, 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 3ccb3e0808129d..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 @@ -12,8 +12,8 @@ import type { ReferenceBasedIndexPatternColumn, } from '../column_types'; import { getErrorsForDateReference } from './utils'; -import type { OperationDefinition } from '..'; -import { combineErrorMessages, getFormatFromPreviousColumn } from '../helpers'; +import type { FieldBasedOperationErrorMessage, OperationDefinition } from '..'; +import { getFormatFromPreviousColumn } from '../helpers'; import { FormBasedLayer } from '../../../types'; export type TimeScaleIndexPatternColumn = FormattedIndexPatternColumn & @@ -90,26 +90,37 @@ export const timeScaleOperation: OperationDefinition { - return combineErrorMessages([ - getErrorsForDateReference(layer, columnId, NORMALIZE_BY_UNIT_NAME), - !(layer.columns[columnId] as TimeScaleIndexPatternColumn).params.unit - ? [ - i18n.translate('xpack.lens.indexPattern.timeScale.missingUnit', { - defaultMessage: 'No unit specified for normalize by unit.', - }), - ] - : [], + const errors: FieldBasedOperationErrorMessage[] = getErrorsForDateReference( + layer, + columnId, + NORMALIZE_BY_UNIT_NAME + ); + + if (!(layer.columns[columnId] as TimeScaleIndexPatternColumn).params.unit) { + errors.push({ + uniqueId: '', + message: i18n.translate('xpack.lens.indexPattern.timeScale.missingUnit', { + defaultMessage: 'No unit specified for normalize by unit.', + }), + }); + } + + if ( ['s', 'm', 'h', 'd'].indexOf( - (layer.columns[columnId] as TimeScaleIndexPatternColumn).params.unit || 's' + (layer.columns[columnId] as TimeScaleIndexPatternColumn).params.unit ?? 's' ) === -1 - ? [ - i18n.translate('xpack.lens.indexPattern.timeScale.wrongUnit', { - defaultMessage: 'Unknown unit specified: use s, m, h or d.', - }), - ] - : [], - ]); + ) { + errors.push({ + uniqueId: '', + message: i18n.translate('xpack.lens.indexPattern.timeScale.wrongUnit', { + defaultMessage: 'Unknown unit specified: use s, m, h or d.', + }), + }); + } + + return errors; }, + filterable: false, shiftable: false, }; diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.test.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.test.ts index 1d27c0db935c83..a83013cac2aa79 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.test.ts @@ -10,6 +10,10 @@ import { operationDefinitionMap } from '..'; import { createMockedFullReference } from '../../mocks'; import { LayerTypes } from '@kbn/expression-xy-plugin/public'; import { DateHistogramIndexPatternColumn } from '../date_histogram'; +import { + CALCULATIONS_MISSING_COLUMN_REFERENCE, + CALCULATIONS_WRONG_DIMENSION_CONFIG, +} from '../../../../../user_messages_ids'; // Mock prevents issue with circular loading jest.mock('..'); @@ -47,7 +51,12 @@ describe('utils', () => { }, 'ref' ) - ).toEqual(['"Label" is not fully configured']); + ).toEqual([ + { + uniqueId: CALCULATIONS_MISSING_COLUMN_REFERENCE, + message: '"Label" is not fully configured', + }, + ]); }); it('should show an error if the reference is not allowed per the requirements', () => { @@ -76,7 +85,12 @@ describe('utils', () => { }, 'ref' ) - ).toEqual(['Dimension "Label" is configured incorrectly']); + ).toEqual([ + { + uniqueId: CALCULATIONS_WRONG_DIMENSION_CONFIG, + message: 'Dimension "Label" is configured incorrectly', + }, + ]); }); }); }); diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.ts index 99173e5064cb59..5a40b967e6c03a 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/calculations/utils.ts @@ -16,9 +16,12 @@ import type { FormBasedLayer } from '../../../types'; import { adjustTimeScaleLabelSuffix } from '../../time_scale_utils'; import type { ReferenceBasedIndexPatternColumn } from '../column_types'; import { getManagedColumnsFrom, isColumnValidAsReference } from '../../layer_helpers'; -import { operationDefinitionMap } from '..'; -import { FieldBasedIndexPatternColumn } from '../../../types'; -import { IndexPatternField } from '../../../../../types'; +import { FieldBasedOperationErrorMessage, operationDefinitionMap } from '..'; +import { + CALCULATIONS_DATE_HISTOGRAM_REQUIRED, + CALCULATIONS_MISSING_COLUMN_REFERENCE, + CALCULATIONS_WRONG_DIMENSION_CONFIG, +} from '../../../../../user_messages_ids'; export const buildLabelFunction = (ofName: (name?: string) => string) => @@ -51,22 +54,28 @@ export function checkForDataLayerType(layerType: LayerType, name: string) { /** * Checks whether the current layer includes a date histogram and returns an error otherwise */ -export function checkForDateHistogram(layer: FormBasedLayer, name: string) { +export function checkForDateHistogram( + layer: FormBasedLayer, + name: string +): FieldBasedOperationErrorMessage[] { const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed); const hasDateHistogram = buckets.some( (colId) => layer.columns[colId].operationType === 'date_histogram' ); if (hasDateHistogram) { - return undefined; + return []; } return [ - i18n.translate('xpack.lens.indexPattern.calculations.dateHistogramErrorMessage', { - defaultMessage: - '{name} requires a date histogram to work. Add a date histogram or select a different function.', - values: { - name, - }, - }), + { + uniqueId: CALCULATIONS_DATE_HISTOGRAM_REQUIRED, + message: i18n.translate('xpack.lens.indexPattern.calculations.dateHistogramErrorMessage', { + defaultMessage: + '{name} requires a date histogram to work. Add a date histogram or select a different function.', + values: { + name, + }, + }), + }, ]; } @@ -87,21 +96,25 @@ const getFullyManagedColumnIds = memoizeOne((layer: FormBasedLayer) => { return managedColumnIds; }); -export function checkReferences(layer: FormBasedLayer, columnId: string) { +export function checkReferences( + layer: FormBasedLayer, + columnId: string +): FieldBasedOperationErrorMessage[] { const column = layer.columns[columnId] as ReferenceBasedIndexPatternColumn; - const errors: string[] = []; + const errors: FieldBasedOperationErrorMessage[] = []; column.references.forEach((referenceId, index) => { if (!layer.columns[referenceId]) { - errors.push( - i18n.translate('xpack.lens.indexPattern.missingReferenceError', { + errors.push({ + uniqueId: CALCULATIONS_MISSING_COLUMN_REFERENCE, + message: i18n.translate('xpack.lens.indexPattern.missingReferenceError', { defaultMessage: '"{dimensionLabel}" is not fully configured', values: { dimensionLabel: column.label, }, - }) - ); + }), + }); } else { const referenceColumn = layer.columns[referenceId]!; const definition = operationDefinitionMap[column.operationType]; @@ -116,27 +129,29 @@ export function checkReferences(layer: FormBasedLayer, columnId: string) { // do not enforce column validity if current column is part of managed subtree if (!isValid && !getFullyManagedColumnIds(layer).has(columnId)) { - errors.push( - i18n.translate('xpack.lens.indexPattern.invalidReferenceConfiguration', { + errors.push({ + uniqueId: CALCULATIONS_WRONG_DIMENSION_CONFIG, + message: i18n.translate('xpack.lens.indexPattern.invalidReferenceConfiguration', { defaultMessage: 'Dimension "{dimensionLabel}" is configured incorrectly', values: { dimensionLabel: column.label, }, - }) - ); + }), + }); } } }); - return errors.length ? errors : undefined; + return errors; } -export function getErrorsForDateReference(layer: FormBasedLayer, columnId: string, name: string) { - const dateErrors = checkForDateHistogram(layer, name) ?? []; - const referenceErrors = checkReferences(layer, columnId) ?? []; - if (dateErrors.length || referenceErrors.length) { - return [...dateErrors, ...referenceErrors]; - } - return; +export function getErrorsForDateReference( + layer: FormBasedLayer, + columnId: string, + name: string +): FieldBasedOperationErrorMessage[] { + const dateErrors = checkForDateHistogram(layer, name); + const referenceErrors = checkReferences(layer, columnId); + return dateErrors.concat(referenceErrors); } export function hasDateField(indexPattern: IndexPattern) { @@ -177,7 +192,7 @@ export function dateBasedOperationToExpression( /** * Creates an expression ast for a date based operation (cumulative sum, derivative, moving average, counter rate) */ -export function optionallHistogramBasedOperationToExpression( +export function optionalHistogramBasedOperationToExpression( layer: FormBasedLayer, columnId: string, functionName: string, @@ -205,45 +220,3 @@ export function optionallHistogramBasedOperationToExpression( }, ]; } - -function isMetricCounterField(field?: IndexPatternField) { - return field?.timeSeriesMetric === 'counter'; -} - -function checkReferencedColumnMetric( - layer: FormBasedLayer, - columnId: string, - indexPattern: IndexPattern -) { - const column = layer.columns[columnId] as ReferenceBasedIndexPatternColumn; - return column.references - .filter((referencedId) => 'sourceField' in layer.columns[referencedId]) - .map((referencedId) => { - const fieldName = (layer.columns[referencedId] as FieldBasedIndexPatternColumn).sourceField; - if (!isMetricCounterField(indexPattern.getFieldByName(fieldName))) { - return i18n.translate('xpack.lens.indexPattern.invalidReferenceConfiguration', { - defaultMessage: 'Dimension "{dimensionLabel}" is configured incorrectly', - values: { - dimensionLabel: layer.columns[referencedId].label, - }, - }); - } - }); -} - -export function getErrorForRateReference( - layer: FormBasedLayer, - columnId: string, - name: string, - indexPattern: IndexPattern -) { - const dateErrors = checkForDateHistogram(layer, name) ?? []; - const referenceErrors = checkReferences(layer, columnId) ?? []; - const metricCounterErrors = checkReferencedColumnMetric(layer, columnId, indexPattern) ?? []; - if (metricCounterErrors.length) { - return metricCounterErrors.concat(referenceErrors); - } - if (dateErrors.length) { - return dateErrors.concat(referenceErrors); - } -} 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 index 028f8016d40673..14ae32b0bf5953 100644 --- 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 @@ -20,7 +20,6 @@ import { getInvalidFieldMessage, getSafeName, getFilter, - combineErrorMessages, isColumnOfType, } from './helpers'; import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; @@ -93,11 +92,10 @@ export const cardinalityOperation: OperationDefinition< return { dataType: 'number', isBucketed: IS_BUCKETED, scale: SCALE }; } }, - getErrorMessage: (layer, columnId, indexPattern) => - combineErrorMessages([ - getInvalidFieldMessage(layer, columnId, indexPattern), - getColumnReducedTimeRangeError(layer, columnId, indexPattern), - ]), + getErrorMessage: (layer, columnId, indexPattern) => [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getColumnReducedTimeRangeError(layer, columnId, indexPattern), + ], isTransferable: (column, newIndexPattern) => { const newField = newIndexPattern.getFieldByName(column.sourceField); 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 index 94b1b36193aed1..c53fbc28b78876 100644 --- 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 @@ -19,7 +19,6 @@ import type { IndexPatternField } from '../../../../types'; import { getInvalidFieldMessage, getFilter, - combineErrorMessages, getFormatFromPreviousColumn, isColumnOfType, } from './helpers'; @@ -87,11 +86,11 @@ export const countOperation: OperationDefinition - combineErrorMessages([ - getInvalidFieldMessage(layer, columnId, indexPattern), - getColumnReducedTimeRangeError(layer, columnId, indexPattern), - ]), + getErrorMessage: (layer, columnId, indexPattern) => [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getColumnReducedTimeRangeError(layer, columnId, indexPattern), + ], + allowAsReference: true, onFieldChange: (oldColumn, field) => { return { 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 index 0d6556344a7102..cedf2f976b2cb0 100644 --- 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 @@ -31,10 +31,11 @@ import { extendedBoundsToAst, intervalOptions } from '@kbn/data-plugin/common'; import { buildExpressionFunction } from '@kbn/expressions-plugin/public'; import { TooltipWrapper } from '@kbn/visualization-utils'; import { updateColumnParam } from '../layer_helpers'; -import { OperationDefinition, ParamEditorProps } from '.'; +import { FieldBasedOperationErrorMessage, OperationDefinition, ParamEditorProps } from '.'; import { FieldBasedIndexPatternColumn } from './column_types'; import { getInvalidFieldMessage, getSafeName } from './helpers'; import { FormBasedLayer } from '../../types'; +import { TIME_SHIFT_MULTIPLE_DATE_HISTOGRAMS } from '../../../../user_messages_ids'; const { isValidInterval } = search.aggs; const autoInterval = 'auto'; @@ -50,26 +51,34 @@ export interface DateHistogramIndexPatternColumn extends FieldBasedIndexPatternC }; } -function getMultipleDateHistogramsErrorMessage(layer: FormBasedLayer, columnId: string) { +function getMultipleDateHistogramsErrorMessage( + layer: FormBasedLayer, + columnId: string +): FieldBasedOperationErrorMessage[] { const usesTimeShift = Object.values(layer.columns).some( (col) => col.timeShift && col.timeShift !== '' ); if (!usesTimeShift) { - return undefined; + return []; } const dateHistograms = layer.columnOrder.filter( (colId) => layer.columns[colId].operationType === 'date_histogram' ); if (dateHistograms.length < 2) { - return undefined; + return []; } - return i18n.translate('xpack.lens.indexPattern.multipleDateHistogramsError', { - defaultMessage: - '"{dimensionLabel}" is not the only date histogram. When using time shifts, make sure to only use one date histogram.', - values: { - dimensionLabel: layer.columns[columnId].label, + return [ + { + uniqueId: TIME_SHIFT_MULTIPLE_DATE_HISTOGRAMS, + message: i18n.translate('xpack.lens.indexPattern.multipleDateHistogramsError', { + defaultMessage: + '"{dimensionLabel}" is not the only date histogram. When using time shifts, make sure to only use one date histogram.', + values: { + dimensionLabel: layer.columns[columnId].label, + }, + }), }, - }); + ]; } export const dateHistogramOperation: OperationDefinition< @@ -84,11 +93,10 @@ export const dateHistogramOperation: OperationDefinition< input: 'field', priority: 5, // Highest priority level used operationParams: [{ name: 'interval', type: 'string', required: false }], - getErrorMessage: (layer, columnId, indexPattern) => - [ - ...(getInvalidFieldMessage(layer, columnId, indexPattern) || []), - getMultipleDateHistogramsErrorMessage(layer, columnId) || '', - ].filter(Boolean), + getErrorMessage: (layer, columnId, indexPattern) => [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getMultipleDateHistogramsErrorMessage(layer, columnId), + ], getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( (type === 'date' || type === 'date_range') && diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/__snapshots__/formula.test.tsx.snap b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/__snapshots__/formula.test.tsx.snap index 1eabd45dc86b6f..310101aec67f85 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/__snapshots__/formula.test.tsx.snap +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/__snapshots__/formula.test.tsx.snap @@ -32,6 +32,7 @@ Array [ } } />, + "uniqueId": "field_not_found", }, ] `; @@ -68,6 +69,7 @@ Array [ } } />, + "uniqueId": "field_not_found", }, ] `; @@ -104,6 +106,7 @@ Array [ } } />, + "uniqueId": "field_not_found", }, ] `; @@ -140,6 +143,7 @@ Array [ } } />, + "uniqueId": "field_not_found", }, ] `; @@ -182,6 +186,7 @@ Array [ } } />, + "uniqueId": "field_not_found", }, ] `; @@ -224,6 +229,7 @@ Array [ } } />, + "uniqueId": "field_not_found", }, ] `; diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.test.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.test.ts index db38e18d3bd194..f6278ed8a69f71 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.test.ts @@ -6,6 +6,13 @@ */ import type { FormBasedLayer } from '../../../../..'; +import { + INTERVAL_OP_MISSING_DATE_HISTOGRAM_TO_COMPUTE_INTERVAL, + INTERVAL_OP_MISSING_TIME_RANGE, + INTERVAL_OP_MISSING_UI_SETTINGS_HISTOGRAM_BAR_TARGET, + TIMERANGE_OP_DATAVIEW_NOT_TIME_BASED, + TIMERANGE_OP_MISSING_TIME_RANGE, +} from '../../../../../user_messages_ids'; import { createMockedIndexPattern } from '../../../mocks'; import { DateHistogramIndexPatternColumn } from '../date_histogram'; import { @@ -49,7 +56,10 @@ describe('context variables', () => { ) ).toEqual( expect.arrayContaining([ - 'Cannot compute an interval without a date histogram column configured', + { + uniqueId: INTERVAL_OP_MISSING_DATE_HISTOGRAM_TO_COMPUTE_INTERVAL, + message: 'Cannot compute an interval without a date histogram column configured', + }, ]) ); }); @@ -64,7 +74,14 @@ describe('context variables', () => { {}, 100 ) - ).toEqual(expect.arrayContaining(['The current time range interval is not available'])); + ).toEqual( + expect.arrayContaining([ + { + uniqueId: INTERVAL_OP_MISSING_TIME_RANGE, + message: 'The current time range interval is not available', + }, + ]) + ); }); it('should return error if no targetBar is passed over', () => { @@ -76,7 +93,14 @@ describe('context variables', () => { { fromDate: new Date().toISOString(), toDate: new Date().toISOString() }, {} ) - ).toEqual(expect.arrayContaining(['Missing "histogram:barTarget" value'])); + ).toEqual( + expect.arrayContaining([ + { + uniqueId: INTERVAL_OP_MISSING_UI_SETTINGS_HISTOGRAM_BAR_TARGET, + message: 'Missing "histogram:barTarget" value', + }, + ]) + ); }); it('should not return errors if all context is provided', () => { @@ -107,7 +131,7 @@ describe('context variables', () => { {}, 100 ) - ).toBeUndefined(); + ).toHaveLength(0); }); }); }); @@ -123,7 +147,14 @@ describe('context variables', () => { {}, 100 ) - ).toEqual(expect.arrayContaining(['The current time range interval is not available'])); + ).toEqual( + expect.arrayContaining([ + { + message: 'The current time range interval is not available', + uniqueId: TIMERANGE_OP_MISSING_TIME_RANGE, + }, + ]) + ); }); it('should return error if dataView is not time-based', () => { @@ -138,7 +169,14 @@ describe('context variables', () => { {}, 100 ) - ).toEqual(expect.arrayContaining(['The current time range interval is not available'])); + ).toEqual( + expect.arrayContaining([ + { + message: 'The current dataView is not time based', + uniqueId: TIMERANGE_OP_DATAVIEW_NOT_TIME_BASED, + }, + ]) + ); }); }); }); @@ -147,7 +185,7 @@ describe('context variables', () => { it('should return no error even without context', () => { expect( nowOperation.getErrorMessage!(createLayer('now'), 'col1', createMockedIndexPattern()) - ).toBeUndefined(); + ).toHaveLength(0); }); }); }); diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.tsx index 1509940486379a..3bb1935e24223b 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/context_variables.tsx @@ -24,9 +24,20 @@ import type { GenericIndexPatternColumn, } from '../../../../..'; import type { DateRange } from '../../../../../../common/types'; -import type { GenericOperationDefinition, OperationDefinition } from '..'; +import type { + FieldBasedOperationErrorMessage, + GenericOperationDefinition, + OperationDefinition, +} from '..'; import type { ReferenceBasedIndexPatternColumn } from '../column_types'; import { IndexPattern } from '../../../../../types'; +import { + INTERVAL_OP_MISSING_DATE_HISTOGRAM_TO_COMPUTE_INTERVAL, + INTERVAL_OP_MISSING_TIME_RANGE, + INTERVAL_OP_MISSING_UI_SETTINGS_HISTOGRAM_BAR_TARGET, + TIMERANGE_OP_DATAVIEW_NOT_TIME_BASED, + TIMERANGE_OP_MISSING_TIME_RANGE, +} from '../../../../../user_messages_ids'; // copied over from layer_helpers // TODO: split layer_helpers util into pure/non-pure functions to avoid issues with tests @@ -72,23 +83,25 @@ function getTimeRangeErrorMessages( _columnId: string, indexPattern: IndexPattern, dateRange?: DateRange | undefined -) { - const errors = []; +): FieldBasedOperationErrorMessage[] { + const errors: FieldBasedOperationErrorMessage[] = []; if (!indexPattern.timeFieldName) { - errors.push( - i18n.translate('xpack.lens.indexPattern.dateRange.dataViewNoTimeBased', { + errors.push({ + uniqueId: TIMERANGE_OP_DATAVIEW_NOT_TIME_BASED, + message: i18n.translate('xpack.lens.indexPattern.dateRange.dataViewNoTimeBased', { defaultMessage: 'The current dataView is not time based', - }) - ); + }), + }); } if (!dateRange) { - errors.push( - i18n.translate('xpack.lens.indexPattern.dateRange.noTimeRange', { + errors.push({ + uniqueId: TIMERANGE_OP_MISSING_TIME_RANGE, + message: i18n.translate('xpack.lens.indexPattern.dateRange.noTimeRange', { defaultMessage: 'The current time range interval is not available', - }) - ); + }), + }); } - return errors.length ? errors : undefined; + return errors; } export const timeRangeOperation = createContextValueBasedOperation({ @@ -107,7 +120,7 @@ export interface NowIndexPatternColumn extends ReferenceBasedIndexPatternColumn } function getNowErrorMessage() { - return undefined; + return []; } export const nowOperation = createContextValueBasedOperation({ @@ -132,37 +145,40 @@ function getIntervalErrorMessages( dateRange?: DateRange | undefined, operationDefinitionMap?: Record | undefined, targetBars?: number -) { - const errors = []; +): FieldBasedOperationErrorMessage[] { + const errors: FieldBasedOperationErrorMessage[] = []; if (!targetBars) { - errors.push( - i18n.translate('xpack.lens.indexPattern.interval.noTargetBars', { + errors.push({ + uniqueId: INTERVAL_OP_MISSING_UI_SETTINGS_HISTOGRAM_BAR_TARGET, + message: i18n.translate('xpack.lens.indexPattern.interval.noTargetBars', { defaultMessage: `Missing "{uiSettingVar}" value`, values: { uiSettingVar: UI_SETTINGS.HISTOGRAM_BAR_TARGET, }, - }) - ); + }), + }); } if (!dateRange) { - errors.push( - i18n.translate('xpack.lens.indexPattern.interval.noTimeRange', { + errors.push({ + uniqueId: INTERVAL_OP_MISSING_TIME_RANGE, + message: i18n.translate('xpack.lens.indexPattern.interval.noTimeRange', { defaultMessage: 'The current time range interval is not available', - }) - ); + }), + }); } if ( !Object.values(layer.columns).some((column) => isColumnOfType('date_histogram', column) ) ) { - errors.push( - i18n.translate('xpack.lens.indexPattern.interval.noDateHistogramColumn', { + errors.push({ + uniqueId: INTERVAL_OP_MISSING_DATE_HISTOGRAM_TO_COMPUTE_INTERVAL, + message: i18n.translate('xpack.lens.indexPattern.interval.noDateHistogramColumn', { defaultMessage: 'Cannot compute an interval without a date histogram column configured', - }) - ); + }), + }); } - return errors.length ? errors : undefined; + return errors; } export const intervalOperation = createContextValueBasedOperation({ diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/editor/formula_editor.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/editor/formula_editor.tsx index e182a3a84b9793..011b824fd17665 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/editor/formula_editor.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/editor/formula_editor.tsx @@ -232,12 +232,12 @@ export function FormulaEditor({ let errors: ErrorWrapper[] = []; - const { root, error } = tryToParse(text, visibleOperationsMap); - if (error) { - errors = [error]; - } else if (root) { + const parseResponse = tryToParse(text, visibleOperationsMap); + if ('error' in parseResponse) { + errors = [parseResponse.error]; + } else { const validationErrors = runASTValidation( - root, + parseResponse.root, layer, indexPattern, visibleOperationsMap, @@ -359,11 +359,11 @@ export function FormulaEditor({ visibleOperationsMap, uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET) ); - if (messages) { + if (messages.length) { const startPosition = offsetToRowColumn(text, locations[id].min); const endPosition = offsetToRowColumn(text, locations[id].max); newWarnings.push({ - message: messages.join(', '), + message: messages.map((e) => e.message).join(', '), startColumn: startPosition.column + 1, startLineNumber: startPosition.lineNumber, endColumn: endPosition.column + 1, 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 26703a2eebbf3d..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 @@ -21,6 +21,7 @@ import type { MovingAverageIndexPatternColumn } from '../calculations'; import { StaticValueIndexPatternColumn } from '../static_value'; import { getFilter } from '../helpers'; import { createOperationDefinitionMock } from './mocks/operation_mocks'; +import { FORMULA_LAYER_ONLY_STATIC_VALUES } from '../../../../../user_messages_ids'; jest.mock('../../layer_helpers', () => { return { @@ -62,7 +63,7 @@ const operationDefinitionMap: Record = { input: 'fullReference', operationParams: [{ name: 'window', type: 'number', required: true }], filterable: true, - getErrorMessage: jest.fn(() => ['mock error']), + getErrorMessage: jest.fn(() => []), buildColumn: ({ referenceIds }, columnsParams) => ({ label: 'moving_average', dataType: 'number', @@ -1044,7 +1045,7 @@ describe('[Lens] formula', () => { indexPattern = createMockedIndexPattern(); }); - it('returns undefined if count is passed without arguments', () => { + it('returns empty array if count is passed without arguments', () => { expect( formulaOperation.getErrorMessage!( getNewLayerWithFormula('count()'), @@ -1053,10 +1054,10 @@ describe('[Lens] formula', () => { undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); }); - it('returns undefined if count is passed with only a named argument', () => { + it('returns empty array if count is passed with only a named argument', () => { expect( formulaOperation.getErrorMessage!( getNewLayerWithFormula(`count(kql='*')`, false), @@ -1064,8 +1065,8 @@ describe('[Lens] formula', () => { indexPattern, undefined, operationDefinitionMap - ) - ).toEqual(undefined); + ).map((e) => e.message) + ).toHaveLength(0); }); it('returns a syntax error if the kql argument does not parse', () => { @@ -1076,7 +1077,7 @@ describe('[Lens] formula', () => { indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ `Expected "(", "{", value, whitespace but """ found. invalid: " @@ -1084,7 +1085,7 @@ invalid: " ]); }); - it('returns undefined if a field operation is passed with the correct first argument', () => { + it('returns empty array if a field operation is passed with the correct first argument', () => { expect( formulaOperation.getErrorMessage!( getNewLayerWithFormula('average(bytes)'), @@ -1093,7 +1094,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); // note that field names can be wrapped in quotes as well expect( formulaOperation.getErrorMessage!( @@ -1103,10 +1104,10 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); }); - it('returns undefined if a fullReference operation is passed with the correct first argument', () => { + it('returns empty array if a fullReference operation is passed with the correct first argument', () => { expect( formulaOperation.getErrorMessage!( getNewLayerWithFormula('derivative(average(bytes))'), @@ -1115,7 +1116,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); expect( formulaOperation.getErrorMessage!( @@ -1125,10 +1126,10 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); }); - it('returns undefined 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)'), @@ -1137,7 +1138,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); }); it('returns an error if field is used with no Lens wrapping operation', () => { @@ -1148,7 +1149,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The field bytes cannot be used without operation`]); expect( @@ -1158,7 +1159,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The operation add does not accept any field as argument`]); }); @@ -1180,7 +1181,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The Formula ${formula} cannot be parsed`]); } }); @@ -1233,7 +1234,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['Operation noFn not found']); } @@ -1247,7 +1248,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['Operations noFn, noFnTwo not found']); } }); @@ -1263,7 +1264,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['Operation formula not found']); } @@ -1277,7 +1278,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['Operation math not found']); } }); @@ -1301,7 +1302,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual( // some formulas may contain more errors expect.arrayContaining([ @@ -1321,8 +1322,8 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) - ).toEqual(undefined); + ).map((e) => e.message) + ).toHaveLength(0); }); it('returns an error if an operation with required parameters does not receive them', () => { @@ -1333,7 +1334,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ 'The operation moving_average in the Formula is missing the following parameters: window', ]); @@ -1345,7 +1346,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ 'The operation moving_average in the Formula is missing the following parameters: window', ]); @@ -1359,7 +1360,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['The operation average does not accept any parameter']); }); @@ -1380,7 +1381,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual( expect.arrayContaining([ expect.stringMatching( @@ -1411,7 +1412,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual( expect.arrayContaining([ expect.stringMatching( @@ -1430,7 +1431,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ 'The parameters for the operation moving_average in the Formula are of the wrong type: window', ]); @@ -1451,7 +1452,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); }); it('returns no error for a query edge case', () => { @@ -1472,11 +1473,11 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); } }); - it('returns an error for a query not wrapped in single quotes', () => { + it('returns an error for a query not wrapped in single quotes: %s', () => { const formulas = [ `count(kql="")`, `count(kql='")`, @@ -1510,7 +1511,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(expect.arrayContaining([expect.stringMatching(`Single quotes are required`)])); } }); @@ -1541,7 +1542,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The Formula ${formula} cannot be parsed`]); } }); @@ -1560,8 +1561,8 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) - ).toEqual(undefined); + ).map((e) => e.message) + ).toHaveLength(0); } }); @@ -1573,7 +1574,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['Use only one of kql= or lucene=, not both']); }); @@ -1586,7 +1587,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The first argument for ${fn} should be a field name. Found no field`]); } expect( @@ -1596,7 +1597,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The first argument for sum should be a field name. Found category.keyword: *`]); }); @@ -1609,7 +1610,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([`The first argument for ${fn} should be a operation name. Found no operation`]); } }); @@ -1642,6 +1643,7 @@ invalid: " ) ).toEqual([ { + uniqueId: FORMULA_LAYER_ONLY_STATIC_VALUES, message: 'A layer with only static values will not show results, use at least one dynamic metric', }, @@ -1657,7 +1659,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); }); it('returns no error if the formula contains comparison operator within the ifelse operation', () => { @@ -1674,7 +1676,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); } }); @@ -1693,7 +1695,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); } }); @@ -1712,7 +1714,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); } }); @@ -1734,7 +1736,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ `The return value type of the operation ${ errorFormula ?? formula @@ -1808,7 +1810,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toContain(errors[i](fn)); }); }); @@ -1829,7 +1831,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ `The return value type of the operation ${formula} is not supported in Formula`, `The operation ${fn} in the Formula is missing ${expectedCount} arguments: ${expectedArgs}`, @@ -1874,7 +1876,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual( indexReverseMap[expectedFail].map((i) => { const arg = tinymathFunctions[fn].positionalArguments[i]; @@ -1901,7 +1903,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([errorsWithSuggestions[i]]); }); }); @@ -1930,7 +1932,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the ${operation} operation`, ]); @@ -1951,7 +1953,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual([ `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the count operation`, `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the sum operation`, @@ -1977,7 +1979,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); } }); @@ -2002,7 +2004,7 @@ invalid: " undefined, operationDefinitionMap ) - ).toEqual(undefined); + ).toHaveLength(0); expect( formulaOperation.getErrorMessage!( @@ -2011,7 +2013,7 @@ invalid: " indexPattern, undefined, operationDefinitionMap - ) + ).map((e) => e.message) ).toEqual(['Operation operation_not_available not found']); }); }); 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 ee415ba9304b30..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 @@ -7,7 +7,6 @@ import { i18n } from '@kbn/i18n'; import { uniqBy } from 'lodash'; -import { nonNullable } from '../../../../../utils'; import type { BaseIndexPatternColumn, FieldBasedOperationErrorMessage, @@ -22,6 +21,7 @@ import { generateFormula } from './generate'; import { filterByVisibleOperation } from './util'; import { getManagedColumnsFrom } from '../../layer_helpers'; import { generateMissingFieldMessage, getFilter, isColumnFormatted } from '../helpers'; +import { FORMULA_LAYER_ONLY_STATIC_VALUES } from '../../../../../user_messages_ids'; const defaultLabel = i18n.translate('xpack.lens.indexPattern.formulaLabel', { defaultMessage: 'Formula', @@ -71,17 +71,17 @@ export const formulaOperation: OperationDefinition message).map(({ type, message, extraInfo }) => - type === 'missingField' && extraInfo?.missingFields - ? generateMissingFieldMessage(extraInfo.missingFields, columnId) - : message + return uniqBy(errors, ({ message }) => message).map(({ id, message, meta }) => + id === 'missingField' && meta.fieldList.length > 0 + ? generateMissingFieldMessage(meta.fieldList, columnId) // TODO: add missing field List + : { + uniqueId: id, + message, + } ); } @@ -103,21 +106,18 @@ 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 []; + ) ?? [] + ); }) - .filter(nonNullable) // dedup messages with the same content .reduce((memo, message) => { memo.add(message); @@ -142,6 +142,7 @@ export const formulaOperation: OperationDefinition = ValidationErrors[K]['type']; - -export interface ErrorWrapper { - type?: ErrorTypes; // TODO - make this required? +export type ErrorWrapper = ValidationErrors & { message: string; locations: TinymathLocation[]; severity?: 'error' | 'warning'; - extraInfo?: { missingFields: string[] }; -} +}; const DEFAULT_RETURN_TYPE = getTypeI18n('number'); @@ -172,7 +94,10 @@ export function hasInvalidOperations( }; } -export const getRawQueryValidationError = (text: string, operations: Record) => { +export const getRawQueryValidationError = ( + text: string, + operations: Record +): (InvalidQueryError & { message: string }) | undefined => { // try to extract the query context here const singleLine = text.split('\n').join(''); const languagesRegexp = /(kql|lucene)/; @@ -183,7 +108,7 @@ export const getRawQueryValidationError = (text: string, operations: Record languagesRegexp.test(subArg)) ); const [kqlQueries, luceneQueries] = partition(flattenArgs, (arg) => /kql/.test(arg)); - const errors = []; for (const kqlQuery of kqlQueries) { - const result = validateQueryQuotes(kqlQuery, 'kql'); - if (result) { - errors.push(result); + const message = validateQueryQuotes(kqlQuery, 'kql'); + if (message) { + return { + id: 'invalidQuery', + meta: { + language: 'kql', + }, + message, + }; } } for (const luceneQuery of luceneQueries) { - const result = validateQueryQuotes(luceneQuery, 'lucene'); - if (result) { - errors.push(result); + const message = validateQueryQuotes(luceneQuery, 'lucene'); + if (message) { + return { + id: 'invalidQuery', + meta: { + language: 'lucene', + }, + message, + }; } } - return errors.length ? errors : undefined; }; const validateQueryQuotes = (rawQuery: string, language: 'kql' | 'lucene') => { @@ -233,8 +168,11 @@ export const getQueryValidationError = ( { value: query, name: language, text }: TinymathNamedArgument, indexPattern: IndexPattern ): string | undefined => { + if (language !== 'kql' && language !== 'lucene') { + return; + } // check if the raw argument has the minimal requirements - const result = validateQueryQuotes(text, language as 'kql' | 'lucene'); + const result = validateQueryQuotes(text, language); // forward the error here is ok? if (result) { return result; @@ -251,172 +189,309 @@ export const getQueryValidationError = ( } }; -function getMessageFromId({ - messageId, - values, - locations, -}: { - messageId: K; - values: ErrorValues; - locations: TinymathLocation[]; -}): ErrorWrapper { - let message: string; - // Use a less strict type instead of doing a typecast on each message type - const out = values as unknown as Record; - switch (messageId) { +function getMessageFromId( + { id, meta }: ValidationErrors, + locations: TinymathLocation[] +): ErrorWrapper { + switch (id) { + case 'invalidQuery': + return { + id, + meta, + locations, + // 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: { language: meta.language }, + }), + }; case 'wrongFirstArgument': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationWrongFirstArgument', { - defaultMessage: - 'The first argument for {operation} should be a {type} name. Found {argument}', - values: { operation: out.operation, type: out.type, argument: out.argument }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationWrongFirstArgument', { + defaultMessage: + 'The first argument for {operation} should be a {type} name. Found {argument}', + values: { operation: meta.operation, type: meta.type, argument: meta.argument }, + }), + }; case 'shouldNotHaveField': - message = i18n.translate('xpack.lens.indexPattern.formulaFieldNotRequired', { - defaultMessage: 'The operation {operation} does not accept any field as argument', - values: { operation: out.operation }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaFieldNotRequired', { + defaultMessage: 'The operation {operation} does not accept any field as argument', + values: { operation: meta.operation }, + }), + }; case 'cannotAcceptParameter': - message = i18n.translate('xpack.lens.indexPattern.formulaParameterNotRequired', { - defaultMessage: 'The operation {operation} does not accept any parameter', - values: { operation: out.operation }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaParameterNotRequired', { + defaultMessage: 'The operation {operation} does not accept any parameter', + values: { operation: meta.operation }, + }), + }; case 'missingParameter': - message = i18n.translate('xpack.lens.indexPattern.formulaExpressionNotHandled', { - defaultMessage: - 'The operation {operation} in the Formula is missing the following parameters: {params}', - values: { operation: out.operation, params: out.params }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaExpressionNotHandled', { + defaultMessage: + 'The operation {operation} in the Formula is missing the following parameters: {params}', + values: { operation: meta.operation, params: meta.params }, + }), + }; case 'wrongTypeParameter': - message = i18n.translate('xpack.lens.indexPattern.formulaExpressionWrongType', { - defaultMessage: - 'The parameters for the operation {operation} in the Formula are of the wrong type: {params}', - values: { operation: out.operation, params: out.params }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaExpressionWrongType', { + defaultMessage: + 'The parameters for the operation {operation} in the Formula are of the wrong type: {params}', + values: { operation: meta.operation, params: meta.params }, + }), + }; case 'wrongTypeArgument': - message = i18n.translate('xpack.lens.indexPattern.formulaExpressionWrongTypeArgument', { - defaultMessage: - 'The {name} argument for the operation {operation} in the Formula is of the wrong type: {type} instead of {expectedType}', - values: { - operation: out.operation, - name: out.name, - type: out.type, - expectedType: out.expectedType, - }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaExpressionWrongTypeArgument', { + defaultMessage: + 'The {name} argument for the operation {operation} in the Formula is of the wrong type: {type} instead of {expectedType}', + values: { + operation: meta.operation, + name: meta.name, + type: meta.type, + expectedType: meta.expectedType, + }, + }), + }; case 'duplicateArgument': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationDuplicateParams', { - defaultMessage: - 'The parameters for the operation {operation} have been declared multiple times: {params}', - values: { operation: out.operation, params: out.params }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationDuplicateParams', { + defaultMessage: + 'The parameters for the operation {operation} have been declared multiple times: {params}', + values: { operation: meta.operation, params: meta.params }, + }), + }; case 'missingField': - message = i18n.translate('xpack.lens.indexPattern.formulaFieldNotFound', { - defaultMessage: - '{variablesLength, plural, one {Field} other {Fields}} {variablesList} not found', - values: { variablesLength: out.variablesLength, variablesList: out.variablesList }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaFieldNotFound', { + defaultMessage: + '{missingFieldCount, plural, one {Field} other {Fields}} {missingFieldList} not found', + values: { + missingFieldCount: meta.fieldList.length, + missingFieldList: meta.fieldList.join(', '), + }, + }), + }; case 'missingOperation': - message = i18n.translate('xpack.lens.indexPattern.operationsNotFound', { - defaultMessage: - '{operationLength, plural, one {Operation} other {Operations}} {operationsList} not found', - values: { operationLength: out.operationLength, operationsList: out.operationsList }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.operationsNotFound', { + defaultMessage: + '{operationLength, plural, one {Operation} other {Operations}} {operationsList} not found', + values: { operationLength: meta.operationLength, operationsList: meta.operationsList }, + }), + }; case 'fieldWithNoOperation': - message = i18n.translate('xpack.lens.indexPattern.fieldNoOperation', { - defaultMessage: 'The field {field} cannot be used without operation', - values: { field: out.field }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.fieldNoOperation', { + defaultMessage: 'The field {field} cannot be used without operation', + values: { field: meta.field }, + }), + }; case 'failedParsing': - message = i18n.translate('xpack.lens.indexPattern.formulaExpressionParseError', { - defaultMessage: 'The Formula {expression} cannot be parsed', - values: { expression: out.expression }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaExpressionParseError', { + defaultMessage: 'The Formula {expression} cannot be parsed', + values: { expression: meta.expression }, + }), + }; case 'tooManyArguments': - message = i18n.translate('xpack.lens.indexPattern.formulaWithTooManyArguments', { - defaultMessage: 'The operation {operation} has too many arguments', - values: { operation: out.operation }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaWithTooManyArguments', { + defaultMessage: 'The operation {operation} has too many arguments', + values: { operation: meta.operation }, + }), + }; case 'missingMathArgument': - message = i18n.translate('xpack.lens.indexPattern.formulaMathMissingArgument', { - defaultMessage: - 'The operation {operation} in the Formula is missing {count} arguments: {params}', - values: { operation: out.operation, count: out.count, params: out.params }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaMathMissingArgument', { + defaultMessage: + 'The operation {operation} in the Formula is missing {count} arguments: {params}', + values: { operation: meta.operation, count: meta.count, params: meta.params }, + }), + }; case 'tooManyQueries': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationDoubleQueryError', { - defaultMessage: 'Use only one of kql= or lucene=, not both', - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationDoubleQueryError', { + defaultMessage: 'Use only one of kql= or lucene=, not both', + }), + }; case 'tooManyFirstArguments': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationTooManyFirstArguments', { - defaultMessage: - 'The operation {operation} in the Formula requires a {supported, plural, one {single} other {supported}} {type}, found: {text}', - values: { - operation: out.operation, - text: out.text, - type: out.type, - supported: out.supported || 1, - }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationTooManyFirstArguments', { + defaultMessage: + 'The operation {operation} in the Formula requires a {supported, plural, one {single} other {supported}} {type}, found: {text}', + values: { + operation: meta.operation, + text: meta.text, + type: meta.type, + supported: meta.supported || 1, + }, + }), + }; case 'wrongArgument': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationwrongArgument', { - defaultMessage: - 'The operation {operation} in the Formula does not support {type} parameters, found: {text}', - values: { operation: out.operation, text: out.text, type: out.type }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationwrongArgument', { + defaultMessage: + 'The operation {operation} in the Formula does not support {type} parameters, found: {text}', + values: { operation: meta.operation, text: meta.text, type: meta.type }, + }), + }; case 'wrongReturnedType': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationWrongReturnedType', { - defaultMessage: 'The return value type of the operation {text} is not supported in Formula', - values: { text: out.text }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationWrongReturnedType', { + defaultMessage: + 'The return value type of the operation {text} is not supported in Formula', + values: { text: meta.text }, + }), + }; case 'filtersTypeConflict': - message = i18n.translate('xpack.lens.indexPattern.formulaOperationFiltersTypeConflicts', { - defaultMessage: - 'The Formula filter of type "{outerType}" is not compatible with the inner filter of type "{innerType}" from the {operation} operation', - values: { operation: out.operation, outerType: out.outerType, innerType: out.innerType }, - }); - break; + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaOperationFiltersTypeConflicts', { + defaultMessage: + 'The Formula filter of type "{outerType}" is not compatible with the inner filter of type "{innerType}" from the {operation} operation', + values: { + operation: meta.operation, + outerType: meta.outerType, + innerType: meta.innerType, + }, + }), + }; case 'useAlternativeFunction': - message = i18n.translate('xpack.lens.indexPattern.formulaUseAlternative', { - defaultMessage: `The operation {operation} in the Formula is missing the {params} argument: use the {alternativeFn} operation instead`, - values: { operation: out.operation, params: out.params, alternativeFn: out.alternativeFn }, - }); - break; - // case 'mathRequiresFunction': - // message = i18n.translate('xpack.lens.indexPattern.formulaMathRequiresFunctionLabel', { - // defaultMessage; 'The function {name} requires an Elasticsearch function', - // values: { ...values }, - // }); - // break; - default: - message = 'no Error found'; - break; - } + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.formulaUseAlternative', { + defaultMessage: `The operation {operation} in the Formula is missing the {params} argument: use the {alternativeFn} operation instead`, + values: { + operation: meta.operation, + params: meta.params, + alternativeFn: meta.alternativeFn, + }, + }), + }; + case REASON_IDS.missingTimerange: + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.absoluteMissingTimeRange', { + defaultMessage: 'Invalid time shift. No time range found as reference', + }), + }; + case REASON_IDS.invalidDate: + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.absoluteInvalidDate', { + defaultMessage: 'Invalid time shift. The date is not of the correct format', + }), + }; + case REASON_IDS.shiftAfterTimeRange: + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.absoluteAfterTimeRange', { + defaultMessage: 'Invalid time shift. The provided date is after the current time range', + }), + }; - return { type: messageId, message, locations }; + case REASON_IDS.notAbsoluteTimeShift: + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.notAbsoluteTimeShift', { + defaultMessage: 'Invalid time shift.', + }), + }; + case 'invalidTimeShift': + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.invalidTimeShift', { + defaultMessage: + 'Invalid time shift. Enter positive integer amount followed by one of the units s, m, h, d, w, M, y. For example 3h for 3 hours', + }), + }; + case 'invalidReducedTimeRange': + return { + id, + meta, + locations, + message: i18n.translate('xpack.lens.indexPattern.invalidReducedTimeRange', { + defaultMessage: + 'Invalid reduced time range. Enter positive integer amount followed by one of the units s, m, h, d, w, M, y. For example 3h for 3 hours', + }), + }; + } } export function tryToParse( formula: string, operations: Record -): { root: TinymathAST; error: null } | { root: null; error: ErrorWrapper } { - let root; +): { root: TinymathAST } | { error: ErrorWrapper } { + let root: TinymathAST; try { root = parse(formula); } catch (e) { @@ -426,21 +501,13 @@ export function tryToParse( // * if the formula contains at least one existing operation, check for query problems const maybeQueryProblems = getRawQueryValidationError(formula, operations); if (maybeQueryProblems) { - // need to emulate an error shape here - return { root: null, error: { message: maybeQueryProblems[0], locations: [] } }; + return { error: { ...maybeQueryProblems, locations: [] } }; } return { - root: null, - error: getMessageFromId({ - messageId: 'failedParsing', - values: { - expression: formula, - }, - locations: [], - }), + error: getMessageFromId({ id: 'failedParsing', meta: { expression: formula } }, []), }; } - return { root, error: null }; + return { root }; } export function runASTValidation( @@ -462,13 +529,7 @@ function checkVariableEdgeCases(ast: TinymathAST, missingVariables: Set) const invalidVariableErrors = []; if (isObject(ast) && ast.type === 'variable' && !missingVariables.has(ast.value)) { invalidVariableErrors.push( - getMessageFromId({ - messageId: 'fieldWithNoOperation', - values: { - field: ast.value, - }, - locations: [ast.location], - }) + getMessageFromId({ id: 'fieldWithNoOperation', meta: { field: ast.value } }, [ast.location]) ); } return invalidVariableErrors; @@ -485,14 +546,16 @@ function checkMissingVariableOrFunctions( if (missingOperations.names.length) { missingErrors.push( - getMessageFromId({ - messageId: 'missingOperation', - values: { - operationLength: missingOperations.names.length, - operationsList: missingOperations.names.join(', '), + getMessageFromId( + { + id: 'missingOperation', + meta: { + operationLength: missingOperations.names.length, + operationsList: missingOperations.names.join(', '), + }, }, - locations: missingOperations.locations, - }) + missingOperations.locations + ) ); } const missingVariables = findVariables(ast).filter( @@ -503,15 +566,15 @@ function checkMissingVariableOrFunctions( // need to check the arguments here: check only strings for now if (missingVariables.length) { missingErrors.push({ - ...getMessageFromId({ - messageId: 'missingField', - values: { - variablesLength: missingVariables.length, - variablesList: missingVariables.map(({ value }) => value).join(', '), + ...getMessageFromId( + { + id: 'missingField', + meta: { + fieldList: [...new Set(missingVariables.map(({ value }) => value))], + }, }, - locations: missingVariables.map(({ location }) => location), - }), - extraInfo: { missingFields: [...new Set(missingVariables.map(({ value }) => value))] }, + missingVariables.map(({ location }) => location) + ), }); } const invalidVariableErrors = checkVariableEdgeCases( @@ -521,27 +584,6 @@ function checkMissingVariableOrFunctions( return [...missingErrors, ...invalidVariableErrors]; } -function getAbsoluteTimeShiftErrorMessage(reason: REASON_ID_TYPES) { - switch (reason) { - case REASON_IDS.missingTimerange: - return i18n.translate('xpack.lens.indexPattern.absoluteMissingTimeRange', { - defaultMessage: 'Invalid time shift. No time range found as reference', - }); - case REASON_IDS.invalidDate: - return i18n.translate('xpack.lens.indexPattern.absoluteInvalidDate', { - defaultMessage: 'Invalid time shift. The date is not of the correct format', - }); - case REASON_IDS.shiftAfterTimeRange: - return i18n.translate('xpack.lens.indexPattern.absoluteAfterTimeRange', { - defaultMessage: 'Invalid time shift. The provided date is after the current time range', - }); - case REASON_IDS.notAbsoluteTimeShift: - return i18n.translate('xpack.lens.indexPattern.notAbsoluteTimeShift', { - defaultMessage: 'Invalid time shift.', - }); - } -} - function getQueryValidationErrors( namedArguments: TinymathNamedArgument[] | undefined, indexPattern: IndexPattern, @@ -550,13 +592,14 @@ function getQueryValidationErrors( const errors: ErrorWrapper[] = []; (namedArguments ?? []).forEach((arg) => { if (arg.name === 'kql' || arg.name === 'lucene') { - const message = getQueryValidationError( - arg as TinymathNamedArgument & { name: 'kql' | 'lucene' }, - indexPattern - ); + const message = getQueryValidationError(arg, indexPattern); if (message) { errors.push({ + id: 'invalidQuery', message, + meta: { + language: arg.name, + }, locations: [arg.location], }); } @@ -567,7 +610,7 @@ function getQueryValidationErrors( if (parsedShift === 'invalid') { if (isAbsoluteTimeShift(arg.value)) { // try to parse as absolute time shift - const error = validateAbsoluteTimeShift( + const errorId = validateAbsoluteTimeShift( arg.value, dateRange ? { @@ -576,33 +619,18 @@ function getQueryValidationErrors( } : undefined ); - if (error) { - errors.push({ - message: getAbsoluteTimeShiftErrorMessage(error), - locations: [arg.location], - }); + if (errorId) { + errors.push(getMessageFromId({ id: errorId }, [arg.location])); } } else { - errors.push({ - message: i18n.translate('xpack.lens.indexPattern.invalidTimeShift', { - defaultMessage: - 'Invalid time shift. Enter positive integer amount followed by one of the units s, m, h, d, w, M, y. For example 3h for 3 hours', - }), - locations: [arg.location], - }); + errors.push(getMessageFromId({ id: 'invalidTimeShift' }, [arg.location])); } } } if (arg.name === 'reducedTimeRange') { const parsedReducedTimeRange = parseTimeShift(arg.value || ''); if (parsedReducedTimeRange === 'invalid' || parsedReducedTimeRange === 'previous') { - errors.push({ - message: i18n.translate('xpack.lens.indexPattern.invalidReducedTimeRange', { - defaultMessage: - 'Invalid reduced time range. Enter positive integer amount followed by one of the units s, m, h, d, w, M, y. For example 3h for 3 hours', - }), - locations: [arg.location], - }); + errors.push(getMessageFromId({ id: 'invalidReducedTimeRange' }, [arg.location])); } } }); @@ -622,8 +650,8 @@ function validateFiltersArguments( | OperationDefinition, namedArguments: TinymathNamedArgument[] | undefined, globalFilters?: Query -) { - const errors = []; +): ErrorWrapper[] { + const errors: ErrorWrapper[] = []; const { conflicts, innerType, outerType } = hasFiltersConflicts( nodeOperation, namedArguments, @@ -632,15 +660,17 @@ function validateFiltersArguments( if (conflicts) { if (innerType && outerType) { errors.push( - getMessageFromId({ - messageId: 'filtersTypeConflict', - values: { - operation: node.name, - innerType, - outerType, + getMessageFromId( + { + id: 'filtersTypeConflict', + meta: { + operation: node.name, + innerType, + outerType, + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } } @@ -655,60 +685,59 @@ function validateNameArguments( namedArguments: TinymathNamedArgument[] | undefined, indexPattern: IndexPattern, dateRange: DateRange | undefined -) { - const errors = []; +): ErrorWrapper[] { + const errors: ErrorWrapper[] = []; const missingParams = getMissingParams(nodeOperation, namedArguments); if (missingParams.length) { errors.push( - getMessageFromId({ - messageId: 'missingParameter', - values: { - operation: node.name, - params: missingParams.map(({ name }) => name).join(', '), + getMessageFromId( + { + id: 'missingParameter', + meta: { + operation: node.name, + params: missingParams.map(({ name }) => name).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } const wrongTypeParams = getWrongTypeParams(nodeOperation, namedArguments); if (wrongTypeParams.length) { errors.push( - getMessageFromId({ - messageId: 'wrongTypeParameter', - values: { - operation: node.name, - params: wrongTypeParams.map(({ name }) => name).join(', '), + getMessageFromId( + { + id: 'wrongTypeParameter', + meta: { + operation: node.name, + params: wrongTypeParams.map(({ name }) => name).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } const duplicateParams = getDuplicateParams(namedArguments); if (duplicateParams.length) { errors.push( - getMessageFromId({ - messageId: 'duplicateArgument', - values: { - operation: node.name, - params: duplicateParams.join(', '), + getMessageFromId( + { + id: 'duplicateArgument', + meta: { + operation: node.name, + params: duplicateParams.join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } const queryValidationErrors = getQueryValidationErrors(namedArguments, indexPattern, dateRange); - if (queryValidationErrors.length) { - errors.push(...queryValidationErrors); - } + errors.push(...queryValidationErrors); + const hasTooManyQueries = checkSingleQuery(namedArguments); if (hasTooManyQueries) { - errors.push( - getMessageFromId({ - messageId: 'tooManyQueries', - values: {}, - locations: getNodeLocation(node), - }) - ); + errors.push(getMessageFromId({ id: 'tooManyQueries' }, getNodeLocation(node))); } return errors; } @@ -721,13 +750,7 @@ function checkTopNodeReturnType(ast: TinymathAST): ErrorWrapper[] { (tinymathFunctions[ast.name]?.outputType || DEFAULT_RETURN_TYPE) !== DEFAULT_RETURN_TYPE ) { return [ - getMessageFromId({ - messageId: 'wrongReturnedType', - values: { - text: ast.text, - }, - locations: getNodeLocation(ast), - }), + getMessageFromId({ id: 'wrongReturnedType', meta: { text: ast.text } }, getNodeLocation(ast)), ]; } return []; @@ -768,36 +791,40 @@ function runFullASTValidation( if (!isArgumentValidType(firstArg, 'variable')) { if (isMathNode(firstArg)) { errors.push( - getMessageFromId({ - messageId: 'wrongFirstArgument', - values: { - operation: node.name, - type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { - defaultMessage: 'field', - }), - argument: `math operation`, + getMessageFromId( + { + id: 'wrongFirstArgument', + meta: { + operation: node.name, + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), + argument: `math operation`, + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } else { if (shouldHaveFieldArgument(node)) { errors.push( - getMessageFromId({ - messageId: 'wrongFirstArgument', - values: { - operation: node.name, - type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { - defaultMessage: 'field', - }), - argument: - getValueOrName(firstArg) || - i18n.translate('xpack.lens.indexPattern.formulaNoFieldForOperation', { - defaultMessage: 'no field', + getMessageFromId( + { + id: 'wrongFirstArgument', + meta: { + operation: node.name, + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', }), + argument: + getValueOrName(firstArg) || + i18n.translate('xpack.lens.indexPattern.formulaNoFieldForOperation', { + defaultMessage: 'no field', + }), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } } @@ -824,13 +851,10 @@ function runFullASTValidation( } if (!canHaveParams(nodeOperation) && namedArguments.length) { errors.push( - getMessageFromId({ - messageId: 'cannotAcceptParameter', - values: { - operation: node.name, - }, - locations: getNodeLocation(node), - }) + getMessageFromId( + { id: 'cannotAcceptParameter', meta: { operation: node.name } }, + getNodeLocation(node) + ) ); } else { const argumentsErrors = validateNameArguments( @@ -862,21 +886,23 @@ function runFullASTValidation( // First field has a special handling if (isFirstArgumentNotValid) { errors.push( - getMessageFromId({ - messageId: 'wrongFirstArgument', - values: { - operation: node.name, - type: i18n.translate('xpack.lens.indexPattern.formulaOperationValue', { - defaultMessage: 'operation', - }), - argument: - getValueOrName(firstArg) || - i18n.translate('xpack.lens.indexPattern.formulaNoOperation', { - defaultMessage: 'no operation', + getMessageFromId( + { + id: 'wrongFirstArgument', + meta: { + operation: node.name, + type: i18n.translate('xpack.lens.indexPattern.formulaOperationValue', { + defaultMessage: 'operation', }), + argument: + getValueOrName(firstArg) || + i18n.translate('xpack.lens.indexPattern.formulaNoOperation', { + defaultMessage: 'no operation', + }), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } // Check for multiple function passed @@ -896,13 +922,15 @@ function runFullASTValidation( if (!canHaveParams(nodeOperation) && namedArguments.length) { errors.push( - getMessageFromId({ - messageId: 'cannotAcceptParameter', - values: { - operation: node.name, + getMessageFromId( + { + id: 'cannotAcceptParameter', + meta: { + operation: node.name, + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } else { // check for fields passed at any position @@ -1075,27 +1103,26 @@ export function validateMathNodes( if (!node.args.length) { // we can stop here return errors.push( - getMessageFromId({ - messageId: 'missingMathArgument', - values: { - operation: node.name, - count: mandatoryArguments.length, - params: mandatoryArguments.map(({ name }) => name).join(', '), + getMessageFromId( + { + id: 'missingMathArgument', + meta: { + operation: node.name, + count: mandatoryArguments.length, + params: mandatoryArguments.map(({ name }) => name).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } if (node.args.length > positionalArguments.length) { errors.push( - getMessageFromId({ - messageId: 'tooManyArguments', - values: { - operation: node.name, - }, - locations: getNodeLocation(node), - }) + getMessageFromId( + { id: 'tooManyArguments', meta: { operation: node.name } }, + getNodeLocation(node) + ) ); } @@ -1108,13 +1135,10 @@ export function validateMathNodes( }); if (hasFieldAsArgument) { errors.push( - getMessageFromId({ - messageId: 'shouldNotHaveField', - values: { - operation: node.name, - }, - locations: getNodeLocation(node), - }) + getMessageFromId( + { id: 'shouldNotHaveField', meta: { operation: node.name } }, + getNodeLocation(node) + ) ); } @@ -1133,30 +1157,34 @@ export function validateMathNodes( if (missingArgsWithoutAlternative.length) { errors.push( - getMessageFromId({ - messageId: 'missingMathArgument', - values: { - operation: node.name, - count: mandatoryArguments.length - node.args.length, - params: missingArgsWithoutAlternative.map(({ name }) => name).join(', '), + getMessageFromId( + { + id: 'missingMathArgument', + meta: { + operation: node.name, + count: mandatoryArguments.length - node.args.length, + params: missingArgsWithoutAlternative.map(({ name }) => name).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } if (missingArgsWithAlternatives.length) { // pick only the first missing argument alternative const [firstArg] = missingArgsWithAlternatives; errors.push( - getMessageFromId({ - messageId: 'useAlternativeFunction', - values: { - operation: node.name, - params: firstArg.name, - alternativeFn: firstArg.alternativeWhenMissing, + getMessageFromId( + { + id: 'useAlternativeFunction', + meta: { + operation: node.name, + params: firstArg.name, + alternativeFn: firstArg.alternativeWhenMissing, + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } } @@ -1174,16 +1202,18 @@ export function validateMathNodes( for (const wrongTypeArgumentIndex of wrongTypeArgumentIndexes) { const arg = node.args[wrongTypeArgumentIndex]; errors.push( - getMessageFromId({ - messageId: 'wrongTypeArgument', - values: { - operation: node.name, - name: positionalArguments[wrongTypeArgumentIndex].name, - type: getArgumentType(arg, operations) || DEFAULT_RETURN_TYPE, - expectedType: positionalArguments[wrongTypeArgumentIndex].type || '', + getMessageFromId( + { + id: 'wrongTypeArgument', + meta: { + operation: node.name, + name: positionalArguments[wrongTypeArgumentIndex].name, + type: getArgumentType(arg, operations) || DEFAULT_RETURN_TYPE, + expectedType: positionalArguments[wrongTypeArgumentIndex].type || '', + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } }); @@ -1209,46 +1239,52 @@ function validateFieldArguments( const errors = []; if (isFieldOperation && (fields.length > 1 || (fields.length === 1 && fields[0] !== firstArg))) { errors.push( - getMessageFromId({ - messageId: 'tooManyFirstArguments', - values: { - operation: node.name, - type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { - defaultMessage: 'field', - }), - supported: 1, - text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '), + getMessageFromId( + { + id: 'tooManyFirstArguments', + meta: { + operation: node.name, + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), + supported: 1, + text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } if (isFieldOperation && fields.length === 1 && fields[0] === firstArg) { if (returnedType === 'ordinal') { errors.push( - getMessageFromId({ - messageId: 'wrongReturnedType', - values: { - text: node.text ?? `${node.name}(${getValueOrName(firstArg)})`, + getMessageFromId( + { + id: 'wrongReturnedType', + meta: { + text: node.text ?? `${node.name}(${getValueOrName(firstArg)})`, + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } } if (!isFieldOperation && fields.length) { errors.push( - getMessageFromId({ - messageId: 'wrongArgument', - values: { - operation: node.name, - text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '), - type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { - defaultMessage: 'field', - }), + getMessageFromId( + { + id: 'wrongArgument', + meta: { + operation: node.name, + text: (fields as TinymathVariable[]).map(({ text }) => text).join(', '), + type: i18n.translate('xpack.lens.indexPattern.formulaFieldValue', { + defaultMessage: 'field', + }), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } return errors; @@ -1270,30 +1306,34 @@ function validateFunctionArguments( if (esOperations.length > requiredFunctions) { if (isFieldOperation) { errors.push( - getMessageFromId({ - messageId: 'wrongArgument', - values: { - operation: node.name, - text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), - type: i18n.translate('xpack.lens.indexPattern.formulaMetricValue', { - defaultMessage: 'metric', - }), + getMessageFromId( + { + id: 'wrongArgument', + meta: { + operation: node.name, + text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + type: i18n.translate('xpack.lens.indexPattern.formulaMetricValue', { + defaultMessage: 'metric', + }), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } else { errors.push( - getMessageFromId({ - messageId: 'tooManyFirstArguments', - values: { - operation: node.name, - type, - supported: requiredFunctions, - text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + getMessageFromId( + { + id: 'tooManyFirstArguments', + meta: { + operation: node.name, + type, + supported: requiredFunctions, + text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } } @@ -1303,15 +1343,17 @@ function validateFunctionArguments( ((!firstArgValidation && mathOperations.length) || mathOperations.length > 1) ) { errors.push( - getMessageFromId({ - messageId: 'wrongArgument', - values: { - operation: node.name, - type, - text: (mathOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + getMessageFromId( + { + id: 'wrongArgument', + meta: { + operation: node.name, + type, + text: (mathOperations as TinymathFunction[]).map(({ text }) => text).join(', '), + }, }, - locations: getNodeLocation(node), - }) + getNodeLocation(node) + ) ); } return errors; diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation_errors.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation_errors.ts new file mode 100644 index 00000000000000..a16b214f7f1571 --- /dev/null +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/validation_errors.ts @@ -0,0 +1,183 @@ +/* + * 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. + */ + +import { REASON_IDS } from '@kbn/data-plugin/common'; + +export interface MissingFieldError { + id: 'missingField'; + meta: { fieldList: string[] }; +} + +export interface MissingOperationError { + id: 'missingOperation'; + meta: { operationLength: number; operationsList: string }; +} + +export interface MissingParameterError { + id: 'missingParameter'; + meta: { operation: string; params: string }; +} + +export interface WrongTypeParameterError { + id: 'wrongTypeParameter'; + meta: { operation: string; params: string }; +} + +export interface WrongTypeArgumentError { + id: 'wrongTypeArgument'; + meta: { operation: string; name: string; type: string; expectedType: string }; +} + +export interface WrongFirstArgument { + id: 'wrongFirstArgument'; + meta: { operation: string; type: string; argument: string | number }; +} + +export interface CannotAcceptParameterError { + id: 'cannotAcceptParameter'; + meta: { operation: string }; +} + +export interface ShouldNotHaveFieldError { + id: 'shouldNotHaveField'; + meta: { operation: string }; +} + +export interface TooManyArgumentsError { + id: 'tooManyArguments'; + meta: { operation: string }; +} + +export interface FieldWithNoOperationError { + id: 'fieldWithNoOperation'; + meta: { field: string }; +} + +export interface FailedParsingError { + id: 'failedParsing'; + meta: { expression: string }; +} + +export interface DuplicateArgumentError { + id: 'duplicateArgument'; + meta: { operation: string; params: string }; +} + +export interface DuplicateArgumentError { + id: 'duplicateArgument'; + meta: { operation: string; params: string }; +} + +export interface MissingMathArgumentError { + id: 'missingMathArgument'; + meta: { operation: string; count: number; params: string }; +} + +export interface TooManyQueriesError { + id: 'tooManyQueries'; + meta?: {}; +} + +export interface TooManyFirstArgumentsError { + id: 'tooManyFirstArguments'; + meta: { + operation: string; + type: string; + text: string; + supported?: number; + }; +} + +export interface WrongArgumentError { + id: 'wrongArgument'; + meta: { operation: string; text: string; type: string }; +} + +export interface WrongReturnedTypeError { + id: 'wrongReturnedType'; + meta: { text: string }; +} + +export interface FiltersTypeConflictError { + id: 'filtersTypeConflict'; + meta: { operation: string; outerType: string; innerType: string }; +} + +export interface UseAlternativeFunctionError { + id: 'useAlternativeFunction'; + meta: { + operation: string; + params: string; + alternativeFn: string; + }; +} + +export interface InvalidQueryError { + id: 'invalidQuery'; + meta: { + language: 'kql' | 'lucene'; + }; +} + +export interface MissingTimerangeError { + id: typeof REASON_IDS.missingTimerange; + meta?: {}; +} + +export interface InvalidDateError { + id: typeof REASON_IDS.invalidDate; + meta?: {}; +} + +export interface ShiftAfterTimeRangeError { + id: typeof REASON_IDS.shiftAfterTimeRange; + meta?: {}; +} + +export interface NotAbsoluteTimeShiftError { + id: typeof REASON_IDS.notAbsoluteTimeShift; + meta?: {}; +} + +export interface InvalidTimeShift { + id: 'invalidTimeShift'; + meta?: {}; +} + +export interface InvalidReducedTimeRange { + id: 'invalidReducedTimeRange'; + meta?: {}; +} + +export type ValidationErrors = + | MissingFieldError + | MissingOperationError + | MissingParameterError + | WrongTypeParameterError + | WrongTypeArgumentError + | WrongFirstArgument + | CannotAcceptParameterError + | ShouldNotHaveFieldError + | TooManyArgumentsError + | FieldWithNoOperationError + | FailedParsingError + | DuplicateArgumentError + | DuplicateArgumentError + | MissingMathArgumentError + | TooManyQueriesError + | TooManyFirstArgumentsError + | WrongArgumentError + | WrongReturnedTypeError + | FiltersTypeConflictError + | UseAlternativeFunctionError + | InvalidQueryError + | MissingTimerangeError + | InvalidDateError + | ShiftAfterTimeRangeError + | NotAbsoluteTimeShiftError + | InvalidTimeShift + | InvalidReducedTimeRange; diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/helpers.test.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/helpers.test.ts index e9212a74d45a98..5c10597a3a9ac4 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/helpers.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/helpers.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { FIELD_WRONG_TYPE } from '../../../../user_messages_ids'; import { createMockedIndexPattern } from '../../mocks'; import type { FormBasedLayer } from '../../types'; import type { GenericIndexPatternColumn } from './column_types'; @@ -67,6 +68,7 @@ describe('helpers', () => { } } />, + "uniqueId": "field_not_found", } `); }); @@ -84,7 +86,10 @@ describe('helpers', () => { createMockedIndexPattern() ); expect(messages).toHaveLength(1); - expect(messages![0]).toEqual('Field timestamp is of the wrong type'); + expect(messages![0]).toEqual({ + uniqueId: FIELD_WRONG_TYPE, + message: 'Field timestamp is of the wrong type', + }); }); it('returns an error if one field amongst multiples does not exist', () => { @@ -134,6 +139,7 @@ describe('helpers', () => { } } />, + "uniqueId": "field_not_found", } `); }); @@ -191,6 +197,7 @@ describe('helpers', () => { } } />, + "uniqueId": "field_not_found", } `); }); @@ -211,7 +218,10 @@ describe('helpers', () => { createMockedIndexPattern() ); expect(messages).toHaveLength(1); - expect(messages![0]).toEqual('Field timestamp is of the wrong type'); + expect(messages![0]).toEqual({ + uniqueId: FIELD_WRONG_TYPE, + message: 'Field timestamp is of the wrong type', + }); }); it('returns an error if multiple fields are of the wrong type', () => { @@ -230,7 +240,10 @@ describe('helpers', () => { createMockedIndexPattern() ); expect(messages).toHaveLength(1); - expect(messages![0]).toEqual('Fields start_date, timestamp are of the wrong type'); + expect(messages![0]).toEqual({ + uniqueId: FIELD_WRONG_TYPE, + message: 'Fields start_date, timestamp are of the wrong type', + }); }); it('returns no message if all fields are matching', () => { @@ -245,7 +258,7 @@ describe('helpers', () => { columnId, createMockedIndexPattern() ); - expect(messages).toBeUndefined(); + expect(messages).toHaveLength(0); }); }); 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 index 11a4e16a39f444..b65deebec963ae 100644 --- 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 @@ -23,14 +23,15 @@ import { } from './column_types'; import type { FormBasedLayer, LastValueIndexPatternColumn } from '../../types'; import { hasField } from '../../pure_utils'; +import { FIELD_NOT_FOUND, FIELD_WRONG_TYPE } from '../../../../user_messages_ids'; export function getInvalidFieldMessage( layer: FormBasedLayer, columnId: string, indexPattern?: IndexPattern -): FieldBasedOperationErrorMessage[] | undefined { +): FieldBasedOperationErrorMessage[] { if (!indexPattern) { - return; + return []; } const column = layer.columns[columnId] as FieldBasedIndexPatternColumn; @@ -77,25 +78,29 @@ export function getInvalidFieldMessage( 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(', '), - }, - }), + { + uniqueId: FIELD_WRONG_TYPE, + message: 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; + return []; } export const generateMissingFieldMessage = ( missingFields: string[], columnId: string ): FieldBasedOperationErrorMessage => ({ + uniqueId: FIELD_NOT_FOUND, message: ( -): FieldBasedOperationErrorMessage[] | undefined { - const messages = (errorMessages.filter(Boolean) as FieldBasedOperationErrorMessage[][]).flat(); - return messages.length ? messages : undefined; -} - export function getSafeName(name: string, indexPattern: IndexPattern | undefined): string { const field = indexPattern?.getFieldByName(name); return field diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/index.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/index.ts index c7f8ed1e6923de..791024e4c93686 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/index.ts @@ -324,7 +324,7 @@ interface BaseOperationDefinitionProps< dateRange?: DateRange, operationDefinitionMap?: Record, targetBars?: number - ) => FieldBasedOperationErrorMessage[] | undefined; + ) => FieldBasedOperationErrorMessage[]; /* * Flag whether this operation can be scaled by time unit if a date histogram is available. @@ -463,21 +463,20 @@ interface FilterParams { lucene?: string; } -export type FieldBasedOperationErrorMessage = - | { - message: string | React.ReactNode; - displayLocations?: UserMessage['displayLocations']; - fixAction?: { - label: string; - newState: ( - data: DataPublicPluginStart, - core: CoreStart, - frame: FramePublicAPI, - layerId: string - ) => Promise; - }; - } - | string; +export interface FieldBasedOperationErrorMessage { + uniqueId: string; + message: string | React.ReactNode; + displayLocations?: UserMessage['displayLocations']; + fixAction?: { + label: string; + newState: ( + data: DataPublicPluginStart, + core: CoreStart, + frame: FramePublicAPI, + layerId: string + ) => Promise; + }; +} interface FieldlessOperationDefinition { input: 'none'; @@ -581,7 +580,7 @@ interface FieldBasedOperationDefinition - ) => FieldBasedOperationErrorMessage[] | undefined; + ) => FieldBasedOperationErrorMessage[]; } export interface RequiredReference { diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.test.tsx index 47777b7c0f6f75..61748339d2a723 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.test.tsx @@ -914,10 +914,10 @@ describe('last_value', () => { indexPatternId: '', }; }); - it('returns undefined if sourceField exists and sortField is of type date ', () => { + it('returns empty array if sourceField exists and sortField is of type date ', () => { expect( lastValueOperation.getErrorMessage!(errorLayer, 'col1', createMockedIndexPattern()) - ).toEqual(undefined); + ).toHaveLength(0); }); it('shows error message if the sourceField does not exist in index pattern', () => { errorLayer = { @@ -962,6 +962,7 @@ describe('last_value', () => { } } />, + "uniqueId": "field_not_found", }, ] `); @@ -1007,6 +1008,7 @@ describe('last_value', () => { } } />, + "uniqueId": "last_value_op_sort_field_not_found", }, ] `); @@ -1043,9 +1045,9 @@ describe('last_value', () => { } as LastValueIndexPatternColumn, }, }; - expect(lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern)).toEqual([ - 'Field start_date is of the wrong type', - ]); + expect( + lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern).map((e) => e.message) + ).toEqual(['Field start_date is of the wrong type']); }); it('shows error message if the sortField is not date', () => { errorLayer = { @@ -1061,7 +1063,9 @@ describe('last_value', () => { }, }; expect( - lastValueOperation.getErrorMessage!(errorLayer, 'col1', createMockedIndexPattern()) + lastValueOperation.getErrorMessage!(errorLayer, 'col1', createMockedIndexPattern()).map( + (e) => e.message + ) ).toEqual(['Field bytes is not a date field and cannot be used for sorting']); }); }); 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 index 0861148aacc3d0..b508534a19800a 100644 --- 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 @@ -36,6 +36,10 @@ import { isRuntimeField, isScriptedField } from './terms/helpers'; import { FormRow } from './shared_components/form_row'; import { getColumnReducedTimeRangeError } from '../../reduced_time_range_utils'; import { getGroupByKey } from './get_group_by_key'; +import { + LAST_VALUE_OP_SORT_FIELD_INVALID_TYPE, + LAST_VALUE_OP_SORT_FIELD_NOT_FOUND, +} from '../../../../user_messages_ids'; function ofName(name: string, timeShift: string | undefined, reducedTimeRange: string | undefined) { return adjustTimeScaleLabelSuffix( @@ -69,35 +73,44 @@ function getInvalidSortFieldMessage( sortField: string, columnId: string, indexPattern?: IndexPattern -): FieldBasedOperationErrorMessage | undefined { +): FieldBasedOperationErrorMessage[] { if (!indexPattern) { - return; + return []; } const field = indexPattern.getFieldByName(sortField); if (!field) { - return { - message: ( - {sortField}, - }} - /> - ), - displayLocations: [ - { id: 'toolbar' }, - { id: 'dimensionButton', dimensionId: columnId }, - { id: 'embeddableBadge' }, - ], - }; + return [ + { + uniqueId: LAST_VALUE_OP_SORT_FIELD_NOT_FOUND, + message: ( + {sortField}, + }} + /> + ), + displayLocations: [ + { id: 'toolbar' }, + { id: 'dimensionButton', dimensionId: columnId }, + { id: 'embeddableBadge' }, + ], + }, + ]; } 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 }, - }); + return [ + { + uniqueId: LAST_VALUE_OP_SORT_FIELD_INVALID_TYPE, + message: i18n.translate('xpack.lens.indexPattern.lastValue.invalidTypeSortField', { + defaultMessage: 'Field {invalidField} is not a date field and cannot be used for sorting', + values: { invalidField: sortField }, + }), + }, + ]; } + return []; } function isTimeFieldNameDateField(indexPattern: IndexPattern) { @@ -211,24 +224,11 @@ export const lastValueOperation: OperationDefinition< }, getErrorMessage(layer, columnId, indexPattern) { const column = layer.columns[columnId] as LastValueIndexPatternColumn; - const errorMessages: FieldBasedOperationErrorMessage[] = []; - - const invalidSourceFieldMessage = getInvalidFieldMessage(layer, columnId, indexPattern); - if (invalidSourceFieldMessage) { - errorMessages.push(...invalidSourceFieldMessage); - } - - const invalidSortFieldMessage = getInvalidSortFieldMessage( - column.params.sortField, - columnId, - indexPattern - ); - if (invalidSortFieldMessage) { - errorMessages.push(invalidSortFieldMessage); - } - - errorMessages.push(...(getColumnReducedTimeRangeError(layer, columnId, indexPattern) || [])); - return errorMessages.length ? errorMessages : undefined; + return [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getInvalidSortFieldMessage(column.params.sortField, columnId, indexPattern), + ...getColumnReducedTimeRangeError(layer, columnId, indexPattern), + ]; }, buildColumn({ field, previousColumn, indexPattern }, columnParams) { const lastValueParams = columnParams as LastValueIndexPatternColumn['params']; 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 index 87e3432d54e2fa..304698b1be5e9e 100644 --- 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 @@ -30,7 +30,6 @@ import { getInvalidFieldMessage, getSafeName, getFilter, - combineErrorMessages, isColumnOfType, } from './helpers'; import { @@ -234,11 +233,10 @@ function buildMetricOperation>({ ); }, - getErrorMessage: (layer, columnId, indexPattern) => - combineErrorMessages([ - getInvalidFieldMessage(layer, columnId, indexPattern), - getColumnReducedTimeRangeError(layer, columnId, indexPattern), - ]), + getErrorMessage: (layer, columnId, indexPattern) => [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getColumnReducedTimeRangeError(layer, columnId, indexPattern), + ], filterable: true, canReduceTimeRange: true, quickFunctionDocumentation, 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 index be40f31c770aab..97bb7270d34310 100644 --- 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 @@ -25,7 +25,6 @@ import { isValidNumber, getFilter, isColumnOfType, - combineErrorMessages, } from './helpers'; import { FieldBasedIndexPatternColumn } from './column_types'; import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; @@ -325,11 +324,10 @@ export const percentileOperation: OperationDefinition< aggs, }; }, - getErrorMessage: (layer, columnId, indexPattern) => - combineErrorMessages([ - getInvalidFieldMessage(layer, columnId, indexPattern), - getColumnReducedTimeRangeError(layer, columnId, indexPattern), - ]), + getErrorMessage: (layer, columnId, indexPattern) => [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getColumnReducedTimeRangeError(layer, columnId, indexPattern), + ], paramEditor: function PercentileParamEditor({ paramEditorUpdater, currentColumn, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile_ranks.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile_ranks.tsx index 3eb867442ef176..f1abee52f68c9a 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile_ranks.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile_ranks.tsx @@ -20,7 +20,6 @@ import { isValidNumber, getFilter, isColumnOfType, - combineErrorMessages, } from './helpers'; import { FieldBasedIndexPatternColumn } from './column_types'; import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; @@ -169,11 +168,10 @@ export const percentileRanksOperation: OperationDefinition< } ).toAst(); }, - getErrorMessage: (layer, columnId, indexPattern) => - combineErrorMessages([ - getInvalidFieldMessage(layer, columnId, indexPattern), - getColumnReducedTimeRangeError(layer, columnId, indexPattern), - ]), + getErrorMessage: (layer, columnId, indexPattern) => [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getColumnReducedTimeRangeError(layer, columnId, indexPattern), + ], paramEditor: function PercentileParamEditor({ paramEditorUpdater, currentColumn, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.test.tsx index 91727f0d56ea79..ce53ca04ea44ca 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.test.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.test.tsx @@ -158,7 +158,7 @@ describe('static_value', () => { createMockedIndexPattern(), dateRange ) - ).toBeUndefined(); + ).toHaveLength(0); // test for potential falsy value expect( staticValueOperation.getErrorMessage!( @@ -167,7 +167,7 @@ describe('static_value', () => { createMockedIndexPattern(), dateRange ) - ).toBeUndefined(); + ).toHaveLength(0); }); it.each(['NaN', 'Infinity', 'string'])( @@ -179,7 +179,7 @@ describe('static_value', () => { 'col2', createMockedIndexPattern(), dateRange - ) + ).map((e) => e.message) ).toEqual(expect.arrayContaining([expect.stringMatching('is not a valid number')])); } ); @@ -192,7 +192,7 @@ describe('static_value', () => { createMockedIndexPattern(), dateRange ) - ).toBe(undefined); + ).toHaveLength(0); }); }); diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.tsx index af00ba7cb5c69c..eef9ee2e9017d5 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/static_value.tsx @@ -17,6 +17,7 @@ import { import type { IndexPattern } from '../../../../types'; import { getFormatFromPreviousColumn, isValidNumber } from './helpers'; import { getColumnOrder } from '../layer_helpers'; +import { STATIC_VALUE_NOT_VALID_NUMBER } from '../../../../user_messages_ids'; const defaultLabel = i18n.translate('xpack.lens.indexPattern.staticValueLabelDefault', { defaultMessage: 'Static value', @@ -70,12 +71,15 @@ export const staticValueOperation: OperationDefinition< return column.params.value != null && !isValid ? [ - i18n.translate('xpack.lens.indexPattern.staticValueError', { - defaultMessage: 'The static value of {value} is not a valid number', - values: { value: column.params.value }, - }), + { + uniqueId: STATIC_VALUE_NOT_VALID_NUMBER, + message: i18n.translate('xpack.lens.indexPattern.staticValueError', { + defaultMessage: 'The static value of {value} is not a valid number', + values: { value: column.params.value }, + }), + }, ] - : undefined; + : []; }, getPossibleOperation() { return { diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.test.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.test.ts index 583f3ff5015c2d..f71e724ed5d255 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.test.ts @@ -93,52 +93,58 @@ function getCountOperationColumn( describe('getMultiTermsScriptedFieldErrorMessage()', () => { it('should return no error message for a single field', () => { - expect( - getMultiTermsScriptedFieldErrorMessage(getLayer(), 'col1', indexPattern) - ).toBeUndefined(); + expect(getMultiTermsScriptedFieldErrorMessage(getLayer(), 'col1', indexPattern)).toHaveLength( + 0 + ); }); it('should return no error message for a scripted field when single', () => { const col = getStringBasedOperationColumn('scripted'); expect( getMultiTermsScriptedFieldErrorMessage(getLayer(col), 'col1', indexPattern) - ).toBeUndefined(); + ).toHaveLength(0); }); it('should return an error message for a scripted field when there are multiple fields', () => { const col = getStringBasedOperationColumn('scripted', { secondaryFields: ['bytes'] }); - expect(getMultiTermsScriptedFieldErrorMessage(getLayer(col), 'col1', indexPattern)).toBe( - 'Scripted fields are not supported when using multiple fields, found scripted' - ); + expect( + getMultiTermsScriptedFieldErrorMessage(getLayer(col), 'col1', indexPattern).map( + (e) => e.message + ) + ).toEqual(['Scripted fields are not supported when using multiple fields, found scripted']); }); it('should return no error message for multiple "native" fields', () => { const col = getStringBasedOperationColumn('source', { secondaryFields: ['dest'] }); expect( getMultiTermsScriptedFieldErrorMessage(getLayer(col), 'col1', indexPattern) - ).toBeUndefined(); + ).toHaveLength(0); }); it('should list all scripted fields in the error message', () => { const col = getStringBasedOperationColumn('scripted', { secondaryFields: ['scripted', 'scripted', 'scripted'], }); - expect(getMultiTermsScriptedFieldErrorMessage(getLayer(col), 'col1', indexPattern)).toBe( - 'Scripted fields are not supported when using multiple fields, found scripted, scripted, scripted, scripted' - ); + expect( + getMultiTermsScriptedFieldErrorMessage(getLayer(col), 'col1', indexPattern).map( + (e) => e.message + ) + ).toEqual([ + 'Scripted fields are not supported when using multiple fields, found scripted, scripted, scripted, scripted', + ]); }); }); describe('getDisallowedTermsMessage()', () => { it('should return no error if no shifted dimensions are defined', () => { - expect(getDisallowedTermsMessage(getLayer(), 'col1', indexPattern)).toBeUndefined(); + expect(getDisallowedTermsMessage(getLayer(), 'col1', indexPattern)).toHaveLength(0); expect( getDisallowedTermsMessage( getLayer(getStringBasedOperationColumn(), [getCountOperationColumn()]), 'col1', indexPattern ) - ).toBeUndefined(); + ).toHaveLength(0); }); it('should return no error for a single dimension shifted', () => { @@ -148,7 +154,7 @@ describe('getDisallowedTermsMessage()', () => { 'col1', indexPattern ) - ).toBeUndefined(); + ).toHaveLength(0); }); it('should return no error for a single dimension shifted which is wrapped in a referencing column', () => { @@ -174,18 +180,18 @@ describe('getDisallowedTermsMessage()', () => { 'col1', indexPattern ) - ).toBeUndefined(); + ).toHaveLength(0); }); it('should return no for multiple fields with no shifted dimensions', () => { - expect(getDisallowedTermsMessage(getLayer(), 'col1', indexPattern)).toBeUndefined(); + expect(getDisallowedTermsMessage(getLayer(), 'col1', indexPattern)).toHaveLength(0); expect( getDisallowedTermsMessage( getLayer(getStringBasedOperationColumn(), [getCountOperationColumn()]), 'col1', indexPattern ) - ).toBeUndefined(); + ).toHaveLength(0); }); it('should return an error for multiple dimensions shifted for a single term', () => { @@ -197,7 +203,7 @@ describe('getDisallowedTermsMessage()', () => { ]), 'col1', indexPattern - ) + )[0] ).toEqual( expect.objectContaining({ message: @@ -216,7 +222,7 @@ describe('getDisallowedTermsMessage()', () => { ]), 'col1', indexPattern - ) + )[0] ).toEqual( expect.objectContaining({ message: @@ -234,7 +240,7 @@ describe('getDisallowedTermsMessage()', () => { ]), 'col1', indexPattern - )!.fixAction.newState; + )[0]!.fixAction!.newState; const newLayer = await fixAction( dataMock, coreMock, @@ -282,7 +288,7 @@ describe('getDisallowedTermsMessage()', () => { ]), 'col1', indexPattern - )!.fixAction.newState; + )[0]!.fixAction!.newState; const newLayer = await fixAction( dataMock, coreMock, @@ -324,7 +330,7 @@ describe('getDisallowedTermsMessage()', () => { ]), 'col1', indexPattern - )!.fixAction.newState; + )[0]!.fixAction!.newState; const newLayer = await fixAction( dataMock, coreMock, @@ -365,7 +371,7 @@ describe('getDisallowedTermsMessage()', () => { ]), 'col1', indexPattern - )!.fixAction.newState; + )[0]!.fixAction!.newState; const newLayer = await fixAction( dataMock, coreMock, diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.ts index cb3ed583d7cdaf..268e8a61c9f904 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/helpers.ts @@ -13,7 +13,11 @@ import { getEsQueryConfig, DataPublicPluginStart } from '@kbn/data-plugin/public import type { DataViewField } from '@kbn/data-views-plugin/common'; import { type FieldStatsResponse } from '@kbn/unified-field-list/src/types'; import { loadFieldStats } from '@kbn/unified-field-list/src/services/field_stats'; -import { GenericIndexPatternColumn, operationDefinitionMap } from '..'; +import { + FieldBasedOperationErrorMessage, + GenericIndexPatternColumn, + operationDefinitionMap, +} from '..'; import { defaultLabel } from '../filters'; import { isReferenced } from '../../layer_helpers'; @@ -27,6 +31,10 @@ import type { PercentileIndexPatternColumn } from '../percentile'; import type { FormBasedLayer } from '../../../types'; import { MULTI_KEY_VISUAL_SEPARATOR, supportedTypes, MAX_TERMS_OTHER_ENABLED } from './constants'; import { isColumnOfType } from '../helpers'; +import { + TERMS_MULTI_TERMS_AND_SCRIPTED_FIELDS, + TERMS_WITH_MULTIPLE_TIMESHIFT, +} from '../../../../../user_messages_ids'; const fullSeparatorString = ` ${MULTI_KEY_VISUAL_SEPARATOR} `; @@ -34,21 +42,27 @@ export function getMultiTermsScriptedFieldErrorMessage( layer: FormBasedLayer, columnId: string, indexPattern: IndexPattern -) { +): FieldBasedOperationErrorMessage[] { const currentColumn = layer.columns[columnId] as TermsIndexPatternColumn; const usedFields = [currentColumn.sourceField, ...(currentColumn.params.secondaryFields ?? [])]; const scriptedFields = usedFields.filter((field) => indexPattern.getFieldByName(field)?.scripted); if (usedFields.length < 2 || !scriptedFields.length) { - return; + return []; } - return i18n.translate('xpack.lens.indexPattern.termsWithMultipleTermsAndScriptedFields', { - defaultMessage: 'Scripted fields are not supported when using multiple fields, found {fields}', - values: { - fields: scriptedFields.join(', '), + return [ + { + uniqueId: TERMS_MULTI_TERMS_AND_SCRIPTED_FIELDS, + message: i18n.translate('xpack.lens.indexPattern.termsWithMultipleTermsAndScriptedFields', { + defaultMessage: + 'Scripted fields are not supported when using multiple fields, found {fields}', + values: { + fields: scriptedFields.join(', '), + }, + }), }, - }); + ]; } function getQueryForMultiTerms(fieldNames: string[], term: string) { @@ -92,7 +106,7 @@ export function getDisallowedTermsMessage( layer: FormBasedLayer, columnId: string, indexPattern: IndexPattern -) { +): FieldBasedOperationErrorMessage[] { const referenced: Set = new Set(); Object.entries(layer.columns).forEach(([cId, c]) => { if ('references' in c) { @@ -112,116 +126,119 @@ export function getDisallowedTermsMessage( .map(([colId, col]) => col.timeShift || '') ).length > 1; if (!hasMultipleShifts) { - return undefined; + return []; } - return { - message: i18n.translate('xpack.lens.indexPattern.termsWithMultipleShifts', { - defaultMessage: - 'In a single layer, you are unable to combine metrics with different time shifts and dynamic top values. Use the same time shift value for all metrics, or use filters instead of top values.', - }), - fixAction: { - label: i18n.translate('xpack.lens.indexPattern.termsWithMultipleShiftsFixActionLabel', { - defaultMessage: 'Use filters', + return [ + { + uniqueId: TERMS_WITH_MULTIPLE_TIMESHIFT, + message: i18n.translate('xpack.lens.indexPattern.termsWithMultipleShifts', { + defaultMessage: + 'In a single layer, you are unable to combine metrics with different time shifts and dynamic top values. Use the same time shift value for all metrics, or use filters instead of top values.', }), - newState: async ( - data: DataPublicPluginStart, - core: CoreStart, - frame: FramePublicAPI, - layerId: string - ) => { - const currentColumn = layer.columns[columnId] as TermsIndexPatternColumn; - const fieldNames = [ - currentColumn.sourceField, - ...(currentColumn.params?.secondaryFields ?? []), - ]; - const activeDataFieldNameMatch = - frame.activeData?.[layerId].columns.find(({ id }) => id === columnId)?.meta.field === - fieldNames[0]; + fixAction: { + label: i18n.translate('xpack.lens.indexPattern.termsWithMultipleShiftsFixActionLabel', { + defaultMessage: 'Use filters', + }), + newState: async ( + data: DataPublicPluginStart, + core: CoreStart, + frame: FramePublicAPI, + layerId: string + ) => { + const currentColumn = layer.columns[columnId] as TermsIndexPatternColumn; + const fieldNames = [ + currentColumn.sourceField, + ...(currentColumn.params?.secondaryFields ?? []), + ]; + const activeDataFieldNameMatch = + frame.activeData?.[layerId].columns.find(({ id }) => id === columnId)?.meta.field === + fieldNames[0]; - let currentTerms = uniq( - frame.activeData?.[layerId].rows - .map((row) => row[columnId] as string | MultiFieldKeyFormat) - .filter((term) => - fieldNames.length > 1 - ? isMultiFieldValue(term) && term.keys[0] !== '__other__' - : typeof term === 'string' && term !== '__other__' - ) - .map((term: string | MultiFieldKeyFormat) => - isMultiFieldValue(term) ? term.keys.join(fullSeparatorString) : term - ) || [] - ); - if (!activeDataFieldNameMatch || currentTerms.length === 0) { - if (fieldNames.length === 1) { - const currentDataView = await data.dataViews.get(indexPattern.id); - const response: FieldStatsResponse = await loadFieldStats({ - services: { data }, - dataView: currentDataView, - field: indexPattern.getFieldByName(fieldNames[0])! as DataViewField, - dslQuery: buildEsQuery( - indexPattern, - frame.query, - frame.filters, - getEsQueryConfig(core.uiSettings) - ), - fromDate: frame.dateRange.fromDate, - toDate: frame.dateRange.toDate, - size: currentColumn.params.size, - }); - currentTerms = response.topValues?.buckets.map(({ key }) => String(key)) || []; + let currentTerms = uniq( + frame.activeData?.[layerId].rows + .map((row) => row[columnId] as string | MultiFieldKeyFormat) + .filter((term) => + fieldNames.length > 1 + ? isMultiFieldValue(term) && term.keys[0] !== '__other__' + : typeof term === 'string' && term !== '__other__' + ) + .map((term: string | MultiFieldKeyFormat) => + isMultiFieldValue(term) ? term.keys.join(fullSeparatorString) : term + ) || [] + ); + if (!activeDataFieldNameMatch || currentTerms.length === 0) { + if (fieldNames.length === 1) { + const currentDataView = await data.dataViews.get(indexPattern.id); + const response: FieldStatsResponse = await loadFieldStats({ + services: { data }, + dataView: currentDataView, + field: indexPattern.getFieldByName(fieldNames[0])! as DataViewField, + dslQuery: buildEsQuery( + indexPattern, + frame.query, + frame.filters, + getEsQueryConfig(core.uiSettings) + ), + fromDate: frame.dateRange.fromDate, + toDate: frame.dateRange.toDate, + size: currentColumn.params.size, + }); + currentTerms = response.topValues?.buckets.map(({ key }) => String(key)) || []; + } + } + // when multi terms the meta.field will always be undefined, so limit the check to no data + if (fieldNames.length > 1 && currentTerms.length === 0) { + // this will produce a query like `field1: * AND field2: * ...etc` + // which is the best we can do for multiple terms when no data is available + currentTerms = [Array(fieldNames.length).fill('*').join(fullSeparatorString)]; } - } - // when multi terms the meta.field will always be undefined, so limit the check to no data - if (fieldNames.length > 1 && currentTerms.length === 0) { - // this will produce a query like `field1: * AND field2: * ...etc` - // which is the best we can do for multiple terms when no data is available - currentTerms = [Array(fieldNames.length).fill('*').join(fullSeparatorString)]; - } - return { - ...layer, - columns: { - ...layer.columns, - [columnId]: { - label: i18n.translate('xpack.lens.indexPattern.pinnedTopValuesLabel', { - defaultMessage: 'Filters of {field}', - values: { - field: - fieldNames.length > 1 ? fieldNames.join(fullSeparatorString) : fieldNames[0], - }, - }), - customLabel: true, - isBucketed: layer.columns[columnId].isBucketed, - dataType: 'string', - operationType: 'filters', - params: { - filters: - currentTerms.length > 0 - ? currentTerms.map((term) => ({ - input: { - query: - fieldNames.length === 1 - ? `${fieldNames[0]}: "${term}"` - : getQueryForMultiTerms(fieldNames, term), - language: 'kuery', - }, - label: getQueryLabel(fieldNames, term), - })) - : [ - { + return { + ...layer, + columns: { + ...layer.columns, + [columnId]: { + label: i18n.translate('xpack.lens.indexPattern.pinnedTopValuesLabel', { + defaultMessage: 'Filters of {field}', + values: { + field: + fieldNames.length > 1 ? fieldNames.join(fullSeparatorString) : fieldNames[0], + }, + }), + customLabel: true, + isBucketed: layer.columns[columnId].isBucketed, + dataType: 'string', + operationType: 'filters', + params: { + filters: + currentTerms.length > 0 + ? currentTerms.map((term) => ({ input: { - query: '*', + query: + fieldNames.length === 1 + ? `${fieldNames[0]}: "${term}"` + : getQueryForMultiTerms(fieldNames, term), language: 'kuery', }, - label: defaultLabel, - }, - ], - }, - } as FiltersIndexPatternColumn, - }, - }; + label: getQueryLabel(fieldNames, term), + })) + : [ + { + input: { + query: '*', + language: 'kuery', + }, + label: defaultLabel, + }, + ], + }, + } as FiltersIndexPatternColumn, + }, + }; + }, }, }, - }; + ]; } function checkLastValue(column: GenericIndexPatternColumn) { diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/index.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/index.tsx index 0ffc918844f755..62c5b777bf37b1 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/index.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/index.tsx @@ -206,12 +206,11 @@ export const termsOperation: OperationDefinition< } }, getErrorMessage: (layer, columnId, indexPattern) => { - const messages = [ - ...(getInvalidFieldMessage(layer, columnId, indexPattern) || []), - getDisallowedTermsMessage(layer, columnId, indexPattern) || '', - getMultiTermsScriptedFieldErrorMessage(layer, columnId, indexPattern) || '', - ].filter(Boolean); - return messages.length ? messages : undefined; + return [ + ...getInvalidFieldMessage(layer, columnId, indexPattern), + ...getDisallowedTermsMessage(layer, columnId, indexPattern), + ...getMultiTermsScriptedFieldErrorMessage(layer, columnId, indexPattern), + ]; }, getNonTransferableFields: (column, newIndexPattern) => { return getFieldsByValidationState(newIndexPattern, column).invalidFields; @@ -574,9 +573,8 @@ export const termsOperation: OperationDefinition< return ; } - const showScriptedFieldError = Boolean( - getMultiTermsScriptedFieldErrorMessage(layer, columnId, indexPattern) - ); + const showScriptedFieldError = + getMultiTermsScriptedFieldErrorMessage(layer, columnId, indexPattern).length > 0; const { invalidFields } = getFieldsByValidationState(indexPattern, selectedColumn); return ( diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/terms.test.tsx index ef80e79f291e9e..141906c712076d 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/terms.test.tsx @@ -41,6 +41,7 @@ import { ReferenceEditor } from '../../../dimension_panel/reference_editor'; import { IndexPattern } from '../../../../../types'; import { cloneDeep } from 'lodash'; import { IncludeExcludeRow } from './include_exclude_options'; +import { TERMS_MULTI_TERMS_AND_SCRIPTED_FIELDS } from '../../../../../user_messages_ids'; jest.mock('@kbn/unified-field-list/src/services/field_stats', () => ({ loadFieldStats: jest.fn().mockResolvedValue({ @@ -1375,8 +1376,7 @@ describe('terms', () => { it('should show an error message when any field but the first is invalid', () => { const updateLayerSpy = jest.fn(); const operationSupportMatrix = getDefaultOperationSupportMatrix('col1'); - - layer.columns.col1 = { + const col1: TermsIndexPatternColumn = { label: 'Top value of geo.src + 1 other', dataType: 'string', isBucketed: true, @@ -1388,7 +1388,8 @@ describe('terms', () => { secondaryFields: ['unsupported'], }, sourceField: 'geo.src', - } as TermsIndexPatternColumn; + }; + layer.columns.col1 = col1; const instance = mount( { updateLayer={updateLayerSpy} columnId="col1" operationSupportMatrix={operationSupportMatrix} - selectedColumn={layer.columns.col1 as TermsIndexPatternColumn} + selectedColumn={col1} /> ); expect( @@ -2736,7 +2737,7 @@ describe('terms', () => { }; }); it('returns undefined for no errors found', () => { - expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual(undefined); + expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toHaveLength(0); }); it('returns error message if the sourceField does not exist in index pattern', () => { layer = { @@ -2780,6 +2781,7 @@ describe('terms', () => { } } />, + "uniqueId": "field_not_found", }, ] `); @@ -2795,7 +2797,7 @@ describe('terms', () => { } as TermsIndexPatternColumn, }, }; - expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toBeUndefined(); + expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toHaveLength(0); }); it('return error for scripted field when in multi terms mode', () => { @@ -2814,7 +2816,10 @@ describe('terms', () => { }, }; expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ - 'Scripted fields are not supported when using multiple fields, found scripted', + { + uniqueId: TERMS_MULTI_TERMS_AND_SCRIPTED_FIELDS, + message: 'Scripted fields are not supported when using multiple fields, found scripted', + }, ]); }); diff --git a/x-pack/plugins/lens/public/datasources/form_based/reduced_time_range_utils.tsx b/x-pack/plugins/lens/public/datasources/form_based/reduced_time_range_utils.tsx index aaa5a446b514cc..ff5465dabee722 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/reduced_time_range_utils.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/reduced_time_range_utils.tsx @@ -9,6 +9,10 @@ import { i18n } from '@kbn/i18n'; import type { FormBasedLayer } from './types'; import type { IndexPattern } from '../../types'; import type { FieldBasedOperationErrorMessage } from './operations/definitions'; +import { + REDUCED_TIME_RANGE_DEFAULT_DATE_FIELD, + REDUCED_TIME_RANGE_NO_DATE_HISTOGRAM, +} from '../../user_messages_ids'; export const reducedTimeRangeOptions = [ { @@ -57,33 +61,39 @@ export function getColumnReducedTimeRangeError( layer: FormBasedLayer, columnId: string, indexPattern: IndexPattern -): FieldBasedOperationErrorMessage[] | undefined { +): FieldBasedOperationErrorMessage[] { const currentColumn = layer.columns[columnId]; if (!currentColumn.reducedTimeRange) { - return; + return []; } const hasDateHistogram = Object.values(layer.columns).some( (column) => column.operationType === 'date_histogram' ); const hasTimeField = Boolean(indexPattern.timeFieldName); - const errors: FieldBasedOperationErrorMessage[] = [ - hasDateHistogram && - i18n.translate('xpack.lens.indexPattern.reducedTimeRangeWithDateHistogram', { + const errors: FieldBasedOperationErrorMessage[] = []; + if (hasDateHistogram) { + errors.push({ + uniqueId: REDUCED_TIME_RANGE_NO_DATE_HISTOGRAM, + message: i18n.translate('xpack.lens.indexPattern.reducedTimeRangeWithDateHistogram', { defaultMessage: 'Reduced time range can only be used without a date histogram. Either remove the date histogram dimension or remove the reduced time range from {column}.', values: { column: currentColumn.label, }, }), - !hasTimeField && - i18n.translate('xpack.lens.indexPattern.reducedTimeRangeWithoutTimefield', { + }); + } + if (!hasTimeField) { + errors.push({ + uniqueId: REDUCED_TIME_RANGE_DEFAULT_DATE_FIELD, + message: i18n.translate('xpack.lens.indexPattern.reducedTimeRangeWithoutTimefield', { defaultMessage: 'Reduced time range can only be used with a specified default time field on the data view. Either use a different data view with default time field or remove the reduced time range from {column}.', values: { column: currentColumn.label, }, }), - ].filter(Boolean) as FieldBasedOperationErrorMessage[]; - + }); + } return errors; } diff --git a/x-pack/plugins/lens/public/datasources/form_based/time_shift_utils.tsx b/x-pack/plugins/lens/public/datasources/form_based/time_shift_utils.tsx index c287a93675f64c..ef34542539468c 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/time_shift_utils.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/time_shift_utils.tsx @@ -20,7 +20,8 @@ import { } from '@kbn/data-plugin/common'; import type { DateRange } from '../../../common/types'; import type { FormBasedLayer, FormBasedPrivateState } from './types'; -import type { FramePublicAPI, IndexPattern } from '../../types'; +import type { FramePublicAPI, IndexPattern, UserMessage } from '../../types'; +import { TIMESHIFT_LT_INTERVAL, TIMESHIFT_NOT_MULTIPLE_INTERVAL } from '../../user_messages_ids'; export function parseTimeShiftWrapper(timeShiftString: string, dateRange: DateRange) { if (isAbsoluteTimeShift(timeShiftString.trim())) { @@ -175,9 +176,9 @@ export function getStateTimeShiftWarningMessages( datatableUtilities: DatatableUtilitiesService, state: FormBasedPrivateState, { activeData, dataViews }: FramePublicAPI -) { - if (!state) return; - const warningMessages: React.ReactNode[] = []; +): UserMessage[] { + if (!state) return []; + const warningMessages: UserMessage[] = []; Object.entries(state.layers).forEach(([layerId, layer]) => { const layerIndexPattern = dataViews.indexPatterns[layer.indexPatternId]; if (!layerIndexPattern) { @@ -198,6 +199,7 @@ export function getStateTimeShiftWarningMessages( const timeShifts = new Set(); const timeShiftMap: Record = {}; Object.entries(layer.columns).forEach(([columnId, column]) => { + // TODO: I believe this can be replaced with a similar code like getColumnTimeShiftWarnings if (column.isBucketed) return; let duration: number = 0; // skip absolute time shifts as underneath it will be converted to be round @@ -224,33 +226,47 @@ export function getStateTimeShiftWarningMessages( if (timeShift === 0) return; if (timeShift < shiftInterval) { timeShiftMap[timeShift].forEach((columnId) => { - warningMessages.push( - {layer.columns[columnId].label}, - interval: {dateHistogramIntervalExpression}, - columnTimeShift: {layer.columns[columnId].timeShift}, - }} - /> - ); + warningMessages.push({ + uniqueId: TIMESHIFT_LT_INTERVAL, + severity: 'warning', + fixableInEditor: true, + displayLocations: [{ id: 'toolbar' }], + shortMessage: '', + longMessage: ( + {layer.columns[columnId].label}, + interval: {dateHistogramIntervalExpression}, + columnTimeShift: {layer.columns[columnId].timeShift}, + }} + /> + ), + }); }); } else if (!Number.isInteger(timeShift / shiftInterval)) { timeShiftMap[timeShift].forEach((columnId) => { - warningMessages.push( - {layer.columns[columnId].label}, - interval: {dateHistogramIntervalExpression}, - columnTimeShift: {layer.columns[columnId].timeShift!}, - }} - /> - ); + warningMessages.push({ + uniqueId: TIMESHIFT_NOT_MULTIPLE_INTERVAL, + severity: 'warning', + fixableInEditor: true, + displayLocations: [{ id: 'toolbar' }], + shortMessage: '', + longMessage: ( + {layer.columns[columnId].label}, + interval: {dateHistogramIntervalExpression}, + columnTimeShift: {layer.columns[columnId].timeShift!}, + }} + /> + ), + }); }); } }); @@ -261,12 +277,12 @@ export function getStateTimeShiftWarningMessages( export function getColumnTimeShiftWarnings( dateHistogramInterval: ReturnType, timeShift: string | undefined -) { +): string[] { const { isValueTooSmall, isValueNotMultiple } = getLayerTimeShiftChecks(dateHistogramInterval); const warnings: string[] = []; if (isAbsoluteTimeShift(timeShift)) { - return warnings; + return []; } const parsedLocalValue = timeShift && parseTimeShift(timeShift); 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 280e2d6a19a1ac..8d74c44c40db62 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/utils.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/utils.tsx @@ -64,6 +64,16 @@ import { DEFAULT_MAX_DOC_COUNT } from './operations/definitions/terms/constants' import { getOriginalId } from '../../../common/expressions/datatable/transpose_helpers'; import { ReducedSamplingSectionEntries } from './info_badges'; import { IgnoredGlobalFiltersEntries } from '../../shared_components/ignore_global_filter'; +import { + INCOMPLETE_ES_RESULTS, + LAYER_SETTINGS_IGNORE_GLOBAL_FILTERS, + LAYER_SETTINGS_RANDOM_SAMPLING_INFO, + PRECISION_ERROR_ACCURACY_MODE_DISABLED, + PRECISION_ERROR_ACCURACY_MODE_ENABLED, + PRECISION_ERROR_ASC_COUNT_PRECISION, + TSDB_UNSUPPORTED_COUNTER_OP, + UNSUPPORTED_DOWNSAMPLED_INDEX_AGG_PREFIX, +} from '../../user_messages_ids'; function isMinOrMaxColumn( column?: GenericIndexPatternColumn @@ -103,62 +113,56 @@ export function getSamplingValue(layer: FormBasedLayer) { export function isColumnInvalid( layer: FormBasedLayer, + column: GenericIndexPatternColumn, columnId: string, indexPattern: IndexPattern, - dateRange: DateRange | undefined, + dateRange: DateRange, targetBars: number -) { - const column: GenericIndexPatternColumn | undefined = layer.columns[columnId]; - if (!column || !indexPattern) return; - - const operationDefinition = column.operationType && operationDefinitionMap[column.operationType]; +): boolean { // check also references for errors const referencesHaveErrors = - true && 'references' in column && - Boolean( - getReferencesErrors(layer, column, indexPattern, dateRange, targetBars).filter(Boolean).length - ); - - const operationErrorMessages = - operationDefinition && - operationDefinition.getErrorMessage?.( - layer, - columnId, - indexPattern, - dateRange, - operationDefinitionMap, - targetBars - ); + hasReferencesErrors(layer, column, indexPattern, dateRange, targetBars); + + const operationHasErrorMessages = + ( + operationDefinitionMap[column.operationType]?.getErrorMessage?.( + layer, + columnId, + indexPattern, + dateRange, + operationDefinitionMap, + targetBars + ) ?? [] + ).length > 0; // it looks like this is just a back-stop since we prevent // invalid filters from being set at the UI level const filterHasError = column.filter ? !isQueryValid(column.filter, indexPattern) : false; - - return ( - (operationErrorMessages && operationErrorMessages.length > 0) || - referencesHaveErrors || - filterHasError - ); + return operationHasErrorMessages || referencesHaveErrors || filterHasError; } -function getReferencesErrors( +function hasReferencesErrors( layer: FormBasedLayer, column: ReferenceBasedIndexPatternColumn, indexPattern: IndexPattern, - dateRange: DateRange | undefined, + dateRange: DateRange, targetBars: number ) { - return column.references?.map((referenceId: string) => { + return column.references?.some((referenceId: string) => { const referencedOperation = layer.columns[referenceId]?.operationType; const referencedDefinition = operationDefinitionMap[referencedOperation]; - return referencedDefinition?.getErrorMessage?.( - layer, - referenceId, - indexPattern, - dateRange, - operationDefinitionMap, - targetBars + return ( + ( + referencedDefinition?.getErrorMessage?.( + layer, + referenceId, + indexPattern, + dateRange, + operationDefinitionMap, + targetBars + ) ?? [] + ).length > 0 ); }); } @@ -173,7 +177,7 @@ export function fieldIsInvalid( if (!column || !hasField(column)) { return false; } - return !!getInvalidFieldMessage(layer, columnId, indexPattern)?.length; + return getInvalidFieldMessage(layer, columnId, indexPattern).length > 0; } const accuracyModeDisabledWarning = ( @@ -181,6 +185,7 @@ const accuracyModeDisabledWarning = ( columnId: string, enableAccuracyMode: () => void ): UserMessage => ({ + uniqueId: PRECISION_ERROR_ACCURACY_MODE_DISABLED, severity: 'warning', displayLocations: [{ id: 'toolbar' }, { id: 'dimensionButton', dimensionId: columnId }], fixableInEditor: true, @@ -215,6 +220,7 @@ const accuracyModeEnabledWarning = ( columnId: string, docLink: string ): UserMessage => ({ + uniqueId: PRECISION_ERROR_ACCURACY_MODE_ENABLED, severity: 'warning', displayLocations: [{ id: 'toolbar' }, { id: 'dimensionButton', dimensionId: columnId }], fixableInEditor: true, @@ -284,27 +290,25 @@ export function getSearchWarningMessages( ].includes(col.operationType) ) .map((col) => col.label) - ).map( - (label) => - ({ - uniqueId: `unsupported_aggregation_on_downsampled_index--${label}`, - severity: 'warning', - fixableInEditor: true, - displayLocations: [{ id: 'toolbar' }, { id: 'embeddableBadge' }], - shortMessage: '', - longMessage: i18n.translate('xpack.lens.indexPattern.tsdbRollupWarning', { - defaultMessage: - '{label} uses a function that is unsupported by rolled up data. Select a different function or change the time range.', - values: { - label, - }, - }), - } as UserMessage) - ) + ).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, + displayLocations: [{ id: 'toolbar' }, { id: 'embeddableBadge' }], + shortMessage: '', + longMessage: i18n.translate('xpack.lens.indexPattern.tsdbRollupWarning', { + defaultMessage: + '{label} uses a function that is unsupported by rolled up data. Select a different function or change the time range.', + values: { + label, + }, + }), + })) ) : [ { - uniqueId: `incomplete`, + uniqueId: INCOMPLETE_ES_RESULTS, severity: 'warning', fixableInEditor: true, displayLocations: [{ id: 'toolbar' }, { id: 'embeddableBadge' }], @@ -315,7 +319,7 @@ export function getSearchWarningMessages( warnings={[warning]} /> ), - } as UserMessage, + }, ]; } } @@ -376,6 +380,7 @@ export function getUnsupportedOperationsWarningMessage( for (const columnsGrouped of columnsGroupedByField) { const sourceField = columnsGrouped[0][0].sourceField; warningMessages.push({ + uniqueId: TSDB_UNSUPPORTED_COUNTER_OP, severity: 'warning', fixableInEditor: false, displayLocations: [{ id: 'toolbar' }, { id: 'embeddableBadge' }], @@ -507,6 +512,7 @@ export function getPrecisionErrorWarningMessages( ); } else { warningMessages.push({ + uniqueId: PRECISION_ERROR_ASC_COUNT_PRECISION, severity: 'warning', displayLocations: [ { id: 'toolbar' }, @@ -611,7 +617,7 @@ export function getNotifiableFeatures( ); if (layersWithCustomSamplingValues.length) { features.push({ - uniqueId: 'random_sampling_info', + uniqueId: LAYER_SETTINGS_RANDOM_SAMPLING_INFO, severity: 'info', fixableInEditor: false, shortMessage: i18n.translate('xpack.lens.indexPattern.samplingPerLayer', { @@ -630,7 +636,7 @@ export function getNotifiableFeatures( const layersWithIgnoreGlobalFilters = layers.filter(([, layer]) => layer.ignoreGlobalFilters); if (layersWithIgnoreGlobalFilters.length) { features.push({ - uniqueId: 'ignoring-global-filters-layers', + uniqueId: LAYER_SETTINGS_IGNORE_GLOBAL_FILTERS, severity: 'info', fixableInEditor: false, shortMessage: i18n.translate('xpack.lens.xyChart.layerAnnotationsIgnoreTitle', { diff --git a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts index 8904031615bcff..e4bc8c955a4efa 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts +++ b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts @@ -612,6 +612,7 @@ describe('Textbased Data Source', () => { "longMessage": "error 1", "severity": "error", "shortMessage": "error 1", + "uniqueId": "text_based_lang_error", }, Object { "displayLocations": Array [ @@ -626,6 +627,7 @@ describe('Textbased Data Source', () => { "longMessage": "error 2", "severity": "error", "shortMessage": "error 2", + "uniqueId": "text_based_lang_error", }, ] `); diff --git a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx index 85d622380695c3..8cf8ce7ebd3606 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx +++ b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx @@ -54,6 +54,7 @@ import { addColumnsToCache, retrieveLayerColumnsFromCache, } from './fieldlist_cache'; +import { TEXT_BASED_LANGUAGE_ERROR } from '../../user_messages_ids'; function getLayerReferenceName(layerId: string) { return `textBasedLanguages-datasource-layer-${layerId}`; @@ -308,6 +309,7 @@ export function getTextBasedDatasource({ }); return errors.map((err) => { const message: UserMessage = { + uniqueId: TEXT_BASED_LANGUAGE_ERROR, severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'visualization' }, { id: 'textBasedLanguagesQueryInput' }], diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx index e6d67bb4eb3db2..5b8318c58498cb 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx @@ -668,6 +668,7 @@ describe('workspace_panel', () => { it('should show configuration error messages if present', async () => { const messages: UserMessage[] = [ { + uniqueId: 'unique_id_1', severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'visualization' }], @@ -675,6 +676,7 @@ describe('workspace_panel', () => { longMessage: "i'm an error", }, { + uniqueId: 'unique_id_2', severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'visualization' }], diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index c574d575bd5d90..bfca879d18d73c 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -43,7 +43,6 @@ import { UserMessage, UserMessagesGetter, AddUserMessages, - isMessageRemovable, VisualizationDisplayOptions, } from '../../../types'; import { switchToSuggestion } from '../suggestion_helpers'; @@ -267,9 +266,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ } if (requestWarnings.length) { - removeSearchWarningMessagesRef.current = addUserMessages( - requestWarnings.filter(isMessageRemovable) - ); + removeSearchWarningMessagesRef.current = addUserMessages(requestWarnings); } else if (removeSearchWarningMessagesRef.current) { removeSearchWarningMessagesRef.current(); removeSearchWarningMessagesRef.current = undefined; diff --git a/x-pack/plugins/lens/public/editor_frame_service/error_helper.tsx b/x-pack/plugins/lens/public/editor_frame_service/error_helper.tsx index 062cbcc867fa29..c938b5c3ab9406 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/error_helper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/error_helper.tsx @@ -8,11 +8,11 @@ import { ExpressionRenderError } from '@kbn/expressions-plugin/public'; import { renderSearchError } from '@kbn/search-errors'; import React from 'react'; -import { RemovableUserMessage } from '../types'; +import { UserMessage } from '../types'; export function getOriginalRequestErrorMessages( error: ExpressionRenderError | null -): RemovableUserMessage[] { +): UserMessage[] { const errorMessages: Array = []; if (error && 'original' in error && error.original) { const searchErrorDisplay = renderSearchError(error.original); diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx index 391201bafad02c..c162679c6bb892 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx @@ -257,6 +257,7 @@ describe('embeddable', () => { jest.spyOn(embeddable, 'getUserMessages').mockReturnValue([ { + uniqueId: 'error', severity: 'error', fixableInEditor: true, displayLocations: [{ id: 'visualization' }], diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 366b631f7f54b5..041d803baad12d 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -106,7 +106,6 @@ import { IndexPatternRef, FramePublicAPI, AddUserMessages, - isMessageRemovable, UserMessagesGetter, UserMessagesDisplayLocationId, } from '../types'; @@ -1000,9 +999,7 @@ export class Embeddable this.removeActiveDataWarningMessages(); const searchWarningMessages = this.getSearchWarningMessages(adapters); - this.removeActiveDataWarningMessages = this.addUserMessages( - searchWarningMessages.filter(isMessageRemovable) - ); + this.removeActiveDataWarningMessages = this.addUserMessages(searchWarningMessages); this.activeData = newActiveData; diff --git a/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.test.tsx b/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.test.tsx index 6e0e7077b28994..017df48d0bb81e 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.test.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.test.tsx @@ -27,6 +27,7 @@ describe('EmbeddableFeatureBadge', () => { { it('should render a description of the badge in a tooltip on hover', async () => { renderPopup([ { + uniqueId: 'unique_id', shortMessage: 'Short message', longMessage: 'Long text', severity: 'info', @@ -104,46 +106,11 @@ describe('EmbeddableFeatureBadge', () => { expect(await screen.findAllByText('LongText', { exact: false })).toHaveLength(2); }); - it('should render messages without id first, then grouped messages', async () => { - renderPopup( - [ - { - shortMessage: 'Section2', - longMessage:
AnotherText
, - severity: 'info', - fixableInEditor: false, - displayLocations: [], - }, - { - uniqueId: '1', - shortMessage: 'Section1', - longMessage:
LongText1
, - severity: 'info', - fixableInEditor: false, - displayLocations: [], - }, - { - uniqueId: '1', - shortMessage: 'Section1', - longMessage:
LongText2
, - severity: 'info', - fixableInEditor: false, - displayLocations: [], - }, - ], - 2 // last two messages are grouped - ); - expect(await screen.findAllByText('Section', { exact: false })).toHaveLength(2); - // now check the order - const longMessages = await screen.findAllByText('Text', { exact: false }); - expect(longMessages[0]).toHaveTextContent('AnotherText'); - expect(longMessages[1]).toHaveTextContent('LongText1'); - }); - describe('Horizontal rules', () => { it('should render no rule for single message', async () => { renderPopup([ { + uniqueId: 'unique_id', shortMessage: `Section1`, longMessage:
hello
, severity: 'info', @@ -155,45 +122,10 @@ describe('EmbeddableFeatureBadge', () => { await screen.queryByTestId('lns-feature-badges-horizontal-rule') ).not.toBeInTheDocument(); }); - it('should apply an horizontal if there are multiple messages without id', async () => { - const messages = [1, 2, 3].map((id) => ({ - shortMessage: `Section${id}`, - longMessage:
{id}
, - severity: 'info' as const, - fixableInEditor: false, - displayLocations: [], - })); - renderPopup(messages); - expect(await screen.getAllByTestId('lns-feature-badges-horizontal-rule')).toHaveLength( - messages.length - 1 - ); - }); - - it('should apply a rule between messages without id and grouped ones', async () => { - const messages = [ - { - uniqueId: 'myId', - shortMessage: `Section1`, - longMessage:
Grouped
, - severity: 'info' as const, - fixableInEditor: false, - displayLocations: [], - }, - { - shortMessage: `Section2`, - longMessage:
NoId
, - severity: 'info' as const, - fixableInEditor: false, - displayLocations: [], - }, - ]; - renderPopup(messages); - expect(await screen.getAllByTestId('lns-feature-badges-horizontal-rule')).toHaveLength(1); - }); - it('should apply rules taking into account grouped messages', async () => { - const messages = [ + const messages: UserMessage[] = [ { + uniqueId: 'myId0', shortMessage: `Section2`, longMessage:
NoId
, severity: 'info' as const, diff --git a/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.tsx b/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.tsx index a1823201382c2a..b774535113a48b 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable_info_badges.tsx @@ -36,20 +36,13 @@ export const EmbeddableFeatureBadge = ({ messages }: { messages: UserMessage[] } count: messages.length, }, }); - - const messagesWithoutUniqueId = messages.filter(({ uniqueId }) => !uniqueId); // compact messages be grouping longMessage together on matching unique-id - const messagesGroupedByUniqueId: Record = {}; + const groupedMessages: Map = new Map(); for (const message of messages) { - if (message.uniqueId) { - if (!messagesGroupedByUniqueId[message.uniqueId]) { - messagesGroupedByUniqueId[message.uniqueId] = []; - } - messagesGroupedByUniqueId[message.uniqueId].push(message); - } + const group = groupedMessages.get(message.uniqueId) ?? []; + group.push(message); + groupedMessages.set(message.uniqueId, group); } - const messageCount = - messagesWithoutUniqueId.length + Object.keys(messagesGroupedByUniqueId).length; return ( - {messageCount} + {groupedMessages.size} } @@ -86,39 +79,16 @@ export const EmbeddableFeatureBadge = ({ messages }: { messages: UserMessage[] } `} data-test-subj="lns-feature-badges-panel" > - {messagesWithoutUniqueId.map(({ shortMessage, longMessage }, index) => { - return ( - - {index ? ( - - ) : null} - - - ); - })} - {Object.entries(messagesGroupedByUniqueId).map(([uniqueId, messagesByUniqueId], index) => { - const hasHorizontalRule = messagesWithoutUniqueId.length || index; - const [{ shortMessage }] = messagesByUniqueId; + {[...groupedMessages.entries()].map(([uniqueId, messageGroup], index) => { + const [{ shortMessage }] = messageGroup; return ( - {hasHorizontalRule ? ( + {index > 0 && ( - ) : null} + )}