Skip to content

feat(invoices): allow deposits to exceed the invoice amount - #2189

Merged
steilerDev merged 4 commits into
betafrom
feat/2188-deposits-exceed-invoice
Oct 7, 2026
Merged

steilerDev merged 4 commits into
betafrom
feat/2188-deposits-exceed-invoice

Conversation

@steilerDev

Copy link
Copy Markdown
Owner

Summary

  • Removes the DEPOSITS_EXCEED_INVOICE_TOTAL invariant everywhere it was checked: deposit create/update, refund decrease/delete (Deposits: lowering or deleting a refund is not checked against the net-deposit rule #2127), invoice amount decrease, and quotation conversion (server and client). Also removes the error code, its client handling, and the en/de i18n keys.
  • Loosens the refund cap to max(invoice amount, gross deposit sum). It is checked on refund create and on refund increase. REFUND_EXCEEDS_INVOICE details gain depositTotal and refundCap.
  • The deposit form shows a non-blocking warning (bannerWarning, role="status", linked through aria-describedby) when net deposits would exceed the invoice amount. The convert-quotation overpaid banner is now advisory: it uses warning tokens and no longer disables Confirm.
  • Wiki API Contract and Schema are updated (wiki 99016a6), and so are the user guides.

Fixes #2188

Test plan

  • Server unit and integration tests for ACs 1–7: over-deposit on create and update, lowering the invoice amount, conversion, refund cap boundaries including cent precision, refund decrease and delete always allowed
  • Aggregate tests for AC12: residual clamps to 0, deposit fractions sum above 1, finalPaymentAmount and openAmount never negative or NaN
  • Client tests: computeDepositExcess, warning visibility, accessibility and non-blocking save, convert canConfirm with a shortfall, en/de i18n parity
  • E2E: over-deposit warning shown and saved (Scenario 5), advisory overpaid banner (Scenario 6), conversion with overpaid deposits and no refund (Scenario 6b)
  • CI Quality Gates and the E2E shards

Co-Authored-By: Claude backend-developer noreply@anthropic.com
Co-Authored-By: Claude frontend-developer noreply@anthropic.com
Co-Authored-By: Claude translator noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester noreply@anthropic.com
Co-Authored-By: Claude e2e-test-engineer noreply@anthropic.com
Co-Authored-By: Claude docs-writer noreply@anthropic.com
Co-Authored-By: Claude dev-team-lead noreply@anthropic.com

🤖 Generated with Claude Code

- Remove the DEPOSITS_EXCEED_INVOICE_TOTAL invariant everywhere: deposit
  create/update, refund decrease/delete, invoice amount decrease and
  quotation conversion; delete the error code, client handling and en/de keys
- Loosen the refund cap to max(invoice amount, gross deposit sum), checked
  on refund create and increase (REFUND_EXCEEDS_INVOICE details gain
  depositTotal and refundCap)
- Deposit form shows a non-blocking over-deposit warning (role=status);
  the convert-quotation overpaid banner is now advisory and no longer
  disables Confirm
- Tests cover ACs 1-7 and over-deposit aggregates (residual clamps to 0,
  no NaN/negative); wiki API contract/schema and user guides updated

Fixes #2188

Co-Authored-By: Claude backend-developer <noreply@anthropic.com>
Co-Authored-By: Claude frontend-developer <noreply@anthropic.com>
Co-Authored-By: Claude translator <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester <noreply@anthropic.com>
Co-Authored-By: Claude e2e-test-engineer <noreply@anthropic.com>
Co-Authored-By: Claude docs-writer <noreply@anthropic.com>
Co-Authored-By: Claude dev-team-lead <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

VERDICT: APPROVE

[security-engineer] Scoped review (deposit/invoice/quotation-conversion services + tests): no security findings. Removed checks were business-invariant validators, not authz/input guards; auth gates, AJV schemas and parameterized drizzle queries are untouched. Refund cap computed server-side in-transaction. Informational: lowering/deleting a deposit after refunds can leave refunds above the cap (data integrity only, no security impact).

@steilerDev

Copy link
Copy Markdown
Owner Author

VERDICT: APPROVE

[ux-designer] Design review (client scope). Token adherence: the overpaid banner swaps to --color-warning / -bg / -text-on-light, which match shared bannerWarning and switch in dark mode. The deposit form warning reuses sharedStyles.bannerWarning, so there is no duplication. A11y: the advisory banners use role=status (no redundant aria-live), the amount input gets aria-describedby only while the warning shows, and Confirm stays enabled. Copy: en and de are in parity and are advisory in tone. No hardcoded values. No findings.

@steilerDev

Copy link
Copy Markdown
Owner Author

VERDICT: REQUEST_CHANGES

[product-owner] Requirements review of #2188 (ACs 1-13): all functional ACs are met. One documentation finding blocks approval.

Verified:

  • AC 1-5: the DEPOSITS_EXCEED_INVOICE_TOTAL check is gone from deposit create/update, invoice-amount decrease and quotation conversion (server and client). The client pre-check (shortfall === 0 in canConfirm) is removed too. The itemized-sum check still applies, and a test pins that.
  • AC 6-7: refund cap is max(invoice amount, gross deposit sum), checked on create and on increase only. REFUND_EXCEEDS_INVOICE details gain depositTotal and refundCap. Refund decrease and delete are unguarded; this removes the Deposits: lowering or deleting a refund is not checked against the net-deposit rule #2127 guard, which only protected the removed invariant.
  • AC 8-10: the warning reuses sharedStyles.bannerWarning with role="status" and aria-describedby. It is deposit-only and excludes the edited entry. Save stays enabled. Copy exists in en and de via t(). The convert banner is now advisory: warning tokens, role="status", Confirm enabled.
  • AC 11: error code, client handling, and en/de keys are removed, including the now-dead amountBelowNetDeposits and the refund-reduce/delete keys. A repo grep finds no live references. The only remaining mentions are historical, in agent-memory notes and a parity test assertion.
  • AC 12-13: aggregate clamps (Math.max(0, ...)) are covered by new tests. Old rejection tests are inverted rather than deleted. E2E scenarios 5, 6 and 6b are added.

Finding 1 (fix-in-session, low effort): wording in docs/src/guides/budget/vendors-and-invoices.md. In the section about editing an invoice total, the intro still says "Cornerstone enforces two invariants". The second bullet is now the refund cap, which is not triggered by editing the invoice total (lowering the amount never blocks on deposits, and refunds are not touched by an amount edit). The bullet also mixes in a deposit-form warning. Either reword the intro and bullet so they describe what actually happens when the amount is edited (only the itemized floor blocks), or move the refund cap and warning text to the deposits guide, where it already exists. The user-guide text must match the shipped behavior.

Non-blocking, no action: CI shards were still pending at review time; the merge-time ci-wait.sh gate covers them.

Board: issue #2188 stays In Progress until merge, then UAT. The warning and banner are rendered artefacts with visual ACs, so this is not Done-on-merge. Use Refs, not Fixes, for the closing keyword if this is a parent-less cluster item with a UAT disposition. The PR body currently says Fixes #2188, so correct this.

@steilerDev

Copy link
Copy Markdown
Owner Author

VERDICT: REQUEST_CHANGES

[product-architect] Review of PR #2189 (feat(invoices): allow deposits to exceed the invoice amount)

Architecture / contract compliance: looks good.

  • Every DepositsExceedInvoiceTotalError throw site is gone: deposit create/update, the Deposits: lowering or deleting a refund is not checked against the net-deposit rule #2127 refund decrease/delete guards, the updateInvoice amount decrease, and convertQuotation. The class, the ERROR_CODES member, the en/de errors.json keys and all client code === branches are removed together. The only remaining mentions are historical Deviation Log rows in the wiki, which is correct.
  • assertRefundsWithinCap combines the two forked refund-cap copies (create and update) into one helper. It uses getDepositEntryTotals with the entry excluded, keeps the exceedsAmount cent-tolerant comparison, and runs inside the write transaction. The REFUND_EXCEEDS_INVOICE details (depositTotal, refundCap, availableHeadroom) match the API Contract exactly.
  • Wiki: the submodule ref 99016a6 is published (ls-remote matches ls-tree). The API Contract intro, POST/PATCH/DELETE error tables, the convert-quotation check order (renumbered, no gap), the Error Codes table and the Schema invariants all match the code. The page states that the cap is checked only when refunds are written, not when deposits are decreased or the amount is lowered.
  • Client: computeDepositExcess works in cents and excludes the edited entry. The warning uses role="status" and aria-describedby, and its id follows the file's existing static-id convention. canConfirm no longer depends on shortfall. ESLint reports no errors or warnings on all changed production files.
  • Tests: ACs 1–7 and 12 are covered at the service, route and aggregate levels, with refund-cap boundaries to the cent. E2E Scenarios 5, 6 and 6b exercise the non-blocking paths. The removed tests only asserted the removed rejections.

Findings

  1. [Low] [fix-in-session] User guide shows an identity that the PR breaks. In docs/src/guides/budget/invoice-deposits.md, line 16 still states Σ deposits + final payment = invoice total. Line 17, added by this PR, says net deposits may exceed the invoice total. When that happens, finalPaymentAmount is clamped to 0 (AC12, which the new aggregate tests assert), so the identity is false. Line 12 ("whatever is left after the deposits is the final payment") has the same problem. Rewrite the formula as final payment = max(0, invoice total − net deposits) (net deposits = deposits − refunds). Also replace the vague "the excess is treated as an overpaid amount" with what actually happens: the final payment shows 0, and the "Refunds due" figure on the vendor page shows the excess. This is a 2-line docs edit inside a file the PR already changes.

No deferrals.

…ver-deposits (#2188)

- Final payment is max(0, invoice total - deposits - refunds already
  received); over-deposits display as 0 and pending refunds show under
  "Refunds due to you"
- Invoice amount edits enforce only the itemized-amount floor; drop the
  misplaced refund-cap bullet

Fixes #2188

Co-Authored-By: Claude docs-writer <noreply@anthropic.com>
Co-Authored-By: Claude dev-team-lead <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

VERDICT: APPROVE

[product-owner] Round 2: commit 5af573b closes my round-1 finding. The edit-invoice-total section now lists one constraint (itemized amount floor); the misplaced refund-cap bullet and the 'two invariants' wording are gone. The invoice-deposits.md formula (max(0, total - deposits - refunds)) and the 'Refunds due to you' label match the shipped UI string. No new findings.

@steilerDev

Copy link
Copy Markdown
Owner Author

VERDICT: APPROVE

[product-architect] Round 2 re-review of fix commit 5af573b against my round-1 finding (comment 6035535024).

  • docs/src/guides/budget/invoice-deposits.md now states final payment = max(0, invoice total − deposits − refunds already received). This matches computeFinalPaymentAmount (server/src/services/shared/depositAggregateUtils.ts:351) term for term: deposits of every status are subtracted, only received refunds (paid/claimed) are subtracted, pending refunds are not, and the result is clamped at 0. The incorrect Σ deposits + final payment = invoice total identity is gone.
  • Over-deposit behaviour is described correctly: the final payment shows as 0, and the excess appears under Refunds due to you once a pending refund is recorded (matches the summaryRefundsDue UI label).
  • vendors-and-invoices.md: dropping the refund-cap bullet and the "two invariants" wording is consistent with this PR removing that guard. Only the itemized-amount floor is left.

Finding resolved. No new findings.

steilerDev and others added 2 commits October 7, 2026 12:12
…xceedInvoiceTotalError (#2188)

Fixes #2188

Co-Authored-By: Claude qa-integration-tester <noreply@anthropic.com>
Co-Authored-By: Claude dev-team-lead <noreply@anthropic.com>
…val specs

Co-Authored-By: Claude dev-team-lead <noreply@anthropic.com>
@steilerDev
steilerDev merged commit 2923e23 into beta Oct 7, 2026
33 checks passed
@steilerDev
steilerDev deleted the feat/2188-deposits-exceed-invoice branch October 7, 2026 10:25
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.19.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant