diff --git a/.changeset/analytics-window-timedimension-no-bucket.md b/.changeset/analytics-window-timedimension-no-bucket.md new file mode 100644 index 0000000000..6dd92aea74 --- /dev/null +++ b/.changeset/analytics-window-timedimension-no-bucket.md @@ -0,0 +1,60 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(service-analytics): a `timeDimensions` entry used only as a date WINDOW no longer buckets the grid (#5688) + +**Observable behaviour change — read this if you render, page, or assert on +dataset responses.** A selection that used a date dimension only as a window — +`timeDimensions: [{ dimension, dateRange }]` with no `granularity`, and the +dimension NOT listed in `selection.dimensions` — used to have the dataset +dimension's declared `dateGranularity` filled in anyway. That made the entry a +`GROUP BY` item, so the response grew a time column nobody selected and every +row split per bucket. "Count by Owner" plus a dashboard date-range filter came +back as "by Owner × month": + +``` +before fields [owner, close_date, opp_count] + rows [{owner:'u1', close_date:'2026-01', opp_count:1}, + {owner:'u1', close_date:'2026-02', opp_count:1}, + {owner:'u2', close_date:'2026-01', opp_count:1}] + +after fields [owner, opp_count] + rows [{owner:'u1', opp_count:2}, + {owner:'u2', opp_count:1}] +``` + +Both the **row count and the column set** change for such a selection: the extra +month column disappears and rows that were split per bucket collapse back into +one row per selected dimension tuple. A KPI single-value card that was reading +the first of several month rows now reads the only row. Consumers that pinned +the previous shape (a snapshot of `fields`, a row count, a hard-coded column +index) need updating; consumers that render the response's own `fields` do not. + +Three conditions had to hold together to be affected, so a selection outside +them is byte-identical: the dataset dimension declares an explicit +`dateGranularity`, the `timeDimensions` entry states no `granularity`, and +`selection.dateGranularity` is unset. + +**What still buckets, unchanged.** An entry is bucketed when the request says +that date is being bucketed: the dimension is one of the selection's own +`dimensions`, the entry carries its own `granularity` (#4033 — still projected +as a column even when not selected), or `selection.dateGranularity` is set. The +granularity *precedence* chain is untouched. A dataset dimension's +`dateGranularity` says how that date renders **when** grouped — it is no longer +read as a request to group by it. + +**`compareTo` alignment (#3588/#4870) holds by construction.** The comparison +pass re-enters the same query builder with the same grid dimensions, differing +only in the shifted `dateRange`, so both passes bucket an entry alike or not at +all — never one of each, which was the state that left every `__compare` column +empty. For a window-only anchor this **repairs** the comparison rather than +preserving it: the merge has always keyed on `selection.dimensions` alone, so +the backfilled bucket column sat outside the merge key, and with several +month-split rows per group the comparison value landed on whichever row the +index held last while the others read a confident `0`. + +Also fixed, same root cause: a time column that IS projected via +`timeDimensions` (an entry carrying its own `granularity`, never listed under +`dimensions`) now carries its dataset `label` in `fields` instead of a bare +`type` — the label enrichment walked `selection.dimensions` only. diff --git a/packages/services/service-analytics/src/__tests__/dataset-dimension-field-descriptors.test.ts b/packages/services/service-analytics/src/__tests__/dataset-dimension-field-descriptors.test.ts index 98c57baf70..87f3d9fb71 100644 --- a/packages/services/service-analytics/src/__tests__/dataset-dimension-field-descriptors.test.ts +++ b/packages/services/service-analytics/src/__tests__/dataset-dimension-field-descriptors.test.ts @@ -53,6 +53,17 @@ * the change ADDS field entries that were absent, it narrows no rule and * removes no `??` limb, so nothing downstream can gain a finding from it. * Predicted 8 red / 4 green; measured exactly that. + * + * ## Amended by #5688 + * + * Two cases here were written as a deliberate FLIP TARGET: the pair that fed a + * `timeDimensions` entry carrying only a `dateRange` asserted, verbatim, that + * the entry acquired the dataset's default bucket and became a second GROUP BY. + * #5688 narrowed that backfill, so both now assert the opposite column set and, + * additionally, the grid substance the descriptors describe (one row per owner, + * no month split). The reverse verification above is unaffected — reverting + * #5537's seam still reds exactly the "all base measures filter-scoped" cases, + * and the flipped pair differs only in what the correct column set IS. */ import { describe, it, expect } from 'vitest'; @@ -253,27 +264,26 @@ describe('#5537 — the single-query path already described its dimensions (guar }); /** - * The CONTROL for the `compareTo` case in the next block, and the reason the - * expectation there carries an unlabelled `close_date`. + * The CONTROL for the `compareTo` case in the next block: the two selections + * differ only in whether their measures carry filters, so their descriptors + * must agree column for column. * - * A `timeDimensions` entry that resolves a granularity is GROUPED BY, so it is - * a COLUMN of the result and every producer of this shape projects it (#4033), - * even when the caller never listed it under `dimensions`. It reaches `fields` - * with a `type` and no `label`, because the label enrichment in `queryDataset` - * walks `selection.dimensions` — and that is true on THIS path, which never - * had the #5537 defect. Pinned here so the pair reads as convergence rather - * than as something the fix introduced. + * **This pin was FLIPPED by #5688 and now carries the opposite fact.** It used + * to assert an unlabelled `close_date` descriptor and three month-split rows, + * because a `timeDimensions` entry carrying only a `dateRange` had the + * dataset's default granularity filled in — turning a WINDOW into a second + * GROUP BY. #5688 narrowed that backfill: a window-only entry stays a filter, + * so there is no `close_date` column on either path and `usr_1` is one row + * again. Asserted as substance, not as an absence: the row values pin that the + * three in-window opportunities landed in ONE `usr_1` bucket rather than being + * split across months, which is the defect's actual signature. * - * That the column exists AT ALL is itself a defect, filed as #5688 and - * deliberately not fixed here: a `timeDimensions` entry carrying only a - * `dateRange` gets the dataset's default granularity filled in, which turns a - * WINDOW into a second GROUP BY — so this selection also comes back split by - * month. It reproduces identically on this path, i.e. independently of #5537, - * and settling it changes the response SHAPE (a row count, not a label), which - * is not a call to make as a rider. Both asserted verbatim so the day #5688 - * lands, this pair goes red and gets updated on purpose. + * The projection rule itself is unchanged (#4033: an entry that DOES resolve a + * granularity is grouped, so it is a column) — see + * `dataset-window-timedimension-bucketing.test.ts`, which pins both sides of + * the narrowed criterion plus the label the projected column now carries. */ - it('a granular `timeDimensions` column is projected here too — unlabelled, on this path as well', async () => { + it('a window-only `timeDimensions` entry adds no column here — same as the filtered path', async () => { const result = await svc().queryDataset( dataset, { @@ -286,18 +296,19 @@ describe('#5537 — the single-query path already described its dimensions (guar ); expect(descriptors(result.fields)).toEqual([ { name: 'owner', type: 'string', label: 'Owner' }, - { name: 'close_date', type: 'time' }, { name: 'opp_count', type: 'number', label: 'Opportunities' }, { name: 'opp_count__compare', type: 'number', label: 'Opportunities' }, ]); - // #5688, stated as data rather than as prose: `usr_1` is one owner and comes - // back as two rows because the window entry acquired a `month` bucket. The - // descriptors are honest about the grid — the grid is what is wrong. - expect(result.rows.map((r) => [r.owner, r.close_date])).toEqual([ - ['usr_1', '2026-01'], - ['usr_1', '2026-02'], - ['usr_2', '2026-02'], + // The grid the descriptors describe: one row per owner, the window applied + // as a filter. `usr_1` has three in-window opportunities (two in January, + // one in February) and they aggregate into a single bucket — before #5688 + // this was two rows carrying 2 and 1. + expect(result.rows).toEqual([ + { owner: 'usr_1', opp_count: 3, opp_count__compare: 0 }, + { owner: 'usr_2', opp_count: 1, opp_count__compare: 0 }, ]); + // …and no row carries a column no descriptor mentions. + expect(result.rows.every((r) => !('close_date' in r))).toBe(true); }); }); @@ -398,15 +409,20 @@ describe('#5537 — all base measures filter-scoped: the dimension is described }, CTX, ); - // Identical to the control in the previous block, measure for measure — - // including the unlabelled `close_date` a granular `timeDimensions` entry - // projects on BOTH paths. + // Identical to the control in the previous block, column for column — + // including the ABSENCE of a `close_date` column, which since #5688 a + // window-only `timeDimensions` entry no longer mints on either path. expect(descriptors(result.fields)).toEqual([ { name: 'owner', type: 'string', label: 'Owner' }, - { name: 'close_date', type: 'time' }, { name: 'won_count', type: 'number', label: 'Won' }, { name: 'won_count__compare', type: 'number', label: 'Won' }, ]); + // Same substance as the control: one row per owner, no month split. The two + // paths converge on the grid as well as on the descriptors. + expect(result.rows).toEqual([ + { owner: 'usr_1', won_count: 1, won_count__compare: 0 }, + { owner: 'usr_2', won_count: 1, won_count__compare: 0 }, + ]); }); it('the `totals` grid re-enters the same pass and still describes the dimension', async () => { diff --git a/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts b/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts new file mode 100644 index 0000000000..3ce25279fc --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/dataset-window-timedimension-bucketing.test.ts @@ -0,0 +1,442 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5688 — a `timeDimensions` entry used only as a WINDOW must stay a filter. + * + * A dashboard date-range picker emits `timeDimensions: [{ dimension, dateRange }]` + * — no `granularity` — and does not list that dimension under + * `selection.dimensions`. The executor filled in the dataset dimension's own + * `dateGranularity` anyway, which made the entry a GROUP BY item: the response + * grew a time column nobody selected and every row split per bucket. "Count by + * Owner" plus a date filter came back as "by Owner × month": + * + * ``` + * fields [{"name":"owner",…},{"name":"close_date","type":"time"},{"name":"opp_count",…}] + * rows [{"owner":"u1","close_date":"2026-01","opp_count":1}, + * {"owner":"u1","close_date":"2026-02","opp_count":1}, + * {"owner":"u2","close_date":"2026-01","opp_count":1}] + * ``` + * + * A KPI single-value card then read the first of those rows. The invariant it + * broke is the one every filter obeys: a `dateRange` removes rows — it does not + * mint a column, split a row, or change the column set. + * + * ## The narrowed criterion (all three arms pinned below) + * + * An entry that states NO granularity is bucketed only when the request says + * that date is being bucketed: the dimension is one of the grid dimensions this + * query groups by, or `selection.dateGranularity` is set. An entry carrying its + * own `granularity` was never in question and stays grouped (#4033). The dataset + * dimension's `dateGranularity` alone is NOT such a signal — it says how this + * date renders WHEN grouped, not that it should be grouped. + * + * ## compareTo — the same question, answered on both passes (#3588/#4870) + * + * The backfill existed because the comparison pass must bucket exactly like the + * primary one; a month-bucketed primary grid merged against raw-timestamp + * comparison rows shares no dimension key, and every `__compare` column comes + * back empty. That still holds, and holds BY CONSTRUCTION: `runCompare` re-enters + * `buildQuery` with the same grid dimensions and the same + * `selection.dateGranularity`, differing only in the shifted `dateRange`, so both + * passes reach the same verdict for the same entry. Pinned as a PAIR here — the + * bucketed anchor and the window-only anchor, each asserted on both passes — so + * neither demand can be re-broken in the other's name. + * + * For a window-only anchor the narrowing does not merely preserve the merge, it + * REPAIRS it. `mergeByDimensions` has always keyed on `selection.dimensions` + * alone, never on the projected bucket column, so the backfilled month column + * was outside the merge key: with several month-split rows per owner the + * comparison value landed on whichever row the index happened to hold last and + * the others read a confident `0`. Both numbers are asserted below. + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; + +const CTX = { tenantId: 'org_A' } as ExecutionContext; + +// ── fixture ───────────────────────────────────────────────────────────────── + +/** + * The issue's dataset: a lookup dimension whose label differs from its + * humanized key, and a date dimension declaring an explicit `dateGranularity` — + * the necessary condition for the defect, and the shape HotCRM-class datasets + * ship. + */ +const dataset = DatasetSchema.parse({ + name: 'opportunity_metrics', + label: 'Opportunity Metrics', + object: 'opportunity', + include: [], + dimensions: [ + { name: 'owner', field: 'owner_id', type: 'lookup', label: 'Owner' }, + { name: 'close_date', field: 'close_date', type: 'date', label: 'Close Date', dateGranularity: 'month' }, + // The control: same shape, no declared `dateGranularity`, so nothing was + // ever backfilled for it — which is what makes the declaration the trigger. + { name: 'created_at', field: 'created_at', type: 'date', label: 'Created' }, + ], + measures: [{ name: 'opp_count', aggregate: 'count', label: 'Opps' }], +}); + +/** + * Current window (2026-01-01..2026-02-28): `u1` → 2 (one per month), `u2` → 1. + * Previous year (2025-01-01..2025-02-28): `u1` → 3 (one in Jan, TWO in Feb), + * `u2` → 1. + * + * `u1`'s comparison total deliberately spans two months and is not equal to + * either month alone (3 = 1 + 2), so a per-month merge cannot accidentally + * produce the right answer. + */ +const OPPS: Array<{ owner_id: string; close_date: string; created_at: string }> = [ + { owner_id: 'u1', close_date: '2026-01-05', created_at: '2025-11-01' }, + { owner_id: 'u1', close_date: '2026-02-11', created_at: '2025-11-02' }, + { owner_id: 'u2', close_date: '2026-01-20', created_at: '2025-11-03' }, + { owner_id: 'u1', close_date: '2025-01-07', created_at: '2024-11-01' }, + { owner_id: 'u1', close_date: '2025-02-03', created_at: '2024-11-02' }, + { owner_id: 'u1', close_date: '2025-02-04', created_at: '2024-11-03' }, + { owner_id: 'u2', close_date: '2025-01-15', created_at: '2024-11-04' }, +]; + +type GroupByItem = string | { field: string; dateGranularity?: string }; +type AggOptions = { groupBy?: unknown; aggregations?: unknown; filter?: unknown }; + +function matches(row: Record, filter: unknown): boolean { + if (filter == null || typeof filter !== 'object') return true; + for (const [key, cond] of Object.entries(filter as Record)) { + if (key === '$and') { + if (!(cond as unknown[]).every((c) => matches(row, c))) return false; + continue; + } + if (key === '$or') { + if (!(cond as unknown[]).some((c) => matches(row, c))) return false; + continue; + } + const value = row[key]; + if (cond != null && typeof cond === 'object' && !Array.isArray(cond)) { + const ops = cond as Record; + if ('$gte' in ops && !(String(value) >= String(ops.$gte))) return false; + if ('$lte' in ops && !(String(value) <= String(ops.$lte))) return false; + continue; + } + if (value !== cond) return false; + } + return true; +} + +/** Bucket keys for the granularities this fixture exercises. */ +function bucket(raw: unknown, granularity?: string): unknown { + const s = String(raw); + if (granularity === 'month') return s.slice(0, 7); + if (granularity === 'year') return s.slice(0, 4); + return raw; +} + +/** Group `OPPS` the way a real `GROUP BY` would, over the filtered row set. */ +function evaluateAggregate(opts: AggOptions): Record[] { + const groupBy = (opts.groupBy ?? []) as GroupByItem[]; + const aggregations = (opts.aggregations ?? []) as Array<{ field: string; method: string; alias: string }>; + const buckets = new Map; rows: unknown[] }>(); + for (const opp of OPPS) { + if (!matches(opp as unknown as Record, opts.filter)) continue; + const key: Record = {}; + for (const g of groupBy) { + const field = typeof g === 'string' ? g : g.field; + const raw = (opp as unknown as Record)[field]; + key[field] = bucket(raw, typeof g === 'string' ? undefined : g.dateGranularity); + } + const id = JSON.stringify(groupBy.map((g) => key[typeof g === 'string' ? g : g.field] ?? null)); + let b = buckets.get(id); + if (!b) { + b = { key, rows: [] }; + buckets.set(id, b); + } + b.rows.push(opp); + } + return [...buckets.values()].map(({ key, rows }) => { + const row: Record = { ...key }; + for (const a of aggregations) row[a.alias] = rows.length; + return row; + }); +} + +/** + * The ObjectQL-aggregate service — the path every date-bucketed query lands on + * — recording each lowered aggregate call, so the GROUP BY itself is asserted + * rather than inferred from the rows. + */ +function svc() { + const calls: AggOptions[] = []; + const service = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (_object: string, options: Record) => { + calls.push(options as AggOptions); + return evaluateAggregate(options as AggOptions); + }, + }); + return { service, calls }; +} + +const groupByOf = (call: AggOptions) => (call.groupBy ?? []) as GroupByItem[]; +const names = (fields: Array<{ name: string }>) => fields.map((f) => f.name); +const descriptor = (fields: Array<{ name: string; type?: string; label?: string }>, name: string) => { + const f = fields.find((x) => x.name === name); + return f ? { name: f.name, type: f.type, label: f.label } : undefined; +}; + +const WINDOW: [string, string] = ['2026-01-01', '2026-02-28']; + +// ── 1) the window-only entry stays a filter ───────────────────────────────── + +describe('#5688 — a dateRange-only entry filters rows and nothing else', () => { + it('the issue verbatim: "by Owner" + a date filter is by Owner, not Owner × month', async () => { + const { service, calls } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + }, + CTX, + ); + // The column set is the selection's own: no time column nobody asked for. + expect(names(result.fields)).toEqual(['owner', 'opp_count']); + // Two rows, not three — `u1`'s January and February records are one owner. + expect(result.rows).toEqual([ + { owner: 'u1', opp_count: 2 }, + { owner: 'u2', opp_count: 1 }, + ]); + // Where it is settled: the entry never reaches the GROUP BY, and its + // `dateRange` is still applied as a predicate — fewer rows, same columns. + expect(groupByOf(calls[0])).toEqual(['owner_id']); + expect(calls[0].filter).toMatchObject({ close_date: { $gte: WINDOW[0], $lte: WINDOW[1] } }); + }); + + it('adding the window changes the ROW COUNT and nothing about the column set', async () => { + const selection = { dimensions: ['owner'], measures: ['opp_count'] }; + const unfiltered = await svc().service.queryDataset(dataset, selection, CTX); + const filtered = await svc().service.queryDataset( + dataset, + { ...selection, timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }] }, + CTX, + ); + // Identical columns; the window only removed the 2025 records from the counts. + expect(names(filtered.fields)).toEqual(names(unfiltered.fields)); + expect(unfiltered.rows).toEqual([ + { owner: 'u1', opp_count: 5 }, + { owner: 'u2', opp_count: 2 }, + ]); + expect(filtered.rows).toEqual([ + { owner: 'u1', opp_count: 2 }, + { owner: 'u2', opp_count: 1 }, + ]); + }); + + it('a KPI single-value card gets ONE row, not the first of several buckets', async () => { + const { service } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: [], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + }, + CTX, + ); + expect(result.rows).toEqual([{ opp_count: 3 }]); + expect(names(result.fields)).toEqual(['opp_count']); + }); + + it('mints no descriptor for the window dimension — the lookup widened, the projection did not', async () => { + const { service } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + }, + CTX, + ); + // #5688 widened the label enrichment's lookup set to dimensions reached via + // `timeDimensions`. It must still describe only columns that EXIST: an entry + // that stayed a window is not a column, so no `close_date` field appears. + expect(descriptor(result.fields, 'close_date')).toBeUndefined(); + expect(result.rows.every((r) => !('close_date' in r))).toBe(true); + }); +}); + +// ── 2) the entries that ARE asking for a bucket keep it ───────────────────── + +describe('#5688 — an entry asking to be bucketed still is (#3588/#4033/#4870 guard)', () => { + it('the dimension is also a GRID dimension → bucketed, exactly as before', async () => { + const { service, calls } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner', 'close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + }, + CTX, + ); + // Left unbucketed this would group RAW TIMESTAMPS — one bucket per row, + // which is the #3588 defect the backfill was introduced to fix. + expect(groupByOf(calls[0])).toEqual(['owner_id', { field: 'close_date', dateGranularity: 'month' }]); + // Chronological by default on the time axis (#3916), ties in grouping order. + expect(result.rows).toEqual([ + { owner: 'u1', close_date: '2026-01', opp_count: 1 }, + { owner: 'u2', close_date: '2026-01', opp_count: 1 }, + { owner: 'u1', close_date: '2026-02', opp_count: 1 }, + ]); + expect(descriptor(result.fields, 'close_date')).toEqual({ + name: 'close_date', type: 'time', label: 'Close Date', + }); + }); + + it('the entry states its OWN granularity → bucketed and projected, though never selected (#4033)', async () => { + const { service, calls } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW, granularity: 'month' }], + }, + CTX, + ); + expect(groupByOf(calls[0])).toEqual(['owner_id', { field: 'close_date', dateGranularity: 'month' }]); + expect(names(result.fields)).toEqual(['owner', 'close_date', 'opp_count']); + // #5691's blind spot, closed here: the projected time column reaches + // `fields` with a `label` now, not a bare `type`. The enrichment loop used + // to walk `selection.dimensions` only, which this column is not in. + expect(descriptor(result.fields, 'close_date')).toEqual({ + name: 'close_date', type: 'time', label: 'Close Date', + }); + }); + + it('`selection.dateGranularity` is set → the presentation asked for buckets, so it gets them', async () => { + const { service, calls } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + dateGranularity: 'year', + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + }, + CTX, + ); + // The selection's granularity beats the dataset's `month` default — the + // precedence chain in `resolveDimensionGranularity` is untouched by #5688. + expect(groupByOf(calls[0])).toEqual(['owner_id', { field: 'close_date', dateGranularity: 'year' }]); + expect(result.rows).toEqual([ + { owner: 'u1', close_date: '2026', opp_count: 2 }, + { owner: 'u2', close_date: '2026', opp_count: 1 }, + ]); + }); + + it('control — a dimension declaring NO dateGranularity was never backfilled either way', async () => { + const { service, calls } = svc(); + await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'created_at', dateRange: ['2025-01-01', '2025-12-31'] }], + }, + CTX, + ); + // Same outcome as `close_date` now has, reached for a different reason: + // there was no default to fill in. Pinned so the declaration is visibly the + // trigger condition rather than an assumed one. + expect(groupByOf(calls[0])).toEqual(['owner_id']); + }); +}); + +// ── 3) compareTo — the pair ───────────────────────────────────────────────── + +describe('#5688 — compareTo buckets both passes alike, whichever verdict it reaches', () => { + it('window-only anchor: NEITHER pass buckets, and the comparison merges on `owner`', async () => { + const { service, calls } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['owner'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // Both passes ran, both grouped by `owner` alone — same verdict, so the two + // grids are shaped alike (the #4870 invariant, at the other verdict). + expect(calls).toHaveLength(2); + expect(groupByOf(calls[0])).toEqual(['owner_id']); + expect(groupByOf(calls[1])).toEqual(['owner_id']); + expect(calls[1].filter).toMatchObject({ close_date: { $gte: '2025-01-01', $lte: '2025-02-28' } }); + // `u1`: 2 this window against 3 last year — 3 spans two months, so the + // number is only reachable by merging the WHOLE shifted window onto the + // owner. Before #5688 this row set was three month-split rows carrying + // `__compare` 0 / 2 / 1: the January row read a confident 0 and the other + // two months' comparison values were overwritten by whichever landed last. + expect(result.rows).toEqual([ + { owner: 'u1', opp_count: 2, opp_count__compare: 3 }, + { owner: 'u2', opp_count: 1, opp_count__compare: 1 }, + ]); + expect(names(result.fields)).toEqual(['owner', 'opp_count', 'opp_count__compare']); + }); + + it('bucketed anchor: BOTH passes bucket by month — the #4870 fix, unregressed', async () => { + const { service, calls } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: ['close_date'], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + expect(calls).toHaveLength(2); + // The comparison pass must bucket EXACTLY like the primary one. Hand-rolled + // without granularity resolution it grouped raw timestamps, and no dimension + // key ever matched. + expect(groupByOf(calls[0])).toEqual([{ field: 'close_date', dateGranularity: 'month' }]); + expect(groupByOf(calls[1])).toEqual([{ field: 'close_date', dateGranularity: 'month' }]); + expect(calls[1].filter).toMatchObject({ close_date: { $gte: '2025-01-01', $lte: '2025-02-28' } }); + expect(descriptor(result.fields, 'close_date')).toEqual({ + name: 'close_date', type: 'time', label: 'Close Date', + }); + // Long-standing behaviour, unchanged by #5688 and asserted so the pair is + // read as a whole: the shifted grid keys by its OWN buckets, so a comparison + // bucket with no current-window twin arrives as its own row (the append + // `fillEmptyGroups` is documented to expect). + expect(result.rows).toEqual([ + { close_date: '2025-01', opp_count: 0, opp_count__compare: 2 }, + { close_date: '2025-02', opp_count: 0, opp_count__compare: 2 }, + { close_date: '2026-01', opp_count: 2, opp_count__compare: 0 }, + { close_date: '2026-02', opp_count: 1, opp_count__compare: 0 }, + ]); + }); + + it('window-only anchor with NO grid dimensions: one row, both totals comparable', async () => { + const { service } = svc(); + const result = await service.queryDataset( + dataset, + { + dimensions: [], + measures: ['opp_count'], + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW }], + compareTo: { kind: 'previousYear', dimension: 'close_date' }, + }, + CTX, + ); + // The period-over-period tile: 3 this window, 4 last year. Before #5688 this + // came back as two month rows and the tile read the first one. + expect(result.rows).toEqual([{ opp_count: 3, opp_count__compare: 4 }]); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index ef948c3985..d39a779d59 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -1022,9 +1022,31 @@ export class AnalyticsService implements IAnalyticsService { // entry in `fields` for this pass to enrich. Fixed where the grid is // assembled (see that method's #5537 note), which is also the only place // that knows what the active strategy actually projected. - if (result.fields?.length && selectedDims.length) { - const dimByName = new Map(selectedDims.map((d) => [d.name, d])); - const dimByField = new Map(selectedDims.filter((d) => !!d.field).map((d) => [d.field as string, d])); + // + // #5688 — the set to describe FROM is wider than `selection.dimensions`. A + // `timeDimensions` entry that resolves a granularity is GROUPED BY, so it is + // a result column even when the caller never listed it under `dimensions` + // (#4033's `projectedDimensions`) — and reading `selection.dimensions` alone + // left exactly that column carrying a `type` and no `label`, the blind spot + // #5537's PR pinned as a control case. Widening the LOOKUP is not the same + // as widening the projection: the loop still enriches only entries + // `result.fields` already carries, so an entry that stays a pure window + // contributes no column and receives no descriptor. + // + // Kept out of `selectedDims` deliberately. Drill metadata and row-value + // label resolution above answer a different question — which dimensions the + // caller GROUPED THE GRID BY, i.e. what a click can be turned back into + // records — and widening those would change drill payloads and row values, + // not table headers. + const describableDims = [...selectedDims]; + for (const t of selection.timeDimensions ?? []) { + if (describableDims.some((d) => d.name === t.dimension)) continue; + const d = dataset.dimensions?.find((x) => x.name === t.dimension); + if (d) describableDims.push(d); + } + if (result.fields?.length && describableDims.length) { + const dimByName = new Map(describableDims.map((d) => [d.name, d])); + const dimByField = new Map(describableDims.filter((d) => !!d.field).map((d) => [d.field as string, d])); for (const f of result.fields) { if (f.label != null) continue; // Result fields may be keyed by the dataset dimension NAME or the diff --git a/packages/services/service-analytics/src/dataset-executor.ts b/packages/services/service-analytics/src/dataset-executor.ts index 45f30231df..70d6b37a29 100644 --- a/packages/services/service-analytics/src/dataset-executor.ts +++ b/packages/services/service-analytics/src/dataset-executor.ts @@ -286,7 +286,16 @@ export type DateGranularityValue = NonNullable t.dimension)); + const groupedDims = new Set(opts.dimensions); const granularityFor = (name: string): string | undefined => { const cd = compiled.cube.dimensions[name]; if (cd?.type !== 'time') return undefined; const datasetDefault = cd.granularities?.length === 1 ? String(cd.granularities[0]) : undefined; return resolveDimensionGranularity(opts.selection, name, datasetDefault); }; - // Fill in a bucket size for caller-supplied entries that named none. + /** + * Does a caller-supplied entry that stated NO granularity get one filled in? + * + * Only when something in the request says this date is being bucketed: + * - the dimension is one of the grid dimensions this query groups by (so + * it is a column regardless, and leaving it unbucketed would group raw + * timestamps — one bucket per row, the #3588 defect); or + * - `selection.dateGranularity` is set, which is the presentation stating + * a bucket size for its date axes. + * An entry carrying its own `granularity` never reaches here (nothing to + * fill in), and stays grouped. + * + * The dataset dimension's own `dateGranularity` is deliberately NOT such a + * signal on its own: it is how this date renders WHEN grouped, not a request + * to group by it. Reading it as one is what made a date-range filter behave + * like a second GROUP BY. + */ + const bucketsUnstatedEntry = (dimension: string): boolean => + groupedDims.has(dimension) || opts.selection.dateGranularity != null; + // Fill in a bucket size for caller-supplied entries that named none — for + // the entries that are asking to be bucketed at all. const resolvedTimeDims = selTimeDims.map((t) => { if (t.granularity) return t; + if (!bucketsUnstatedEntry(t.dimension)) return t; const granularity = granularityFor(t.dimension); return granularity ? { ...t, granularity } : t; });