feat(expenditures): count only approved expenditures in totals - #324
Draft
nourshoreibah wants to merge 2 commits into
Draft
feat(expenditures): count only approved expenditures in totals#324nourshoreibah wants to merge 2 commits into
nourshoreibah wants to merge 2 commits into
Conversation
Every SUM, chart series, budget percentage and report total counted every expenditure row regardless of status, so a pending or denied request inflated spend the moment it was entered and could show a project as over budget on expenditures nobody had signed off on. Filter to status = 'approved' in the dashboard aggregates, the project list cards, the project overview stats and the report data. The raw lists are deliberately left alone: the project expense table and the admin review queue exist to show what is awaiting review. The seed now sets statuses explicitly, keeping one denied and one pending row so local dev exercises both the review queue and the filter. Seed spend is unchanged at 18000, of which 14000 is approved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nding
Three PATCH /expenditures/{id}/status rejection tests asserted the row
was still 'pending' afterwards, encoding the seeded status rather than
the thing they mean: that the write did not happen. Seeding expenditure
1 as approved broke them.
Capture the status before the call and compare, and ask for a status
the row does not already hold, so a rejection test cannot pass merely
because the request matched what was already there. The approve and
decline cases move to the seeded pending row so they still exercise a
real transition.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Nothing that reports money spent filtered on
expenditures.status. EverySUM(amount), chart series, budget percentage and report total countedpending,deniedandneeds_more_inforows alongsideapprovedones — so an expenditure inflated the spend figures the moment someone entered it, before anyone reviewed it, and a project could render as over budget on the strength of requests that were later denied.The only status-aware code was the write path (
PATCH /expenditures/{id}/status), the status badge, and the optional user-selected filter on the expenses list page.Change
Filter to
status = 'approved'everywhere a figure is computed:projectslambdaGET /dashboard—totalSpent,topExpenseCategory, theaverageSpendPerProjectnumerator, the per-projectspenton the cards, and theexpensesByMonthseries behind the bar chartGET /projects—total_spenton the list cards (loadProjectAggregates)GET /projects/{id}/overview—totalSpent,totalRemaining,spentPercentage,expenditureCountreportslambdafetchReportData— the expenditures the PDF and DOCX tables list, and therefore the total each printsDeliberately left unfiltered
The raw lists — the expenses table on the project detail page,
GET /projects/{id}/expenditures, and the/expendituresadmin review queue. Their job is to show what is awaiting review. On the overview endpoint the table still returns every row while the stats beside it count only approved ones.No frontend change was needed: the only client-side reduction (
ExpensesBarChart) sums the already-filtered series the dashboard returns.Seed
Seeded expenditures had no explicit status, so they all defaulted to
pending— after this change the local dashboard would have read $0. The seed now sets statuses explicitly and keeps onedeniedand onependingrow, which gives the admin review queue something to show and keeps the seed honest about what the totals exclude. Row count and amounts are unchanged; seed spend is 18000, of which 14000 is approved.Tests
projects/test/approved-expenditures.e2e.test.ts(new, 8 tests) — one project, one expenditure per status; asserts every dashboard figure, the list card total and the overview stats see only the approved 1000, and that the overview table still returns all four rows.reports/test/report-service.e2e.test.ts(new, 2 tests) —fetchReportDatareturns approved rows only, and the total the report prints follows.projects/test/projects.e2e.test.ts— the one existing assertion that moved: seedtotalSpent18000 → 14000, average 4500 → 3500.topExpenseCategory(Travel/6800) and project 1's 9200 are unchanged by design, so they still pin the non-status behaviour.expenditures/test/expenditures.e2e.test.ts— threePATCH /expenditures/{id}/statusrejection tests asserted the row was still'pending'afterwards, which encoded the seeded status rather than what they mean: that the write did not happen. They now capture the status before the call and compare, and ask for a status the row does not already hold, so a rejection test cannot pass merely because the request matched what was already there. The approve and decline cases move to the seeded pending row so they still exercise a real transition.reports/jest.config.jsgainsmaxWorkers: 1: it now has two suites that reseed the same database, which truncate each other's fixtures when run in parallel. Theprojectslambda already does this for the same reason.Verification: full CI green — every lambda suite,
frontend-ci,migrations-freshandshared-auth.🤖 Generated with Claude Code