fix(echarts): adaptive formatting labels (#38017)

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

@@ -619,7 +619,7 @@ export default function transformProps(
       : String;
   const xAxisFormatter =
     xAxisDataType === GenericDataType.Temporal
-      ? getXAxisFormatter(xAxisTimeFormat)
+      ? getXAxisFormatter(xAxisTimeFormat, timeGrainSqla)
       : xAxisDataType === GenericDataType.Numeric
         ? getNumberFormatter(xAxisNumberFormat)
         : String;

superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts

@@ -29,13 +29,99 @@ import {
   SMART_DATE_ID,
   SMART_DATE_VERBOSE_ID,
   TimeFormatter,
+  TimeGranularity,
   ValueFormatter,
 } from '@superset-ui/core';
 
 export const getSmartDateDetailedFormatter = () =>
   getTimeFormatter(SMART_DATE_DETAILED_ID);
 
-export const getSmartDateFormatter = () => getTimeFormatter(SMART_DATE_ID);
+export const getSmartDateFormatter = (timeGrain?: string) => {
+  const baseFormatter = getTimeFormatter(SMART_DATE_ID);
+
+  // If no time grain provided, use the standard smart date formatter
+  if (!timeGrain) {
+    return baseFormatter;
+  }
+
+  // Create a wrapper that normalizes dates based on time grain
+  return new TimeFormatter({
+    id: SMART_DATE_ID,
+    label: baseFormatter.label,
+    formatFunc: (date: Date) => {
+      // Create a normalized date based on time grain to ensure consistent smart formatting
+      const normalizedDate = new Date(date);
+
+      // Always remove milliseconds to prevent .XXXms format
+      normalizedDate.setMilliseconds(0);
+
+      // For all time grains, normalize using UTC methods to avoid timezone issues
+      if (timeGrain === TimeGranularity.YEAR) {
+        // Set to January 1st at midnight UTC - smart formatter will show year
+        const year = normalizedDate.getUTCFullYear();
+        const cleanDate = new Date(Date.UTC(year, 0, 1, 0, 0, 0, 0));
+        return baseFormatter(cleanDate);
+      } else if (timeGrain === TimeGranularity.QUARTER) {
+        // Set to first month of quarter, first day, midnight UTC
+        const year = normalizedDate.getUTCFullYear();
+        const month = normalizedDate.getUTCMonth();
+        const quarterStartMonth = Math.floor(month / 3) * 3;
+        const cleanDate = new Date(
+          Date.UTC(year, quarterStartMonth, 1, 0, 0, 0, 0),
+        );
+        return baseFormatter(cleanDate);
+      } else if (timeGrain === TimeGranularity.MONTH) {
+        // Set to first of month at midnight UTC - smart formatter will show month name or year
+        const year = normalizedDate.getUTCFullYear();
+        const month = normalizedDate.getUTCMonth();
+        const cleanDate = new Date(Date.UTC(year, month, 1, 0, 0, 0, 0));
+        return baseFormatter(cleanDate);
+      } else if (
+        timeGrain === TimeGranularity.WEEK ||
+        timeGrain === TimeGranularity.WEEK_STARTING_SUNDAY ||
+        timeGrain === TimeGranularity.WEEK_STARTING_MONDAY ||
+        timeGrain === TimeGranularity.WEEK_ENDING_SATURDAY ||
+        timeGrain === TimeGranularity.WEEK_ENDING_SUNDAY
+      ) {
+        // Set to midnight UTC, keep the day
+        const year = normalizedDate.getUTCFullYear();
+        const month = normalizedDate.getUTCMonth();
+        const day = normalizedDate.getUTCDate();
+        const cleanDate = new Date(Date.UTC(year, month, day, 0, 0, 0, 0));
+        return baseFormatter(cleanDate);
+      } else if (
+        timeGrain === TimeGranularity.DAY ||
+        timeGrain === TimeGranularity.DATE
+      ) {
+        // Set to midnight UTC
+        const year = normalizedDate.getUTCFullYear();
+        const month = normalizedDate.getUTCMonth();
+        const day = normalizedDate.getUTCDate();
+        const cleanDate = new Date(Date.UTC(year, month, day, 0, 0, 0, 0));
+        return baseFormatter(cleanDate);
+      } else if (
+        timeGrain === TimeGranularity.HOUR ||
+        timeGrain === TimeGranularity.THIRTY_MINUTES ||
+        timeGrain === TimeGranularity.FIFTEEN_MINUTES ||
+        timeGrain === TimeGranularity.TEN_MINUTES ||
+        timeGrain === TimeGranularity.FIVE_MINUTES ||
+        timeGrain === TimeGranularity.MINUTE ||
+        timeGrain === TimeGranularity.SECOND
+      ) {
+        // Set to top of hour UTC
+        const year = normalizedDate.getUTCFullYear();
+        const month = normalizedDate.getUTCMonth();
+        const day = normalizedDate.getUTCDate();
+        const hour = normalizedDate.getUTCHours();
+        const cleanDate = new Date(Date.UTC(year, month, day, hour, 0, 0, 0));
+        return baseFormatter(cleanDate);
+      }
+
+      // Use the base formatter on the normalized date
+      return baseFormatter(normalizedDate);
+    },
+  });
+};
 
 export const getSmartDateVerboseFormatter = () =>
   getTimeFormatter(SMART_DATE_VERBOSE_ID);
@@ -88,9 +174,10 @@ export function getTooltipTimeFormatter(
 
 export function getXAxisFormatter(
   format?: string,
+  timeGrain?: string,
 ): TimeFormatter | StringConstructor | undefined {
   if (format === SMART_DATE_ID || !format) {
-    return undefined;
+    return getSmartDateFormatter(timeGrain);
   }
   if (format) {
     return getTimeFormatter(format);

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

@@ -76,7 +76,7 @@ describe('Scatter Chart X-axis Time Formatting', () => {
     expect(transformedProps.echartOptions.xAxis).toHaveProperty('axisLabel');
     const xAxis = transformedProps.echartOptions.xAxis as any;
     expect(xAxis.axisLabel).toHaveProperty('formatter');
-    expect(xAxis.axisLabel.formatter).toBeUndefined();
+    expect(typeof xAxis.axisLabel.formatter).toBe('function');
   });
 
   test.each(D3_TIME_FORMAT_OPTIONS.map(([id]) => id))(
@@ -96,10 +96,8 @@ describe('Scatter Chart X-axis Time Formatting', () => {
 
       const xAxis = transformedProps.echartOptions.xAxis as any;
       expect(xAxis.axisLabel).toHaveProperty('formatter');
-      if (format === SMART_DATE_ID) {
-        expect(xAxis.axisLabel.formatter).toBeUndefined();
-      } else {
-        expect(typeof xAxis.axisLabel.formatter).toBe('function');
+      expect(typeof xAxis.axisLabel.formatter).toBe('function');
+      if (format !== SMART_DATE_ID) {
         expect(xAxis.axisLabel.formatter.id).toBe(format);
       }
     },

superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts

@@ -16,22 +16,166 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { NumberFormats } from '@superset-ui/core';
-import { getPercentFormatter } from '../../src/utils/formatters';
+import {
+  NumberFormats,
+  SMART_DATE_ID,
+  TimeFormatter,
+  TimeGranularity,
+} from '@superset-ui/core';
+import {
+  getPercentFormatter,
+  getXAxisFormatter,
+} from '../../src/utils/formatters';
 
-describe('getPercentFormatter', () => {
+test('getPercentFormatter should format as percent if no format is specified', () => {
   const value = 0.6;
-  test('should format as percent if no format is specified', () => {
-    expect(getPercentFormatter().format(value)).toEqual('60%');
-  });
-  test('should format as percent if SMART_NUMBER is specified', () => {
-    expect(
-      getPercentFormatter(NumberFormats.SMART_NUMBER).format(value),
-    ).toEqual('60%');
-  });
-  test('should format using a provided format', () => {
-    expect(
-      getPercentFormatter(NumberFormats.PERCENT_2_POINT).format(value),
-    ).toEqual('60.00%');
+  expect(getPercentFormatter().format(value)).toEqual('60%');
+});
+
+test('getPercentFormatter should format as percent if SMART_NUMBER is specified', () => {
+  const value = 0.6;
+  expect(getPercentFormatter(NumberFormats.SMART_NUMBER).format(value)).toEqual(
+    '60%',
+  );
+});
+
+test('getPercentFormatter should format using a provided format', () => {
+  const value = 0.6;
+  expect(
+    getPercentFormatter(NumberFormats.PERCENT_2_POINT).format(value),
+  ).toEqual('60.00%');
+});
+
+test('getXAxisFormatter should return smart date formatter for SMART_DATE_ID format', () => {
+  const formatter = getXAxisFormatter(SMART_DATE_ID);
+  expect(formatter).toBeDefined();
+  expect(formatter).toBeInstanceOf(TimeFormatter);
+  expect((formatter as TimeFormatter).id).toBe(SMART_DATE_ID);
+});
+
+test('getXAxisFormatter should return smart date formatter for undefined format', () => {
+  const formatter = getXAxisFormatter();
+  expect(formatter).toBeDefined();
+  expect(formatter).toBeInstanceOf(TimeFormatter);
+  expect((formatter as TimeFormatter).id).toBe(SMART_DATE_ID);
+});
+
+test('getXAxisFormatter should return custom time formatter for custom format', () => {
+  const customFormat = '%Y-%m-%d';
+  const formatter = getXAxisFormatter(customFormat);
+  expect(formatter).toBeDefined();
+  expect(formatter).toBeInstanceOf(TimeFormatter);
+  expect((formatter as TimeFormatter).id).toBe(customFormat);
+});
+
+test('getXAxisFormatter smart date formatter should be returned and not undefined', () => {
+  const formatter = getXAxisFormatter(SMART_DATE_ID);
+  expect(formatter).toBeDefined();
+  expect(formatter).toBeInstanceOf(TimeFormatter);
+  expect((formatter as TimeFormatter).id).toBe(SMART_DATE_ID);
+
+  const undefinedFormatter = getXAxisFormatter(undefined);
+  expect(undefinedFormatter).toBeDefined();
+  expect(undefinedFormatter).toBeInstanceOf(TimeFormatter);
+  expect((undefinedFormatter as TimeFormatter).id).toBe(SMART_DATE_ID);
+
+  const emptyFormatter = getXAxisFormatter();
+  expect(emptyFormatter).toBeDefined();
+  expect(emptyFormatter).toBeInstanceOf(TimeFormatter);
+  expect((emptyFormatter as TimeFormatter).id).toBe(SMART_DATE_ID);
+});
+
+test('getXAxisFormatter time grain aware formatter should prevent millisecond and timestamp formats', () => {
+  const formatter = getXAxisFormatter(SMART_DATE_ID, TimeGranularity.MONTH);
+
+  // Test that dates with milliseconds don't show millisecond format
+  const dateWithMs = new Date('2025-03-15T21:13:32.389Z');
+  const result = (formatter as TimeFormatter).format(dateWithMs);
+  expect(result).not.toContain('.389ms');
+  expect(result).not.toMatch(/\.\d+ms/);
+  expect(result).not.toContain('PM');
+  expect(result).not.toContain('AM');
+  expect(result).not.toMatch(/\d{1,2}:\d{2}/); // No time format
+});
+
+test('getXAxisFormatter time grain aware formatting should prevent problematic formats', () => {
+  // Test that time grain aware formatter prevents the specific issues we solved
+  const monthFormatter = getXAxisFormatter(
+    SMART_DATE_ID,
+    TimeGranularity.MONTH,
+  );
+  const yearFormatter = getXAxisFormatter(SMART_DATE_ID, TimeGranularity.YEAR);
+  const dayFormatter = getXAxisFormatter(SMART_DATE_ID, TimeGranularity.DAY);
+
+  // Test dates that previously caused issues
+  const problematicDates = [
+    new Date('2025-03-15T21:13:32.389Z'), // Had .389ms issue
+    new Date('2025-04-01T02:30:00.000Z'), // Timezone edge case
+    new Date('2025-07-01T00:00:00.000Z'), // Month boundary
+  ];
+
+  problematicDates.forEach(date => {
+    // Month formatter should not show milliseconds or PM/AM
+    const monthResult = (monthFormatter as TimeFormatter).format(date);
+    expect(monthResult).not.toMatch(/\.\d+ms/);
+    expect(monthResult).not.toMatch(/PM|AM/);
+    expect(monthResult).not.toMatch(/\d{1,2}:\d{2}:\d{2}/);
+
+    // Year formatter should not show milliseconds or PM/AM
+    const yearResult = (yearFormatter as TimeFormatter).format(date);
+    expect(yearResult).not.toMatch(/\.\d+ms/);
+    expect(yearResult).not.toMatch(/PM|AM/);
+    expect(yearResult).not.toMatch(/\d{1,2}:\d{2}:\d{2}/);
+
+    // Day formatter should not show milliseconds or seconds
+    const dayResult = (dayFormatter as TimeFormatter).format(date);
+    expect(dayResult).not.toMatch(/\.\d+ms/);
+    expect(dayResult).not.toMatch(/:\d{2}:\d{2}/); // No seconds
   });
 });
+
+test('getXAxisFormatter time grain parameter should be passed correctly', () => {
+  // Test that formatter with time grain is different from formatter without
+  const formatterWithGrain = getXAxisFormatter(
+    SMART_DATE_ID,
+    TimeGranularity.MONTH,
+  );
+  const formatterWithoutGrain = getXAxisFormatter(SMART_DATE_ID);
+
+  expect(formatterWithGrain).toBeDefined();
+  expect(formatterWithoutGrain).toBeDefined();
+  expect(formatterWithGrain).toBeInstanceOf(TimeFormatter);
+  expect(formatterWithoutGrain).toBeInstanceOf(TimeFormatter);
+
+  // Both should be valid formatters
+  const testDate = new Date('2025-04-15T12:30:45.789Z');
+  const resultWithGrain = (formatterWithGrain as TimeFormatter).format(
+    testDate,
+  );
+  const resultWithoutGrain = (formatterWithoutGrain as TimeFormatter).format(
+    testDate,
+  );
+
+  expect(typeof resultWithGrain).toBe('string');
+  expect(typeof resultWithoutGrain).toBe('string');
+  expect(resultWithGrain.length).toBeGreaterThan(0);
+  expect(resultWithoutGrain.length).toBeGreaterThan(0);
+});
+
+test('getXAxisFormatter without time grain should use standard smart date behavior', () => {
+  const standardFormatter = getXAxisFormatter(SMART_DATE_ID);
+  const timeGrainFormatter = getXAxisFormatter(SMART_DATE_ID, undefined);
+
+  // Both should be equivalent when no time grain is provided
+  expect(standardFormatter).toBeDefined();
+  expect(timeGrainFormatter).toBeDefined();
+
+  // Test with a date that has time components
+  const testDate = new Date('2025-01-01T00:00:00.000Z');
+  const standardResult = (standardFormatter as TimeFormatter).format(testDate);
+  const timeGrainResult = (timeGrainFormatter as TimeFormatter).format(
+    testDate,
+  );
+
+  expect(standardResult).toBe(timeGrainResult);
+});