Skip to content
Snippets Groups Projects
Unverified Commit 7e2fcddc authored by shaun's avatar shaun Committed by GitHub
Browse files

Fix Filter Pill display names for dates (#32066)

parent e1bec602
No related branches found
No related tags found
No related merge requests found
......@@ -62,9 +62,11 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
// new filter applied
// Note: Test was flaking because apparently mouseup doesn't always happen at the same position.
// It is enough that we assert that the filter exists and that it starts with May, 2022
// It is enough that we assert that the filter exists and that it starts with May 2022.
// The date range formatter sometimes omits the year of the first month (e.g. May–July 2022),
// so checking that 2022 occurs after May ensures that May 2022 is in fact the first date.
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.contains(/^Created At between May, 2022/);
cy.contains(/^Created At is May.*2022/);
// more granular axis labels
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.contains("June, 2022");
......@@ -111,7 +113,7 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
granularity === "month"
? // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.findByText("Created At between September, 2022 February, 2023")
cy.findByText("Created At is September 2022 February 2023")
: // Once the issue gets fixed, figure out the positive assertion for the "month-of-year" granularity
null;
......@@ -325,7 +327,7 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
cy.log("Filter should show the range between two dates");
// Now click on the filter widget to see if the proper parameters got passed in
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.contains("Created At between").click();
cy.contains(/^Created At is .*–/).click(); // en-dash to detect date range
});
it.skip("should drill-through on filtered aggregated results (metabase#13504)", () => {
......
......@@ -24,7 +24,7 @@ describe("issue 25007", () => {
it("should display weeks correctly in tooltips for native questions (metabase#25007)", () => {
cy.createNativeQuestion(questionDetails, { visitQuestion: true });
clickLineDot({ index: 1 });
popover().findByTextEnsureVisible("May 17, 2022");
popover().findByTextEnsureVisible("May 17, 2022");
});
});
......
......@@ -2,11 +2,13 @@
// @ts-nocheck
import { t, ngettext, msgid } from "ttag";
import _ from "underscore";
import moment from "moment-timezone";
import type {
Filter as FilterObject,
FieldFilter,
FieldReference,
} from "metabase-types/api";
import { formatDateTimeRangeWithUnit } from "metabase/lib/formatting/date";
import { isExpression } from "metabase-lib/expressions";
import { getFilterArgumentFormatOptions } from "metabase-lib/operators/utils";
import {
......@@ -61,6 +63,42 @@ export default class Filter extends MBQLClause {
return this._query.removeFilter(this._index);
}
betterDateLabel() {
const args = this.arguments();
if (!args.every(arg => typeof arg === "string")) {
return undefined;
}
const unit = this.dimension()?.temporalUnit();
const isSupportedDateRangeUnit = [
"day",
"week",
"month",
"quarter",
"year",
].includes(unit);
const op = this.operatorName();
const betweenDates = op === "between" && isSupportedDateRangeUnit;
const equalsWeek = op === "=" && unit === "week";
if (betweenDates || equalsWeek) {
return formatDateTimeRangeWithUnit(args, unit, {
type: "tooltip",
date_resolution: unit === "week" ? "day" : unit,
});
}
const sliceFormat = {
// modified from DEFAULT_DATE_FORMATS in date.tsx to show extra context
"hour-of-day": "[hour] H",
"minute-of-hour": "[minute] m",
"day-of-month": "Do [day of month]",
"day-of-year": "DDDo [day of year]",
"week-of-year": "wo [week of year]",
}[unit];
const m = moment(args[0]);
if (op === "=" && sliceFormat && m.isValid()) {
return m.format(sliceFormat);
}
}
/**
* Returns the display name for the filter
*/
......@@ -72,17 +110,18 @@ export default class Filter extends MBQLClause {
const segment = this.segment();
return segment ? segment.displayName() : t`Unknown Segment`;
} else if (this.isStandard()) {
const dimension = this.dimension();
const operator = this.operator();
const dimensionName =
dimension && includeDimension && dimension.displayName();
const operatorName =
operator &&
includeOperator &&
!isStartingFrom(this) &&
operator.moreVerboseName;
const argumentNames = this.formattedArguments().join(" ");
return `${dimensionName || ""} ${operatorName || ""} ${argumentNames}`;
if (isStartingFrom(this)) {
includeOperator = false;
}
const betterDate = this.betterDateLabel();
const op = betterDate ? "=" : this.operatorName();
return [
includeDimension && this.dimension()?.displayName(),
includeOperator && this.operator(op)?.moreVerboseName,
betterDate ?? this.formattedArguments().join(" "),
]
.map(s => s || "")
.join(" ");
} else if (this.isCustom()) {
return this._query.formatExpression(this);
} else {
......@@ -203,9 +242,9 @@ export default class Filter extends MBQLClause {
return this[0];
}
operator(): FilterOperator | null | undefined {
operator(opName = this.operatorName()): FilterOperator | null | undefined {
const dimension = this.dimension();
return dimension ? dimension.filterOperator(this.operatorName()) : null;
return dimension ? dimension.filterOperator(opName) : null;
}
setOperator(operatorName: string) {
......
......@@ -13,7 +13,7 @@ import {
import type { OptionsType } from "./types";
const RANGE_SEPARATOR = ``;
const EN_DASH = ``;
type DEFAULT_DATE_FORMATS_TYPE = { [key: string]: string };
const DEFAULT_DATE_FORMATS: DEFAULT_DATE_FORMATS_TYPE = {
......@@ -125,56 +125,91 @@ export function formatDateTimeForParameter(value: string, unit: DatetimeUnit) {
}
}
type DateVal = string | number;
/** This formats a time with unit as a date range */
export function formatDateTimeRangeWithUnit(
value: string | number,
value: DateVal | [DateVal] | [DateVal, DateVal],
unit: DatetimeUnit,
options: OptionsType = {},
) {
const m = parseTimestamp(value, unit, options.local);
if (!m.isValid()) {
return String(value);
const values = Array.isArray(value) ? value : [value];
const [a, b] = [values[0], values[1] ?? values[0]].map(d =>
parseTimestamp(d, unit, options.local),
);
if (!a.isValid() || !b.isValid()) {
return String(a);
}
// Tooltips should show full month name, but condense "MMMM D, YYYY - MMMM D, YYYY" to "MMMM D - D, YYYY" etc
const monthFormat =
options.type === "tooltip" ? "MMMM" : getMonthFormat(options);
const condensed = options.compact || options.type === "tooltip";
// The client's unit boundaries might not line up with the data returned from the server.
// We shift the range so that the start lines up with the value.
const start = m.clone().startOf(unit);
const end = m.clone().endOf(unit);
const shift = m.diff(start, "days");
const start = a.clone().startOf(unit);
const end = b.clone().endOf(unit);
const shift = a.diff(start, "days");
[start, end].forEach(d => d.add(shift, "days"));
if (start.isValid() && end.isValid()) {
if (!condensed || start.year() !== end.year()) {
// January 1, 2018 - January 2, 2019
return (
start.format(`${monthFormat} D, YYYY`) +
RANGE_SEPARATOR +
end.format(`${monthFormat} D, YYYY`)
);
} else if (start.month() !== end.month()) {
// January 1 - Feburary 2, 2018
return (
start.format(`${monthFormat} D`) +
RANGE_SEPARATOR +
end.format(`${monthFormat} D, YYYY`)
);
} else {
// January 1 - 2, 2018
return (
start.format(`${monthFormat} D`) +
RANGE_SEPARATOR +
end.format(`D, YYYY`)
);
}
} else {
if (!start.isValid() || !end.isValid()) {
// TODO: when is this used?
return formatWeek(m, options);
return formatWeek(a, options);
}
// Tooltips should show full month name, but condense "MMMM D, YYYY - MMMM D, YYYY" to "MMMM D - D, YYYY" etc
const monthFormat =
options.type === "tooltip" ? "MMMM" : getMonthFormat(options);
const condensed = options.compact || options.type === "tooltip";
const sameYear = start.year() === end.year();
const sameQuarter = start.quarter() === end.quarter();
const sameMonth = start.month() === end.month();
const sameDayOfMonth = start.date() === end.date();
const Y = "YYYY";
const Q = "[Q]Q";
const QY = "[Q]Q YYYY";
const M = monthFormat;
const MY = `${monthFormat} YYYY`;
const MDY = `${monthFormat} D, YYYY`;
const MD = `${monthFormat} D`;
const DY = `D, YYYY`;
// Drop down to day resolution if shift causes misalignment with desired resolution boundaries
const date_resolution =
(shift === 0 ? options.date_resolution : null) ?? "day";
// Use Wikipedia’s date range formatting guidelines
// https://en.wikipedia.org/wiki/Wikipedia:Manual_of_Style/Dates_and_numbers#Ranges
const [startFormat, endFormat, pad = ""] = {
year:
!sameYear || !condensed
? [Y, Y] // 2018–2019
: [Y], // 2018
quarter:
!sameYear || !condensed
? [QY, QY, " "] // Q2 2018 – Q3 2019
: !sameQuarter
? [Q, QY] // Q2–Q4 2019
: [QY], // Q2 2018
month:
!sameYear || !condensed
? [MY, MY, " "] // September 2018 – January 2019
: !sameMonth
? [M, MY] // September–December 2018
: [MY], // September 2018
day:
!sameYear || !condensed
? [MDY, MDY, " "] // January 1, 2018 – January 2, 2019
: !sameMonth
? [MD, MDY, " "] // January 1 – February 2, 2018
: !sameDayOfMonth
? [MD, DY] // January 1–2, 2018
: [MDY], // January 1, 2018
}[date_resolution];
const startStr = start.format(startFormat);
const endStr = end.format(endFormat ?? startFormat);
return startStr === endStr
? startStr
: startStr + pad + EN_DASH + pad + endStr;
}
export function formatRange(
......@@ -186,11 +221,11 @@ export function formatRange(
if ((options.jsx && typeof start !== "string") || typeof end !== "string") {
return (
<span>
{start} {RANGE_SEPARATOR} {end}
{start} {EN_DASH} {end}
</span>
);
} else {
return `${start} ${RANGE_SEPARATOR} ${end}`;
return `${start} ${EN_DASH} ${end}`;
}
}
......
import { formatDateTimeRangeWithUnit } from "metabase/lib/formatting/date";
import { OptionsType } from "metabase/lib/formatting/types";
describe("formatDateTimeRangeWithUnit", () => {
const format = formatDateTimeRangeWithUnit;
// use this to test that the variants of a single date (not a date range) will all be equal
const singleDateVariants = (date: any) => [date, [date], [date, date]];
// we use the tooltip type to test abbreviated dates
const abbrev: OptionsType = { type: "tooltip" };
it("should display year ranges", () => {
const opts: OptionsType = { date_resolution: "year" };
const unit = "year";
singleDateVariants("2018").forEach(d =>
expect(format(d, unit, opts)).toBe("2018"),
);
expect(format(["2018", "2020"], unit, opts)).toBe("2018–2020");
});
it("should display quarter ranges", () => {
const opts: OptionsType = { date_resolution: "quarter" };
const unit = "quarter";
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, unit, opts)).toBe("Q1 2018"),
);
expect(format(["2018-01-01", "2019-04-01"], unit, opts)).toBe(
"Q1 2018 – Q2 2019",
);
expect(format(["2018-01-01", "2018-04-01"], unit, opts)).toBe(
"Q1 2018 – Q2 2018",
);
expect(
format(["2018-01-01", "2018-04-01"], unit, { ...opts, ...abbrev }),
).toBe("Q1–Q2 2018");
});
it("should display month ranges", () => {
const opts: OptionsType = { date_resolution: "month" };
const unit = "month";
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, unit, opts)).toBe("January 2018"),
);
expect(format(["2018-01-01", "2019-04-01"], unit, opts)).toBe(
"January 2018 – April 2019",
);
expect(format(["2018-01-01", "2018-04-01"], unit, opts)).toBe(
"January 2018 – April 2018",
);
expect(
format(["2018-01-01", "2018-04-01"], unit, { ...opts, ...abbrev }),
).toBe("January–April 2018");
});
it("should display day ranges for a single unit", () => {
const opts: OptionsType = { ...abbrev };
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, "day", opts)).toBe("January 1, 2018"),
);
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, "week", opts)).toBe("January 1–7, 2018"),
);
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, "month", opts)).toBe("January 1–31, 2018"),
);
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, "quarter", opts)).toBe("January 1 – March 31, 2018"),
);
singleDateVariants("2018-01-01").forEach(d =>
expect(format(d, "year", opts)).toBe("January 1 – December 31, 2018"),
);
});
it("should display day ranges between two units", () => {
const opts: OptionsType = { ...abbrev };
expect(format(["2018-01-01", "2018-01-02"], "day", opts)).toBe(
"January 1–2, 2018",
);
expect(format(["2018-01-01", "2018-01-08"], "week", opts)).toBe(
"January 1–14, 2018",
);
expect(format(["2018-01-01", "2018-02-01"], "month", opts)).toBe(
"January 1 – February 28, 2018",
);
expect(format(["2018-01-01", "2018-04-01"], "quarter", opts)).toBe(
"January 1 – June 30, 2018",
);
expect(format(["2018-01-01", "2019-01-01"], "year", opts)).toBe(
"January 1, 2018 – December 31, 2019",
);
});
});
......@@ -24,6 +24,7 @@ export interface OptionsType {
rich?: boolean;
suffix?: string;
time_enabled?: "minutes" | "milliseconds" | "seconds" | null;
date_resolution?: "day" | "month" | "quarter" | "year";
time_format?: string;
time_style?: string;
type?: string;
......
......@@ -158,7 +158,7 @@ describe("LineAreaBarRenderer", () => {
const hover = onHoverChange.mock.calls[0][0];
const [formattedWeek] = getFormattedTooltips(hover, settings);
expect(formattedWeek).toEqual("January 511, 2020");
expect(formattedWeek).toEqual("January 511, 2020");
const ticks = qsa(".axis.x .tick text").map(e => e.textContent);
expect(ticks).toEqual([
......
......@@ -32,6 +32,63 @@ describe("Filter", () => {
it("should return the correct string for a segment filter", () => {
expect(filter(["segment", 1]).displayName()).toEqual("Expensive Things");
});
describe("betterDateLabel", () => {
function createdAtFilter(op, unit, ...args) {
return filter([
op,
[
"field",
ORDERS.CREATED_AT,
{
"base-type": "type/DateTime",
"temporal-unit": unit,
},
],
...args,
]);
}
it("should display is-week filter as a day range", () => {
expect(
createdAtFilter("=", "week", "2026-10-04").displayName(),
).toEqual("Created At is October 4–10, 2026");
});
it("should display between-weeks filter as day range", () => {
expect(
createdAtFilter(
"between",
"week",
"2026-10-04",
"2026-10-11",
).displayName(),
).toEqual("Created At is October 4–17, 2026");
});
it("should display slice filters with enough context for understanding them", () => {
expect(
createdAtFilter(
"=",
"minute-of-hour",
"2023-07-03T18:31:00-05:00",
).displayName(),
).toEqual("Created At is minute 31");
expect(
createdAtFilter(
"=",
"hour-of-day",
"2023-07-03T10:00:00-05:00",
).displayName(),
).toMatch(/^Created At is hour \d+$/); // GitHub CI is in different time zone
expect(
createdAtFilter("=", "day-of-month", "2016-01-17").displayName(),
).toEqual("Created At is 17th day of month");
expect(
createdAtFilter("=", "day-of-year", "2016-07-19").displayName(),
).toEqual("Created At is 201st day of year");
expect(
createdAtFilter("=", "week-of-year", "2023-07-02").displayName(),
).toEqual("Created At is 27th week of year");
});
});
});
describe("isValid", () => {
describe("with a field filter", () => {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment