diff --git a/.changeset/11632-dataset-compare-label.md b/.changeset/11632-dataset-compare-label.md new file mode 100644 index 0000000000..2386d5677e --- /dev/null +++ b/.changeset/11632-dataset-compare-label.md @@ -0,0 +1,9 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +A dataset-bound dashboard widget names its comparison window from `compareTo.kind` (objectui#11632). `previousPeriod` reads "vs previous period" and `previousYear` reads "vs last year", in every locale the `dashboard.trend.*` keys already cover. The label used to be guessed from the widget filter's date-macro tokens. A dashboard date range of `last_30_days` (`{30_days_ago}` to `{today}`) with `compareTo: { kind: 'previousPeriod' }` therefore read "vs yesterday", although the analytics executor had compared the previous 30 days. A quarter's macros read "vs last quarter" in the same way, although the executor compares the equal-length window before the quarter. The fix applies to every place the widget names the window: the KPI delta, the table's comparison column header and its CSV export, the cross-tab caption, and the chart's comparison series. The compared values were already right and do not change. + +Inline (non-dataset) metric and chart widgets keep their filter-based label. There the comparison filter really does swap `{today}` for `{yesterday}` and `current_*` tokens for `last_*`, so the label matches what was compared. + +**Clause-②: no.** No export, prop, type member or i18n key is added or removed. diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index a27ff8c340..d998dc62fa 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -65,7 +65,6 @@ import { pivotBucketId, pivotDimensionValue, pivotCellKey, - compareToTrendLabelKey, // Which chart families ignore `compareTo` — ONE declaration, read by the // inline chart path too (objectui#7495). See `compareTo` below. chartTypeIgnoresCompareTo, @@ -228,6 +227,35 @@ const TREND_LABEL_DEFAULTS: Record = { vsPreviousPeriod: 'vs previous period', }; +/** + * The `dashboard.trend.*` key a comparison is labelled with on THIS path, read + * off `compareTo.kind` alone (objectui#11632). + * + * The dataset executor decides the comparison window from the kind: for + * `previousPeriod` it is the equal-length window immediately before the + * resolved one, and for `previousYear` the same window one calendar year back + * (`DatasetCompareTo` in `@objectstack/spec`). So each kind's label holds for + * every window it is applied to. "vs previous period" is true of a 30-day + * window and of a quarter alike. + * + * ⛔ Not `compareToTrendLabelKey` from `@object-ui/core`. That helper guesses + * the window from the RAW filter's date-macro tokens (`{today}` reads + * "vs yesterday"). The guess is faithful on the inline path, where + * `shiftFilterByCompareTo` really does swap `{today}` for `{yesterday}`. Here + * no token is swapped, because the executor shifts the whole resolved window. + * A dashboard date range of `last_30_days` (`{30_days_ago}` to `{today}`) + * compared the previous 30 days and was labelled "vs yesterday". + * + * Both keys already exist (`TREND_LABEL_DEFAULTS` above), so a dataset-bound + * and an inline KPI comparing one year back still read the same. `satisfies` + * makes a third kind added to `CompareToConfig` a compile error here instead + * of a comparison with no label of its own. + */ +const COMPARE_KIND_TREND_KEY = { + previousPeriod: 'vsPreviousPeriod', + previousYear: 'vsLastYear', +} as const satisfies Record; + /** * ISO calendar date, optionally carrying a time part — `2026-01-15`, * `2026-01-15T08:30:00Z`. Deliberately narrower than `Date.parse` (which also @@ -1091,10 +1119,13 @@ export function DatasetWidget({ widget, dataSource, subCaption }: { widget: any; ? values.filter((m) => state.rows.some((r) => r[compareColumn(m)] != null)) : []; // Window label from the SAME `dashboard.trend.*` vocabulary the inline metric - // widget uses. `compareToTrendLabelKey` reads `compareTo.kind` and — for - // `previousPeriod` — the RAW filter's macro tokens, so "vs last quarter" - // survives the resolution that turned those tokens into dates. - const compareTrendKey = compareTo ? compareToTrendLabelKey(compareTo, rawFilter) : ''; + // widget uses, read off `compareTo.kind` and never off the filter's macro + // tokens (objectui#11632). The executor shifts the whole resolved window, so + // the kind is all there is to say about which window was compared. See + // `COMPARE_KIND_TREND_KEY`. This one label names the window on every surface + // below: the KPI delta, the table's comparison column header, the cross-tab + // caption and the chart's comparison series. + const compareTrendKey = compareTo ? COMPARE_KIND_TREND_KEY[compareTo.kind] : ''; const compareLabel = compareTo ? tt(`dashboard.trend.${compareTrendKey}`, TREND_LABEL_DEFAULTS[compareTrendKey] ?? 'vs previous period') : ''; diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.compareTo.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.compareTo.test.tsx index de415cc897..3f2fa9ef2f 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.compareTo.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.compareTo.test.tsx @@ -17,6 +17,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, screen, cleanup, waitFor, within, fireEvent } from '@testing-library/react'; +import { DashboardWidgetSchema as SpecDashboardWidgetSchema } from '@objectstack/spec/ui'; let lastChartSchema: any = null; @@ -223,24 +224,6 @@ describe('DatasetWidget — showing the comparison the executor returned', () => expect(trend).toHaveTextContent(/vs last year/i); }); - it('labels the window from the RAW filter’s macros, which resolution erased', async () => { - // `previousPeriod` derives its label by sniffing the filter's date macros. - // The resolved filter has none left — passing that instead would silently - // downgrade every label to the generic "vs previous period". - const src = makeSource(async () => ({ rows: [{ revenue: 120, revenue__compare: 100 }] })); - render( - , - ); - expect(await screen.findByTestId('dataset-compare-trend')).toHaveTextContent(/vs last quarter/i); - }); - it('shows no trend when the executor attached no comparison', async () => { const src = makeSource(async () => ({ rows: [{ revenue: 120 }] })); render( @@ -324,6 +307,119 @@ describe('DatasetWidget — showing the comparison the executor returned', () => }); }); +/** + * objectui#11632 — on this path the label names the window the EXECUTOR + * compared, and `compareTo.kind` alone decides that window: `previousPeriod` is + * the equal-length window right before the resolved one, `previousYear` the + * same window a calendar year back. The label used to be guessed from the RAW + * filter's date-macro tokens, a guess that only holds on the inline path, where + * `shiftFilterByCompareTo` really does swap `{today}` for `{yesterday}`. So a + * dashboard's `last_30_days` range compared the previous 30 days and read + * "vs yesterday". The inline path keeps its guess; its pins live with + * `compareToTrendLabelKey` in `@object-ui/core` and with `ObjectMetricWidget`. + */ +describe('DatasetWidget — the comparison label comes from compareTo.kind (objectui#11632)', () => { + // The dashboard's `last_30_days` date range as `DashboardRenderer` merges it + // into a widget's filter: `{30_days_ago}` to `{today}`. + const LAST_30_DAYS = { created_at: { $gte: '{30_days_ago}', $lte: '{today}' } }; + const compared = async () => ({ + rows: [{ revenue: 120, revenue__compare: 100 }], + fields: [{ name: 'revenue', type: 'number', label: 'Revenue' }], + }); + + it('labels previousPeriod over a {today} window "vs previous period", not "vs yesterday"', async () => { + const src = makeSource(compared); + render( + , + ); + const trend = await screen.findByTestId('dataset-compare-trend'); + expect(trend).toHaveTextContent(/vs previous period/i); + expect(trend).not.toHaveTextContent(/yesterday/i); + // The whole resolved window went to the executor to shift. A one-day + // window would be the only one whose previous period is a single day. + const [td] = selectionOf(src).timeDimensions; + expect(td.dimension).toBe('created_at'); + expect(td.dateRange[0]).not.toBe(td.dateRange[1]); + }); + + it('names the chart’s comparison series from the kind too (the line legend)', async () => { + const src = makeSource(async () => ({ + rows: [ + { stage: 'won', revenue: 120, revenue__compare: 100 }, + { stage: 'lost', revenue: 20, revenue__compare: 40 }, + ], + fields: [{ name: 'revenue', type: 'number', label: 'Revenue' }], + })); + render( + , + ); + await waitFor(() => expect(lastChartSchema).not.toBeNull()); + expect(lastChartSchema.series[1]).toMatchObject({ dataKey: 'revenue__compare', variant: 'comparison' }); + expect(lastChartSchema.series[1].label).toMatch(/vs previous period/i); + expect(lastChartSchema.series[1].label).not.toMatch(/yesterday/i); + }); + + it('does not read a calendar unit off the macros: a quarter’s previousPeriod is "vs previous period"', async () => { + // The equal-length window before a quarter is a calendar quarter only by + // coincidence: Q1 (90 days) shifted back 90 days starts in October. + const src = makeSource(compared); + render( + , + ); + const trend = await screen.findByTestId('dataset-compare-trend'); + expect(trend).toHaveTextContent(/vs previous period/i); + expect(trend).not.toHaveTextContent(/quarter/i); + }); + + // Every kind the spec declares for a widget's `compareTo`, read from the spec + // itself, so a kind added there fails the coverage check below until it has + // a label of its own. + const SPEC_KINDS: readonly string[] = SpecDashboardWidgetSchema.shape.compareTo.unwrap().shape.kind.options; + const LABEL_OF_KIND: Record = { + previousPeriod: /vs previous period/i, + previousYear: /vs last year/i, + }; + + it('has an expected label for every kind the spec declares', () => { + expect([...SPEC_KINDS].sort()).toEqual(Object.keys(LABEL_OF_KIND).sort()); + }); + + it.each(SPEC_KINDS)('labels %s over a {today} window from the kind alone', async (kind) => { + const src = makeSource(compared); + render( + , + ); + const trend = await screen.findByTestId('dataset-compare-trend'); + expect(trend).toHaveTextContent(LABEL_OF_KIND[kind]); + expect(trend).not.toHaveTextContent(/yesterday/i); + }); +}); + /** * objectui#3614 — the cross-tab was the one render path PR #3612 left out, so a * `type: 'pivot'` widget with ≥2 dimensions ran a correct comparison query,