[Lens] Visualization validation and better error messages

x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx

@@ -356,20 +356,77 @@ describe('Datatable Visualization', () => {
       datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
       datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
         dataType: 'string',
-        isBucketed: true,
+        isBucketed: false, // <= make them metrics
         label: 'label',
       });
 
       const expression = datatableVisualization.toExpression(
         { layers: [layer] },
         frame.datasourceLayers
       ) as Ast;
+
       const tableArgs = buildExpression(expression).findFunction('lens_datatable_columns');
 
       expect(tableArgs).toHaveLength(1);
       expect(tableArgs[0].arguments).toEqual({
         columnIds: ['c', 'b'],
       });
     });
+
+    it('returns no expression if the metric dimension is not defined', () => {
+      const datasource = createMockDatasource('test');
+      const layer = { layerId: 'a', columns: ['b', 'c'] };
+      const frame = mockFrame();
+      frame.datasourceLayers = { a: datasource.publicAPIMock };
+      datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
+      datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
+        dataType: 'string',
+        isBucketed: true, // move it from the metric to the break down by side
+        label: 'label',
+      });
+
+      const expression = datatableVisualization.toExpression(
+        { layers: [layer] },
+        frame.datasourceLayers
+      );
+
+      expect(expression).toEqual(null);
+    });
+  });
+
+  describe('#getErrorMessages', () => {
+    it('returns undefined if the datasource is missing a metric dimension', () => {
+      const datasource = createMockDatasource('test');
+      const layer = { layerId: 'a', columns: ['b', 'c'] };
+      const frame = mockFrame();
+      frame.datasourceLayers = { a: datasource.publicAPIMock };
+      datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
+      datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
+        dataType: 'string',
+        isBucketed: true, // move it from the metric to the break down by side
+        label: 'label',
+      });
+
+      const error = datatableVisualization.getErrorMessages({ layers: [layer] }, frame);
+
+      expect(error).not.toBeDefined();
+    });
+
+    it('returns undefined if the metric dimension is defined', () => {
+      const datasource = createMockDatasource('test');
+      const layer = { layerId: 'a', columns: ['b', 'c'] };
+      const frame = mockFrame();
+      frame.datasourceLayers = { a: datasource.publicAPIMock };
+      datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'c' }, { columnId: 'b' }]);
+      datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
+        dataType: 'string',
+        isBucketed: false, // keep it a metric
+        label: 'label',
+      });
+
+      const error = datatableVisualization.getErrorMessages({ layers: [layer] }, frame);
+
+      expect(error).not.toBeDefined();
+    });
   });
 });

x-pack/plugins/lens/public/datatable_visualization/visualization.tsx

@@ -6,7 +6,13 @@
 
 import { Ast } from '@kbn/interpreter/common';
 import { i18n } from '@kbn/i18n';
-import { SuggestionRequest, Visualization, VisualizationSuggestion, Operation } from '../types';
+import {
+  SuggestionRequest,
+  Visualization,
+  VisualizationSuggestion,
+  Operation,
+  DatasourcePublicAPI,
+} from '../types';
 import { LensIconChartDatatable } from '../assets/chart_datatable';
 
 export interface LayerState {
@@ -128,16 +134,13 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
   },
 
   getConfiguration({ state, frame, layerId }) {
-    const layer = state.layers.find((l) => l.layerId === layerId);
-    if (!layer) {
+    const { sortedColumns, datasource } =
+      getDataSourceAndSortedColumns(state, frame.datasourceLayers, layerId) || {};
+
+    if (!sortedColumns) {
       return { groups: [] };
     }
 
-    const datasource = frame.datasourceLayers[layer.layerId];
-    const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
-    // When we add a column it could be empty, and therefore have no order
-    const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));
-
     return {
       groups: [
         {
@@ -146,7 +149,9 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
             defaultMessage: 'Break down by',
           }),
           layerId: state.layers[0].layerId,
-          accessors: sortedColumns.filter((c) => datasource.getOperationForColumnId(c)?.isBucketed),
+          accessors: sortedColumns.filter(
+            (c) => datasource!.getOperationForColumnId(c)?.isBucketed
+          ),
           supportsMoreColumns: true,
           filterOperations: (op) => op.isBucketed,
           dataTestSubj: 'lnsDatatable_column',
@@ -158,7 +163,7 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
           }),
           layerId: state.layers[0].layerId,
           accessors: sortedColumns.filter(
-            (c) => !datasource.getOperationForColumnId(c)?.isBucketed
+            (c) => !datasource!.getOperationForColumnId(c)?.isBucketed
           ),
           supportsMoreColumns: true,
           filterOperations: (op) => !op.isBucketed,
@@ -194,14 +199,19 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
     };
   },
 
-  toExpression(state, datasourceLayers, { title, description } = {}): Ast {
-    const layer = state.layers[0];
-    const datasource = datasourceLayers[layer.layerId];
-    const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
-    // When we add a column it could be empty, and therefore have no order
-    const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));
-    const operations = sortedColumns
-      .map((columnId) => ({ columnId, operation: datasource.getOperationForColumnId(columnId) }))
+  toExpression(state, datasourceLayers, { title, description } = {}): Ast | null {
+    const { sortedColumns, datasource } =
+      getDataSourceAndSortedColumns(state, datasourceLayers, state.layers[0].layerId) || {};
+
+    if (
+      sortedColumns?.length &&
+      sortedColumns.filter((c) => !datasource!.getOperationForColumnId(c)?.isBucketed).length === 0
+    ) {
+      return null;
+    }
+
+    const operations = sortedColumns!
+      .map((columnId) => ({ columnId, operation: datasource!.getOperationForColumnId(columnId) }))
       .filter((o): o is { columnId: string; operation: Operation } => !!o.operation);
 
     return {
@@ -232,4 +242,24 @@ export const datatableVisualization: Visualization<DatatableVisualizationState>
       ],
     };
   },
+
+  getErrorMessages(state, frame) {
+    return undefined;
+  },
 };
+
+function getDataSourceAndSortedColumns(
+  state: DatatableVisualizationState,
+  datasourceLayers: Record<string, DatasourcePublicAPI>,
+  layerId: string
+) {
+  const layer = state.layers.find((l: LayerState) => l.layerId === layerId);
+  if (!layer) {
+    return undefined;
+  }
+  const datasource = datasourceLayers[layer.layerId];
+  const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
+  // When we add a column it could be empty, and therefore have no order
+  const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));
+  return { datasource, sortedColumns };
+}

x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx

@@ -61,41 +61,48 @@ const PreviewRenderer = ({
   withLabel,
   ExpressionRendererComponent,
   expression,
+  hasError,
 }: {
   withLabel: boolean;
-  expression: string;
+  expression: string | null | undefined;
   ExpressionRendererComponent: ReactExpressionRendererType;
+  hasError: boolean;
 }) => {
+  const onErrorMessage = (
+    <div className="lnsSuggestionPanel__suggestionIcon">
+      <EuiIconTip
+        size="xl"
+        color="danger"
+        type="alert"
+        aria-label={i18n.translate('xpack.lens.editorFrame.previewErrorLabel', {
+          defaultMessage: 'Preview rendering failed',
+        })}
+        content={i18n.translate('xpack.lens.editorFrame.previewErrorLabel', {
+          defaultMessage: 'Preview rendering failed',
+        })}
+      />
+    </div>
+  );
   return (
     <div
       className={classNames('lnsSuggestionPanel__chartWrapper', {
         // eslint-disable-next-line @typescript-eslint/naming-convention
         'lnsSuggestionPanel__chartWrapper--withLabel': withLabel,
       })}
     >
-      <ExpressionRendererComponent
-        className="lnsSuggestionPanel__expressionRenderer"
-        padding="s"
-        expression={expression}
-        debounce={2000}
-        renderError={() => {
-          return (
-            <div className="lnsSuggestionPanel__suggestionIcon">
-              <EuiIconTip
-                size="xl"
-                color="danger"
-                type="alert"
-                aria-label={i18n.translate('xpack.lens.editorFrame.previewErrorLabel', {
-                  defaultMessage: 'Preview rendering failed',
-                })}
-                content={i18n.translate('xpack.lens.editorFrame.previewErrorLabel', {
-                  defaultMessage: 'Preview rendering failed',
-                })}
-              />
-            </div>
-          );
-        }}
-      />
+      {!expression || hasError ? (
+        onErrorMessage
+      ) : (
+        <ExpressionRendererComponent
+          className="lnsSuggestionPanel__expressionRenderer"
+          padding="s"
+          expression={expression}
+          debounce={2000}
+          renderError={() => {
+            return onErrorMessage;
+          }}
+        />
+      )}
     </div>
   );
 };
@@ -112,6 +119,7 @@ const SuggestionPreview = ({
     expression?: Ast | null;
     icon: IconType;
     title: string;
+    error?: boolean;
   };
   ExpressionRenderer: ReactExpressionRendererType;
   selected: boolean;
@@ -129,11 +137,12 @@ const SuggestionPreview = ({
           data-test-subj="lnsSuggestion"
           onClick={onSelect}
         >
-          {preview.expression ? (
+          {preview.expression || preview.error ? (
             <PreviewRenderer
               ExpressionRendererComponent={ExpressionRendererComponent}
-              expression={toExpression(preview.expression)}
+              expression={preview.expression && toExpression(preview.expression)}
               withLabel={Boolean(showTitleAsLabel)}
+              hasError={Boolean(preview.error)}
             />
           ) : (
             <span className="lnsSuggestionPanel__suggestionIcon">
@@ -170,14 +179,21 @@ export function SuggestionPanel({
     ? stagedPreview.visualization.activeId
     : activeVisualizationId;
 
-  const { suggestions, currentStateExpression } = useMemo(() => {
+  const { suggestions, currentStateExpression, currentStateError } = useMemo(() => {
     const newSuggestions = getSuggestions({
       datasourceMap,
       datasourceStates: currentDatasourceStates,
       visualizationMap,
       activeVisualizationId: currentVisualizationId,
       visualizationState: currentVisualizationState,
     })
+      .filter(({ visualizationId, visualizationState: suggestionVisualizationState }) => {
+        const suggestionVisualization = visualizationMap[visualizationId];
+        // filter out visualizations with errors
+        return (
+          suggestionVisualization.getErrorMessages(suggestionVisualizationState, frame) == null
+        );
+      })
       .map((suggestion) => ({
         ...suggestion,
         previewExpression: preparePreviewExpression(
@@ -191,8 +207,22 @@ export function SuggestionPanel({
       .filter((suggestion) => !suggestion.hide)
       .slice(0, MAX_SUGGESTIONS_DISPLAYED);
 
+    const activeDatasource = activeDatasourceId ? datasourceMap[activeDatasourceId] : null;
+    const datasourceValidationErrors = activeDatasourceId
+      ? activeDatasource?.getErrorMessages(currentDatasourceStates[activeDatasourceId]?.state)
+      : undefined;
+
+    const visualizationValidationErrors = currentVisualizationId
+      ? visualizationMap[currentVisualizationId]?.getErrorMessages(currentVisualizationState, frame)
+      : undefined;
+
+    const validationErrors =
+      datasourceValidationErrors || visualizationValidationErrors
+        ? [...(datasourceValidationErrors || []), ...(visualizationValidationErrors || [])]
+        : undefined;
+
     const newStateExpression =
-      currentVisualizationState && currentVisualizationId
+      currentVisualizationState && currentVisualizationId && !validationErrors
         ? preparePreviewExpression(
             { visualizationState: currentVisualizationState },
             visualizationMap[currentVisualizationId],
@@ -202,14 +232,19 @@ export function SuggestionPanel({
           )
         : undefined;
 
-    return { suggestions: newSuggestions, currentStateExpression: newStateExpression };
-    // eslint-disable-next-line react-hooks/exhaustive-deps
+    return {
+      suggestions: newSuggestions,
+      currentStateExpression: newStateExpression,
+      currentStateError: validationErrors,
+    };
   }, [
     currentDatasourceStates,
     currentVisualizationState,
     currentVisualizationId,
+    activeDatasourceId,
     datasourceMap,
     visualizationMap,
+    frame,
   ]);
 
   const context: ExecutionContextSearch = useMemo(
@@ -305,6 +340,7 @@ export function SuggestionPanel({
         {currentVisualizationId && (
           <SuggestionPreview
             preview={{
+              error: currentStateError != null,
               expression: currentStateExpression,
               icon:
                 visualizationMap[currentVisualizationId].getDescription(currentVisualizationState)