pt/pf_app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Let clone read reference rows — copying them out is the point

Browse Source 
A selection of five AOP rows cloned nothing, with no explanation. The cause
was exclude_iters, which clone applied along with scale and recode.

It should not. That exclusion exists to stop operations *modifying*
reference rows: scale distributes an increment across its pool, so including
reference would attribute forecast movement to prior-year rows, and recode
writes negative rows that zero the original out. Clone does neither -- it
reads rows and inserts new pf_iter = 'clone' rows, leaving the source
untouched. Copying a plan or a prior year out of reference and into
adjustments is the operation doing exactly what it is for.

So from_logid stops being the only way to reach those rows and becomes what
it should be: a narrowing, for a selection spanning AOP and Prior Year where
only one is wanted.

The "would just duplicate the rows" guard no longer fires when the selection
is entirely non-adjustable, since moving rows from reference into adjustments
changes what they are even at factor 1 with no shift.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-17 02:33:28 -04:00
parent 13e49c14b6
commit 8f96fa3f7f
2 changed files with 21 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
routes/operations.js
View File

@ -749,25 +749,27 @@ module.exports = function(pool) {
}); });
} }
// Cloning from a named segment is how a period with no baseline gets a // exclude_iters deliberately does not apply here. It exists to stop
// shape: pick the ledger line -- prior year, plan -- and copy its mix // operations *modifying* reference rows: scale would attribute forecast
// forward. That means reading rows exclude_iters normally keeps // movement to prior-year rows by distributing across them, and recode
// operations away from, so the entry is named explicitly and replaces // writes negative rows that zero the original out. Clone does neither --
// the exclusion rather than widening it. The rows written are ordinary // it reads rows and writes new pf_iter = 'clone' rows, leaving the source
// clone rows either way, so they are adjustable afterwards. // untouched. Copying a plan or a prior year out of reference and into
let excludeClause; // adjustments is the operation working as intended, and excluding them
// meant a visible, deliberate selection silently produced nothing.
//
// from_logid narrows instead: a selection spanning AOP and Prior Year
// where only one is wanted.
let excludeClause = '';
if (from_logid != null) { if (from_logid != null) {
const srcLog = await pool.query( const srcLog = await pool.query(
`SELECT id, operation, coalesce(nullif(tag, ''), note) AS label `SELECT id FROM pf.log WHERE id = $1 AND version_id = $2`,
FROM pf.log WHERE id = $1 AND version_id = $2`,
[parseInt(from_logid), ctx.version.id] [parseInt(from_logid), ctx.version.id]
); );
if (!srcLog.rows.length) { if (!srcLog.rows.length) {
return res.status(400).json({ error: `No log entry ${from_logid} on this version` }); return res.status(400).json({ error: `No log entry ${from_logid} on this version` });
} }
excludeClause = `AND pf_logid = ${parseInt(from_logid)}`; excludeClause = `AND pf_logid = ${parseInt(from_logid)}`;
} else {
excludeClause = buildExcludeClause(ctx.version.exclude_iters);
} }
// Period dimensions come from the calendar against the shifted date, not // Period dimensions come from the calendar against the shifted date, not

9
ui/src/views/Forecast.jsx
View File

@ -1067,11 +1067,18 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
flash('Enter at least one new dimension value', 'error') flash('Enter at least one new dimension value', 'error')
return return
} }
// A clone that changes nothing is only pointless when the source and the
// destination are the same band. Copying a plan or a prior year out of
// reference and into adjustments changes what the rows *are*, even at factor 1
// with no shift -- that is the whole operation. Only warn when the selection
// is already adjustable.
if (op === 'clone') { if (op === 'clone') {
const shifts = body.date_offset && body.date_offset !== '0 days' const shifts = body.date_offset && body.date_offset !== '0 days'
const scales = body.scale != null && body.scale !== 1 const scales = body.scale != null && body.scale !== 1
const changes = Object.keys(body.set || {}).length > 0 const changes = Object.keys(body.set || {}).length > 0
if (!shifts && !scales && !changes) { const fromRef = !!body.from_logid ||
(currentTotals?.excluded?.rows > 0 && !(currentTotals?.total?.value || currentTotals?.total?.units))
if (!shifts && !scales && !changes && !fromRef) {
flash('A clone with no override, no date shift and a factor of 1 would just duplicate the rows', 'error') flash('A clone with no override, no date shift and a factor of 1 would just duplicate the rows', 'error')
return return
} }