[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: #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: #151526) Subsequent possible steps, outside this PR: - add metadata where required for #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
elastic · Jun 5, 2024 · e12c7e3 · e12c7e3
1 parent db9e530
commit e12c7e3
Show file tree

Hide file tree

Showing 68 changed files with 1,880 additions and 1,380 deletions.
diff --git a/.../plugins/lens/public/app_plugin/__snapshots__/get_application_user_messages.test.tsx.snap b/.../plugins/lens/public/app_plugin/__snapshots__/get_application_user_messages.test.tsx.snap
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
@@ -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,41 +213,47 @@ describe('filtering user messages', () => {
 
   const userMessages: UserMessage[] = [
     {
+      uniqueId: 'unique_id_1',
       severity: 'error',
       fixableInEditor: true,
       displayLocations: [{ id: 'dimensionButton', dimensionId: dimensionId1 }],
       shortMessage: 'Warning on dimension 1!',
       longMessage: '',
     },
     {
+      uniqueId: 'unique_id_2',
       severity: 'warning',
       fixableInEditor: true,
       displayLocations: [{ id: 'dimensionButton', dimensionId: dimensionId2 }],
       shortMessage: 'Warning on dimension 2!',
       longMessage: '',
     },
     {
+      uniqueId: 'unique_id_3',
       severity: 'warning',
       fixableInEditor: true,
       displayLocations: [{ id: 'banner' }],
       shortMessage: 'Deprecation notice!',
       longMessage: '',
     },
     {
+      uniqueId: 'unique_id_4',
       severity: 'error',
       fixableInEditor: true,
       displayLocations: [{ id: 'visualization' }],
       shortMessage: 'Visualization error!',
       longMessage: '',
     },
     {
+      uniqueId: 'unique_id_5',
       severity: 'error',
       fixableInEditor: true,
       displayLocations: [{ id: 'visualizationInEditor' }],
       shortMessage: 'Visualization editor error!',
       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
@@ -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
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
@@ -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
@@ -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', () => {
                 </p>,
                 "severity": "error",
                 "shortMessage": "",
+                "uniqueId": "editor_invalid_dimension",
               },
             ]
           `);
@@ -3286,6 +3291,7 @@ describe('IndexPattern Data Source', () => {
                 </React.Fragment>,
                 "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
@@ -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<FormBasedPrivateState, unknown>,
   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,