diff --git a/apps/backend/db/seed.sql b/apps/backend/db/seed.sql index ae73432e..794d708c 100644 --- a/apps/backend/db/seed.sql +++ b/apps/backend/db/seed.sql @@ -42,13 +42,17 @@ INSERT INTO branch.project_memberships (project_id, user_id, role, start_date, h (1, 2, 'Director', '2025-02-01', 80.00), (2, 3, 'Student', '2025-03-15', 60.00); -INSERT INTO branch.expenditures (project_id, entered_by, amount, category, description, spent_on) VALUES -(1, 1, 5000, 'Travel', 'Domestic conference attendance', '2025-02-10'), -(1, 1, 4200, 'Travel Foreign', 'International collaborator meeting in London', '2025-03-22'), -(2, 2, 3000, 'General', 'Recording device supplies', '2025-04-05'), -(2, 2, 1500, 'Visitor / Honorarium', 'Guest lecturer honorarium', '2025-05-18'), -(3, 3, 2500, 'General', 'Educational materials', '2025-07-12'), -(3, 3, 1800, 'Travel', 'Local outreach travel', '2025-08-03'); +-- Statuses are load-bearing too. Totals, charts and reports count 'approved' +-- rows only, so the seed keeps one denied and one pending row: they give the +-- admin review queue something to show, and they keep the dashboard honest +-- about what it excludes. 14000 of the 18000 below is approved spend. +INSERT INTO branch.expenditures (project_id, entered_by, amount, category, description, status, spent_on) VALUES +(1, 1, 5000, 'Travel', 'Domestic conference attendance', 'approved', '2025-02-10'), +(1, 1, 4200, 'Travel Foreign', 'International collaborator meeting in London', 'approved', '2025-03-22'), +(2, 2, 3000, 'General', 'Recording device supplies', 'approved', '2025-04-05'), +(2, 2, 1500, 'Visitor / Honorarium', 'Guest lecturer honorarium', 'denied', '2025-05-18'), +(3, 3, 2500, 'General', 'Educational materials', 'pending', '2025-07-12'), +(3, 3, 1800, 'Travel', 'Local outreach travel', 'approved', '2025-08-03'); INSERT INTO branch.reports (project_id, title, object_url) VALUES (1, 'Clinician Communication Study Report', 'https://s3.amazonaws.com/branch-reports/clinician_communication_study_report.pdf'), diff --git a/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts b/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts index 0646134f..81a49aea 100644 --- a/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts +++ b/apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts @@ -628,39 +628,53 @@ describe('Expenditures integration tests', () => { return result.rows[0]?.status; } + // Seeded pending row, so the two success cases below really do move a + // status. Expenditure 1 is seeded approved and is used for the rejection + // cases, where the point is that nothing changes. + const PENDING_ID = 5; + test('200: admin approves a pending expenditure', async () => { mockAuthenticateRequest.mockResolvedValue(adminUser); - const res = await handler(patchStatusEvent(1, { status: 'approved' })); + expect(await getStatus(PENDING_ID)).toBe('pending'); + const res = await handler(patchStatusEvent(PENDING_ID, { status: 'approved' })); expect(res.statusCode).toBe(200); expect(JSON.parse(res.body).body.status).toBe('approved'); // confirms it persisted to the database - expect(await getStatus(1)).toBe('approved'); + expect(await getStatus(PENDING_ID)).toBe('approved'); }); test('200: admin declines a pending expenditure', async () => { mockAuthenticateRequest.mockResolvedValue(adminUser); - const res = await handler(patchStatusEvent(1, { status: 'denied' })); + expect(await getStatus(PENDING_ID)).toBe('pending'); + const res = await handler(patchStatusEvent(PENDING_ID, { status: 'denied' })); expect(res.statusCode).toBe(200); expect(JSON.parse(res.body).body.status).toBe('denied'); - expect(await getStatus(1)).toBe('denied'); + expect(await getStatus(PENDING_ID)).toBe('denied'); }); + // The rejection cases each ask for a status the row does not already hold + // and assert the stored value is untouched, so they cannot pass merely + // because the request happened to match the seed. test('401: unauthenticated request is rejected', async () => { mockAuthenticateRequest.mockResolvedValue({ isAuthenticated: false }); - const res = await handler(patchStatusEvent(1, { status: 'approved' })); + const before = await getStatus(1); + const res = await handler(patchStatusEvent(1, { status: 'denied' })); expect(res.statusCode).toBe(401); - expect(await getStatus(1)).toBe('pending'); + expect(before).not.toBe('denied'); + expect(await getStatus(1)).toBe(before); }); test('403: non-admin user is rejected', async () => { mockAuthenticateRequest.mockResolvedValue(studentUser); - const res = await handler(patchStatusEvent(1, { status: 'approved' })); + const before = await getStatus(1); + const res = await handler(patchStatusEvent(1, { status: 'denied' })); expect(res.statusCode).toBe(403); - expect(await getStatus(1)).toBe('pending'); + expect(before).not.toBe('denied'); + expect(await getStatus(1)).toBe(before); }); test('404: expenditure not found', async () => { @@ -672,10 +686,11 @@ describe('Expenditures integration tests', () => { test('400: status not valid is rejected', async () => { mockAuthenticateRequest.mockResolvedValue(adminUser); + const before = await getStatus(1); const res = await handler(patchStatusEvent(1, { status: 'pend' })); expect(res.statusCode).toBe(400); - expect(await getStatus(1)).toBe('pending'); + expect(await getStatus(1)).toBe(before); }); test('400: invalid id is rejected', async () => { diff --git a/apps/backend/lambdas/projects/handler.ts b/apps/backend/lambdas/projects/handler.ts index ecbe9bae..6af5d66b 100644 --- a/apps/backend/lambdas/projects/handler.ts +++ b/apps/backend/lambdas/projects/handler.ts @@ -2,7 +2,11 @@ import { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import { sql, Transaction } from 'kysely'; import type { DB } from '@branch/types'; import db from './db'; -import { MemberAssignment, ProjectValidationUtils } from './validation-utils'; +import { + APPROVED_EXPENDITURE_STATUS, + MemberAssignment, + ProjectValidationUtils, +} from './validation-utils'; import { authenticateRequest, canAccessProject, @@ -80,6 +84,7 @@ export const handler = async (event: any): Promise => { ] = await Promise.all([ db.selectFrom('branch.expenditures') .select(db.fn.sum('amount').as('total')) + .where('status', '=', APPROVED_EXPENDITURE_STATUS) .where('spent_on', '>=', yearStart as any) .where('spent_on', '<=', yearEnd as any) .executeTakeFirst(), @@ -89,6 +94,7 @@ export const handler = async (event: any): Promise => { .executeTakeFirst(), db.selectFrom('branch.expenditures') .select(['category', db.fn.sum('amount').as('total')]) + .where('status', '=', APPROVED_EXPENDITURE_STATUS) .where('category', 'is not', null) .where('spent_on', '>=', yearStart as any) .where('spent_on', '<=', yearEnd as any) @@ -102,6 +108,7 @@ export const handler = async (event: any): Promise => { db.selectFrom('branch.expenditures as e') .innerJoin('branch.projects as p', 'p.project_id', 'e.project_id') .select((eb) => eb.fn.sum('e.amount').as('total')) + .where('e.status', '=', APPROVED_EXPENDITURE_STATUS) .where('e.spent_on', '>=', yearStart as any) .where('e.spent_on', '<=', yearEnd as any) .where(isActive('p.end_date')) @@ -115,6 +122,7 @@ export const handler = async (event: any): Promise => { eb.selectFrom('branch.expenditures') .select('project_id') .select((sub) => sub.fn.sum('amount').as('total')) + .where('status', '=', APPROVED_EXPENDITURE_STATUS) .groupBy('project_id') .as('spend'), (join) => join.onRef('spend.project_id', '=', 'p.project_id'), @@ -140,6 +148,7 @@ export const handler = async (event: any): Promise => { .execute(), db.selectFrom('branch.expenditures') .select([monthExpr.as('month'), 'category', db.fn.sum('amount').as('total')]) + .where('status', '=', APPROVED_EXPENDITURE_STATUS) .where('category', 'is not', null) .where('spent_on', '>=', yearStart as any) .where('spent_on', '<=', yearEnd as any) @@ -325,7 +334,10 @@ export const handler = async (event: any): Promise => { ]); const totalBudget = project.total_budget !== null ? Number(project.total_budget) : 0; - const totalSpent = expenditures.reduce((sum, e) => sum + Number(e.amount), 0); + // The table below lists every expenditure, including the ones still in + // review; the stats beside it count only what was approved. + const approved = expenditures.filter((e) => e.status === APPROVED_EXPENDITURE_STATUS); + const totalSpent = approved.reduce((sum, e) => sum + Number(e.amount), 0); // Guarded because a project may legitimately have no budget set yet, and // 0/0 would render as NaN% in the donut. const spentPercentage = totalBudget > 0 ? (totalSpent / totalBudget) * 100 : 0; @@ -339,7 +351,7 @@ export const handler = async (event: any): Promise => { spentPercentage: Number(spentPercentage.toFixed(2)), totalDonated: Number(donationRow?.total ?? 0), memberCount: members.length, - expenditureCount: expenditures.length, + expenditureCount: approved.length, }, members, expenditures, @@ -683,6 +695,7 @@ async function loadProjectAggregates(projectIds: number[]): Promise<{ db .selectFrom('branch.expenditures') .select(['project_id', db.fn.sum('amount').as('total')]) + .where('status', '=', APPROVED_EXPENDITURE_STATUS) .where('project_id', 'in', projectIds) .groupBy('project_id') .execute(), diff --git a/apps/backend/lambdas/projects/openapi.yaml b/apps/backend/lambdas/projects/openapi.yaml index 417b8230..8e7d11b3 100644 --- a/apps/backend/lambdas/projects/openapi.yaml +++ b/apps/backend/lambdas/projects/openapi.yaml @@ -81,6 +81,7 @@ paths: properties: total_spent: type: number + description: Approved expenditures only. member_count: type: integer is_active: @@ -142,6 +143,9 @@ paths: $ref: '#/components/schemas/Project' stats: type: object + description: >- + Counts approved expenditures only; `expenditures` below + still lists every row, including those under review. properties: totalBudget: type: number @@ -285,6 +289,10 @@ paths: /dashboard: get: summary: GET /dashboard + description: >- + Admin-only roll-up: this year's totals, the per-project breakdown and + the monthly expense series. Every spend figure counts approved + expenditures only. responses: '200': description: OK diff --git a/apps/backend/lambdas/projects/test/approved-expenditures.e2e.test.ts b/apps/backend/lambdas/projects/test/approved-expenditures.e2e.test.ts new file mode 100644 index 00000000..f7a0d9bf --- /dev/null +++ b/apps/backend/lambdas/projects/test/approved-expenditures.e2e.test.ts @@ -0,0 +1,153 @@ +/** + * Every figure the app reports as money spent must count approved + * expenditures only. A pending or denied row is a request, not a spend, so + * letting it into a total overstates the budget burn and can show a project + * as over budget on the strength of expenditures nobody signed off on. + * + * The raw lists (the project expense table, the admin review queue) stay + * unfiltered — they exist to show what is awaiting review. + */ +import { describe, test, expect, beforeAll, beforeEach, afterAll, jest } from '@jest/globals'; +import { Pool } from 'pg'; +import { ensureSchema, resetData } from '../../../db/testkit'; + +jest.mock('../auth', () => ({ + ...jest.requireActual('../auth'), + authenticateRequest: jest.fn(), +})); + +import { handler } from '../handler'; +import db from '../db'; +import { authenticateRequest } from '../auth'; + +const mockAuthenticateRequest = authenticateRequest as jest.MockedFunction; + +const adminAuthResult = { + isAuthenticated: true as const, + user: { cognitoSub: 'admin-sub', userId: 1, email: 'ashley@branch.org', isAdmin: true }, +}; + +const pool = new Pool({ + host: 'localhost', + port: 5432, + user: 'branch_dev', + password: 'password', + database: 'branch_db', + ssl: false, +}); + +beforeAll(async () => { + const client = await pool.connect(); + try { + await ensureSchema(client); + } finally { + client.release(); + } +}); + +// One project, one budget, one of each status. Every assertion below reduces +// to "did the 1000 survive and the other 3000 stay out". +beforeEach(async () => { + jest.clearAllMocks(); + mockAuthenticateRequest.mockResolvedValue(adminAuthResult); + + const client = await pool.connect(); + try { + await resetData(client); + await client.query('DELETE FROM branch.expenditures'); + await client.query(` + INSERT INTO branch.expenditures + (project_id, entered_by, amount, category, description, status, spent_on) + VALUES + (1, 1, 1000, 'Travel', 'approved', 'approved', CURRENT_DATE), + (1, 1, 1000, 'Travel', 'pending', 'pending', CURRENT_DATE), + (1, 1, 1000, 'Travel', 'denied', 'denied', CURRENT_DATE), + (1, 1, 1000, 'Travel', 'needs info', 'needs_more_info', CURRENT_DATE) + `); + await client.query(`UPDATE branch.projects SET end_date = '2099-12-31' WHERE end_date IS NOT NULL`); + } finally { + client.release(); + } +}); + +afterAll(async () => { + await pool.end(); + await db.destroy(); +}); + +function getEvent(rawPath: string) { + return { + rawPath, + requestContext: { http: { method: 'GET' } }, + headers: { Authorization: 'Bearer fake-token' }, + queryStringParameters: {}, + } as any; +} + +async function get(rawPath: string) { + const res = await handler(getEvent(rawPath)); + expect(res.statusCode).toBe(200); + return JSON.parse(res.body); +} + +describe('GET /dashboard counts approved expenditures only', () => { + test('totalSpent leaves out pending, denied and needs_more_info', async () => { + const body = await get('/dashboard'); + expect(body.summary.totalSpent).toBe(1000); + }); + + test('topExpenseCategory sums approved rows only', async () => { + const body = await get('/dashboard'); + expect(body.summary.topExpenseCategory).toEqual({ + category: 'Travel', + amount: 1000, + percentage: 100, + }); + }); + + test('averageSpendPerProject divides approved spend across active projects', async () => { + const body = await get('/dashboard'); + // 1000 approved over the 4 active seed projects. + expect(body.summary.averageSpendPerProject).toBe(250); + }); + + test('per-project spend on the dashboard cards is approved-only', async () => { + const body = await get('/dashboard'); + const p1 = body.projects.find((p: any) => p.project_id === 1); + expect(p1.spent).toBe(1000); + }); + + test('the expenses bar chart series is approved-only', async () => { + const body = await get('/dashboard'); + const total = body.expensesByMonth.reduce((sum: number, r: any) => sum + r.amount, 0); + expect(total).toBe(1000); + }); +}); + +describe('GET /projects counts approved expenditures only', () => { + test('total_spent on the list cards leaves out unapproved rows', async () => { + const projects = await get('/'); + const p1 = projects.find((p: any) => p.project_id === 1); + expect(Number(p1.total_spent)).toBe(1000); + }); +}); + +describe('GET /projects/{id}/overview counts approved expenditures only', () => { + test('stats are computed from approved rows', async () => { + const body = await get('/1/overview'); + expect(body.stats.totalSpent).toBe(1000); + expect(body.stats.totalRemaining).toBe(body.stats.totalBudget - 1000); + expect(body.stats.expenditureCount).toBe(1); + }); + + test('the expenditures table still lists every row so reviewers can see them', async () => { + const body = await get('/1/overview'); + expect(body.expenditures).toHaveLength(4); + expect(body.expenditures.map((e: any) => e.status).sort()).toEqual([ + 'approved', + 'denied', + 'needs_more_info', + 'pending', + ]); + }); +}); diff --git a/apps/backend/lambdas/projects/test/projects.e2e.test.ts b/apps/backend/lambdas/projects/test/projects.e2e.test.ts index 4cbcea64..f48afb00 100644 --- a/apps/backend/lambdas/projects/test/projects.e2e.test.ts +++ b/apps/backend/lambdas/projects/test/projects.e2e.test.ts @@ -357,8 +357,10 @@ describe('GET /dashboard (e2e)', () => { const body = JSON.parse(res.body); expect(body.summary.totalProjects).toBe(4); - expect(body.summary.totalSpent).toBe(18000); - expect(body.summary.averageSpendPerProject).toBe(4500); + // 14000, not the 18000 the seed spends: one denied and one pending row are + // requests rather than spend and stay out of every total. + expect(body.summary.totalSpent).toBe(14000); + expect(body.summary.averageSpendPerProject).toBe(3500); }); test('topExpenseCategory is highest-summed category 🌞', async () => { diff --git a/apps/backend/lambdas/projects/validation-utils.ts b/apps/backend/lambdas/projects/validation-utils.ts index ba29e288..f212c0df 100644 --- a/apps/backend/lambdas/projects/validation-utils.ts +++ b/apps/backend/lambdas/projects/validation-utils.ts @@ -18,6 +18,13 @@ export const DEFAULT_PROJECT_ROLE: ProjectRole = 'Student'; export type MemberAssignment = { user_id: number; role: ProjectRole }; +/** + * A pending or denied expenditure is a request, not a spend. Every total, + * chart series and budget percentage filters on this; the raw lists do not, + * because their job is to show what is still awaiting review. + */ +export const APPROVED_EXPENDITURE_STATUS = 'approved'; + // Utility class for validating project-related input fields export class ProjectValidationUtils { // Parses numeric input (number or string), converts to fixed 2-decimal string for database storage diff --git a/apps/backend/lambdas/reports/jest.config.js b/apps/backend/lambdas/reports/jest.config.js index 3ed2e70d..cb28368c 100644 --- a/apps/backend/lambdas/reports/jest.config.js +++ b/apps/backend/lambdas/reports/jest.config.js @@ -2,6 +2,9 @@ module.exports = { preset: 'ts-jest', testEnvironment: 'node', testMatch: ['**/*.test.ts'], + // Two suites reseed the same database; in parallel they truncate each + // other's fixtures mid-test. Matches the projects lambda. + maxWorkers: 1, extensionsToTreatAsEsm: ['.ts'], moduleNameMapper: { '^(\\.{1,2}/.*)\\.js$': '$1', diff --git a/apps/backend/lambdas/reports/openapi.yaml b/apps/backend/lambdas/reports/openapi.yaml index 84ca41de..e7b69dc6 100644 --- a/apps/backend/lambdas/reports/openapi.yaml +++ b/apps/backend/lambdas/reports/openapi.yaml @@ -163,10 +163,12 @@ paths: summary: Auto-generate a PDF report from project data description: > Generates a report for the given project containing project info, - participants and roles, donations, and expenditures. Supports PDF and - DOCX output via the `file_type` field (defaults to `pdf`). Uploads the - file to S3 and records it in the database. Requires the caller to be a - member of the project or a global admin. + participants and roles, donations, and approved expenditures. Rows + still under review, or denied, appear in neither the expenditures + table nor the total beneath it. Supports PDF and DOCX output via the + `file_type` field (defaults to `pdf`). Uploads the file to S3 and + records it in the database. Requires the caller to be a member of the + project or a global admin. requestBody: required: true content: diff --git a/apps/backend/lambdas/reports/report-service.ts b/apps/backend/lambdas/reports/report-service.ts index b43c334d..26fa7657 100644 --- a/apps/backend/lambdas/reports/report-service.ts +++ b/apps/backend/lambdas/reports/report-service.ts @@ -24,6 +24,13 @@ const URLResolver = require('pdfmake/js/URLResolver').default; const s3 = new S3Client({ region: process.env.AWS_REGION ?? 'us-east-2' }); +/** + * A report states what a project spent. Expenditures still in review, or + * denied, are requests rather than spend, so they stay out of the table and + * out of the total under it. + */ +const APPROVED_EXPENDITURE_STATUS = 'approved'; + function getBucketName(): string { const bucket = process.env.REPORTS_BUCKET_NAME; if (!bucket) { @@ -178,6 +185,7 @@ export async function fetchReportData(projectId: number): Promise { + const client = await pool.connect(); + try { + await ensureSchema(client); + } finally { + client.release(); + } +}); + +beforeEach(async () => { + const client = await pool.connect(); + try { + await resetData(client); + await client.query('DELETE FROM branch.expenditures'); + await client.query(` + INSERT INTO branch.expenditures + (project_id, entered_by, amount, category, description, status, spent_on) + VALUES + (1, 1, 1000, 'Travel', 'approved', 'approved', '2025-02-10'), + (1, 1, 2000, 'Travel', 'pending', 'pending', '2025-02-11'), + (1, 1, 4000, 'Travel', 'denied', 'denied', '2025-02-12'), + (1, 1, 8000, 'Travel', 'needs info', 'needs_more_info', '2025-02-13') + `); + } finally { + client.release(); + } +}); + +afterAll(async () => { + await pool.end(); + await db.destroy(); +}); + +describe('fetchReportData', () => { + test('returns approved expenditures only', async () => { + const data = await fetchReportData(1); + expect(data).not.toBeNull(); + expect(data!.expenditures).toHaveLength(1); + expect(data!.expenditures[0].description).toBe('approved'); + }); + + test('the total the report prints covers approved expenditures only', async () => { + const data = await fetchReportData(1); + const total = data!.expenditures.reduce((sum, e) => sum + parseFloat(e.amount), 0); + expect(total).toBe(1000); + }); +});