[Lens] Improvements on the suggestions api (#153931)

## Summary This is a follow up PR from my PR that enables Lens suggestions in Discover. Specifically: - it moves the filtering from unified_histogram to lens api - it solves a bug that exists atm. If I have a query that it doesn't return a valid chart (e.g. select <string_field> from index) the chart is rendered wrongly (we should render nothing at this case). For this reason we add the incomplete flag in each visualization suggest which is set to true every time my chart is incomplete (missing required dimensions)
2025-04-24 09:48:58 -04:00 · 2023-04-04 14:06:24 +03:00 · 2023-04-04 14:06:24 +03:00 · d718066d75
commit d718066d75
parent 48ded93bc7
13 changed files with 166 additions and 26 deletions
--- a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts
+++ b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts
@ -36,21 +36,15 @@ export const useLensSuggestions = ({
      contextualFields: columns,
      query: query && isOfAggregateQueryType(query) ? query : undefined,
    };
-    const lensSuggestions = isPlainRecord ? lensSuggestionsApi(context, dataView) : undefined;
-    const firstSuggestion = lensSuggestions?.length ? lensSuggestions[0] : undefined;
-    const restSuggestions = lensSuggestions?.filter((sug) => {
-      return !sug.hide && sug.visualizationId !== 'lnsLegacyMetric';
-    });
-    const firstSuggestionExists = restSuggestions?.find(
-      (sug) => sug.title === firstSuggestion?.title
-    );
-    if (firstSuggestion && !firstSuggestionExists) {
-      restSuggestions?.push(firstSuggestion);
-    }
-    return { firstSuggestion, restSuggestions };
+    const allSuggestions = isPlainRecord
+      ? lensSuggestionsApi(context, dataView, ['lnsDatatable']) ?? []
+      : [];
+    const [firstSuggestion] = allSuggestions;
+
+    return { firstSuggestion, allSuggestions };
  }, [columns, dataView, isPlainRecord, lensSuggestionsApi, query]);

-  const [allSuggestions, setAllSuggestions] = useState(suggestions.restSuggestions);
+  const [allSuggestions, setAllSuggestions] = useState(suggestions.allSuggestions);
  const currentSuggestion = originalSuggestion ?? suggestions.firstSuggestion;
  const suggestionDeps = useRef(getSuggestionDeps({ dataView, query, columns }));

@ -58,7 +52,7 @@ export const useLensSuggestions = ({
    const newSuggestionsDeps = getSuggestionDeps({ dataView, query, columns });

    if (!isEqual(suggestionDeps.current, newSuggestionsDeps)) {
-      setAllSuggestions(suggestions.restSuggestions);
+      setAllSuggestions(suggestions.allSuggestions);
      onSuggestionChange?.(suggestions.firstSuggestion);

      suggestionDeps.current = newSuggestionsDeps;
@ -69,15 +63,13 @@ export const useLensSuggestions = ({
    onSuggestionChange,
    query,
    suggestions.firstSuggestion,
-    suggestions.restSuggestions,
+    suggestions.allSuggestions,
  ]);

  return {
    allSuggestions,
    currentSuggestion,
-    suggestionUnsupported:
-      isPlainRecord &&
-      (!currentSuggestion || currentSuggestion?.visualizationId === 'lnsDatatable'),
+    suggestionUnsupported: isPlainRecord && !currentSuggestion,
  };
 };

--- a/x-pack/plugins/lens/public/lens_suggestions_api.test.ts
+++ b/x-pack/plugins/lens/public/lens_suggestions_api.test.ts
@ -102,4 +102,101 @@ describe('suggestionsApi', () => {
    expect(datasourceMap.textBased.getDatasourceSuggestionsFromCurrentState).toHaveBeenCalled();
    expect(suggestions?.length).toEqual(1);
  });
+
+  test('filters out legacy metric and incomplete suggestions', async () => {
+    const dataView = { id: 'index1' } as unknown as DataView;
+    const visualizationMap = {
+      testVis: {
+        ...mockVis,
+        getSuggestions: () => [
+          {
+            score: 0.2,
+            title: 'Test',
+            state: {},
+            previewIcon: 'empty',
+            visualizationId: 'lnsLegacyMetric',
+          },
+          {
+            score: 0.8,
+            title: 'Test2',
+            state: {},
+            previewIcon: 'empty',
+          },
+          {
+            score: 0.8,
+            title: 'Test2',
+            state: {},
+            previewIcon: 'empty',
+            incomplete: true,
+          },
+        ],
+      },
+    };
+    datasourceMap.textBased.getDatasourceSuggestionsForVisualizeField.mockReturnValue([
+      generateSuggestion(),
+    ]);
+    const context = {
+      dataViewSpec: {
+        id: 'index1',
+        title: 'index1',
+        name: 'DataView',
+      },
+      fieldName: '',
+      contextualFields: ['field1', 'field2'],
+      query: {
+        sql: 'SELECT field1, field2 FROM "index1"',
+      },
+    };
+    const suggestions = suggestionsApi({ context, dataView, datasourceMap, visualizationMap });
+    expect(datasourceMap.textBased.getDatasourceSuggestionsFromCurrentState).toHaveBeenCalled();
+    expect(suggestions?.length).toEqual(1);
+  });
+
+  test('filters out the suggestion if exists on excludedVisualizations', async () => {
+    const dataView = { id: 'index1' } as unknown as DataView;
+    const visualizationMap = {
+      testVis: {
+        ...mockVis,
+        getSuggestions: () => [
+          {
+            score: 0.2,
+            title: 'Test',
+            state: {},
+            previewIcon: 'empty',
+            visualizationId: 'lnsXY',
+          },
+          {
+            score: 0.8,
+            title: 'Test2',
+            state: {},
+            previewIcon: 'empty',
+          },
+        ],
+      },
+    };
+    datasourceMap.textBased.getDatasourceSuggestionsForVisualizeField.mockReturnValue([
+      generateSuggestion(),
+    ]);
+    const context = {
+      dataViewSpec: {
+        id: 'index1',
+        title: 'index1',
+        name: 'DataView',
+      },
+      fieldName: '',
+      contextualFields: ['field1', 'field2'],
+      query: {
+        sql: 'SELECT field1, field2 FROM "index1"',
+      },
+    };
+    const suggestions = suggestionsApi({
+      context,
+      dataView,
+      datasourceMap,
+      visualizationMap,
+      excludedVisualizations: ['lnsXY'],
+    });
+    expect(datasourceMap.textBased.getDatasourceSuggestionsFromCurrentState).toHaveBeenCalled();
+    expect(suggestions?.length).toEqual(1);
+  });
 });
--- a/x-pack/plugins/lens/public/lens_suggestions_api.ts
+++ b/x-pack/plugins/lens/public/lens_suggestions_api.ts
@ -15,6 +15,7 @@ interface SuggestionsApi {
  dataView: DataView;
  visualizationMap?: VisualizationMap;
  datasourceMap?: DatasourceMap;
+  excludedVisualizations?: string[];
 }

 export const suggestionsApi = ({
@ -22,6 +23,7 @@ export const suggestionsApi = ({
  dataView,
  datasourceMap,
  visualizationMap,
+  excludedVisualizations,
 }: SuggestionsApi) => {
  if (!datasourceMap || !visualizationMap || !dataView.id) return undefined;
  const datasourceStates = {
@ -63,7 +65,13 @@ export const suggestionsApi = ({
  });
  if (!suggestions.length) return [];
  const activeVisualization = suggestions[0];
-  // compute the rest suggestions depending on the active one
+  if (
+    activeVisualization.incomplete ||
+    excludedVisualizations?.includes(activeVisualization.visualizationId)
+  ) {
+    return [];
+  }
+  // compute the rest suggestions depending on the active one and filter out the lnsLegacyMetric
  const newSuggestions = getSuggestions({
    datasourceMap,
    datasourceStates: {
@ -76,7 +84,7 @@ export const suggestionsApi = ({
    activeVisualization: visualizationMap[activeVisualization.visualizationId],
    visualizationState: activeVisualization.visualizationState,
    dataViews,
-  });
+  }).filter((sug) => !sug.hide && sug.visualizationId !== 'lnsLegacyMetric');

  return [activeVisualization, ...newSuggestions];
 };
--- a/x-pack/plugins/lens/public/plugin.ts
+++ b/x-pack/plugins/lens/public/plugin.ts
@ -241,7 +241,8 @@ export interface LensPublicStart {

 export type LensSuggestionsApi = (
  context: VisualizeFieldContext | VisualizeEditorContext,
-  dataViews: DataView
+  dataViews: DataView,
+  excludedVisualizations?: string[]
 ) => Suggestion[] | undefined;

 export class LensPlugin {
@ -607,15 +608,13 @@ export class LensPlugin {
        return {
          formula: createFormulaPublicApi(),
          chartInfo: createChartInfoApi(startDependencies.dataViews, this.editorFrameService),
-          suggestions: (
-            context: VisualizeFieldContext | VisualizeEditorContext,
-            dataView: DataView
-          ) => {
+          suggestions: (context, dataView, excludedVisualizations) => {
            return suggestionsApi({
              datasourceMap,
              visualizationMap,
              context,
              dataView,
+              excludedVisualizations,
            });
          },
        };
--- a/x-pack/plugins/lens/public/types.ts
+++ b/x-pack/plugins/lens/public/types.ts
@ -874,6 +874,8 @@ export interface Suggestion<T = unknown, V = unknown> {
  previewExpression?: Ast | string;
  previewIcon: IconType;
  hide?: boolean;
+  // flag to indicate if the visualization is incomplete
+  incomplete?: boolean;
  changeType: TableChangeType;
  keptLayerIds: string[];
 }
@ -926,6 +928,10 @@ export interface VisualizationSuggestion<T = unknown> {
   * directly.
   */
  hide?: boolean;
+  /**
+   * Flag indicating whether this suggestion is incomplete
+   */
+  incomplete?: boolean;
  /**
   * Descriptive title of the suggestion. Should be as short as possible. This title is shown if
   * the suggestion is advertised to the user and will also show either the `previewExpression` or
--- a/x-pack/plugins/lens/public/visualizations/gauge/suggestions.test.ts
+++ b/x-pack/plugins/lens/public/visualizations/gauge/suggestions.test.ts
@ -158,11 +158,13 @@ describe('shows suggestions', () => {
        },
        title: 'Gauge',
        hide: true,
+        incomplete: true,
        previewIcon: IconChartHorizontalBullet,
        score: 0.5,
      },
      {
        hide: true,
+        incomplete: true,
        previewIcon: IconChartVerticalBullet,
        title: 'Gauge',
        score: 0.5,
@ -208,6 +210,7 @@ describe('shows suggestions', () => {
        previewIcon: IconChartVerticalBullet,
        title: 'Gauge',
        hide: false, // shows suggestion when current is gauge
+        incomplete: false,
        score: 0.5,
      },
    ]);
--- a/x-pack/plugins/lens/public/visualizations/gauge/suggestions.ts
+++ b/x-pack/plugins/lens/public/visualizations/gauge/suggestions.ts
@ -69,6 +69,7 @@ export const getSuggestions: Visualization<GaugeVisualizationState>['getSuggesti
      shape === GaugeShapes.VERTICAL_BULLET ? IconChartVerticalBullet : IconChartHorizontalBullet,
    score: 0.5,
    hide: !isGauge || state?.metricAccessor === undefined, // only display for gauges for beta
+    incomplete: state?.metricAccessor === undefined,
  };

  const suggestions = isGauge
--- a/x-pack/plugins/lens/public/visualizations/heatmap/suggestions.test.ts
+++ b/x-pack/plugins/lens/public/visualizations/heatmap/suggestions.test.ts
@ -299,6 +299,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: true,
+          incomplete: false,
          previewIcon: IconChartHeatmap,
          score: 0.3,
        },
@ -352,6 +353,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: true,
+          incomplete: true,
          previewIcon: IconChartHeatmap,
          score: 0,
        },
@ -405,6 +407,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: true,
+          incomplete: true,
          previewIcon: IconChartHeatmap,
          score: 0.3,
        },
@ -469,6 +472,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: true,
+          incomplete: false,
          previewIcon: IconChartHeatmap,
          score: 0.3,
        },
@ -535,6 +539,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: false,
+          incomplete: false,
          previewIcon: IconChartHeatmap,
          score: 0.6,
        },
@ -609,6 +614,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: false,
+          incomplete: false,
          previewIcon: IconChartHeatmap,
          score: 0.3,
        },
@ -683,6 +689,7 @@ describe('heatmap suggestions', () => {
          },
          title: 'Heat map',
          hide: false,
+          incomplete: false,
          previewIcon: IconChartHeatmap,
          score: 0.9,
        },
--- a/x-pack/plugins/lens/public/visualizations/heatmap/suggestions.ts
+++ b/x-pack/plugins/lens/public/visualizations/heatmap/suggestions.ts
@ -130,6 +130,7 @@ export const getSuggestions: Visualization<HeatmapVisualizationState>['getSugges
      hide,
      previewIcon: IconChartHeatmap,
      score: Number(score.toFixed(1)),
+      incomplete: isSingleBucketDimension || isOnlyMetricDimension,
    },
  ];
 };
--- a/x-pack/plugins/lens/public/visualizations/partition/suggestions.test.ts
+++ b/x-pack/plugins/lens/public/visualizations/partition/suggestions.test.ts
@ -455,6 +455,7 @@ describe('suggestions', () => {
      });
      expect(currentSuggestions).toHaveLength(5);
      expect(currentSuggestions.every((s) => s.hide)).toEqual(true);
+      expect(currentSuggestions.every((s) => s.incomplete)).toEqual(true);
    });

    it('should suggest a donut chart as initial state when only one bucket', () => {
@ -1039,6 +1040,7 @@ describe('suggestions', () => {
        Array [
          Object {
            "hide": false,
+            "incomplete": false,
            "previewIcon": [Function],
            "score": 0.61,
            "state": Object {
@ -1148,6 +1150,7 @@ describe('suggestions', () => {
        Array [
          Object {
            "hide": false,
+            "incomplete": false,
            "previewIcon": [Function],
            "score": 0.46,
            "state": Object {
--- a/x-pack/plugins/lens/public/visualizations/partition/suggestions.ts
+++ b/x-pack/plugins/lens/public/visualizations/partition/suggestions.ts
@ -330,5 +330,6 @@ export function suggestions({
    .map((suggestion) => ({
      ...suggestion,
      hide: shouldHideSuggestion || incompleteConfiguration || suggestion.hide,
+      incomplete: incompleteConfiguration,
    }));
 }
--- a/x-pack/plugins/lens/public/visualizations/xy/xy_suggestions.test.ts
+++ b/x-pack/plugins/lens/public/visualizations/xy/xy_suggestions.test.ts
@ -149,6 +149,25 @@ describe('xy_suggestions', () => {
    );
  });

+  test('marks incomplete as true when no metric is provided', () => {
+    expect(
+      (
+        [
+          {
+            isMultiRow: true,
+            columns: [strCol('foo')],
+            layerId: 'first',
+            changeType: 'unchanged',
+          },
+        ] as TableSuggestion[]
+      ).map((table) => {
+        const suggestions = getSuggestions({ table, keptLayerIds: [] });
+        expect(suggestions.every((suggestion) => suggestion.incomplete)).toEqual(true);
+        expect(suggestions).toHaveLength(10);
+      })
+    );
+  });
+
  test('rejects the configuration when metric isStaticValue', () => {
    (generateId as jest.Mock).mockReturnValueOnce('aaa');
    const suggestions = getSuggestions({
--- a/x-pack/plugins/lens/public/visualizations/xy/xy_suggestions.ts
+++ b/x-pack/plugins/lens/public/visualizations/xy/xy_suggestions.ts
@ -573,6 +573,8 @@ function buildSuggestion({
      existingLayer && Object.keys(existingLayer).length ? keptLayers : [...keptLayers, newLayer],
  };

+  const isIncomplete = yValues.length === 0;
+
  return {
    title,
    score: getScore(yValues, splitBy, changeType),
@ -583,10 +585,11 @@ function buildSuggestion({
        // Don't advertise removing dimensions
        (currentState && changeType === 'reduced') ||
        // Don't advertise charts without y axis
-        yValues.length === 0 ||
+        isIncomplete ||
        // Don't advertise charts without at least one split
        (!xValue && !splitBy)),
    state,
+    incomplete: isIncomplete,
    previewIcon: getIconForSeries(seriesType),
  };
 }