[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

@@ -178,6 +178,13 @@ export function SuggestionPanel({
       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(

x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx

@@ -454,6 +454,132 @@ describe('workspace_panel', () => {
     expect(expressionRendererMock).toHaveBeenCalledTimes(2);
   });
 
+  it('should show an error message if validation on datasource does not pass', () => {
+    mockDatasource.getErrorMessages.mockReturnValue([
+      { shortMessage: 'An error occurred', longMessage: 'An long description here' },
+    ]);
+    mockDatasource.getLayers.mockReturnValue(['first']);
+    const framePublicAPI = createMockFramePublicAPI();
+    framePublicAPI.datasourceLayers = {
+      first: mockDatasource.publicAPIMock,
+    };
+
+    instance = mount(
+      <WorkspacePanel
+        activeDatasourceId={'mock'}
+        datasourceStates={{
+          mock: {
+            state: {},
+            isLoading: false,
+          },
+        }}
+        datasourceMap={{
+          mock: mockDatasource,
+        }}
+        framePublicAPI={framePublicAPI}
+        activeVisualizationId="vis"
+        visualizationMap={{
+          vis: { ...mockVisualization, toExpression: () => 'vis' },
+        }}
+        visualizationState={{}}
+        dispatch={() => {}}
+        ExpressionRenderer={expressionRendererMock}
+        core={coreMock.createSetup()}
+        plugins={{ uiActions: uiActionsMock, data: dataMock }}
+      />
+    );
+
+    expect(instance.find('[data-test-subj="configuration-failure"]').exists()).toBeTruthy();
+    expect(instance.find(expressionRendererMock)).toHaveLength(0);
+  });
+
+  it('should show an error message if validation on visualization does not pass', () => {
+    mockDatasource.getErrorMessages.mockReturnValue(undefined);
+    mockDatasource.getLayers.mockReturnValue(['first']);
+    mockVisualization.getErrorMessages.mockReturnValue([
+      { shortMessage: 'Some error happened', longMessage: 'Some long description happened' },
+    ]);
+    mockVisualization.toExpression.mockReturnValue('vis');
+    const framePublicAPI = createMockFramePublicAPI();
+    framePublicAPI.datasourceLayers = {
+      first: mockDatasource.publicAPIMock,
+    };
+
+    instance = mount(
+      <WorkspacePanel
+        activeDatasourceId={'mock'}
+        datasourceStates={{
+          mock: {
+            state: {},
+            isLoading: false,
+          },
+        }}
+        datasourceMap={{
+          mock: mockDatasource,
+        }}
+        framePublicAPI={framePublicAPI}
+        activeVisualizationId="vis"
+        visualizationMap={{
+          vis: mockVisualization,
+        }}
+        visualizationState={{}}
+        dispatch={() => {}}
+        ExpressionRenderer={expressionRendererMock}
+        core={coreMock.createSetup()}
+        plugins={{ uiActions: uiActionsMock, data: dataMock }}
+      />
+    );
+
+    expect(instance.find('[data-test-subj="configuration-failure"]').exists()).toBeTruthy();
+    expect(instance.find(expressionRendererMock)).toHaveLength(0);
+  });
+
+  it('should show an error message if validation on both datasource and visualization do not pass', () => {
+    mockDatasource.getErrorMessages.mockReturnValue([
+      { shortMessage: 'An error occurred', longMessage: 'An long description here' },
+    ]);
+    mockDatasource.getLayers.mockReturnValue(['first']);
+    mockVisualization.getErrorMessages.mockReturnValue([
+      { shortMessage: 'Some error happened', longMessage: 'Some long description happened' },
+    ]);
+    mockVisualization.toExpression.mockReturnValue('vis');
+    const framePublicAPI = createMockFramePublicAPI();
+    framePublicAPI.datasourceLayers = {
+      first: mockDatasource.publicAPIMock,
+    };
+
+    instance = mount(
+      <WorkspacePanel
+        activeDatasourceId={'mock'}
+        datasourceStates={{
+          mock: {
+            state: {},
+            isLoading: false,
+          },
+        }}
+        datasourceMap={{
+          mock: mockDatasource,
+        }}
+        framePublicAPI={framePublicAPI}
+        activeVisualizationId="vis"
+        visualizationMap={{
+          vis: mockVisualization,
+        }}
+        visualizationState={{}}
+        dispatch={() => {}}
+        ExpressionRenderer={expressionRendererMock}
+        core={coreMock.createSetup()}
+        plugins={{ uiActions: uiActionsMock, data: dataMock }}
+      />
+    );
+
+    // EuiFlexItem duplicates internally the attribute, so we need to filter only the most inner one here
+    expect(
+      instance.find('[data-test-subj="configuration-failure-more-errors"]').last().text()
+    ).toEqual(' +1 error');
+    expect(instance.find(expressionRendererMock)).toHaveLength(0);
+  });
+
   it('should show an error message if the expression fails to parse', () => {
     mockDatasource.toExpression.mockReturnValue('|||');
     mockDatasource.getLayers.mockReturnValue(['first']);
@@ -487,7 +613,7 @@ describe('workspace_panel', () => {
       />
     );
 
-    expect(instance.find('[data-test-subj="expression-failure"]').first()).toBeTruthy();
+    expect(instance.find('[data-test-subj="expression-failure"]').exists()).toBeTruthy();
     expect(instance.find(expressionRendererMock)).toHaveLength(0);
   });