diff --git a/CLAUDE.md b/CLAUDE.md index 839fcc3..26d0c6f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -755,6 +755,15 @@ near-unique. - **Recode** — inserts negative rows (zero out original) + positive rows with `{{set_clause}}` dimension overrides; both share the same logid - **Clone** — copies the slice with `{{set_clause}}` overrides and `{{scale_factor}}` multiplier; original untouched +**Recode takes a date shift too.** `date_offset` moves the *put-back* leg only — +the negative rows stay where the business was — so a shifted recode is a timing +move (pull forward, push out), and with a shift `set` may be empty. The moved +rows' period dimensions are re-derived from `pf.dim_period` against the shifted +date, as clone's are; a row whose date is null keeps the label it had, since a +calendar lookup on nothing would blank it (v34 has 1,033 baseline rows with no +`sdate_e`). Without a shift nothing is derived and recode copies as it always +did. Clone still blanks those labels on a null date; it has not been changed. + `build_where()` validates every slice key against col_meta (only `role = dimension` allowed). Values are escaped but not parameterized — consistent with existing patterns, debuggable in pg logs. --- diff --git a/lib/sql_generator.js b/lib/sql_generator.js index 39e812f..40de10c 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -447,11 +447,14 @@ ilog AS ( FROM src RETURNING * ) +-- the put-back leg, moved by {{date_offset}} ('0 days' for a plain recode). The +-- route resolves {{set_clause}} against alias s, deriving the period dimensions +-- from the dim_period joins below only when the offset is non-zero. ,ins AS ( INSERT INTO {{fc_table}} (${insertCols}) - SELECT {{set_clause}}, ${datesJoined}, ${effectiveValue ? q(effectiveValue) : '0'}${effectiveUnits ? `, ${q(effectiveUnits)}` : ''}, + SELECT {{set_clause}}, ${dateCols.map(c => `(s.${q(c)} + '{{date_offset}}'::interval)::date`).join(', ')}, ${effectiveValue ? `s.${q(effectiveValue)}` : '0'}${effectiveUnits ? `, s.${q(effectiveUnits)}` : ''}, 'recode', (SELECT id FROM ilog), '{{pf_user}}', now() - FROM src + FROM src s${hasDimPeriod ? dimPeriodJoins(dateGroups) : ''} RETURNING * ) ${grain ? `,allrows AS ( diff --git a/routes/operations.js b/routes/operations.js index 826da16..74223b9 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -906,12 +906,22 @@ module.exports = function(pool) { // recode dimension values on one or more slices // inserts negative rows to zero out the original, positive rows with new dimension values + // + // date_offset moves the positive rows in time as well -- the negative leg stays + // where the rows were, so the pair is a timing move (pull forward, push out) + // rather than a copy. With an offset, set may be empty: moving a slice without + // renaming anything is the whole request. router.post('/versions/:id/recode', async (req, res) => { - const { note, set, apply_mode } = req.body; + const { note, set, apply_mode, date_offset } = req.body; const pf_user = sessionUser(req); const slices = normalizeSlices(req.body); if (slices.length === 0) return res.status(400).json({ error: 'slice is required' }); - if (!set || Object.keys(set).length === 0) return res.status(400).json({ error: 'set is required' }); + const dateOffset = String(date_offset ?? '').trim() || '0 days'; + if (!await assertInterval(dateOffset, res)) return; + const shifted = (await pool.query(`SELECT $1::interval <> '0'::interval AS s`, [dateOffset])).rows[0].s; + if ((!set || Object.keys(set).length === 0) && !shifted) { + return res.status(400).json({ error: 'Recode needs a new value or a date offset' }); + } try { const ctx = await getContext(parseInt(req.params.id), 'recode'); @@ -920,7 +930,20 @@ module.exports = function(pool) { if (!assertMayRecodeTerritory(req, ctx, set, res)) return; const excludeClause = buildExcludeClause(ctx.version.exclude_iters); - const setClause = buildSetClause(ctx.dimCols, set); + + // Moved rows take their period dimensions from the calendar against the + // shifted date, as clone does; an explicit set still wins. A row whose + // date is null has nothing to shift and keeps the label it had, rather + // than having it blanked by a calendar lookup that found nothing. Not + // derived at all without an offset, so a plain recode copies them as + // it always has. + const dateGroups = dateGroupsOf(ctx.colMeta); + const dateColOf = Object.fromEntries(dateGroups.map(g => [g.alias, g.dateCol])); + const derivedExprs = shifted ? Object.fromEntries( + [...dimPeriodMapOf(dateGroups)].map(([cname, { alias, periodCol }]) => + [cname, `CASE WHEN s."${dateColOf[alias]}" IS NULL THEN s."${cname}" ELSE ${alias}."${periodCol}" END`]) + ) : undefined; + const setClause = buildSetClause(ctx.dimCols, set, { derivedExprs, alias: 's' }); const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope, req); const names = adjustmentNames(ctx.version); @@ -940,11 +963,12 @@ module.exports = function(pool) { note: esc(note || ''), label: esc(names.pf_segment), bucket: esc(names.pf_bucket), - params: esc(JSON.stringify({ slices: unit.slices, set, apply_mode: unit.mode })), + params: esc(JSON.stringify({ slices: unit.slices, set, apply_mode: unit.mode, date_offset: dateOffset })), slice: esc(JSON.stringify(loggedSlice)), where_clause: unit.where, exclude_clause: excludeClause, - set_clause: setClause + set_clause: setClause, + date_offset: esc(dateOffset) }); const result = await runSQL(sql, client); await tagLog(client, result.rows, req.body.tag); diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 87c0729..b628888 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -1380,6 +1380,7 @@ export default function OperationPanel({ scaleNote, setScaleNote, recodeSet, setRecodeSet, recodeNote, setRecodeNote, + recodeOffset, setRecodeOffset, cloneSet, setCloneSet, cloneScale, setCloneScale, cloneFrom, setCloneFrom, cloneOffset, setCloneOffset, cloneSources, @@ -1392,7 +1393,8 @@ export default function OperationPanel({ const horizontal = dock === 'bottom' const note = activeOp === 'scale' ? scaleNote : activeOp === 'recode' ? recodeNote : cloneNote - const shifting = !!cloneOffset && cloneOffset.trim() !== '' && cloneOffset.trim() !== '0 days' + const isShift = off => !!off && off.trim() !== '' && off.trim() !== '0 days' + const shifting = isShift(cloneOffset) const setNote = activeOp === 'scale' ? setScaleNote : activeOp === 'recode' ? setRecodeNote : setCloneNote // Slices are confirmation, not the work. The list used to hold a whole column @@ -1439,7 +1441,26 @@ export default function OperationPanel({ +
+ {/* The shift applies to the rows put back, not the ones taken + out, so a recode with a shift is a timing move: select the + period the business is leaving and the shift lands it in the + one it is going to. Nothing else set is fine. */} + + setRecodeOffset(e.target.value)} + placeholder="0 days" className={`${TEXT} w-28`} /> + + +
} /> diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index b07fad1..e3f6ab9 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -164,6 +164,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const [scaleNote, setScaleNote] = useState('') const [recodeSet, setRecodeSet] = useState({}) const [recodeNote, setRecodeNote] = useState('') + // blank, unlike clone's: a recode is a rename first, and a remembered shift + // would quietly move the next one in time as well + const [recodeOffset, setRecodeOffset] = useState('') const [cloneSet, setCloneSet] = useState({}) const [cloneScale, setCloneScale] = useState('1') const [cloneNote, setCloneNote] = useState('') @@ -1731,13 +1734,13 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio .some(k => body[k] !== undefined) if (!has) { flash('Enter a target or increment', 'error'); return } } - // Recode with nothing set would rewrite rows as themselves. Clone would not: + // Recode with nothing set and no shift would rewrite rows as themselves. Clone would not: // copying a segment forward in time changes the dates and the period // dimensions, which is the whole point of cloning from prior year, so it needs // no dimension override at all. It only needs to be doing *something* -- // an override, a shift, or a factor. - if (op === 'recode' && !Object.keys(body.set || {}).length) { - flash('Enter at least one new dimension value', 'error') + if (op === 'recode' && !Object.keys(body.set || {}).length && !body.date_offset) { + flash('Enter at least one new dimension value, or a date shift', 'error') return } // A clone that changes nothing is only pointless when the source and the @@ -1789,7 +1792,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // The rows just moved under the highlight — see relocateAreas. relocateAreas() if (op === 'scale') { setScaleInputs({}); setScaleNote('') } - if (op === 'recode') { setRecodeNote('') } + if (op === 'recode') { setRecodeNote(''); setRecodeOffset('') } if (op === 'clone') { setCloneNote(''); setCloneScale('1') } } catch (err) { flash(err.message, 'error') @@ -1943,6 +1946,8 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio } else if (op === 'recode') { const set = Object.fromEntries(Object.entries(recodeSet).filter(([, v]) => v.trim())) body = { ...body, note: recodeNote || undefined, set } + const off = recodeOffset.trim() + if (off && off !== '0 days') body.date_offset = off } else if (op === 'clone') { const set = Object.fromEntries(Object.entries(cloneSet).filter(([, v]) => v.trim())) body = { ...body, note: cloneNote || undefined, set, scale: parseFloat(cloneScale) || 1 } @@ -2070,6 +2075,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio scaleNote, setScaleNote, recodeSet, setRecodeSet, recodeNote, setRecodeNote, + recodeOffset, setRecodeOffset, cloneSet, setCloneSet, cloneScale, setCloneScale, cloneFrom, setCloneFrom, cloneOffset, setCloneOffset, cloneSources,