diff --git a/.changeset/pivot-null-bucket-4056.md b/.changeset/pivot-null-bucket-4056.md new file mode 100644 index 0000000000..32412ce23e --- /dev/null +++ b/.changeset/pivot-null-bucket-4056.md @@ -0,0 +1,50 @@ +--- +"@object-ui/core": patch +"@object-ui/plugin-dashboard": patch +"@object-ui/plugin-report": patch +--- + +Pivot buckets encode an empty dimension value as JSON `null`, so it no longer collides with a row whose value is literally the placeholder character + +objectstack#5473 / objectstack#5665 replaced the pivot's delimiter-joined ids +with `JSON.stringify`, because every delimiter that had been tried — an empty +string, a plain space, a control character — assumed the data would not contain +it, and each assumption failed on ordinary data. This closes the last place the +same assumption survived: the ids were JSON, but the VALUES fed into them were +spelled `String(row[d] ?? '∅')`, so an absent dimension value became the +ordinary string `"∅"` and shared a bucket with a row whose value literally is +that character (U+2205). One bucket, later row overwriting the earlier one — the +cell showed a different row's measure, the overwritten row was unreachable, and +drill-through followed the same wrong index into the wrong records, all without +an error. The trigger requires that character to appear as a dimension value, so +this is the assumption being removed rather than a defect users hit today. + +An empty value now encodes as JSON `null`, which `JSON.stringify` renders as a +bare `null` that no string can spell. The normalization lives in +`@object-ui/core` as `pivotDimensionValue` (absent ⇒ `null`, everything else ⇒ +its string form) rather than at each call site, because a placeholder spelled by +a caller is a placeholder that can collide again — which is exactly how this one +survived the previous fix. `pivotBucketId` accepts `Array` +accordingly; that is a widening, so existing callers passing `string[]` are +unaffected. + +Both renderers' bucket keys move together, which the fix requires: a bucket id +and the subtotal map keyed by it are built from the same expression, so changing +one alone would split the headers while the subtotal map still merged, landing +every column subtotal under the wrong header. In `plugin-dashboard`'s +`DatasetWidget` that is the row bucket id, the column bucket id, the cell key, +and both the `rowTotalById` and `colTotalById` lookups; in `plugin-report`'s +`DatasetReportRenderer` the single `bucketId` helper already feeds all five. + +The dashboard's column bucket id also stops being a bare string and becomes a +one-element tuple through the same shared encoder. It was the one id in the +family still built by hand, on the reasoning that a single value needs no +boundary — true of the boundary, false of everything else the encoder does, and +it is why the across axis kept carrying this collision after the row ids were +fixed. + +No display change: these placeholders only ever entered ids, never labels. An +unset dimension still renders through `formatDimensionValue` exactly as before, +and data containing neither an absent value nor that character buckets +identically — the ids are opaque lookup keys, never parsed back into a value, +never shown, never persisted. diff --git a/packages/core/src/utils/__tests__/dataset-pivot.test.ts b/packages/core/src/utils/__tests__/dataset-pivot.test.ts new file mode 100644 index 0000000000..271db2d867 --- /dev/null +++ b/packages/core/src/utils/__tests__/dataset-pivot.test.ts @@ -0,0 +1,74 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { pivotBucketId, pivotCellKey, pivotDimensionValue } from '../dataset-pivot.js'; + +/** + * The encoders' whole job is that DIFFERENT dimension-value tuples never spell + * the SAME id. Every historical defect in this family was one pair of tuples + * that did (objectstack#5473 a space join, objectstack#5665 an empty join, + * objectui#4056 a null placeholder), so the tests are written as pairs that + * used to collide — not as literal id spellings, which is what let the old + * encodings read as correct. + */ +describe('pivotDimensionValue', () => { + it('maps an absent value to null and everything else to its string form', () => { + expect(pivotDimensionValue(null)).toBeNull(); + expect(pivotDimensionValue(undefined)).toBeNull(); + expect(pivotDimensionValue('North')).toBe('North'); + expect(pivotDimensionValue(0)).toBe('0'); + expect(pivotDimensionValue(false)).toBe('false'); + // The empty string is a VALUE a dimension can hold, not an absent value — + // `?? ` never coerced it and neither does this. + expect(pivotDimensionValue('')).toBe(''); + }); + + it('does not map any string to null — including the placeholder it replaced', () => { + // objectui#4056: the old spelling was `String(v ?? '∅')`, which mapped an + // absent value ONTO a string, so this character had two meanings. + expect(pivotDimensionValue('∅')).toBe('∅'); + expect(pivotDimensionValue('null')).toBe('null'); + }); +}); + +describe('pivotBucketId', () => { + it('keeps an absent value apart from every string spelling of it (objectui#4056)', () => { + expect(pivotBucketId([null])).not.toBe(pivotBucketId(['∅'])); + expect(pivotBucketId([null])).not.toBe(pivotBucketId(['null'])); + expect(pivotBucketId([null])).not.toBe(pivotBucketId([''])); + // …and the pair as the callers actually build it, raw value in. + expect(pivotBucketId([pivotDimensionValue(null)])).not.toBe( + pivotBucketId([pivotDimensionValue('∅')]), + ); + }); + + it('keeps tuples apart that concatenate to the same string', () => { + expect(pivotBucketId(['x', 'yz'])).not.toBe(pivotBucketId(['xy', 'z'])); + expect(pivotBucketId(['New', 'York Q1'])).not.toBe(pivotBucketId(['New York', 'Q1'])); + }); + + it('distinguishes an absent value by POSITION, not just by presence', () => { + expect(pivotBucketId([null, 'a'])).not.toBe(pivotBucketId(['a', null])); + expect(pivotBucketId([null, 'a'])).not.toBe(pivotBucketId([null, null, 'a'])); + }); + + it('is deterministic, and unchanged for all-string tuples', () => { + expect(pivotBucketId(['North', 'Q1'])).toBe(pivotBucketId(['North', 'Q1'])); + // Regrouping stability: data with no absent values encodes exactly as it did + // before objectui#4056, so existing buckets do not re-key. + expect(pivotBucketId(['North', 'Q1'])).toBe(JSON.stringify(['North', 'Q1'])); + }); +}); + +describe('pivotCellKey', () => { + it('keeps cell keys apart whose row/column ids meet at a different point', () => { + expect(pivotCellKey('a', 'bc')).not.toBe(pivotCellKey('ab', 'c')); + }); + + it('keys a null bucket and a placeholder bucket to different cells', () => { + const col = pivotBucketId(['Q1']); + expect(pivotCellKey(pivotBucketId([null]), col)).not.toBe( + pivotCellKey(pivotBucketId(['∅']), col), + ); + }); +}); diff --git a/packages/core/src/utils/dataset-pivot.ts b/packages/core/src/utils/dataset-pivot.ts index da75966fe5..7c2bbb6cfe 100644 --- a/packages/core/src/utils/dataset-pivot.ts +++ b/packages/core/src/utils/dataset-pivot.ts @@ -38,20 +38,44 @@ * * Pure (no React / i18n), like its `dataset-format` neighbour. * - * Known residual, tracked separately in objectstack#5666: callers encode a - * null/undefined dimension value as a placeholder string, so it still collides - * with a value that literally equals that placeholder. That is a property of - * the placeholder, not of the encoding below. + * The empty-value encoding closed the last instance of the same assumption + * (objectui#4056): callers used to spell a null/undefined dimension value as + * the STRING `'∅'` before encoding it, so a row whose value literally IS that + * character shared a bucket with a row whose value is absent — the placeholder + * reintroduced, one level up, exactly the "no value contains this character" + * assumption the encoder below had just removed. An empty value is now JSON + * `null`, which `JSON.stringify` renders as a bare `null` no string can spell, + * and `pivotDimensionValue` owns that normalization so no caller has to hold a + * placeholder literal of its own. */ /** - * Encode a pivot BUCKET id from its dimension values. + * Normalize ONE raw dimension value for `pivotBucketId`: an absent value (null + * or undefined) becomes JSON `null`, everything else its string form. + * + * This lives here rather than at each call site because a placeholder spelled + * by the caller is a placeholder that can collide: `String(v ?? '∅')` put an + * ordinary string into the tuple, so `null` and the character `'∅'` encoded + * identically (objectui#4056). `null` is not a string, so nothing a dimension + * can hold encodes to it — the same reason the tuple is JSON rather than a + * delimiter join. + */ +export const pivotDimensionValue = (value: unknown): string | null => + value == null ? null : String(value); + +/** + * Encode a pivot BUCKET id from its dimension values, each already normalized + * by `pivotDimensionValue` (empty ⇒ `null`). * * Axis-neutral on purpose: a DOWN bucket and an ACROSS bucket are the same kind * of thing (a dimension-value tuple), and a cross-tab with multiple across - * dimensions collides on that axis just as readily as on the down axis. + * dimensions collides on that axis just as readily as on the down axis. That + * holds for a SINGLE-value across bucket too — a bare value is a one-element + * tuple, and spelling it as the raw string instead is what left the dashboard's + * column ids on a second, colliding encoding after the row ids were fixed. */ -export const pivotBucketId = (dimensionValues: string[]): string => JSON.stringify(dimensionValues); +export const pivotBucketId = (dimensionValues: Array): string => + JSON.stringify(dimensionValues); /** * Encode the cell key for a (down bucket, across bucket) pair — the key of the diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index 8e1c5cfb3d..f8e42b732f 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -43,11 +43,12 @@ import { // The pivot key encoders now live in `@object-ui/core` so this widget and the // report renderer's cross-tab share ONE implementation — each having written // its own is why the same collision had to be fixed twice (objectstack#5473, - // objectstack#5665). Aliased to the local name: `pivotRowId` reads right here - // (this widget only ever encodes DOWN buckets — its across axis is a single - // dimension), while the shared helper is axis-neutral because the report's - // cross-tab keys multi-dimension ACROSS buckets with it too. - pivotBucketId as pivotRowId, + // objectstack#5665). Imported under the shared name because BOTH of this + // widget's axes now use it: the across axis used to spell its single-value id + // as a bare string, which was a second encoding of the same kind of id and + // carried the placeholder collision on its own (objectui#4056). + pivotBucketId, + pivotDimensionValue, pivotCellKey, compareToTrendLabelKey, type CompareToConfig, @@ -81,12 +82,18 @@ export const buildDrillFilter = buildDatasetDrillFilter; * live in `@object-ui/core` (`pivotBucketId` / `pivotCellKey`) so this widget * and the report renderer's cross-tab key their buckets identically. See that * module for why both are `JSON.stringify` rather than a delimiter character, - * and for the null-placeholder residual tracked in objectstack#5666. + * and why an empty value encodes as JSON `null` rather than a placeholder + * string (objectui#4056). * - * Every consumer of a row id — the cell index below AND the row-total lookup in - * the cross-tab renderer — must build its key with these; a second, hand-rolled - * encoding of the same id is what made the old bug invisible. + * Every consumer of a bucket id — the cell index below, the row-total lookup + * AND the column-total lookup in the cross-tab renderer — must build its key + * with these, over values normalized by `pivotDimensionValue`; a second, + * hand-rolled encoding of the same id is what made the old bug invisible. + * + * `pivotRowId` is the historical name of the axis-neutral encoder, kept as an + * alias so this package's published surface does not change. */ +const pivotRowId = pivotBucketId; export { pivotRowId, pivotCellKey }; /** @@ -113,13 +120,15 @@ export function buildPivot( const cellIndex = new Map(); rows.forEach((row, index) => { // Both ids are opaque lookup keys, never displayed — the visible text comes - // from `labels`/`label` via formatDimensionValue. The column id stays the - // bare value because a single value needs no boundary; only the row id joins - // several values, and pivotRowId encodes that join unambiguously. (It used to - // join them with a control character no dimension value was ASSUMED to carry; - // pivotRowId needs no such assumption.) - const rid = pivotRowId(rowDims.map((d) => String(row[d] ?? '∅'))); - const cid = String(row[colDim] ?? '∅'); + // from `labels`/`label` via formatDimensionValue. BOTH go through the shared + // encoder, over values normalized by `pivotDimensionValue`. The column id + // used to be the bare value on the reasoning that a single value needs no + // boundary; that is true of the boundary and false of everything else the + // encoder does, and it left the across axis on its own encoding — which then + // carried the null-placeholder collision independently (objectui#4056). A + // one-element tuple costs nothing and keeps one encoding for one kind of id. + const rid = pivotBucketId(rowDims.map((d) => pivotDimensionValue(row[d]))); + const cid = pivotBucketId([pivotDimensionValue(row[colDim])]); if (!rowSeen.has(rid)) { rowSeen.add(rid); rowHeaders.push({ id: rid, labels: rowDims.map((d) => formatDimensionValue(row[d])) }); } if (!colSeen.has(cid)) { colSeen.add(cid); colHeaders.push({ id: cid, label: formatDimensionValue(row[colDim]) }); } cellIndex.set(pivotCellKey(rid, cid), index); @@ -943,9 +952,9 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: const findTotals = (dims: string[]) => state.totals?.find((t) => Array.isArray(t.dimensions) && t.dimensions.join(',') === dims.join(','))?.rows; const rowTotalById = new Map(); - for (const r of findTotals(rowDims) ?? []) rowTotalById.set(pivotRowId(rowDims.map((d) => String(r[d] ?? '∅'))), r); + for (const r of findTotals(rowDims) ?? []) rowTotalById.set(pivotBucketId(rowDims.map((d) => pivotDimensionValue(r[d]))), r); const colTotalById = new Map(); - for (const r of findTotals([colDim]) ?? []) colTotalById.set(String(r[colDim] ?? '∅'), r); + for (const r of findTotals([colDim]) ?? []) colTotalById.set(pivotBucketId([pivotDimensionValue(r[colDim])]), r); const grandTotal = findTotals([])?.[0]; const showTotalCol = rowTotalById.size > 0; const showTotalRow = colTotalById.size > 0; diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx index 387c49d42c..fe80ba5392 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.test.tsx @@ -419,6 +419,76 @@ describe('DatasetWidget', () => { expect(cellAt(p, ['xy', 'z'], 'High')).toBe(1); }); + // objectui#4056 — the LAST encoding in this family that still relied on "the + // data will not contain this character". Every id above is JSON-encoded, but + // the values fed into it were `String(row[d] ?? '∅')`: an absent value became + // the STRING "∅", so a row whose dimension value literally IS that character + // encoded to the same bucket as a row whose value is null. Same shape as + // objectstack#5473 — one bucket, the later row overwriting the earlier one — + // reached through the placeholder rather than through the join. An empty value + // is now JSON `null`, which no string can collide with. + it('buildPivot keeps a null row-dimension value apart from the literal placeholder character', () => { + const rows = [ + { region: null, quarter: 'Q1', amount: 111 }, + { region: '∅', quarter: 'Q1', amount: 222 }, + ]; + const p = buildPivot(rows, ['region'], 'quarter'); + // Two rows in, two buckets out — one bucket for both is the defect. + expect(p.rowHeaders).toHaveLength(2); + expect(p.rowHeaders[0].id).not.toBe(p.rowHeaders[1].id); + expect(p.cellIndex.size).toBe(2); + // Resolved by DISPLAY label, never by id spelling: null renders as the + // em-dash placeholder, the literal character renders as itself. + expect(cellAt(p, ['—'], 'Q1')).toBe(0); + expect(cellAt(p, ['∅'], 'Q1')).toBe(1); + }); + + it('buildPivot keeps a null COLUMN-dimension value apart from the literal placeholder character', () => { + // The across axis is the half the row-id fix could not reach: the column + // bucket id was a bare string built by the same `?? '∅'` expression, so it + // carried the collision independently of the JSON-encoded row id. + const rows = [ + { status: 'Open', priority: null, task_count: 1 }, + { status: 'Open', priority: '∅', task_count: 2 }, + ]; + const p = buildPivot(rows, ['status'], 'priority'); + expect(p.colHeaders).toHaveLength(2); + expect(p.colHeaders[0].id).not.toBe(p.colHeaders[1].id); + expect(p.colHeaders.map((c) => c.label)).toEqual(['—', '∅']); + expect(p.cellIndex.size).toBe(2); + expect(cellAt(p, ['Open'], '—')).toBe(0); + expect(cellAt(p, ['Open'], '∅')).toBe(1); + }); + + it('buildPivot regroups ordinary data exactly as before (no nulls, no placeholder character)', () => { + // Regrouping stability: the encoding changed for BOTH axes, so the control + // is that data containing neither a null nor the placeholder character + // buckets identically — same header count, same first-seen order, same + // labels, same cell→index mapping. The ids are opaque lookup keys (Map keys + // and React keys only — never parsed back into a value, never displayed, + // never persisted), so what has to hold is the structure, not the spelling. + const rows = [ + { region: 'North', segment: 'SMB', quarter: 'Q1', amount: 1 }, + { region: 'North', segment: 'Enterprise', quarter: 'Q1', amount: 2 }, + { region: 'South', segment: 'SMB', quarter: 'Q2', amount: 3 }, + ]; + const p = buildPivot(rows, ['region', 'segment'], 'quarter'); + expect(p.rowHeaders.map((r) => r.labels)).toEqual([ + ['North', 'SMB'], + ['North', 'Enterprise'], + ['South', 'SMB'], + ]); + expect(p.colHeaders.map((c) => c.label)).toEqual(['Q1', 'Q2']); + expect(p.cellIndex.size).toBe(3); + expect(cellAt(p, ['North', 'SMB'], 'Q1')).toBe(0); + expect(cellAt(p, ['North', 'Enterprise'], 'Q1')).toBe(1); + expect(cellAt(p, ['South', 'SMB'], 'Q2')).toBe(2); + expect(cellAt(p, ['North', 'SMB'], 'Q2')).toBeUndefined(); + // The row id for all-string values is still exactly the JSON tuple + // objectstack#5473 introduced — the placeholder fix did not re-spell it. + expect(p.rowHeaders[0].id).toBe(JSON.stringify(['North', 'SMB'])); + }); + it('renders a pivot (≥2 dims) as a true cross-tab, not a flat table', async () => { const src = { queryDataset: vi.fn(async () => ({ rows: [ @@ -495,6 +565,54 @@ describe('DatasetWidget', () => { expect((src.find.mock.calls[0][1] as any).$filter).toEqual({ region: 'new', quarter: 'york_q1' }); }); + it('gives the null bucket its OWN cell and drills it to the null rows, not the placeholder rows (objectui#4056)', async () => { + // End-to-end shape of the placeholder collision: an unset region and a + // region whose value literally IS the placeholder character shared one + // bucket, so the table showed 222 twice, 111 was unreachable, and the null + // row's cell drilled into the OTHER row's records — the objectstack#5473 + // symptom class, reached through the null placeholder. + const src = { + queryDataset: vi.fn(async () => ({ + rows: [ + { region: null, quarter: 'Q1', amount: 111 }, + { region: '∅', quarter: 'Q1', amount: 222 }, + ], + fields: [ + { name: 'region', type: 'string', label: 'Region' }, + { name: 'quarter', type: 'string', label: 'Quarter' }, + { name: 'amount', type: 'number', label: 'Amount' }, + ], + object: 'showcase_deal', + dimensionFields: { region: 'region', quarter: 'quarter' }, + // Deliberately distinguishable raw values: the drill filter names which + // flat index the clicked cell resolved to. + drillRawRows: [ + { region: 'was_null', quarter: 'q1' }, + { region: 'literal_emptyset', quarter: 'q1' }, + ], + })), + find: vi.fn(async () => ({ data: [] })), + getObjectSchema: vi.fn(async () => ({ fields: { region: { type: 'text', label: 'Region' } } })), + }; + render(); + const m = await screen.findByTestId('dataset-matrix'); + + // Control (display layer untouched): the null dimension still renders as the + // em dash `formatDimensionValue` has always produced, and the row whose + // value IS the character renders as that character. The placeholders only + // ever entered the ids. + const rowNull = within(m).getByText('—').closest('tr') as HTMLElement; + expect([...rowNull.querySelectorAll('td')].map((td) => td.textContent)).toEqual(['—', '111']); + const rowLiteral = within(m).getByText('∅').closest('tr') as HTMLElement; + expect([...rowLiteral.querySelectorAll('td')].map((td) => td.textContent)).toEqual(['∅', '222']); + + // Drill-through reads `drillRawRows` by the same index the cell resolved, + // so a merged bucket drilled into the wrong records too. + fireEvent.click(within(rowNull).getByTestId('dataset-drill-cell')); + await waitFor(() => expect(src.find).toHaveBeenCalled()); + expect((src.find.mock.calls[0][1] as any).$filter).toEqual({ region: 'was_null', quarter: 'q1' }); + }); + it('makes matrix cells drillable when the server returns drill metadata', async () => { const src = { queryDataset: vi.fn(async () => ({ rows: [{ status: 'Open', priority: 'High', task_count: 2 }], @@ -550,6 +668,40 @@ describe('DatasetWidget', () => { expect(within(m).getByTestId('matrix-grand-total').textContent).toBe('6'); }); + it('matches column subtotals to the null bucket and the placeholder bucket separately (objectui#4056)', async () => { + // The card's radius warning, pinned: the column bucket id and the + // `colTotalById` key are built by the same expression, so they must change + // together. If either kept the placeholder spelling, the two null-ish + // columns would merge — or worse, the headers would split while the + // subtotal map still merged, and every column subtotal would land under the + // wrong header (the "one id, two encodings" shape objectui#3414 converged + // away). + const src = { queryDataset: vi.fn(async () => ({ + rows: [ + { status: 'Open', priority: null, task_count: 1 }, + { status: 'Open', priority: '∅', task_count: 2 }, + ], + fields: [ + { name: 'status', type: 'string', label: 'Status' }, + { name: 'priority', type: 'string', label: 'Priority' }, + { name: 'task_count', type: 'number', label: 'Tasks' }, + ], + totals: [ + { dimensions: ['status'], rows: [{ status: 'Open', task_count: 3 }] }, + { dimensions: ['priority'], rows: [{ priority: null, task_count: 10 }, { priority: '∅', task_count: 20 }] }, + { dimensions: [], rows: [{ task_count: 30 }] }, + ], + })) }; + render(); + const m = await screen.findByTestId('dataset-matrix'); + // Two column headers, in first-seen order, labelled by the display layer. + const headers = [...m.querySelectorAll('thead th')].map((th) => th.textContent); + expect(headers).toEqual(['Status', '—', '∅', 'Total']); + // Each column subtotal under ITS OWN header, then the grand total. + const totalRow = within(m).getByTestId('matrix-total-row'); + expect([...totalRow.querySelectorAll('td')].map((td) => td.textContent)).toEqual(['Total', '10', '20', '30']); + }); + it('matches server row subtotals to a MULTI-dimension row bucket', async () => { // The row-total lookup keys the same row bucket ids as the row headers, so // it has to build them with the same encoder. It used to roll its own join, diff --git a/packages/plugin-report/src/DatasetReportRenderer.tsx b/packages/plugin-report/src/DatasetReportRenderer.tsx index d832432ec1..5180ccb97f 100644 --- a/packages/plugin-report/src/DatasetReportRenderer.tsx +++ b/packages/plugin-report/src/DatasetReportRenderer.tsx @@ -65,6 +65,7 @@ import { buildDatasetFieldHelpers, buildDatasetDrillFilter, pivotBucketId, + pivotDimensionValue, pivotCellKey, type DatasetResultField, type DatasetDrillRange, @@ -750,9 +751,15 @@ function DatasetReportChart({ * boundary at all between adjacent values, so `'x'` + `'yz'` and `'xy'` + `'z'` * were one bucket and the later row overwrote the earlier one * (objectstack#5665; objectstack#5473 is the same defect in the dashboard). + * + * Each value is normalized by `pivotDimensionValue`, so an absent one encodes + * as JSON `null`. It used to be spelled `String(row[d] ?? '∅')`, which put an + * ordinary string in the tuple and merged a null value with a value that + * literally IS that character — the encoder's own assumption ("no value spells + * this") reintroduced by its caller (objectui#4056). */ function bucketId(dims: string[], row: Row): string { - return pivotBucketId(dims.map((d) => String(row[d] ?? '∅'))); + return pivotBucketId(dims.map((d) => pivotDimensionValue(row[d]))); } function bucketLabel(dims: string[], row: Row): string { diff --git a/packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx b/packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx index 826dc3f279..b52063e6a8 100644 --- a/packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx +++ b/packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx @@ -200,6 +200,58 @@ describe('DatasetReportRenderer', () => { expect(screen.getByTestId('matrix-grand-total')).toHaveTextContent('44'); }); + it('matrix keeps a null bucket apart from the literal placeholder character, on BOTH axes (objectui#4056)', async () => { + // The last encoding in this family that relied on "the data will not + // contain this character": `bucketId` fed `String(row[d] ?? '∅')` into the + // JSON encoder, so an unset dimension became the STRING "∅" and collided + // with a row whose value literally IS that character — one bucket, the later + // row overwriting the earlier one (the objectstack#5473 symptom class, + // reached through the placeholder). An empty value is now JSON `null`. + // + // Both axes in one fixture because this renderer keys row headers, column + // headers, cells and BOTH subtotal maps off the same `bucketId`. + const src = makeSource({ + task_metrics: { + rows: [ + { status: null, priority: null, est_hours: 1 }, + { status: null, priority: '∅', est_hours: 2 }, + { status: '∅', priority: null, est_hours: 3 }, + { status: '∅', priority: '∅', est_hours: 4 }, + ], + totals: [ + { dimensions: ['status'], rows: [{ status: null, est_hours: 10 }, { status: '∅', est_hours: 20 }] }, + { dimensions: ['priority'], rows: [{ priority: null, est_hours: 30 }, { priority: '∅', est_hours: 40 }] }, + { dimensions: [], rows: [{ est_hours: 50 }] }, + ], + }, + }); + render( + , + ); + await waitFor(() => expect(screen.getByTestId('dataset-matrix')).toBeInTheDocument()); + + // Four distinct (row, column) combinations → four cells, none overwritten. + // Display layer untouched: null still renders as the em dash + // `formatDimensionValue` has always produced, the literal character as + // itself — the placeholders only ever entered the ids. + const matrix = screen.getByTestId('dataset-matrix'); + const bodyRows = [...matrix.querySelectorAll('tbody tr')]; + expect(bodyRows.slice(0, 2).map((tr) => [...tr.querySelectorAll('td')].map((td) => td.textContent))).toEqual([ + ['—', '1', '2', '10'], + ['∅', '3', '4', '20'], + ]); + // Column headers split the same way, and each column subtotal lands under + // ITS OWN header — the card's radius: header ids and `colTotalById` keys are + // built by the same expression and must change together. + expect(screen.getByRole('columnheader', { name: '—' })).toBeInTheDocument(); + expect(screen.getByRole('columnheader', { name: '∅' })).toBeInTheDocument(); + const totalRow = screen.getByTestId('matrix-total-row'); + expect([...totalRow.querySelectorAll('td')].map((td) => td.textContent)).toEqual(['Total', '30', '40', '50']); + }); + it('matrix degrades gracefully when the server returns no totals (older server)', async () => { const src = makeSource({ task_metrics: [