feat(reports): add source report data, mark-claimed, and Paperless upload endpoints - #1885
Conversation
…load endpoints Adds GET /api/source-reports (budget-overview/claim/proof-of-funds report generation with Paperless-ngx document enrichment), POST /api/source-reports/mark-claimed (batch invoice + deposit claiming), and POST /api/paperless/documents (PDF upload proxy to Paperless-ngx's consumption pipeline). Also allows the pending -> claimed deposit status transition needed for direct claiming. Fixes a Map-iteration bug in getSourceReport where document enrichment iterated a Map's [key, value] tuples instead of .values(), silently leaving ASN/title fields unpopulated. Fixes #1878 Fixes #1884 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
… transition Scenario 47 asserted the pre-#1878 contract (pending -> claimed disallowed); repurposed to exercise claimed -> pending, which remains disallowed. Added scenario 47b asserting pending -> claimed now succeeds with both paidDate and claimedDate auto-set to today. Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
|
[security-engineer] Security review of PR #1885 (Story #1878, feat/1878-source-report-backend → beta). Scope reviewed: No critical or high findings. Verified clean
Informational (non-blocking)
None of the above block merge. Verdict: APPROVED. |
…ns spec Story #1877 added the Household tab to ManagePage, making 6 tabs total. orientations.spec.ts's secondary tab-count assertion (line ~389) wasn't updated at the time and was failing full-matrix shards 6/11/15. Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
|
[product-owner] Product Owner review of PR #1885 against story #1878. I verified every acceptance criterion against the implementation and the accompanying test suites ( AC coverageReport data —
|
| AC | Verdict | Evidence |
|---|---|---|
budget-overview returns all four statuses incl. quotation |
Met | getSourceReport status switch adds all 4; scenario 8 |
claim = pending+paid only; proof-of-funds = claimed only |
Met | scenarios 9, 10 |
Multi-source invoice → partial allocatedAmount + isSplit: true |
Met | single COUNT(DISTINCT COALESCE(wib…, hib…)) split query; scenarios 11, 12a, 12b (12b covers the COUNT DISTINCT NULL exclusion) |
Matching-status invoice with zero budget lines → unallocatedInvoices[], not main list |
Met | dedicated NOT EXISTS query; scenarios 15, 15b |
Exactly-zero net contribution dropped; negative kept as lineKind: 'refund-adjustment' with negative amount |
Met | drop evaluated post-rounding (correct, matches the aggregation helper's contract); scenarios 13, 14 |
| Stage-based document rule + untagged docs always included, in every report type | Met | quotation-status → quotation; residual (non-quotation, in-slice) → invoice; in-slice deposit/refund fractions → deposit; attachmentType === null always kept. Scenarios 16a–16e |
Batched id__in ASN/title fetch in try/catch; degrades to doc-ID-only, never hard-fails |
Met | getDocuments uses id__in; try/catch returns null ASN/title. Scenarios 17 (reachable → populated), 18 (throws → degrades), 19 (unconfigured → no fetch at all) |
Missing/invalid type/sourceId → 400; unknown source → 404 |
Met | AJV querystring schema with enum + required; NotFoundError. Route tests cover missing type, missing sourceId, invalid enum value, unknown sourceId |
| 2 dp rounding server-side, total summed from rounded values | Met | per-line toCents(...)/100, then total summed from the rounded lines; scenario 21 asserts exact addition on decimal-noisy input |
Mark claimed — POST /api/source-reports/mark-claimed
| AC | Verdict | Evidence |
|---|---|---|
Single transaction; invoices flip to claimed incl. pending → claimed, with onInvoiceStatusChanged; eligible deposits and refunds flip with claimed_date = today, paid_date preserved, via onDepositStatusChanged |
Met | validate-then-write inside one db.transaction; deposit eligibility driven off the shared ALLOWED_TRANSITIONS table rather than a duplicated status list. Scenarios 23, 24 (paidDate stays null), 25 (paidDate preserved), 26 (already-claimed invoice + pending refund → invoice untouched, refund flips), 30 (diaryAutoEvents=false), 31, 32 |
| Stale wizard → 409 listing offending IDs, no partial changes | Met | InvoicesNotClaimableError (409, INVOICES_NOT_CLAIMABLE, details.invoiceIds) thrown before any write. Scenarios 27 and 28 verify rollback by re-querying rather than trusting the throw — good rigour; 29/29b cover the "claimed invoice with nothing sweepable" case |
Paperless upload proxy — POST /api/paperless/documents
| AC | Verdict | Evidence |
|---|---|---|
Multipart PDF + title → forwarded to post_document, filter tag applied when configured, { taskId } returned |
Met | scenarios 31, 35 |
Unconfigured → 503 via requirePaperless; non-PDF rejected |
Met | gating runs before the multipart parse; MIME check before toBuffer(). Scenarios 36, 37 (upload never attempted), 38, 39, 40 |
| Filter-tag resolution failure → uploads untagged with a warning, does not fail | Met | scenarios 32 and the route-level "unresolvable filter tag → 201 untagged" test |
Scope and process
- No scope creep. The two additions beyond the endpoint trio — widening deposit
ALLOWED_TRANSITIONS.pendingto includeclaimed, and fixing getSourceReport: Paperless ASN/title enrichment never populates (Map iteration bug) #1884 — were both explicitly anticipated in the story's implementation notes / arose from it. - The two refinements are accepted as documented deviations:
unallocatedInvoices[]uses a lighter dedicated type instead of a dummy-filledSourceReportInvoice(cleaner — the informational rows genuinely have no allocation, nolineKind, no documents), and the PATCHpending → claimedpath auto-setting both dates ("fully settled" single-deposit semantics) versus the batch sweep preservingpaidDateincluding null ("submitted for reimbursement") is a deliberate, wiki-documented split. Both semantics are now written down inAPI-Contract.md, which is what makes the divergence safe. - Wiki accuracy:
API-Contract.mdwas updated in the same change and matches the implementation I read (degradation behaviour, 409 shape with "NO rows modified",paidDatepreserved/never cleared,pending → claimedauto-set rules, 503 gating). No deviation found. - Browser E2E deferred to Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 is accepted — these endpoints have no UI surface until the wizard consumes them.
- CI:
Quality Gates(the required check for beta) passes. E2E shards 5/6/11/15 fail oninvoices.spec.ts:841("Effective Amount" column, from Deposit refunds with negative claim adjustments #1876) anddashboard.spec.ts:566— both pre-existing and unrelated to this backend-only diff (noclient/ore2e/files changed).
Follow-ups (non-blocking — do NOT hold the merge)
- N+1 Paperless round-trips.
getSourceReportissues thedocumentLinksSELECT and thepaperlessService.getDocuments()call inside the per-invoice loop. The AC's "batchedid__in" is satisfied per invoice, and the try/catch degrades correctly at every iteration, so this is functionally correct. But a claim report over 80 invoices makes 80 sequential HTTP round-trips to Paperless, and the wizard in Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 sits directly on this latency. Recommend hoisting to one document-links query plus onegetDocuments()over the union of IDs before the loop. Say the word and I will file this as a backlog item against the Bank Report Wizard mini-epic so Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 does not inherit a slow report call. - Duplicated filter predicate. The
link.attachmentType === null || stages.has(...)filter runs twice (once to builddocumentIds, once to builddocuments). Harmless today, but two copies of a filtering rule is exactly where a future stage-rule change goes half-applied. - Stale test comment. The comment block above scenario 17 in
sourceReportService.test.tsstill narrates that the test "is skipped (not deleted, not inverted) until getSourceReport: Paperless ASN/title enrichment never populates (Map iteration bug) #1884 is fixed". getSourceReport: Paperless ASN/title enrichment never populates (Map iteration bug) #1884 is fixed in this very PR and the test is active. Worth trimming to a one-line historical note so a future reader does not go hunting for a skip that is not there.
Process note for the orchestrator
Per CLAUDE.md, the architecture, security, and QA reviews are required on every story PR. At the time of this review no other agent reviews or comments are posted on #1885. Those must land before merge — my acceptance covers requirements and AC coverage only, not the security or architectural verdict.
Verdict: APPROVED
|
[product-architect] Architecture review of PR #1885 (story #1878). Reviewed against What I verified as correct
BlockingH1 — Paperless metadata resolution runs inside the per-invoice loop (network N+1)
Two things make this worth fixing before the wizard frontend (#1879) builds on it:
It also contradicts the codebase's own stated convention: Requested change — hoist both fetches out of the loop:
While you are in there: the same Non-blocking (fine for refinement / #1879)
Nothing here touches the schema, so no Schema-page or migration follow-up is owed. No ADR is required: the report is a read-model over existing entities and the transition widening is documented in the contract. Happy to re-review promptly once H1 is addressed — it is a contained change and the existing degradation tests should cover it unmodified. VERDICT: CHANGES_REQUIRED |
…oad response handling Applies product-architect review findings from #1878: - H1: batch document-link lookup and Paperless getDocuments resolution across all report invoices into one query + one call, instead of per-invoice N+1 fetches - M1: document the concrete Paperless document-stage filtering rule in the API contract - L1: use the shared PaperlessUploadResponse type instead of an inline { taskId: string } shape - L2: guard the Paperless upload response, throwing PAPERLESS_ERROR if taskId is not a string instead of trusting an unchecked cast Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com>
|
[product-architect] Round 2 review of PR #1885 — re-reviewed commit H1 (blocking) — verified fixed on the meritsThe restructuring in
Net: an N-invoice report went from ~2–4N sequential Paperless round trips to 2–4 total, regardless of N. Behavioural side-effect worth recording, and it is an improvement: degradation is now uniform across the report. Previously a transient Paperless failure mid-loop could produce a report where invoice 1 carried real ASN/title and invoice 2 carried nulls — an internally inconsistent document to hand to a bank. Now the whole report degrades together. Scenarios 18/19 passing unchanged is the right evidence that the observable contract is intact. Empty-report regression checked explicitly, since the fix introduced a new
So a zero-invoice source returns an empty report rather than a 500. Good. M1 / L1 / L2 — verified fixed
Wiki formatting — fixed by meThe M1 edit had inserted the "Document Filtering Rule" prose inside the response-field table, orphaning the Pushed to the wiki repo directly: Non-blocking findings (for refinement / #1879, not this PR)
Everything I raised in round 1 is addressed, the fix is correct on inspection rather than merely plausible, and the contract now matches the implementation. No schema changes, so no Schema-page or migration follow-up is owed; no ADR required. VERDICT: APPROVED |
|
🎉 This PR is included in version 2.13.0-beta.28 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
GET /api/source-reports— generates budget-overview/claim/proof-of-funds reports for a budget source, enriched with Paperless-ngx document metadata (degrades gracefully to null fields if Paperless is unreachable)POST /api/source-reports/mark-claimed— atomically flips a batch of invoices and their eligible deposits toclaimedPOST /api/paperless/documents— multipart PDF upload proxy into Paperless-ngx's consumption pipeline, returning the async task UUIDpending -> claimeddeposit status transition (previously only reachable viapaid)Fixes #1878
Fixes #1884
Test plan
app.inject())Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com
Co-Authored-By: Claude backend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) noreply@anthropic.com