Skip to content

Commit 4bef659

Browse files
committed
pivot: track row expansion by dimension values, not position
Wire the value-based expand keys through the pivot so expansion survives adding a field, sorting, and data refreshes (#9781). - PivotTable sets a hierarchical value-based getRowId (parent id + this row's value), with a reserved id for the grand-totals row. - getValuesForExpandedKey / addExpandedDataToPivot resolve a key by matching each row's own value (stored under rowDimensions[0]) instead of positional indices, so the totals-row offset is gone. - queryExpandedRowMeasureValues derives depth from the key's segment count. - getFiltersForCell / getValuesForFlatTable / getRawRowValues and the show-more and ancestor-highlight paths use the value keys. - Drop the defensive expanded={} resets on add/sort/columns/rows; stale keys are inert. Stop deserializing the legacy positional pivotExpanded proto field. - Tests updated/added for value-based resolution. Fixes #9781
1 parent 1f3a046 commit 4bef659

11 files changed

Lines changed: 241 additions & 161 deletions

web-common/src/features/dashboards/pivot/PivotTable.svelte

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818
isShowMoreRow,
1919
splitPivotChips,
2020
} from "@rilldata/web-common/features/dashboards/pivot/pivot-utils";
21+
import {
22+
buildExpandKey,
23+
childExpandKey,
24+
encodeExpandKeyValue,
25+
parentExpandKey,
26+
PIVOT_TOTALS_ROW_ID,
27+
} from "@rilldata/web-common/features/dashboards/pivot/pivot-expand-keys";
2128
import { copyToClipboard } from "@rilldata/web-common/lib/actions/copy-to-clipboard";
2229
import {
2330
createVirtualizer,
@@ -89,15 +96,32 @@
8996
export let clickSelection: PivotClickSelectionState | undefined = undefined;
9097
9198
const options: Readable<TableOptions<PivotDataRow>> = derived(
92-
[pivotDataStore, pivotState],
93-
([pivotData, state]) => {
99+
[pivotDataStore, pivotState, config],
100+
([pivotData, state, cfg]) => {
94101
let tableData = [...pivotData.data];
95102
if (pivotData.totalsRowData) {
96103
tableData = [pivotData.totalsRowData, ...pivotData.data];
97104
}
105+
const totalsRowData = pivotData.totalsRowData;
106+
const rowDims = cfg.rowDimensionNames;
98107
return {
99108
data: tableData,
100109
columns: pivotData.columnDef,
110+
// Value-based row ids: a row is keyed by the dimension values from the
111+
// root to it, not by position, so expansion survives adding a field,
112+
// sorting, and data refreshes. Kept hierarchical (parent id + this
113+
// row's value) so depth and parent-address semantics still hold.
114+
getRowId: (row, _index, parent) => {
115+
if (totalsRowData && row === totalsRowData)
116+
return PIVOT_TOTALS_ROW_ID;
117+
if (cfg.isFlat) {
118+
return buildExpandKey(rowDims.map((dim) => row[dim]));
119+
}
120+
const anchor = rowDims[0];
121+
return parent
122+
? childExpandKey(parent.id, row[anchor])
123+
: encodeExpandKeyValue(row[anchor]);
124+
},
101125
state: {
102126
expanded: state.expanded,
103127
sorting: state.sorting,
@@ -237,7 +261,7 @@
237261
if (needsDomains) {
238262
for (const row of flatRows) {
239263
// Always skip the prepended grand-totals row.
240-
if (hasTotalsRow && row.id === "0") continue;
264+
if (hasTotalsRow && row.id === PIVOT_TOTALS_ROW_ID) continue;
241265
const target = row.subRows.length > 0 ? parentValues : leafValues;
242266
for (const cell of row.getAllCells()) {
243267
const meta = cell.column.columnDef.meta;
@@ -340,21 +364,20 @@
340364
341365
if (!nextLimit) return;
342366
343-
// Check if this is the outermost dimension or a nested dimension
344-
// Outermost dimension has rowId like "0", "1", etc. (no dots)
345-
// Nested dimensions have rowId like "0.1", "0.1.2", etc.
346-
const isOutermostDimension = !rowId.includes(".");
367+
// The outermost "Show more" row sits at the top level (its parent key
368+
// is the root ""); a nested one has a real parent node whose child
369+
// limit we raise.
370+
const parentKey = parentExpandKey(rowId);
347371
348-
if (isOutermostDimension) {
372+
if (parentKey === "") {
349373
// Handle outermost dimension "Show more" click
350374
if (setPivotOutermostRowLimit) {
351375
setPivotOutermostRowLimit(nextLimit);
352376
}
353377
} else {
354378
// Handle nested dimension "Show more" click
355-
const expandIndex = rowId.split(".").slice(0, -1).join(".");
356-
if (expandIndex && setPivotRowLimitForExpanded) {
357-
setPivotRowLimitForExpanded(expandIndex, nextLimit);
379+
if (setPivotRowLimitForExpanded) {
380+
setPivotRowLimitForExpanded(parentKey, nextLimit);
358381
}
359382
}
360383
return;

web-common/src/features/dashboards/pivot/pivot-expand-keys.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ export const EXPAND_KEY_SEP = String.fromCharCode(0);
2424
// collide with a genuinely-absent deeper level. Contains no separator.
2525
const NULL_SENTINEL = "<null>";
2626

27+
// Reserved id for the prepended grand-totals row. Begins with the separator,
28+
// which no encoded top-level value ever does, so it can't collide with a real
29+
// row id.
30+
export const PIVOT_TOTALS_ROW_ID = EXPAND_KEY_SEP + "totals";
31+
2732
export function encodeExpandKeyValue(value: unknown): string {
2833
if (value === null || value === undefined) return NULL_SENTINEL;
2934
return String(value).replaceAll(EXPAND_KEY_SEP, "");
Lines changed: 100 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
import { describe, expect, it } from "vitest";
22
import { LOADING_CELL } from "@rilldata/web-common/features/dashboards/pivot/pivot-constants";
33
import { createAndExpression } from "@rilldata/web-common/features/dashboards/stores/filter-utils";
4-
import { addExpandedDataToPivot } from "./pivot-expansion";
4+
import { buildExpandKey } from "./pivot-expand-keys";
5+
import {
6+
addExpandedDataToPivot,
7+
getValuesForExpandedKey,
8+
} from "./pivot-expansion";
59
import { type PivotDataRow, type PivotDataStoreConfig } from "./types";
610

7-
function getConfig(showTotalsRow: boolean): PivotDataStoreConfig {
11+
function getConfig(
12+
showTotalsRow: boolean,
13+
rowDimensionNames = ["publisher", "campaign"],
14+
): PivotDataStoreConfig {
815
return {
916
measureNames: ["impressions"],
10-
rowDimensionNames: ["publisher", "campaign"],
17+
rowDimensionNames,
1118
colDimensionNames: [],
1219
allMeasures: [],
1320
allDimensions: [],
@@ -38,76 +45,129 @@ function getConfig(showTotalsRow: boolean): PivotDataStoreConfig {
3845
} as unknown as PivotDataStoreConfig;
3946
}
4047

41-
describe("pivot expansion", () => {
42-
it("adds expanded rows at the correct index when totals row is hidden", () => {
48+
describe("pivot expansion (value-based keys)", () => {
49+
it("fills the node matched by dimension value, regardless of totals row", () => {
50+
for (const showTotals of [false, true]) {
51+
const tableData: PivotDataRow[] = [
52+
{ publisher: "A", subRows: [{ publisher: LOADING_CELL }] },
53+
{ publisher: "B", subRows: [{ publisher: LOADING_CELL }] },
54+
];
55+
56+
addExpandedDataToPivot(
57+
getConfig(showTotals),
58+
tableData,
59+
["publisher", "campaign"],
60+
{},
61+
[
62+
{
63+
isFetching: false,
64+
expandIndex: buildExpandKey(["B"]),
65+
rowDimensionValues: ["B"],
66+
totals: [{ campaign: "campaign-1", impressions: 10 }],
67+
data: [],
68+
},
69+
],
70+
);
71+
72+
// Row "A" is untouched; row "B" (matched by value, not position) is filled.
73+
expect(tableData[0].subRows?.[0]?.publisher).toBe(LOADING_CELL);
74+
expect(tableData[1].subRows?.[0]).toMatchObject({
75+
publisher: "campaign-1",
76+
campaign: "campaign-1",
77+
impressions: 10,
78+
});
79+
}
80+
});
81+
82+
it("resolves a nested value path to the correct deep node", () => {
4383
const tableData: PivotDataRow[] = [
4484
{
4585
publisher: "A",
46-
subRows: [{ publisher: LOADING_CELL }],
47-
},
48-
{
49-
publisher: "B",
50-
subRows: [{ publisher: LOADING_CELL }],
86+
subRows: [
87+
{ publisher: "camp1", subRows: [{ publisher: LOADING_CELL }] },
88+
],
5189
},
5290
];
5391

5492
addExpandedDataToPivot(
55-
getConfig(false),
93+
getConfig(true, ["publisher", "campaign", "adgroup"]),
5694
tableData,
57-
["publisher", "campaign"],
95+
["publisher", "campaign", "adgroup"],
5896
{},
5997
[
6098
{
6199
isFetching: false,
62-
expandIndex: "1",
63-
rowDimensionValues: ["B"],
64-
totals: [{ campaign: "campaign-1", impressions: 10 }],
100+
expandIndex: buildExpandKey(["A", "camp1"]),
101+
rowDimensionValues: ["A", "camp1"],
102+
totals: [{ adgroup: "ad-1", impressions: 5 }],
65103
data: [],
66104
},
67105
],
68106
);
69107

70-
expect(tableData[0].subRows?.[0]?.publisher).toBe(LOADING_CELL);
71-
expect(tableData[1].subRows?.[0]).toMatchObject({
72-
publisher: "campaign-1",
73-
campaign: "campaign-1",
74-
impressions: 10,
108+
expect(tableData[0].subRows?.[0]?.subRows?.[0]).toMatchObject({
109+
publisher: "ad-1",
110+
adgroup: "ad-1",
111+
impressions: 5,
75112
});
76113
});
77114

78-
it("keeps the totals row offset when totals row is visible", () => {
115+
it("does nothing when the value path matches no row", () => {
79116
const tableData: PivotDataRow[] = [
80-
{
81-
publisher: "A",
82-
subRows: [{ publisher: LOADING_CELL }],
83-
},
84-
{
85-
publisher: "B",
86-
subRows: [{ publisher: LOADING_CELL }],
87-
},
117+
{ publisher: "A", subRows: [{ publisher: LOADING_CELL }] },
88118
];
89-
90119
addExpandedDataToPivot(
91-
getConfig(true),
120+
getConfig(false),
92121
tableData,
93122
["publisher", "campaign"],
94123
{},
95124
[
96125
{
97126
isFetching: false,
98-
expandIndex: "2",
99-
rowDimensionValues: ["B"],
100-
totals: [{ campaign: "campaign-1", impressions: 10 }],
127+
expandIndex: buildExpandKey(["does-not-exist"]),
128+
rowDimensionValues: ["does-not-exist"],
129+
totals: [{ campaign: "c", impressions: 1 }],
101130
data: [],
102131
},
103132
],
104133
);
105-
106134
expect(tableData[0].subRows?.[0]?.publisher).toBe(LOADING_CELL);
107-
expect(tableData[1].subRows?.[0]).toMatchObject({
108-
publisher: "campaign-1",
109-
campaign: "campaign-1",
110-
impressions: 10,
111-
});
135+
});
136+
});
137+
138+
describe("getValuesForExpandedKey", () => {
139+
const tableData: PivotDataRow[] = [
140+
{
141+
publisher: "A",
142+
subRows: [{ publisher: "camp1" }, { publisher: "camp2" }],
143+
},
144+
{ publisher: "B", subRows: [{ publisher: "camp3" }] },
145+
];
146+
147+
it("returns the actual values along the matched path", () => {
148+
expect(
149+
getValuesForExpandedKey(
150+
tableData,
151+
["publisher", "campaign"],
152+
buildExpandKey(["B"]),
153+
),
154+
).toEqual(["B"]);
155+
expect(
156+
getValuesForExpandedKey(
157+
tableData,
158+
["publisher", "campaign"],
159+
buildExpandKey(["A", "camp2"]),
160+
),
161+
).toEqual(["A", "camp2"]);
162+
});
163+
164+
it("stops at the deepest resolvable segment", () => {
165+
expect(
166+
getValuesForExpandedKey(
167+
tableData,
168+
["publisher", "campaign"],
169+
buildExpandKey(["A", "missing"]),
170+
),
171+
).toEqual(["A"]);
112172
});
113173
});

0 commit comments

Comments
 (0)