diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts index b2fcb7b7cdb..eabd63cc901 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/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; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts index 65961e0889c..4582253a496 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts +++ b/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); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts index f69ac1f4d93..4f799eeb41e 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts +++ b/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); } }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts index daa15d9aa01..a4e4dea5a4b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts +++ b/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); +});