diff --git a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js index 1c55395d5e34ee9e9a867495c2d825c497dfb694..e113927e07dca3e6f51e2c56c9de62a3ea4b5a37 100644 --- a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js @@ -373,6 +373,58 @@ describe("scenarios > visualizations > table > conditional formatting", () => { }); }); +describe("scenarios > visualizations > table > time formatting (#11398)", () => { + const singleTimeQuery = ` + WITH t1 AS (SELECT TIMESTAMP '2023-01-01 18:34:00' AS time_value), + t2 AS (SELECT CAST(time_value AS TIME) AS creation_time + FROM t1) + SELECT * + FROM t2; + `; + + beforeEach(() => { + restore(); + cy.signInAsNormalUser(); + }); + + it("should work with time columns", { tags: ["@external"] }, () => { + cy.createNativeQuestion( + { + name: "11398", + native: { + query: singleTimeQuery, + }, + }, + { visitQuestion: true }, + ); + + // Open the formatting menu + cy.findByTestId("field-info-popover").click(); + + cy.findByTestId("drill-through-section").within(() => { + cy.icon("gear").click(); + }); + + cy.findByTestId("column-formatting-settings").within(() => { + // Set to hours, minutes, seconds, 24-hour clock + cy.findByText("HH:MM:SS").click(); + cy.findByText("17:24 (24-hour clock)").click(); + }); + + // And you should find the result + cy.findByRole("gridcell").findByText("18:34:00"); + + cy.findByTestId("column-formatting-settings").within(() => { + // Add millisecond display and change back to 12 hours + cy.findByText("HH:MM:SS.MS").click(); + cy.findByText("5:24 PM (12-hour clock)").click(); + }); + + // And you should find the result + cy.findByRole("gridcell").findByText("6:34:00.000 PM"); + }); +}); + function headerCells() { return cy.findAllByTestId("header-cell"); } diff --git a/frontend/src/metabase/lib/formatting/time.ts b/frontend/src/metabase/lib/formatting/time.ts index 926debbe9861a27b06b7ae63f7f39c550a280d8c..f2db3622d4b536daf103a6400f70e19f5dff7509 100644 --- a/frontend/src/metabase/lib/formatting/time.ts +++ b/frontend/src/metabase/lib/formatting/time.ts @@ -4,6 +4,7 @@ import type { Moment } from "moment-timezone"; import { parseTime, parseTimestamp } from "metabase/lib/time"; import type { DatetimeUnit } from "metabase-types/api/query"; +import type { TimeOnlyOptions } from "metabase/lib/formatting/types"; import { DEFAULT_TIME_STYLE, getTimeFormatFromStyle, @@ -28,12 +29,6 @@ export function duration(milliseconds: number) { return ngettext(msgid`${seconds} second`, `${seconds} seconds`, seconds); } -export function formatTime(time: Moment) { - const parsedTime = parseTime(time); - - return parsedTime.isValid() ? parsedTime.format("LT") : String(time); -} - interface TimeWithUnitType { local?: boolean; time_enabled?: "minutes" | "milliseconds" | "seconds" | boolean; @@ -67,3 +62,28 @@ export function formatTimeWithUnit( return m.format(timeFormat); } + +export function formatTime( + time: Moment, + unit: DatetimeUnit = "default", + options: TimeOnlyOptions = {}, +) { + const parsedTime = parseTime(time); + + const timeStyle = options.time_style ?? DEFAULT_TIME_STYLE; + + let timeEnabled; + if (options.time_enabled) { + timeEnabled = options.time_enabled; + } else if (hasHour(unit)) { + timeEnabled = "minute"; + } else { + timeEnabled = null; + } + + const timeFormat = + options.time_format ?? + getTimeFormatFromStyle(timeStyle, unit, timeEnabled as any); + + return parsedTime.isValid() ? parsedTime.format(timeFormat) : String(time); +} diff --git a/frontend/src/metabase/lib/formatting/types.ts b/frontend/src/metabase/lib/formatting/types.ts index 3056b615ef2ce2427f686b0338fbd96c3e232611..21414dfbe0b3cb4400822d4e6e2b674758913fcf 100644 --- a/frontend/src/metabase/lib/formatting/types.ts +++ b/frontend/src/metabase/lib/formatting/types.ts @@ -1,4 +1,11 @@ -export interface OptionsType { +export interface TimeOnlyOptions { + local?: boolean; + time_enabled?: "minutes" | "milliseconds" | "seconds" | null; + time_format?: string; + time_style?: string; +} + +export interface OptionsType extends TimeOnlyOptions { click_behavior?: any; clicked?: any; column?: any; @@ -12,7 +19,6 @@ export interface OptionsType { jsx?: boolean; link_text?: string; link_url?: string; - local?: boolean; majorWidth?: number; markdown_template?: any; maximumFractionDigits?: number; @@ -25,9 +31,6 @@ export interface OptionsType { removeYear?: boolean; rich?: boolean; suffix?: string; - time_enabled?: "minutes" | "milliseconds" | "seconds" | null; - time_format?: string; - time_style?: string; type?: string; view_as?: string | null; weekday_enabled?: boolean; diff --git a/frontend/src/metabase/lib/formatting/value.tsx b/frontend/src/metabase/lib/formatting/value.tsx index 0d5cf6c8c8e349560057eee6d749d1c8fe8b402e..2e34ad9a18db1b437827223c6f22dcb05d337b3a 100644 --- a/frontend/src/metabase/lib/formatting/value.tsx +++ b/frontend/src/metabase/lib/formatting/value.tsx @@ -177,7 +177,7 @@ export function formatValueRaw( } else if (isEmail(column)) { return formatEmail(value as string, options); } else if (isTime(column)) { - return formatTime(value as Moment); + return formatTime(value as Moment, column.unit, options); } else if (column && column.unit != null) { return formatDateTimeWithUnit( value as string | number, diff --git a/frontend/src/metabase/static-viz/lib/format.ts b/frontend/src/metabase/static-viz/lib/format.ts index 0cb042789bfc90cde385fa769d4e4fb9c29e0ed0..69bc1ebceed8ae95a823ed94ebb394bb2b01e561 100644 --- a/frontend/src/metabase/static-viz/lib/format.ts +++ b/frontend/src/metabase/static-viz/lib/format.ts @@ -57,7 +57,7 @@ export const formatStaticValue = (value: unknown, options: OptionsType) => { if (value == null) { formattedValue = JSON.stringify(null); } else if (isTime(column)) { - formattedValue = formatTime(value as Moment); + formattedValue = formatTime(value as Moment, column.unit, options); } else if (column?.unit != null) { formattedValue = formatDateTimeWithUnit( value as string | number, diff --git a/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx b/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx index 1ec775a5bdd29799261f9a604e0a97ffa7fdf510..1c9eabbcac75fc0174b39cacbcb9715ea6680801 100644 --- a/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx +++ b/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx @@ -698,6 +698,8 @@ class TableInteractive extends Component { const isSorted = sortDirection != null; const isAscending = sortDirection === "asc"; + const fieldInfoPopoverTestId = "field-info-popover"; + return ( <TableDraggable /* needs to be index+name+counter so Draggable resets after each drag */ @@ -793,6 +795,7 @@ class TableInteractive extends Component { className="Icon mr1" name={isAscending ? "chevronup" : "chevrondown"} size={10} + data-testid={fieldInfoPopoverTestId} /> )} {columnTitle} @@ -801,6 +804,7 @@ class TableInteractive extends Component { className="Icon ml1" name={isAscending ? "chevronup" : "chevrondown"} size={10} + data-testid={fieldInfoPopoverTestId} /> )} </Ellipsified>, diff --git a/frontend/test/metabase/lib/formatting.unit.spec.js b/frontend/test/metabase/lib/formatting.unit.spec.js index bf9a482dccaf0eea69e5bf5260dd85ff137d90a1..9a29fc090916ee81edfb1d37ad9adbf85a12403b 100644 --- a/frontend/test/metabase/lib/formatting.unit.spec.js +++ b/frontend/test/metabase/lib/formatting.unit.spec.js @@ -612,12 +612,23 @@ describe("formatting", () => { ]; test.each(FORMAT_TIME_TESTS)( - `parseTime(%p) to be %p`, + `formatTime(%p) to be %p`, (value, resultStr) => { const result = formatTime(value); expect(result).toBe(resultStr); }, ); + + it("should use options when formatting times", () => { + const value = "20:34:56"; + const t12 = formatTime(value, "default", {}); + expect(t12).toBe("8:34 PM"); + const t24 = formatTime(value, "default", { + time_enabled: "minutes", + time_style: "HH:mm", + }); + expect(t24).toBe("20:34"); + }); }); describe("formatTimeWithUnit", () => {