Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions apps/backend/db/seed.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
33 changes: 24 additions & 9 deletions apps/backend/lambdas/expenditures/test/expenditures.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
19 changes: 16 additions & 3 deletions apps/backend/lambdas/projects/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -80,6 +84,7 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
] = 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(),
Expand All @@ -89,6 +94,7 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
.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)
Expand All @@ -102,6 +108,7 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
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'))
Expand All @@ -115,6 +122,7 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
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'),
Expand All @@ -140,6 +148,7 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
.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)
Expand Down Expand Up @@ -325,7 +334,10 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
]);

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;
Expand All @@ -339,7 +351,7 @@ export const handler = async (event: any): Promise<APIGatewayProxyResult> => {
spentPercentage: Number(spentPercentage.toFixed(2)),
totalDonated: Number(donationRow?.total ?? 0),
memberCount: members.length,
expenditureCount: expenditures.length,
expenditureCount: approved.length,
},
members,
expenditures,
Expand Down Expand Up @@ -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(),
Expand Down
8 changes: 8 additions & 0 deletions apps/backend/lambdas/projects/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ paths:
properties:
total_spent:
type: number
description: Approved expenditures only.
member_count:
type: integer
is_active:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
153 changes: 153 additions & 0 deletions apps/backend/lambdas/projects/test/approved-expenditures.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof import('../auth')>('../auth'),
authenticateRequest: jest.fn(),
}));

import { handler } from '../handler';
import db from '../db';
import { authenticateRequest } from '../auth';

const mockAuthenticateRequest = authenticateRequest as jest.MockedFunction<typeof authenticateRequest>;

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',
]);
});
});
6 changes: 4 additions & 2 deletions apps/backend/lambdas/projects/test/projects.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
7 changes: 7 additions & 0 deletions apps/backend/lambdas/projects/validation-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading