Skip to content

fix(templates): a user update must not take system-owned fields from the payload - #6689

Merged
delchev merged 1 commit into
masterfrom
fix/dao-update-preserves-system-fields-pr
Aug 13, 2026
Merged

fix(templates): a user update must not take system-owned fields from the payload#6689
delchev merged 1 commit into
masterfrom
fix/dao-update-preserves-system-fields-pr

Conversation

@delchev

@delchev delchev commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What

A generated update() persisted the caller's partial payload whole, so a header-only edit erased the system-owned money columns.

The UI form PUTs only the fields it edits. Paid (the allocation roll-up target) took the payload's null, and Balance was recomputed before recalculate() from those nulls and persisted as 0.

Observed in production: invoice 0000000181 printed a total due of 0 on an 8,556.00 invoice after exactly one draft header edit. The data was repaired by hand.

Fix

In Repository.java.template:

  • Collect the system-owned properties at generation time — an author's readOnly, roll-up/aggregate targets, the document number and uuid — into $preservedOnUpdate.
  • In update(), read the stored row once (findById) and take those values from there, never from the payload.
  • Move recalculate(entity) before the update-time calculated fields, so an expression like Balance = Total - Paid reads the freshly resummed Total and the preserved Paid.

This is the #6226/#6306 lost-update family, user-update edition.

Deliberately not preserved

An aggregate that is itself recomputed on update (an expressionUpdate such as Balance) is exempt — update() reassigns it after recalculate(). A calculated create-only field that stays user-editable (a Currency relation defaulted from the company) carries neither flag, so a user's edit of it is respected.

Safe against workflow writes by construction: every system writer (number stamp, status transitions, roll-ups) uses the targeted updateProperty / updateProperties / updateDerived primitives, which bypass update() entirely.

Verification

On a live instance (registry wipe + template reseed + regen of sales-invoices and timesheets):

  • the emitted update() preserves Number / Net / Vat / Discount / Total / Paid / Uuid / ProcessId / audit, and recomputes Balance after recalculate();
  • runtime reproduction — create invoice + item, then PUT a header-only payload — keeps Paid 0.00 / Balance 120.00 / Number intact, where the previous build persisted Paid null / Balance 0;
  • client-Java batch: 256 units, 261 class files;
  • audit-dsl-emission.py: 729 OK, 1 pre-existing unrelated FAIL (timesheets personal-group, regen owed).

🤖 Generated with Claude Code

…the payload

The generated update() persisted the whole incoming row, so a partial
payload - the UI header form PUTs only the fields it edits - erased every
column the write path owns. Reproduced on production (invoice 0000000181,
2026-08-12): a draft header edit nulled Paid and wrote Balance 0, and the
customer-facing PDF printed "amount due: 0" on an 8,556 invoice. Two defects
compounded:

- nothing preserved the system-owned columns, so the roll-up target Paid
  (written only by the allocation listeners) took the payload's null;
- the expressionUpdate recompute (Balance = Total - Paid) ran BEFORE
  recalculate(), reading the payload's nulls as zeros - Balance persisted 0
  while recalculate() then repaired Net/Vat/Total from the reloaded items,
  which is exactly why the corruption looked impossible: correct totals
  next to a nulled Paid and a zero Balance.

update() now reloads the stored row once and takes the system-owned values
from THERE, never from the payload: isReadOnlyProperty fields (author
readOnly, roll-up targets, the document number and uuid - which also covers
ProcessId and the audit columns) and aggregate footer fields with no
update-time recompute (Net/Vat/Discount/Total/Paid). The expressionUpdate /
actionUpdate assigns move AFTER recalculate(), so Balance = Total - Paid
reads the resummed Total and the preserved Paid.

Deliberately NOT preserved: a calculated create-only field that stays
user-editable (a Currency relation defaulted from the company) carries
neither flag, so a user's edit of it is respected. Safe against workflow
writes by construction: every system writer (number stamp, status
transitions, roll-ups) uses the targeted updateProperty / updateProperties /
updateDerived primitives, which bypass update() entirely - the #6226/#6306
lost-update family, user-update edition.

Verified on a live instance (registry wipe + template reseed + regen of
sales-invoices and timesheets): emitted update() preserves
Number/Net/Vat/Discount/Total/Paid/Uuid/ProcessId/audit and recomputes
Balance after recalculate; runtime reproduction (create invoice + item,
PUT a header-only payload) keeps Paid 0.00 / Balance 120.00 / Number intact
where the previous build persisted Paid null / Balance 0. Client-Java batch
256 units, 261 class files. audit-dsl-emission.py: 729 OK, 1 pre-existing
unrelated FAIL (timesheets personal-group, regen owed).
@delchev
delchev merged commit 4ce03ca into master Aug 13, 2026
8 of 10 checks passed
@delchev
delchev deleted the fix/dao-update-preserves-system-fields-pr branch August 13, 2026 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant