Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/olive-poets-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@object-ui/plugin-report': patch
'@object-ui/plugin-dashboard': patch
'@object-ui/core': patch
---

Fix matrix report cells showing another bucket's numbers when dimension values run together.

The cross-tab in `DatasetReportRenderer` built its bucket ids by joining dimension values with the EMPTY string, so adjacent values had no boundary at all: `"x"` + `"yz"` and `"xy"` + `"z"` were the same bucket on both axes, and the later row silently overwrote the earlier one. Its cell key then joined the two bucket ids with a plain space, while dimension values contain spaces constantly ("New York", "In Progress"), so `"New"` × `"York Q1"` and `"New York"` × `"Q1"` also met in one key. A merged bucket showed a different row's measure, the overwritten row's value was unreachable, the per-row and per-column subtotals matched the wrong header, and drill-through followed the same wrong index into another record's list — none of it with an error.

Bucket ids and cell keys are now encoded with `JSON.stringify`, which carries the boundary in its own quoting rather than in a character the data is assumed never to contain. All four lookups in the renderer (row headers, column headers, row subtotals, column subtotals) share the one encoder, so they agree by construction.

The encoders moved to `@object-ui/core` as `pivotBucketId` / `pivotCellKey` and are now shared with the dashboard `DatasetWidget`, which carried the same defect and fixed it separately: two packages each hand-rolling the same key is why one fix left the other broken. The dashboard keeps its existing exports and behaviour.
4 changes: 4 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ export * from './utils/merge-filters.js';
export * from './utils/compare-to.js';
export * from './utils/chart-series.js';
export * from './utils/dataset-format.js';
// Pivot lookup-key encoders, shared by every cross-tab renderer so the
// dashboard widget and the report renderer key their buckets identically
// (objectstack#5473 / objectstack#5665).
export * from './utils/dataset-pivot.js';
export * from './utils/record-title.js';
export * from './utils/export-filename.js';
export * from './utils/reference-keys.js';
Expand Down
60 changes: 60 additions & 0 deletions packages/core/src/utils/dataset-pivot.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* dataset-pivot — the lookup-key encoders every cross-tab over a semantic-layer
* `queryDataset` result shares (ADR-0021).
*
* A pivot keys three lookups off dimension-value tuples: the DOWN bucket, the
* ACROSS bucket, and the cell where the two meet. Every one of them used to be
* spelled by concatenating values with a character the data was ASSUMED never
* to contain — an empty string, a plain space, U+0001 — and each assumption
* failed on ordinary data: `"x"` + `"yz"` and `"xy"` + `"z"` are one id under an
* empty join, and `"New"` × `"York Q1"` and `"New York"` × `"Q1"` are one cell
* key under a space join. The later row silently overwrote the earlier one, so
* the cell showed a different row's measure, the overwritten row's value was
* unreachable, and drill-through followed the same wrong index into the wrong
* records — all without an error (objectstack#5473, objectstack#5665).
*
* `JSON.stringify` carries the boundary in its own quoting rather than in a
* character the values must avoid, so these are unambiguous for ANY value a
* dimension can hold. The ids are opaque lookup keys, never displayed — visible
* text comes from the separately-formatted labels.
*
* Both encoders live here, in `@object-ui/core`, because the same pivot is
* rendered by two packages (`plugin-dashboard`'s `DatasetWidget` and
* `plugin-report`'s `DatasetReportRenderer`). Each having written its own is
* exactly why both carried the defect and why fixing one left the other broken.
* The same rule applies WITHIN a renderer: the cell index and the subtotal
* lookups key the same buckets, so they must all call these — a second
* hand-rolled encoding of the same id agrees with the first only for the
* dimension counts someone happened to test, which is what kept the original
* bug invisible.
*
* 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.
*/

/**
* Encode a pivot BUCKET id from its dimension values.
*
* 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.
*/
export const pivotBucketId = (dimensionValues: string[]): string => JSON.stringify(dimensionValues);

/**
* Encode the cell key for a (down bucket, across bucket) pair — the key of the
* map a renderer reads to place a measure and to resolve a drill-through.
*/
export const pivotCellKey = (rowId: string, colId: string): string => JSON.stringify([rowId, colId]);
44 changes: 17 additions & 27 deletions packages/plugin-dashboard/src/DatasetWidget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ import {
formatDimensionValue,
buildDatasetFieldHelpers,
buildDatasetDrillFilter,
// 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,
pivotCellKey,
type DatasetResultField,
type DatasetDrillRange,
} from '@object-ui/core';
Expand All @@ -64,36 +73,17 @@ interface DatasetCapableSource {
export const buildDrillFilter = buildDatasetDrillFilter;

/**
* Encode a pivot ROW bucket id from its dimension values.
*
* `JSON.stringify` of the value array — not a delimiter character — carries the
* boundary between two values, so the id is unambiguous for ANY value a
* dimension can hold. The previous encodings both relied on a character the
* values were assumed never to contain, which is exactly how two different
* rows collapsed into one bucket (objectstack#5473; same treatment as the
* include key in objectui#3388 and the warning-dedupe key in objectstack#5450).
* The pivot key encoders, re-exported for back-compat; the implementations now
* 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.
*
* 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 this function; a second,
* hand-rolled encoding of the same id is what made the old bug invisible.
*
* Known residual, tracked separately in objectstack#5666: a null/undefined
* dimension value is encoded as the placeholder character below, so it still
* collides with a value that literally equals that placeholder.
*/
export const pivotRowId = (dimensionValues: string[]): string => JSON.stringify(dimensionValues);

/**
* Encode the `cellIndex` key for a (row bucket, column bucket) pair.
*
* Was `${rowId} ${colId}` — a plain space, while dimension values contain
* spaces all the time ("New York", "In Progress"). Two rows whose ids met at a
* different point of the same string ("New" + "York Q1" vs "New York" + "Q1")
* produced ONE key: the later row silently overwrote the earlier one, the cell
* showed another row's measure, and drill-through followed the same wrong
* index. `JSON.stringify` of the pair has no such boundary (objectstack#5473).
* 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.
*/
export const pivotCellKey = (rowId: string, colId: string): string => JSON.stringify([rowId, colId]);
export { pivotRowId, pivotCellKey };

/**
* Pivot flat dataset rows into a cross-tab: `rowDims` go DOWN, `colDim` spreads
Expand Down
24 changes: 20 additions & 4 deletions packages/plugin-report/src/DatasetReportRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ import {
formatDimensionValue,
buildDatasetFieldHelpers,
buildDatasetDrillFilter,
pivotBucketId,
pivotCellKey,
type DatasetResultField,
type DatasetDrillRange,
} from '@object-ui/core';
Expand Down Expand Up @@ -738,9 +740,19 @@ function DatasetReportChart({
);
}

/** Stable bucket id for a dimension-value tuple. */
/**
* Stable bucket id for a dimension-value tuple — the id every lookup in the
* cross-tab below is keyed by (row headers, column headers, and both subtotal
* maps), so they all agree by construction.
*
* The values go through `pivotBucketId` (shared with the dashboard's pivot)
* rather than being concatenated: this used to `join('')`, which left NO
* 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).
*/
function bucketId(dims: string[], row: Row): string {
return dims.map((d) => String(row[d] ?? '∅')).join('');
return pivotBucketId(dims.map((d) => String(row[d] ?? '∅')));
}

function bucketLabel(dims: string[], row: Row): string {
Expand Down Expand Up @@ -816,7 +828,11 @@ function DatasetMatrixTable({
for (const d of columnsAcross) key[d] = r[d];
colHeaders.push({ id: cid, label: bucketLabel(columnsAcross, r), key });
}
cells.set(`${rid} ${cid}`, { row: r, index });
// Keyed by pivotCellKey, not `${rid} ${cid}`: a plain space is a boundary
// only while no dimension value contains one, and they do constantly
// ("New York", "In Progress"). `index` is also what drill-through reads
// `drillRawRows` by, so a merged key drilled to the wrong records too.
cells.set(pivotCellKey(rid, cid), { row: r, index });
});
return { rowHeaders, colHeaders, cells };
}, [state, rows, columnsAcross]);
Expand Down Expand Up @@ -903,7 +919,7 @@ function DatasetMatrixTable({
</td>
))}
{cellCols.map((cc) => {
const entry = pivot.cells.get(`${rh.id} ${cc.col.id}`);
const entry = pivot.cells.get(pivotCellKey(rh.id, cc.col.id));
const value = entry?.row[cc.measure];
const clickable = canDrill && entry != null;
return (
Expand Down
168 changes: 168 additions & 0 deletions packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -787,3 +787,171 @@ describe('DatasetReportRenderer', () => {
expect(screen.getByText(/does not support dataset queries/i)).toBeInTheDocument();
});
});

/**
* Matrix bucket / cell-key encoding (objectstack#5665).
*
* The cross-tab keys three lookups off dimension-value tuples: the DOWN bucket,
* the ACROSS bucket, and the (row, column) cell that meets them. All three were
* spelled by concatenation — the bucket id joined its values with the EMPTY
* string, the cell key joined the two bucket ids with a PLAIN SPACE — so a
* boundary existed only where the data happened not to reach across it. Two
* different buckets then produced ONE key: the later row silently overwrote the
* earlier one, the cell showed another row's measure, the overwritten row was
* unreachable, and drill-through followed the same wrong index into the wrong
* records. Same defect as the dashboard widget's (objectstack#5473).
*
* These cases assert BEHAVIOUR — which bucket renders which measure, which raw
* record a cell drills to — not key spelling. The pre-existing pivot cases use
* one row dimension and space-free values, where an empty separator is
* indistinguishable from a correct one, which is why they stayed green
* throughout.
*/
describe('DatasetReportRenderer — matrix bucket encoding (objectstack#5665)', () => {
beforeEach(() => vi.clearAllMocks());

/** Data rows of the rendered cross-tab, as text — the totals row excluded. */
const matrixBodyRows = (): string[][] =>
[...screen.getByTestId('dataset-matrix').querySelectorAll('tbody tr')]
.filter((tr) => tr.getAttribute('data-testid') !== 'matrix-total-row')
.map((tr) => [...tr.querySelectorAll('td')].map((td) => td.textContent ?? ''));

it('keeps two DOWN buckets whose values concatenate identically apart ("x"+"yz" vs "xy"+"z")', async () => {
// The empty join gave these two rows one id, "xyz". Nothing about the values
// is exotic — any two adjacent dimensions can spell each other's boundary.
const src = makeSource({
sales: [
{ region: 'x', segment: 'yz', priority: 'High', amount: 1 },
{ region: 'xy', segment: 'z', priority: 'High', amount: 2 },
],
});
render(
<DatasetReportRenderer
report={{ name: 'm', type: 'matrix', dataset: 'sales', rows: ['region', 'segment'], columns: ['priority'], values: ['amount'] }}
dataSource={src}
/>,
);
await waitFor(() => expect(screen.getByTestId('dataset-matrix')).toBeInTheDocument());
expect(matrixBodyRows()).toEqual([
['x', 'yz', '1'],
['xy', 'z', '2'],
]);
});

it('keeps two ACROSS buckets whose values concatenate identically apart', async () => {
// The across axis runs through the same encoder, so it collides the same
// way: "Q"+"1x" and "Q1"+"x" both spelled "Q1x", collapsing two columns into
// one and making the first bucket's measure unreachable.
const src = makeSource({
sales: {
rows: [
{ region: 'East', quarter: 'Q', channel: '1x', amount: 1 },
{ region: 'East', quarter: 'Q1', channel: 'x', amount: 2 },
],
totals: [
{ dimensions: ['quarter', 'channel'], rows: [{ quarter: 'Q', channel: '1x', amount: 10 }, { quarter: 'Q1', channel: 'x', amount: 20 }] },
],
},
});
render(
<DatasetReportRenderer
report={{ name: 'm', type: 'matrix', dataset: 'sales', rows: ['region'], columns: ['quarter', 'channel'], values: ['amount'] }}
dataSource={src}
/>,
);
await waitFor(() => expect(screen.getByTestId('dataset-matrix')).toBeInTheDocument());
expect(screen.getByRole('columnheader', { name: 'Q / 1x' })).toBeInTheDocument();
expect(screen.getByRole('columnheader', { name: 'Q1 / x' })).toBeInTheDocument();
expect(matrixBodyRows()).toEqual([['East', '1', '2']]);
// Column subtotals key the across buckets with the SAME encoder, so they
// must land under their own column rather than share one.
expect(screen.getByTestId('matrix-total-row').textContent).toContain('10');
expect(screen.getByTestId('matrix-total-row').textContent).toContain('20');
});

it('does not merge cells when a dimension value contains a space ("New" × "York Q1" vs "New York" × "Q1")', async () => {
// The space-joined cell key spelled both pairs "New York Q1" — and values
// with spaces are the norm, not the exception ("New York", "In Progress").
const src = makeSource({
sales: [
{ region: 'New', quarter: 'York Q1', amount: 111 },
{ region: 'New York', quarter: 'Q1', amount: 222 },
],
});
render(
<DatasetReportRenderer
report={{ name: 'm', type: 'matrix', dataset: 'sales', rows: ['region'], columns: ['quarter'], values: ['amount'] }}
dataSource={src}
/>,
);
await waitFor(() => expect(screen.getByTestId('dataset-matrix')).toBeInTheDocument());
// Each bucket pair holds its own measure; the two absent pairs render '—'.
expect(matrixBodyRows()).toEqual([
['New', '111', '—'],
['New York', '—', '222'],
]);
});

it('drills a colliding cell to ITS raw record, not the one that overwrote it', async () => {
// The cell entry carries the flat row INDEX that drill-through reads
// `drillRawRows` by, so a merged key drilled to another row's records with
// no error — the quiet half of the same bug.
const src = makeSource({
sales: {
rows: [
{ region: 'New', quarter: 'York Q1', amount: 111 },
{ region: 'New York', quarter: 'Q1', amount: 222 },
],
object: 'opportunity',
dimensionFields: { region: 'billing_state', quarter: 'close_quarter' },
drillRawRows: [
{ region: 'NEW', quarter: 'YORK-Q1' },
{ region: 'NEW-YORK', quarter: 'Q1' },
],
},
});
const onDrill = vi.fn();
render(
<DatasetReportRenderer
report={{ name: 'm', type: 'matrix', dataset: 'sales', rows: ['region'], columns: ['quarter'], values: ['amount'] }}
dataSource={src}
onDrill={onDrill}
/>,
);
await waitFor(() => expect(screen.getByTestId('dataset-matrix')).toBeInTheDocument());
// First clickable cell = row "New" × column "York Q1" (flat row 0).
fireEvent.click(screen.getAllByTestId('dataset-drill-cell')[0]);
expect(onDrill).toHaveBeenCalledWith(expect.objectContaining({
groupKey: { region: 'New', quarter: 'York Q1' },
objectFilter: { billing_state: 'NEW', close_quarter: 'YORK-Q1' },
}));
});

it('matches per-row subtotals to multi-dimension row buckets', async () => {
// `rowTotalById` keys the server's subtotal rows with the same encoder as
// the row headers, so the collision reached the Total column too: two
// subtotals under one id, one of them unreachable.
const src = makeSource({
sales: {
rows: [
{ region: 'x', segment: 'yz', priority: 'High', amount: 1 },
{ region: 'xy', segment: 'z', priority: 'High', amount: 2 },
],
totals: [
{ dimensions: ['region', 'segment'], rows: [{ region: 'x', segment: 'yz', amount: 1 }, { region: 'xy', segment: 'z', amount: 2 }] },
{ dimensions: ['priority'], rows: [{ priority: 'High', amount: 3 }] },
{ dimensions: [], rows: [{ amount: 3 }] },
],
},
});
render(
<DatasetReportRenderer
report={{ name: 'm', type: 'matrix', dataset: 'sales', rows: ['region', 'segment'], columns: ['priority'], values: ['amount'] }}
dataSource={src}
/>,
);
await waitFor(() => expect(screen.getByTestId('dataset-matrix')).toBeInTheDocument());
expect(screen.getAllByTestId('matrix-row-total').map((el) => el.textContent)).toEqual(['1', '2']);
expect(screen.getByTestId('matrix-grand-total')).toHaveTextContent('3');
});
});
Loading