refactor(invoices): remove old budget-line pickers (#608)#618
Conversation
…age and InvoicesPage Budget lines are now linked to invoices via the dedicated InvoiceBudgetLinesSection (Story #606) and from item detail pages (Story #607). Remove the old single-budget-line picker fields that referenced columns no longer on the invoices table. - Remove workItemBudgetId/householdItemBudgetId from invoice form state - Remove WorkItemPicker/HouseholdItemPicker from invoice create/edit modals - Remove budget line loading state and useEffect hooks - Update VendorDetailPage tests to remove old picker assertions Fixes #608 Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
steilerDev
left a comment
There was a problem hiding this comment.
[security-engineer]
PR #618 — refactor: remove old budget-line pickers from invoice forms (Story #608)
Summary: Pure deletion PR — 639 lines removed, zero new code introduced. All removed code consists of dead frontend state, imports, UI controls (WorkItemPicker, HouseholdItemPicker, budget-line select dropdowns), and the corresponding Jest unit tests. No backend changes.
Security Checklist
- No SQL/command/XSS injection vectors in new code — N/A (deletion only)
- Authentication/authorization enforced on all new endpoints — N/A (no new endpoints)
- No sensitive data (secrets, tokens, PII) exposed in logs, errors, or client responses — verified; removed code did not expose sensitive data and no replacement code introduced
- User input validated and sanitized at API boundaries — N/A (no new input paths)
- New dependencies have no known CVEs — no dependency changes
- No hardcoded credentials or secrets — confirmed absent
- CORS configuration remains restrictive — unchanged
- Error responses do not leak internal details — unchanged
Verified:
workItemBudgetId,selectedWorkItemId,selectedHouseholdItemId,householdItemBudgetIdfields have been fully removed from bothInvoicesPageandVendorDetailPageform state. A grep acrossclient/src/pages/confirms no lingering references outside the correctly scopedInvoiceBudgetLinesSection(reviewed in PRs #615/#617).- The removed
listWorkItems({ pageSize: 100 })call inVendorDetailPagethat fetched all work items on every vendor page load (open informational finding from PR #193/#151 — swallowed promise rejections and unbounded pre-fetch) is now eliminated, which is a minor security hygiene improvement. - No
dangerouslySetInnerHTML,innerHTML, orevalpatterns introduced. - No new API surface, no new authorization decisions, no new data flows.
Conclusion: No security findings. This refactor reduces attack surface by removing an unnecessary API pre-fetch (listWorkItems) and simplifying component state. No issues to raise.
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect] Approved (via comment -- cannot self-approve).
Review Summary
Pure deletion PR that removes the old single-budget-line picker UI from VendorDetailPage and InvoicesPage. Budget line linking is now handled exclusively through the invoice_budget_lines junction table (EPIC-15, ADR-018).
Verified
- No stale FK references: Searched all production client code for
workItemBudgetId,householdItemBudgetId,selectedWorkItemId,selectedHouseholdItemId, andbudgetLinkTouched-- no remaining references outside the new junction table system (InvoiceBudgetLinesSection, InvoiceLinkModal, invoiceBudgetLinesApi). - Server route clean:
server/src/routes/invoices.tsdoes not reference the old FK fields. - Shared types clean:
workItemBudgetId/householdItemBudgetIdexist only onInvoiceBudgetLinetypes (junction table), not onInvoiceorCreateInvoiceRequest. - Test cleanup matches: 354 lines of tests removed from VendorDetailPage correspond exactly to the deleted UI (work item picker, budget line dropdown, budget link touched tracking).
- Wiki consistency: API Contract and Schema wiki pages already reflect the junction table model. No wiki updates needed for this PR.
Informational
The Security Audit wiki still references the old "workItemBudgetId Cross-Vendor Boundary" finding and the "Link to Work Item dropdown" bug -- both are now moot since the old UI and FK are removed. The security-engineer should mark these as resolved/superseded in a future audit update.
steilerDev
left a comment
There was a problem hiding this comment.
[product-owner] Reviewing PR #618 — Story 15.6: Budget Overview & Projected Budget Updates.
Verdict: APPROVE (submitted as comment due to same-account authorship limitation)
Acceptance Criteria Verification
All 7 ACs pass. The backend aggregation changes (ACs 1-4, 6-7) were completed in prior stories (15.1 via PR #612, 15.2 via PR #613, 15.3 via PR #614). This PR handles the remaining frontend cleanup (AC 5, AC 7 frontend portion):
| AC | Description | Verdict |
|---|---|---|
| 1 | Budget overview "Actual Cost" uses invoice_budget_lines.itemized_amount |
PASS (budgetOverviewService.ts, prior story) |
| 2 | "Actual Paid" sums only paid/claimed invoices via junction | PASS (budgetOverviewService.ts, prior story) |
| 3 | Projected Budget blended model uses itemized amounts for invoiced lines | PASS (budgetOverviewService.ts, prior story) |
| 4 | Category detail shows invoices with itemized amounts per category | PASS (budgetBreakdownService.ts, prior story) |
| 5 | Vendor detail shows invoices with per-budget-line itemized amounts | PASS (old picker removed, linking via InvoiceBudgetLinesSection) |
| 6 | Budget source utilization uses itemized amounts for claimed/unclaimed | PASS (budgetSourceService.ts computeClaimedAmount/computeUnclaimedAmount use junction) |
| 7 | No endpoint references old workItemBudgetId/householdItemBudgetId columns on invoices |
PASS (invoices schema clean, all references are on invoice_budget_lines entity) |
Scope & Quality
- Scope discipline: PR stays within the story boundary — removes old single-budget-line picker fields from InvoicesPage and VendorDetailPage invoice create/edit forms, plus corresponding test cleanup. No scope creep.
- Dead code removal: No orphan imports or unused state variables remain after the removal.
- Test changes: Only deletions of tests that validated the now-removed picker UI. No new test authorship required.
- Commit attribution: frontend-developer co-author trailer present as expected.
Notes
- CI is still running at time of review. Approval is conditional on all quality gates passing.
- No other agent reviews are present yet. This approval covers the product-owner scope only (requirements coverage and scope validation).
|
🎉 This PR is included in version 1.14.0-beta.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Fixes #608
Test plan
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com