Let a recode shift its dates, as clone does

date_offset moves the put-back leg of a recode while the negative leg
stays put, so a shifted recode is a timing move (pull forward, push
out); with a shift, set may be empty. Period dimensions on the moved
rows are re-derived from pf.dim_period against the shifted date, except
where the date is null, where the existing label is kept. No shift, no
derivation: a plain recode is unchanged.

The Recode form gets the same "Shift dates" field as Clone, blank by
default and cleared after each post.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-24 23:03:03 -04:00
parent 2337951b65
commit 94af6a5f13
5 changed files with 76 additions and 13 deletions

View File

@ -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 - **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 - **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. `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.
--- ---

View File

@ -447,11 +447,14 @@ ilog AS (
FROM src FROM src
RETURNING * 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 ( ,ins AS (
INSERT INTO {{fc_table}} (${insertCols}) 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() 'recode', (SELECT id FROM ilog), '{{pf_user}}', now()
FROM src FROM src s${hasDimPeriod ? dimPeriodJoins(dateGroups) : ''}
RETURNING * RETURNING *
) )
${grain ? `,allrows AS ( ${grain ? `,allrows AS (

View File

@ -906,12 +906,22 @@ module.exports = function(pool) {
// recode dimension values on one or more slices // recode dimension values on one or more slices
// inserts negative rows to zero out the original, positive rows with new dimension values // 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) => { 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 pf_user = sessionUser(req);
const slices = normalizeSlices(req.body); const slices = normalizeSlices(req.body);
if (slices.length === 0) return res.status(400).json({ error: 'slice is required' }); 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 { try {
const ctx = await getContext(parseInt(req.params.id), 'recode'); const ctx = await getContext(parseInt(req.params.id), 'recode');
@ -920,7 +930,20 @@ module.exports = function(pool) {
if (!assertMayRecodeTerritory(req, ctx, set, res)) return; if (!assertMayRecodeTerritory(req, ctx, set, res)) return;
const excludeClause = buildExcludeClause(ctx.version.exclude_iters); 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 units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope, req);
const names = adjustmentNames(ctx.version); const names = adjustmentNames(ctx.version);
@ -940,11 +963,12 @@ module.exports = function(pool) {
note: esc(note || ''), note: esc(note || ''),
label: esc(names.pf_segment), label: esc(names.pf_segment),
bucket: esc(names.pf_bucket), 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)), slice: esc(JSON.stringify(loggedSlice)),
where_clause: unit.where, where_clause: unit.where,
exclude_clause: excludeClause, exclude_clause: excludeClause,
set_clause: setClause set_clause: setClause,
date_offset: esc(dateOffset)
}); });
const result = await runSQL(sql, client); const result = await runSQL(sql, client);
await tagLog(client, result.rows, req.body.tag); await tagLog(client, result.rows, req.body.tag);

View File

@ -1380,6 +1380,7 @@ export default function OperationPanel({
scaleNote, setScaleNote, scaleNote, setScaleNote,
recodeSet, setRecodeSet, recodeSet, setRecodeSet,
recodeNote, setRecodeNote, recodeNote, setRecodeNote,
recodeOffset, setRecodeOffset,
cloneSet, setCloneSet, cloneSet, setCloneSet,
cloneScale, setCloneScale, cloneScale, setCloneScale,
cloneFrom, setCloneFrom, cloneOffset, setCloneOffset, cloneSources, cloneFrom, setCloneFrom, cloneOffset, setCloneOffset, cloneSources,
@ -1392,7 +1393,8 @@ export default function OperationPanel({
const horizontal = dock === 'bottom' const horizontal = dock === 'bottom'
const note = activeOp === 'scale' ? scaleNote : activeOp === 'recode' ? recodeNote : cloneNote 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 const setNote = activeOp === 'scale' ? setScaleNote : activeOp === 'recode' ? setRecodeNote : setCloneNote
// Slices are confirmation, not the work. The list used to hold a whole column // Slices are confirmation, not the work. The list used to hold a whole column
@ -1439,7 +1441,26 @@ export default function OperationPanel({
<DimForm dimCols={dimCols} setObj={recodeSet} setSet={setRecodeSet} <DimForm dimCols={dimCols} setObj={recodeSet} setSet={setRecodeSet}
slices={slices} lookupDerivedCols={lookupDerivedCols} dimMembers={dimMembers} versionId={versionId} slices={slices} lookupDerivedCols={lookupDerivedCols} dimMembers={dimMembers} versionId={versionId}
extra={ extra={
<div className="pt-1 border-t border-gray-100 mt-1"> <div className="flex flex-col gap-2 pt-1 border-t border-gray-100 mt-1">
{/* 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. */}
<Field label="Shift dates"
hint={isShift(recodeOffset)
? 'moves the rows — select the period they leave'
: undefined}>
<input value={recodeOffset} list="pf-recode-offsets"
onChange={e => setRecodeOffset(e.target.value)}
placeholder="0 days" className={`${TEXT} w-28`} />
<datalist id="pf-recode-offsets">
<option value="-1 month" />
<option value="1 month" />
<option value="-3 months" />
<option value="3 months" />
<option value="12 months" />
</datalist>
</Field>
<MovingTotal currentTotals={currentTotals} verb="Moving" /> <MovingTotal currentTotals={currentTotals} verb="Moving" />
</div> </div>
} /> } />

View File

@ -164,6 +164,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
const [scaleNote, setScaleNote] = useState('') const [scaleNote, setScaleNote] = useState('')
const [recodeSet, setRecodeSet] = useState({}) const [recodeSet, setRecodeSet] = useState({})
const [recodeNote, setRecodeNote] = 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 [cloneSet, setCloneSet] = useState({})
const [cloneScale, setCloneScale] = useState('1') const [cloneScale, setCloneScale] = useState('1')
const [cloneNote, setCloneNote] = useState('') const [cloneNote, setCloneNote] = useState('')
@ -1731,13 +1734,13 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
.some(k => body[k] !== undefined) .some(k => body[k] !== undefined)
if (!has) { flash('Enter a target or increment', 'error'); return } 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 // 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 // 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* -- // no dimension override at all. It only needs to be doing *something* --
// an override, a shift, or a factor. // an override, a shift, or a factor.
if (op === 'recode' && !Object.keys(body.set || {}).length) { if (op === 'recode' && !Object.keys(body.set || {}).length && !body.date_offset) {
flash('Enter at least one new dimension value', 'error') flash('Enter at least one new dimension value, or a date shift', 'error')
return return
} }
// A clone that changes nothing is only pointless when the source and the // 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. // The rows just moved under the highlight — see relocateAreas.
relocateAreas() relocateAreas()
if (op === 'scale') { setScaleInputs({}); setScaleNote('') } if (op === 'scale') { setScaleInputs({}); setScaleNote('') }
if (op === 'recode') { setRecodeNote('') } if (op === 'recode') { setRecodeNote(''); setRecodeOffset('') }
if (op === 'clone') { setCloneNote(''); setCloneScale('1') } if (op === 'clone') { setCloneNote(''); setCloneScale('1') }
} catch (err) { } catch (err) {
flash(err.message, 'error') flash(err.message, 'error')
@ -1943,6 +1946,8 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
} else if (op === 'recode') { } else if (op === 'recode') {
const set = Object.fromEntries(Object.entries(recodeSet).filter(([, v]) => v.trim())) const set = Object.fromEntries(Object.entries(recodeSet).filter(([, v]) => v.trim()))
body = { ...body, note: recodeNote || undefined, set } body = { ...body, note: recodeNote || undefined, set }
const off = recodeOffset.trim()
if (off && off !== '0 days') body.date_offset = off
} else if (op === 'clone') { } else if (op === 'clone') {
const set = Object.fromEntries(Object.entries(cloneSet).filter(([, v]) => v.trim())) const set = Object.fromEntries(Object.entries(cloneSet).filter(([, v]) => v.trim()))
body = { ...body, note: cloneNote || undefined, set, scale: parseFloat(cloneScale) || 1 } body = { ...body, note: cloneNote || undefined, set, scale: parseFloat(cloneScale) || 1 }
@ -2070,6 +2075,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
scaleNote, setScaleNote, scaleNote, setScaleNote,
recodeSet, setRecodeSet, recodeSet, setRecodeSet,
recodeNote, setRecodeNote, recodeNote, setRecodeNote,
recodeOffset, setRecodeOffset,
cloneSet, setCloneSet, cloneSet, setCloneSet,
cloneScale, setCloneScale, cloneScale, setCloneScale,
cloneFrom, setCloneFrom, cloneOffset, setCloneOffset, cloneSources, cloneFrom, setCloneFrom, cloneOffset, setCloneOffset, cloneSources,