fix(chart): Horizontal bar chart value labels cut off (#36989)

superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/helpers.ts

@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Base timestamp used for test data generation.
+ * This provides a consistent starting point for all test timestamps.
+ */
+export const BASE_TIMESTAMP = 599616000000;
+
+/**
+ * Default interval between timestamps (300000000 ms ≈ 3.47 days).
+ * This matches the common pattern used in tests.
+ */
+export const DEFAULT_TIMESTAMP_INTERVAL = 300000000;
+
+/**
+ * Creates a timestamp with an offset from the base timestamp.
+ *
+ * @param offset - Offset in milliseconds from BASE_TIMESTAMP
+ * @returns Timestamp value
+ */
+export function createTimestamp(offset: number = 0): number {
+  return BASE_TIMESTAMP + offset;
+}
+
+/**
+ * Creates an array of timestamps starting from the base timestamp.
+ *
+ * @param count - Number of timestamps to generate
+ * @param intervalMs - Interval between timestamps in milliseconds (default: DEFAULT_TIMESTAMP_INTERVAL)
+ * @returns Array of timestamp values
+ */
+export function createTimestamps(
+  count: number,
+  intervalMs: number = DEFAULT_TIMESTAMP_INTERVAL,
+): number[] {
+  return Array.from({ length: count }, (_, index) =>
+    createTimestamp(index * intervalMs),
+  );
+}
+
+/**
+ * Creates a single test data row with a timestamp.
+ *
+ * @param values - Object containing series values (excluding __timestamp)
+ * @param timestamp - Timestamp value to include
+ * @returns Test data row with __timestamp property
+ */
+export function createTestDataRow(
+  values: Record<string, number>,
+  timestamp: number,
+): Record<string, number> & { __timestamp: number } {
+  return {
+    ...values,
+    __timestamp: timestamp,
+  };
+}
+
+/**
+ * Options for creating test data.
+ */
+export interface CreateTestDataOptions {
+  /** Base timestamp to start from (default: BASE_TIMESTAMP) */
+  baseTimestamp?: number;
+  /** Interval between timestamps in milliseconds (default: DEFAULT_TIMESTAMP_INTERVAL) */
+  intervalMs?: number;
+}
+
+/**
+ * Creates an array of test data rows with auto-generated timestamps.
+ *
+ * @param rows - Array of objects containing series values (without __timestamp)
+ * @param options - Options for timestamp generation
+ * @returns Array of test data rows with __timestamp properties
+ *
+ * @example
+ * ```typescript
+ * const data = createTestData(
+ *   [
+ *     { 'Series A': 15000 },
+ *     { 'Series A': 20000 },
+ *     { 'Series A': 18000 },
+ *   ],
+ *   { intervalMs: 300000000 }
+ * );
+ * // Returns:
+ * // [
+ * //   { 'Series A': 15000, __timestamp: 599616000000 },
+ * //   { 'Series A': 20000, __timestamp: 599916000000 },
+ * //   { 'Series A': 18000, __timestamp: 600216000000 },
+ * // ]
+ * ```
+ */
+export function createTestData(
+  rows: Array<Record<string, number>>,
+  options: CreateTestDataOptions = {},
+): Array<Record<string, number> & { __timestamp: number }> {
+  const {
+    baseTimestamp = BASE_TIMESTAMP,
+    intervalMs = DEFAULT_TIMESTAMP_INTERVAL,
+  } = options;
+
+  return rows.map((row, index) =>
+    createTestDataRow(row, baseTimestamp + index * intervalMs),
+  );
+}

superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts

@@ -30,6 +30,11 @@ import {
 import { supersetTheme } from '@apache-superset/core/ui';
 import { EchartsTimeseriesChartProps } from '../../src/types';
 import transformProps from '../../src/Timeseries/transformProps';
+import {
+  EchartsTimeseriesSeriesType,
+  OrientationType,
+} from '../../src/Timeseries/types';
+import { BASE_TIMESTAMP, createTestData } from './helpers';
 
 type YAxisFormatter = (value: number, index: number) => string;
 
@@ -55,10 +60,13 @@ const formData: SqlaFormData = {
 };
 const queriesData = [
   {
-    data: [
-      { 'San Francisco': 1, 'New York': 2, __timestamp: 599616000000 },
-      { 'San Francisco': 3, 'New York': 4, __timestamp: 599916000000 },
-    ],
+    data: createTestData(
+      [
+        { 'San Francisco': 1, 'New York': 2 },
+        { 'San Francisco': 3, 'New York': 4 },
+      ],
+      { intervalMs: 300000000 },
+    ),
   },
 ];
 const chartPropsConfig = {
@@ -83,15 +91,15 @@ describe('EchartsTimeseries transformProps', () => {
           series: expect.arrayContaining([
             expect.objectContaining({
               data: [
-                [599616000000, 1],
-                [599916000000, 3],
+                [BASE_TIMESTAMP, 1],
+                [BASE_TIMESTAMP + 300000000, 3],
               ],
               name: 'San Francisco',
             }),
             expect.objectContaining({
               data: [
-                [599616000000, 2],
-                [599916000000, 4],
+                [BASE_TIMESTAMP, 2],
+                [BASE_TIMESTAMP + 300000000, 4],
               ],
               name: 'New York',
             }),
@@ -120,15 +128,15 @@ describe('EchartsTimeseries transformProps', () => {
           series: expect.arrayContaining([
             expect.objectContaining({
               data: [
-                [1, 599616000000],
-                [3, 599916000000],
+                [1, BASE_TIMESTAMP],
+                [3, BASE_TIMESTAMP + 300000000],
               ],
               name: 'San Francisco',
             }),
             expect.objectContaining({
               data: [
-                [2, 599616000000],
-                [4, 599916000000],
+                [2, BASE_TIMESTAMP],
+                [4, BASE_TIMESTAMP + 300000000],
               ],
               name: 'New York',
             }),
@@ -165,22 +173,22 @@ describe('EchartsTimeseries transformProps', () => {
           series: expect.arrayContaining([
             expect.objectContaining({
               data: [
-                [599616000000, 1],
-                [599916000000, 3],
+                [BASE_TIMESTAMP, 1],
+                [BASE_TIMESTAMP + 300000000, 3],
               ],
               name: 'San Francisco',
             }),
             expect.objectContaining({
               data: [
-                [599616000000, 2],
-                [599916000000, 4],
+                [BASE_TIMESTAMP, 2],
+                [BASE_TIMESTAMP + 300000000, 4],
               ],
               name: 'New York',
             }),
             expect.objectContaining({
               data: [
-                [599616000000, 599616000001],
-                [599916000000, 599916000001],
+                [BASE_TIMESTAMP, BASE_TIMESTAMP + 1],
+                [BASE_TIMESTAMP + 300000000, BASE_TIMESTAMP + 300000000 + 1],
               ],
               name: 'My Formula',
             }),
@@ -303,64 +311,60 @@ describe('EchartsTimeseries transformProps', () => {
   it('Should add a baseline series for stream graph', () => {
     const streamQueriesData = [
       {
-        data: [
-          {
-            'San Francisco': 120,
-            'New York': 220,
-            Boston: 150,
-            Miami: 270,
-            Denver: 800,
-            __timestamp: 599616000000,
-          },
-          {
-            'San Francisco': 150,
-            'New York': 190,
-            Boston: 240,
-            Miami: 350,
-            Denver: 700,
-            __timestamp: 599616000001,
-          },
-          {
-            'San Francisco': 130,
-            'New York': 300,
-            Boston: 250,
-            Miami: 410,
-            Denver: 650,
-            __timestamp: 599616000002,
-          },
-          {
-            'San Francisco': 90,
-            'New York': 340,
-            Boston: 300,
-            Miami: 480,
-            Denver: 590,
-            __timestamp: 599616000003,
-          },
-          {
-            'San Francisco': 260,
-            'New York': 200,
-            Boston: 420,
-            Miami: 490,
-            Denver: 760,
-            __timestamp: 599616000004,
-          },
-          {
-            'San Francisco': 250,
-            'New York': 250,
-            Boston: 380,
-            Miami: 360,
-            Denver: 400,
-            __timestamp: 599616000005,
-          },
-          {
-            'San Francisco': 160,
-            'New York': 210,
-            Boston: 330,
-            Miami: 440,
-            Denver: 580,
-            __timestamp: 599616000006,
-          },
-        ],
+        data: createTestData(
+          [
+            {
+              'San Francisco': 120,
+              'New York': 220,
+              Boston: 150,
+              Miami: 270,
+              Denver: 800,
+            },
+            {
+              'San Francisco': 150,
+              'New York': 190,
+              Boston: 240,
+              Miami: 350,
+              Denver: 700,
+            },
+            {
+              'San Francisco': 130,
+              'New York': 300,
+              Boston: 250,
+              Miami: 410,
+              Denver: 650,
+            },
+            {
+              'San Francisco': 90,
+              'New York': 340,
+              Boston: 300,
+              Miami: 480,
+              Denver: 590,
+            },
+            {
+              'San Francisco': 260,
+              'New York': 200,
+              Boston: 420,
+              Miami: 490,
+              Denver: 760,
+            },
+            {
+              'San Francisco': 250,
+              'New York': 250,
+              Boston: 380,
+              Miami: 360,
+              Denver: 400,
+            },
+            {
+              'San Francisco': 160,
+              'New York': 210,
+              Boston: 330,
+              Miami: 440,
+              Denver: 580,
+            },
+          ],
+          { intervalMs: 1 },
+        ),
       },
     ];
     const streamFormData = { ...formData, stack: 'Stream' };
@@ -395,13 +399,13 @@ describe('EchartsTimeseries transformProps', () => {
       },
       type: 'line',
       data: [
-        [599616000000, -415.7692307692308],
-        [599616000001, -403.6219915054271],
-        [599616000002, -476.32314093071443],
-        [599616000003, -514.2120298196033],
-        [599616000004, -485.7378514158475],
-        [599616000005, -419.6402904402378],
-        [599616000006, -442.9833136960517],
+        [BASE_TIMESTAMP, -415.7692307692308],
+        [BASE_TIMESTAMP + 1, -403.6219915054271],
+        [BASE_TIMESTAMP + 2, -476.32314093071443],
+        [BASE_TIMESTAMP + 3, -514.2120298196033],
+        [BASE_TIMESTAMP + 4, -485.7378514158475],
+        [BASE_TIMESTAMP + 5, -419.6402904402378],
+        [BASE_TIMESTAMP + 6, -442.9833136960517],
       ],
     });
   });
@@ -434,32 +438,31 @@ describe('Does transformProps transform series correctly', () => {
   };
   const queriesData = [
     {
-      data: [
-        {
-          'San Francisco': 1,
-          'New York': 2,
-          Boston: 1,
-          __timestamp: 599616000000,
-        },
-        {
-          'San Francisco': 3,
-          'New York': 4,
-          Boston: 1,
-          __timestamp: 599916000000,
-        },
-        {
-          'San Francisco': 5,
-          'New York': 8,
-          Boston: 6,
-          __timestamp: 600216000000,
-        },
-        {
-          'San Francisco': 2,
-          'New York': 7,
-          Boston: 2,
-          __timestamp: 600516000000,
-        },
-      ],
+      data: createTestData(
+        [
+          {
+            'San Francisco': 1,
+            'New York': 2,
+            Boston: 1,
+          },
+          {
+            'San Francisco': 3,
+            'New York': 4,
+            Boston: 1,
+          },
+          {
+            'San Francisco': 5,
+            'New York': 8,
+            Boston: 6,
+          },
+          {
+            'San Francisco': 2,
+            'New York': 7,
+            Boston: 2,
+          },
+        ],
+        { intervalMs: 300000000 },
+      ),
     },
   ];
   const chartPropsConfig = {
@@ -643,36 +646,35 @@ describe('Does transformProps transform series correctly', () => {
 describe('legend sorting', () => {
   const legendSortData = [
     {
-      data: [
-        {
-          Milton: 40,
-          'San Francisco': 1,
-          'New York': 2,
-          Boston: 1,
-          __timestamp: 599616000000,
-        },
-        {
-          Milton: 20,
-          'San Francisco': 3,
-          'New York': 4,
-          Boston: 1,
-          __timestamp: 599916000000,
-        },
-        {
-          Milton: 60,
-          'San Francisco': 5,
-          'New York': 8,
-          Boston: 6,
-          __timestamp: 600216000000,
-        },
-        {
-          Milton: 10,
-          'San Francisco': 2,
-          'New York': 7,
-          Boston: 2,
-          __timestamp: 600516000000,
-        },
-      ],
+      data: createTestData(
+        [
+          {
+            Milton: 40,
+            'San Francisco': 1,
+            'New York': 2,
+            Boston: 1,
+          },
+          {
+            Milton: 20,
+            'San Francisco': 3,
+            'New York': 4,
+            Boston: 1,
+          },
+          {
+            Milton: 60,
+            'San Francisco': 5,
+            'New York': 8,
+            Boston: 6,
+          },
+          {
+            Milton: 10,
+            'San Francisco': 2,
+            'New York': 7,
+            Boston: 2,
+          },
+        ],
+        { intervalMs: 300000000 },
+      ),
     },
   ];
 
@@ -855,3 +857,164 @@ test('EchartsTimeseries should preserve static currency format with £ for GBP',
   const formatter = getYAxisFormatter(transformed);
   expect(formatter(1000, 0)).toContain('£');
 });
+
+const baseFormDataHorizontalBar: SqlaFormData = {
+  colorScheme: 'bnbColors',
+  datasource: '3__table',
+  granularity_sqla: '__timestamp',
+  metric: 'sum__num',
+  groupby: [],
+  viz_type: 'echarts_timeseries',
+  seriesType: EchartsTimeseriesSeriesType.Bar,
+  orientation: OrientationType.Horizontal,
+  truncateYAxis: true,
+  yAxisBounds: [null, null],
+};
+
+test('should set yAxis max to actual data max for horizontal bar charts', () => {
+  const queriesData = [
+    {
+      data: createTestData(
+        [{ 'Series A': 15000 }, { 'Series A': 20000 }, { 'Series A': 18000 }],
+        { intervalMs: 300000000 },
+      ),
+    },
+  ];
+
+  const chartProps = new ChartProps({
+    formData: baseFormDataHorizontalBar,
+    width: 800,
+    height: 600,
+    queriesData,
+    theme: supersetTheme,
+  });
+
+  const transformedProps = transformProps(
+    chartProps as EchartsTimeseriesChartProps,
+  );
+
+  // In horizontal orientation, axes are swapped, so yAxis becomes xAxis
+  const xAxisRaw = transformedProps.echartOptions.xAxis as any;
+  expect(xAxisRaw.max).toBe(20000); // Should be the actual max value, not rounded
+});
+
+test('should set yAxis min and max for diverging horizontal bar charts', () => {
+  const queriesData = [
+    {
+      data: createTestData(
+        [{ 'Series A': -21000 }, { 'Series A': 20000 }, { 'Series A': 18000 }],
+        { intervalMs: 300000000 },
+      ),
+    },
+  ];
+
+  const chartProps = new ChartProps({
+    formData: baseFormDataHorizontalBar,
+    width: 800,
+    height: 600,
+    queriesData,
+    theme: supersetTheme,
+  });
+
+  const transformedProps = transformProps(
+    chartProps as EchartsTimeseriesChartProps,
+  );
+
+  // In horizontal orientation, axes are swapped, so yAxis becomes xAxis
+  const xAxisRaw = transformedProps.echartOptions.xAxis as any;
+  expect(xAxisRaw.max).toBe(20000); // Should be the actual max value
+  expect(xAxisRaw.min).toBe(-21000); // Should be the actual min value for diverging bars
+});
+
+test('should not override explicit yAxisBounds for horizontal bar charts', () => {
+  const queriesData = [
+    {
+      data: createTestData(
+        [{ 'Series A': 15000 }, { 'Series A': 20000 }, { 'Series A': 18000 }],
+        { intervalMs: 300000000 },
+      ),
+    },
+  ];
+
+  const chartProps = new ChartProps({
+    formData: {
+      ...baseFormDataHorizontalBar,
+      yAxisBounds: [0, 25000], // Explicit bounds
+    },
+    width: 800,
+    height: 600,
+    queriesData,
+    theme: supersetTheme,
+  });
+
+  const transformedProps = transformProps(
+    chartProps as EchartsTimeseriesChartProps,
+  );
+
+  // In horizontal orientation, axes are swapped, so yAxis becomes xAxis
+  const xAxisRaw = transformedProps.echartOptions.xAxis as any;
+  expect(xAxisRaw.max).toBe(25000); // Should respect explicit bound
+  expect(xAxisRaw.min).toBe(0); // Should respect explicit bound
+});
+
+test('should not apply axis bounds calculation when truncateYAxis is false for horizontal bar charts', () => {
+  const queriesData = [
+    {
+      data: createTestData(
+        [{ 'Series A': 15000 }, { 'Series A': 20000 }, { 'Series A': 18000 }],
+        { intervalMs: 300000000 },
+      ),
+    },
+  ];
+
+  const chartProps = new ChartProps({
+    formData: {
+      ...baseFormDataHorizontalBar,
+      truncateYAxis: false,
+    },
+    width: 800,
+    height: 600,
+    queriesData,
+    theme: supersetTheme,
+  });
+
+  const transformedProps = transformProps(
+    chartProps as EchartsTimeseriesChartProps,
+  );
+
+  // In horizontal orientation, axes are swapped, so yAxis becomes xAxis
+  const xAxis = transformedProps.echartOptions.xAxis as any;
+  // Should not have explicit max set when truncateYAxis is false
+  expect(xAxis.max).toBeUndefined();
+});
+
+test('should not apply axis bounds calculation when seriesType is not Bar for horizontal charts', () => {
+  const queriesData = [
+    {
+      data: createTestData(
+        [{ 'Series A': 15000 }, { 'Series A': 20000 }, { 'Series A': 18000 }],
+        { intervalMs: 300000000 },
+      ),
+    },
+  ];
+
+  const chartProps = new ChartProps({
+    formData: {
+      ...baseFormDataHorizontalBar,
+      seriesType: EchartsTimeseriesSeriesType.Line,
+    },
+    width: 800,
+    height: 600,
+    queriesData,
+    theme: supersetTheme,
+  });
+
+  const transformedProps = transformProps(
+    chartProps as EchartsTimeseriesChartProps,
+  );
+
+  // In horizontal orientation, axes are swapped, so yAxis becomes xAxis
+  const xAxisRaw = transformedProps.echartOptions.xAxis as any;
+  // Should not have explicit max set when seriesType is not Bar
+  expect(xAxisRaw.max).toBeUndefined();
+});