From 0c9746d3bec5450d476d3ad71b108b801810c47a Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 28 Sep 2026 21:05:51 -0400 Subject: [PATCH] Let a total cell be selected inside a filtered pivot A grand-total cell is its displayed amount like any other: under a column it is that column's values, and inside a filtered pivot it is what the filter leaves. Its slice can therefore be {}, which is accepted only alongside a non-empty scope -- without one it would be the whole version, and is still refused on both sides. buildWhereAny reads a union containing {} as TRUE, the scope ANDed on afterwards. The panel names it "Total". Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- lib/sql_generator.js | 7 +++++-- routes/operations.js | 21 +++++++++++++------- ui/src/components/OperationPanel.jsx | 6 +++++- ui/src/views/Forecast.jsx | 29 ++++++++++++++++------------ 5 files changed, 42 insertions(+), 23 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 94f14e9..593c638 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -676,7 +676,7 @@ When the user clicks a pivot cell, `perspective-click` fires. The handler in `Fo Dragging across a block of cells selects a region. The datagrid runs in `edit_mode: SELECT_REGION` (forced on restore, so a saved layout can't switch it off) and reports the region as a `perspective-select` event carrying a Perspective **ViewWindow** — `{ start_row, end_row, start_col, end_col }`, *not* the per-row `insertConfigs` payload an older API used. It fires on every mouseover as the region grows, so the handler only records the latest window and a window-level `mouseup` commits it. A single-cell region is ignored there: `perspective-click` already owns plain and modifier clicks, and handling it in both places would undo a ctrl-click toggle. -Turning a region back into slices re-derives, per cell, the same filters Perspective attaches to a click — row dimensions from the view's `__ROW_PATH__` (raw values, so dates stay epoch millis rather than whatever the grid formatted them as), column dimensions from the split_by segments of the column name. The grand-total row resolves to no dimension at all and is skipped; that would mean "the whole version". +Turning a region back into slices re-derives, per cell, the same filters Perspective attaches to a click — row dimensions from the view's `__ROW_PATH__` (raw values, so dates stay epoch millis rather than whatever the grid formatted them as), column dimensions from the split_by segments of the column name. **A total cell is its displayed amount.** The grand-total row resolves to no row dimension, so its slice is whatever the column axis and the pivot's `==` filters leave — possibly `{}`, meaning *everything the pivot's filter shows*. An empty slice is accepted only alongside a non-empty `scope` (`normalizeSlices()` / `assertSelective()` in `routes/operations.js`); with no filter at all it would be the whole version, and is refused on both sides. `buildWhereAny()` reads a union containing `{}` as `TRUE`, the scope then ANDed on — dropping it would narrow the write to the other cells. **Selection highlight.** The datagrid highlights whatever sits in its own `model._selection_state.selected_areas`, and wipes that list on every mousedown — so a multi-slice selection built up over several ctrl-clicks would only ever show the last cell. `Forecast.jsx` keeps `areasRef`, a `sliceKey -> rectangles` map parallel to `slices`, and an effect pushes the full set back and redraws after every change. Deselecting anywhere (ctrl-click, the panel's ×, Clear selection) prunes the map by live slice key, so the grid and the panel can't disagree. diff --git a/lib/sql_generator.js b/lib/sql_generator.js index b993705..e985ec7 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -727,8 +727,11 @@ function buildTerritoryClause(territory, territoryCol, alias = '') { // {Region:East, State:NY} and {Region:West, State:CA} would become // Region IN (East,West) AND State IN (NY,CA), which also matches East/CA. function buildWhereAny(slices, dimCols, versionId) { - const list = (slices || []).filter(s => s && Object.keys(s).length > 0); - if (list.length === 0) return 'TRUE'; + const list = (slices || []).filter(Boolean); + // An empty slice is a total cell, and the union with a total is the total. + // Dropping it instead would narrow the write to the other cells. The caller + // only lets one through under a scope, which is ANDed on afterwards. + if (list.length === 0 || list.some(s => Object.keys(s).length === 0)) return 'TRUE'; if (list.length === 1) return buildWhere(list[0], dimCols, versionId); const groups = list diff --git a/routes/operations.js b/routes/operations.js index 1c11dea..b25bfe9 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -42,11 +42,14 @@ module.exports = function(pool) { return (client || pool).query(sql); } - // accept either the legacy single `slice` object or the newer `slices` array, - // and drop any empty entries so an empty selection can never widen to TRUE + // accept either the legacy single `slice` object or the newer `slices` array. + // An empty slice is a total cell -- everything the pivot's filter shows -- so + // it is kept only when a scope came with it; without one it would be the + // whole version, and is dropped so an empty selection can never widen to TRUE function normalizeSlices(body) { const raw = Array.isArray(body.slices) && body.slices.length ? body.slices : [body.slice]; - return raw.filter(s => s && typeof s === 'object' && Object.keys(s).length > 0); + const scoped = Array.isArray(body.scope) && body.scope.length > 0; + return raw.filter(s => s && typeof s === 'object' && (scoped || Object.keys(s).length > 0)); } // Stamp the tag onto the log entry the operation just created. @@ -127,9 +130,13 @@ module.exports = function(pool) { // column. buildWhere silently drops unknown keys, so {"typo": "x"} would // otherwise reduce to TRUE and apply the operation to the whole version. // Refuse rather than let a malformed selection rewrite every row. - function assertSelective(slices, ctx) { + // An empty slice passes only under a scope (see normalizeSlices), which + // buildScopeClause refuses to let reduce to nothing. + function assertSelective(slices, ctx, scope) { const allowed = new Set([...ctx.filterCols, ...Object.keys(COMPUTED_SLICE_COLS)]); + const scoped = Array.isArray(scope) && scope.length > 0; slices.forEach((sl, i) => { + if (scoped && Object.keys(sl).length === 0) return; const hits = Object.keys(sl).filter(k => allowed.has(k)); if (hits.length === 0) { const err = new Error( @@ -828,7 +835,7 @@ module.exports = function(pool) { try { const ctx = await getContext(parseInt(req.params.id), 'scale'); if (!guardOpen(ctx.version, res)) return; - assertSelective(slices, ctx); + assertSelective(slices, ctx, req.body.scope); const excludeClause = buildExcludeClause(ctx.version.exclude_iters); @@ -939,7 +946,7 @@ module.exports = function(pool) { try { const ctx = await getContext(parseInt(req.params.id), 'recode'); if (!guardOpen(ctx.version, res)) return; - assertSelective(slices, ctx); + assertSelective(slices, ctx, req.body.scope); if (!assertMayRecodeTerritory(req, ctx, set, res)) return; const excludeClause = buildExcludeClause(ctx.version.exclude_iters); @@ -1033,7 +1040,7 @@ module.exports = function(pool) { try { const ctx = await getContext(parseInt(req.params.id), 'clone'); if (!guardOpen(ctx.version, res)) return; - assertSelective(slices, ctx); + assertSelective(slices, ctx, req.body.scope); const scaleFactor = (scale != null) ? parseFloat(scale) : 1.0; const dateOffset = (date_offset || '0 days').trim() || '0 days'; diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 8353e15..77d4c98 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -39,7 +39,7 @@ function dispVal(v) { function sliceLabel(s) { const entries = Object.entries(s) - return entries.length ? entries.map(([k, v]) => `${k}=${dispVal(v)}`).join(' · ') : '—' + return entries.length ? entries.map(([k, v]) => `${k}=${dispVal(v)}`).join(' · ') : 'Total' } // A light grouping with an optional caption — a rule between blocks in the @@ -275,6 +275,10 @@ function SelectionList({ slices, viewScope = [], distinctSlices, currentTotals, ? {sliceLabel(s)} : (
+ {/* a total cell: everything the pivot's filter shows */} + {!Object.keys(s).length && ( + Total{scopeLine ? ' within the filter' : ''} + )} {Object.entries(s).map(([k, v]) => (
{k} diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 45475f7..dfbd0f7 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -397,7 +397,6 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio if (!viewer) return [] const cfg = await viewer.save() const view = await viewer.getView() - const groupBy = cfg.group_by || [] const base = cfg.filter || [] const rows = await view.to_json({ start_row: win.start_row, end_row: win.end_row }) if (!rows.length) return [] @@ -406,18 +405,15 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const c1 = win.end_col ?? userKeys.length const out = [] for (let i = 0; i < rows.length; i++) { - const path = rows[i].__ROW_PATH__ || [] - // the grand-total row resolves to no dimension at all, which would mean - // "the whole version" — never what dragging over it is asking for - if (groupBy.length && !path.length) continue for (let c = c0; c < c1; c++) { const key = userKeys[c] if (!key) continue const { rowFilters, colFilters } = cellFilters(cfg, rows[i], key) - if (groupBy.length && !rowFilters.length) continue const slice = sliceFromFilters([...base, ...rowFilters, ...colFilters], (cfg.columns || []).filter(Boolean)) - if (!Object.keys(slice).length) continue + // a total cell counts when its column or the pivot's filter narrows it; + // one nothing narrows would mean the whole version (see perspective-click) + if (!Object.keys(slice).length && !base.length) continue const y = win.start_row + i out.push({ slice, area: { x0: c, x1: c, y0: y, y1: y } }) } @@ -1119,12 +1115,16 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio let s if (cell) { const { rowFilters, colFilters } = cellFilters(config, detail.row, colKey) - if (!rowFilters.length) return s = sliceFromFilters([...(config.filter || []), ...rowFilters, ...colFilters], measures) } else { s = sliceFromFilters((detail.config || {}).filter || [], measures) } - if (!Object.keys(s).length) return + // A total cell is its displayed amount like any other: the grand-total + // row under a column is that column's values, and inside a filtered + // pivot it is what the filter leaves -- which travels as `scope`, not in + // the slice. Only a cell nothing narrows at all is refused, since that + // would be the whole version. + if (!Object.keys(s).length && !(config.filter || []).length) return // the CustomEvent carries no modifier flags, so read them off the // mousedown that produced it (captured on window below) const additive = modifierRef.current @@ -1803,8 +1803,10 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // equality: a view filtered to sseas_e <= 2027 has no slice form at all. const body = buildPayload(op) if (!body) return - if (body.slices.some(sl => !Object.keys(sl).length)) { - flash('No dimension or date columns in slice — check col_meta', 'error'); return + // an empty slice is a total cell, and means "everything the pivot's filter + // shows" -- so it needs that filter, or it would be the whole version + if (!body.scope && body.slices.some(sl => !Object.keys(sl).length)) { + flash('That total covers the whole forecast — filter the pivot or pick a narrower cell', 'error'); return } if (op === 'scale') { const has = ['target_value','target_units','target_price', @@ -2761,7 +2763,10 @@ function fmtSliceLines(slice) { const one = (sl) => Object.entries(sl) .map(([k, v]) => `${k} = ${fmtSliceValue(v)}`) .join(' · ') - if (!Array.isArray(slice)) return Object.keys(slice).length ? [one(slice)] : [] + // an empty slice is a total cell: everything the pivot's filter showed + const TOTAL = 'Total within the pivot filter' + if (!Array.isArray(slice)) return [Object.keys(slice).length ? one(slice) : TOTAL] + if (slice.some(sl => sl && !Object.keys(sl).length)) return [TOTAL] const list = slice.filter(sl => sl && Object.keys(sl).length) if (list.length <= 1) return list.map(one)