Barchart: Fix warning not showing (#51190)

public/app/plugins/panel/barchart/BarChartPanel.tsx

@@ -102,6 +102,8 @@ export const BarChartPanel: React.FunctionComponent<Props> = ({
 
   const frame0Ref = useRef<DataFrame>();
   const info = useMemo(() => prepareBarChartDisplayValues(data?.series, theme, options), [data, theme, options]);
+  const chartDisplay = 'viz' in info ? info : null;
+
   const structureRef = useRef(10000);
   useMemo(() => {
     structureRef.current++;
@@ -109,14 +111,14 @@ export const BarChartPanel: React.FunctionComponent<Props> = ({
   }, [options]); // change every time the options object changes (while editing)
 
   const structureRev = useMemo(() => {
-    const f0 = info.viz[0];
+    const f0 = chartDisplay?.viz[0];
     const f1 = frame0Ref.current;
     if (!(f0 && f1 && compareDataFrameStructures(f0, f1, true))) {
       structureRef.current++;
     }
     frame0Ref.current = f0;
     return (data.structureRev ?? 0) + structureRef.current;
-  }, [info, data.structureRev]);
+  }, [chartDisplay, data.structureRev]);
 
   const orientation = useMemo(() => {
     if (!options.orientation || options.orientation === VizOrientation.Auto) {
@@ -142,7 +144,7 @@ export const BarChartPanel: React.FunctionComponent<Props> = ({
     }
   }, [height, options.xTickLabelRotation, options.xTickLabelMaxLength]);
 
-  if (!info.viz[0]?.fields.length) {
+  if ('warn' in info) {
     return (
       <PanelDataErrorView
         panelId={id}

public/app/plugins/panel/barchart/module.tsx

@@ -70,7 +70,7 @@ export const plugin = new PanelPlugin<PanelOptions, BarChartFieldConfig>(BarChar
   .setPanelOptions((builder, context) => {
     const disp = prepareBarChartDisplayValues(context.data, config.theme2, context.options ?? ({} as any));
     let xaxisPlaceholder = 'First string or time field';
-    const viz = disp.viz ? disp.viz[0] : undefined;
+    const viz = 'viz' in disp ? disp.viz[0] : undefined;
     if (viz?.fields?.length) {
       const first = viz.fields[0];
       xaxisPlaceholder += ` (${getFieldDisplayName(first, viz)})`;

public/app/plugins/panel/barchart/types.ts

@@ -1,9 +1,6 @@
 import { DataFrame, Field } from '@grafana/data';
 
 export interface BarChartDisplayValues {
-  /** When the data can not display, this will be returned */
-  warn?: string;
-
   /** All fields joined */
   aligned: DataFrame;
 
@@ -16,3 +13,8 @@ export interface BarChartDisplayValues {
   /** Potentialy color by a field value */
   colorByField?: Field;
 }
+
+export interface BarChartDisplayWarning {
+  /** When the data can not display, this will be returned */
+  warn: string;
+}

public/app/plugins/panel/barchart/utils.test.ts

@@ -1,3 +1,5 @@
+import { assertIsDefined } from 'test/helpers/asserts';
+
 import {
   createTheme,
   DefaultTimeZone,
@@ -67,7 +69,13 @@ function mockDataFrame() {
     state: {},
   });
 
-  return prepareBarChartDisplayValues([df1], createTheme(), {} as any).aligned;
+  const info = prepareBarChartDisplayValues([df1], createTheme(), {} as any);
+
+  if (!('aligned' in info)) {
+    throw new Error('Bar chart not prepared correctly');
+  }
+
+  return info.aligned;
 }
 
 jest.mock('@grafana/data', () => ({
@@ -150,7 +158,9 @@ describe('BarChart utils', () => {
   describe('prepareGraphableFrames', () => {
     it('will warn when there is no data in the response', () => {
       const result = prepareBarChartDisplayValues([], createTheme(), { stacking: StackingMode.None } as any);
-      expect(result.warn).toEqual('No data in response');
+      const warning = assertIsDefined('warn' in result ? result : null);
+
+      expect(warning.warn).toEqual('No data in response');
     });
 
     it('will warn when there is no string or time field', () => {
@@ -161,8 +171,9 @@ describe('BarChart utils', () => {
         ],
       });
       const result = prepareBarChartDisplayValues([df], createTheme(), { stacking: StackingMode.None } as any);
-      expect(result.warn).toEqual('Bar charts requires a string or time field');
-      expect(result.viz).toBeUndefined();
+      const warning = assertIsDefined('warn' in result ? result : null);
+      expect(warning.warn).toEqual('Bar charts requires a string or time field');
+      expect(warning).not.toHaveProperty('viz');
     });
 
     it('will warn when there are no numeric fields in the response', () => {
@@ -173,8 +184,9 @@ describe('BarChart utils', () => {
         ],
       });
       const result = prepareBarChartDisplayValues([df], createTheme(), { stacking: StackingMode.None } as any);
-      expect(result.warn).toEqual('No numeric fields found');
-      expect(result.viz).toBeUndefined();
+      const warning = assertIsDefined('warn' in result ? result : null);
+      expect(warning.warn).toEqual('No numeric fields found');
+      expect(warning).not.toHaveProperty('viz');
     });
 
     it('will convert NaN and Infinty to nulls', () => {
@@ -185,9 +197,10 @@ describe('BarChart utils', () => {
         ],
       });
       const result = prepareBarChartDisplayValues([df], createTheme(), { stacking: StackingMode.None } as any);
+      const displayValues = assertIsDefined('viz' in result ? result : null);
 
-      const field = result.viz[0].fields[1];
-      expect(field!.values.toArray()).toMatchInlineSnapshot(`
+      const field = displayValues.viz[0].fields[1];
+      expect(field.values.toArray()).toMatchInlineSnapshot(`
       Array [
         -10,
         null,
@@ -210,19 +223,21 @@ describe('BarChart utils', () => {
 
       const resultAsc = prepareBarChartDisplayValues([frame], createTheme(), {
         legend: { sortBy: 'Min', sortDesc: false },
-      } as any).viz[0];
-      expect(resultAsc.fields[0].type).toBe(FieldType.string);
-      expect(resultAsc.fields[1].name).toBe('a');
-      expect(resultAsc.fields[2].name).toBe('c');
-      expect(resultAsc.fields[3].name).toBe('b');
+      } as any);
+      const displayValuesAsc = assertIsDefined('viz' in resultAsc ? resultAsc : null).viz[0];
+      expect(displayValuesAsc.fields[0].type).toBe(FieldType.string);
+      expect(displayValuesAsc.fields[1].name).toBe('a');
+      expect(displayValuesAsc.fields[2].name).toBe('c');
+      expect(displayValuesAsc.fields[3].name).toBe('b');
 
       const resultDesc = prepareBarChartDisplayValues([frame], createTheme(), {
         legend: { sortBy: 'Min', sortDesc: true },
-      } as any).viz[0];
-      expect(resultDesc.fields[0].type).toBe(FieldType.string);
-      expect(resultDesc.fields[1].name).toBe('b');
-      expect(resultDesc.fields[2].name).toBe('c');
-      expect(resultDesc.fields[3].name).toBe('a');
+      } as any);
+      const displayValuesDesc = assertIsDefined('viz' in resultDesc ? resultDesc : null).viz[0];
+      expect(displayValuesDesc.fields[0].type).toBe(FieldType.string);
+      expect(displayValuesDesc.fields[1].name).toBe('b');
+      expect(displayValuesDesc.fields[2].name).toBe('c');
+      expect(displayValuesDesc.fields[3].name).toBe('a');
     });
   });
 });

public/app/plugins/panel/barchart/utils.ts

@@ -30,7 +30,7 @@ import { findField } from 'app/features/dimensions';
 
 import { BarsOptions, getConfig } from './bars';
 import { BarChartFieldConfig, PanelOptions, defaultBarChartFieldConfig } from './models.gen';
-import { BarChartDisplayValues } from './types';
+import { BarChartDisplayValues, BarChartDisplayWarning } from './types';
 
 function getBarCharScaleOrientation(orientation: VizOrientation) {
   if (orientation === VizOrientation.Vertical) {
@@ -298,9 +298,9 @@ export function prepareBarChartDisplayValues(
   series: DataFrame[],
   theme: GrafanaTheme2,
   options: PanelOptions
-): BarChartDisplayValues {
+): BarChartDisplayValues | BarChartDisplayWarning {
   if (!series?.length) {
-    return { warn: 'No data in response' } as BarChartDisplayValues;
+    return { warn: 'No data in response' };
   }
 
   // Bar chart requires a single frame
@@ -312,7 +312,7 @@ export function prepareBarChartDisplayValues(
         )
       : outerJoinDataFrames({ frames: series });
   if (!frame) {
-    return { warn: 'Unable to join data' } as BarChartDisplayValues;
+    return { warn: 'Unable to join data' };
   }
 
   // Color by a field different than the input
@@ -320,7 +320,7 @@ export function prepareBarChartDisplayValues(
   if (options.colorByField) {
     colorByField = findField(frame, options.colorByField);
     if (!colorByField) {
-      return { warn: 'Color field not found' } as BarChartDisplayValues;
+      return { warn: 'Color field not found' };
     }
   }
 
@@ -328,7 +328,7 @@ export function prepareBarChartDisplayValues(
   if (options.xField) {
     xField = findField(frame, options.xField);
     if (!xField) {
-      return { warn: 'Configured x field not found' } as BarChartDisplayValues;
+      return { warn: 'Configured x field not found' };
     }
   }
 
@@ -398,13 +398,13 @@ export function prepareBarChartDisplayValues(
   if (!firstField) {
     return {
       warn: 'Bar charts requires a string or time field',
-    } as BarChartDisplayValues;
+    };
   }
 
   if (!fields.length) {
     return {
       warn: 'No numeric fields found',
-    } as BarChartDisplayValues;
+    };
   }
 
   // Show the first number value