From ee1ebb5b00d9d3539ba0832f613fd5201cf9d023 Mon Sep 17 00:00:00 2001 From: nourshoreibah Date: Tue, 11 Aug 2026 22:01:13 -0400 Subject: [PATCH 1/6] feat(dashboard): admin-only dashboard from Figma MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebuilds /dashboard to the Figma design: four summary stat cards, a three-project preview with a View All link, and a stacked monthly expenses chart. The dashboard is now admin-only, matching the design's "Dashboard (admin view only)" frame. /dashboard joins ADMIN_PREFIXES and the nav item is gated on the admin role. That gate breaks the landing route, which was /dashboard for every role: a non-admin would have been dropped on the no-access panel the instant they signed in. The landing path is now role-aware via landingPathFor(), with DEFAULT_LANDING_PATH (/projects) reachable by everyone. The login page hands off to "/" rather than naming a page, because isAdmin is not known until GET /auth/me returns. Backend: GET /projects/dashboard already existed and was already admin-gated. Scoped its spend aggregates to the current calendar year and its project count to projects that have not ended, so the figures match the card captions ("this year", "active projects"), and added the top category's percentage — the client cannot derive it safely because totalSpent is the divisor and may be zero. The per-project budget breakdown stays lifetime-to-date on purpose. Chart is hand-rolled; no charting library is a dependency of this app. Categories are matched with whitespace stripped, because the expenses form writes "Visitor / Honorarium" while the design names the band "Visitor/Honorarium" — comparing literally split one category across two colours. Unrecognised categories still render, with a fallback colour, rather than being silently dropped from the totals. Layout is fluid rather than a transcription of the design's fixed 1280px frame: the stat and project grids reflow at sm/md/xl, the chart legend moves from a column beside the plot to a wrapping row above it, and the plot height is a clamp() so the bars scale with the viewport. Also fixes two pre-existing `next build` failures. Pages may only export Next's own fields, and both accounts/page.tsx and expenses/page.tsx exported extra values, so the production build was broken on main. Drive-by: ProjectCard rendered "NaN%" for a project with no budget set. Co-Authored-By: Claude Opus 5 (1M context) --- apps/backend/lambdas/projects/handler.ts | 39 +++- .../projects/test/dashboard.unit.test.ts | 26 ++- apps/frontend/src/app/accounts/page.tsx | 25 +-- apps/frontend/src/app/accounts/staff.ts | 24 +++ apps/frontend/src/app/components/AuthGate.tsx | 11 +- .../src/app/components/ExpensesBarChart.tsx | 182 ++++++++++++++++++ apps/frontend/src/app/components/Navbar.tsx | 2 +- .../src/app/components/ProjectCard.tsx | 37 ++-- .../src/app/components/SummaryStatCard.tsx | 28 +++ apps/frontend/src/app/dashboard/page.tsx | 146 +++++++++----- apps/frontend/src/app/expenses/page.tsx | 3 +- apps/frontend/src/app/login/page.tsx | 4 +- apps/frontend/src/app/page.tsx | 17 +- apps/frontend/src/lib/routes.ts | 22 ++- apps/frontend/src/types/dashboard.ts | 40 ++++ apps/frontend/src/types/index.ts | 1 + apps/frontend/test/app/RootPage.test.tsx | 12 +- .../test/components/AccountsPage.test.tsx | 3 +- .../test/components/AuthGate.test.tsx | 25 ++- .../test/components/DashboardPage.test.tsx | 129 +++++++++++++ .../test/components/ExpensesBarChart.test.tsx | 90 +++++++++ .../test/components/LoginPage.test.tsx | 10 +- apps/frontend/test/components/Navbar.test.tsx | 8 +- apps/frontend/test/lib/routes.test.ts | 45 +++-- 24 files changed, 796 insertions(+), 133 deletions(-) create mode 100644 apps/frontend/src/app/accounts/staff.ts create mode 100644 apps/frontend/src/app/components/ExpensesBarChart.tsx create mode 100644 apps/frontend/src/app/components/SummaryStatCard.tsx create mode 100644 apps/frontend/src/types/dashboard.ts create mode 100644 apps/frontend/test/components/DashboardPage.test.tsx create mode 100644 apps/frontend/test/components/ExpensesBarChart.test.tsx diff --git a/apps/backend/lambdas/projects/handler.ts b/apps/backend/lambdas/projects/handler.ts index ab5d0d9a..92fb6d85 100644 --- a/apps/backend/lambdas/projects/handler.ts +++ b/apps/backend/lambdas/projects/handler.ts @@ -46,6 +46,15 @@ export const handler = async (event: any): Promise => { } try { + // Cards read "this year" / "active projects", so spend is scoped to the + // calendar year and the count to projects that have not ended. The + // per-project budget breakdown below stays lifetime-to-date. + const now = new Date(); + const year = now.getUTCFullYear(); + const yearStart = `${year}-01-01`; + const yearEnd = `${year}-12-31`; + const today = now.toISOString().slice(0, 10); + const [ totalSpentRow, totalProjectsRow, @@ -57,13 +66,23 @@ export const handler = async (event: any): Promise => { ] = await Promise.all([ db.selectFrom('branch.expenditures') .select(db.fn.sum('amount').as('total')) + .where('spent_on', '>=', yearStart as any) + .where('spent_on', '<=', yearEnd as any) .executeTakeFirst(), db.selectFrom('branch.projects') .select(db.fn.count('project_id').as('count')) + .where((eb) => + eb.or([ + eb('end_date', 'is', null), + eb('end_date', '>=', today as any), + ]), + ) .executeTakeFirst(), db.selectFrom('branch.expenditures') .select(['category', db.fn.sum('amount').as('total')]) .where('category', 'is not', null) + .where('spent_on', '>=', yearStart as any) + .where('spent_on', '<=', yearEnd as any) .groupBy('category') .orderBy(db.fn.sum('amount'), 'desc') .limit(1) @@ -83,6 +102,8 @@ export const handler = async (event: any): Promise => { db.selectFrom('branch.expenditures') .select(['spent_on', 'category', 'amount']) .where('category', 'is not', null) + .where('spent_on', '>=', yearStart as any) + .where('spent_on', '<=', yearEnd as any) .execute(), ]); @@ -120,11 +141,23 @@ export const handler = async (event: any): Promise => { } const expensesByMonth = [...monthMap.values()].sort((a, b) => a.month.localeCompare(b.month)); + // Computed here, not client-side: totalSpent is the divisor and may be 0. + const topCategoryAmount = Number(topCategoryRow?.total ?? 0); + const topExpenseCategory = topCategoryRow + ? { + category: topCategoryRow.category, + amount: topCategoryAmount, + percentage: + totalSpent > 0 + ? Number(((topCategoryAmount / totalSpent) * 100).toFixed(2)) + : 0, + } + : null; + return json(200, { + year, summary: { - topExpenseCategory: topCategoryRow - ? { category: topCategoryRow.category, amount: Number(topCategoryRow.total ?? 0) } - : null, + topExpenseCategory, totalSpent, totalProjects, averageSpendPerProject: Number(averageSpendPerProject.toFixed(2)), diff --git a/apps/backend/lambdas/projects/test/dashboard.unit.test.ts b/apps/backend/lambdas/projects/test/dashboard.unit.test.ts index 5caf455c..683bc6bf 100644 --- a/apps/backend/lambdas/projects/test/dashboard.unit.test.ts +++ b/apps/backend/lambdas/projects/test/dashboard.unit.test.ts @@ -119,7 +119,16 @@ describe('GET /dashboard unit tests', () => { expect(body.summary.totalSpent).toBe(18000); expect(body.summary.totalProjects).toBe(4); expect(body.summary.averageSpendPerProject).toBe(4500); - expect(body.summary.topExpenseCategory).toEqual({ category: 'Travel', amount: 6800 }); + expect(body.summary.topExpenseCategory).toEqual({ + category: 'Travel', + amount: 6800, + percentage: 37.78, + }); + }); + + test('200: response is stamped with the year the aggregates cover', async () => { + const res = await handler(getEvent()); + expect(JSON.parse(res.body).year).toBe(new Date().getUTCFullYear()); }); test('200: projects breakdown joins spent and staff_count by project_id', async () => { @@ -180,6 +189,21 @@ describe('GET /dashboard unit tests', () => { expect(body.expensesByMonth).toEqual([]); }); + test('200: top category percentage is 0 rather than NaN when nothing was spent', async () => { + mockDb.selectFrom = jest.fn(); + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '0' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ count: '2' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ category: 'Travel', total: '0' })); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + + const res = await handler(getEvent()); + const body = JSON.parse(res.body); + expect(body.summary.topExpenseCategory.percentage).toBe(0); + }); + test('500: db failure surfaces as 500', async () => { mockDb.selectFrom = jest.fn().mockImplementation(() => { throw new Error('boom'); diff --git a/apps/frontend/src/app/accounts/page.tsx b/apps/frontend/src/app/accounts/page.tsx index 1c5cd0f4..f84aee69 100644 --- a/apps/frontend/src/app/accounts/page.tsx +++ b/apps/frontend/src/app/accounts/page.tsx @@ -2,30 +2,7 @@ import React from 'react'; import StaffCard from '../components/StaffCard'; -import { User } from '@/types'; - -const mockUsers: User[] = [ - { user_id: 1, name: 'Mehana Nagarur', email: 'nagarur.m@northeastern.edu', is_admin: true }, - { user_id: 2, name: 'Alex Rivera', email: 'rivera.a@northeastern.edu', is_admin: true }, - { user_id: 3, name: 'Jordan Lee', email: 'lee.j@northeastern.edu', is_admin: true }, - { user_id: 4, name: 'Priya Sharma', email: 'sharma.p@northeastern.edu', is_admin: true }, - { user_id: 5, name: 'Chris Nguyen', email: 'nguyen.c@northeastern.edu', is_admin: true }, - { user_id: 6, name: 'Taylor Brooks', email: 'brooks.t@northeastern.edu', is_admin: false }, - { user_id: 7, name: 'Sam Patel', email: 'patel.s@northeastern.edu', is_admin: false }, - { user_id: 8, name: 'Morgan Clarke', email: 'clarke.m@northeastern.edu', is_admin: false }, - { user_id: 9, name: 'Jamie Wu', email: 'wu.j@northeastern.edu', is_admin: false }, - { user_id: 10, name: 'Riley Thompson', email: 'thompson.r@northeastern.edu',is_admin: false }, - { user_id: 11, name: 'Avery Johnson', email: 'johnson.a@northeastern.edu', is_admin: false }, - { user_id: 12, name: 'Casey Martinez', email: 'martinez.c@northeastern.edu',is_admin: false }, - { user_id: 13, name: 'Drew Hassan', email: 'hassan.d@northeastern.edu', is_admin: false }, - { user_id: 14, name: 'Quinn Okafor', email: 'okafor.q@northeastern.edu', is_admin: false }, - { user_id: 15, name: 'Blake Fernandez', email: 'fernandez.b@northeastern.edu',is_admin: false }, -]; - -export const facilitationTeam = mockUsers.filter(u => u.is_admin); -export const teamMembers = mockUsers.filter(u => !u.is_admin); - - +import { facilitationTeam, teamMembers } from './staff'; export default function AccountsPage() { return ( diff --git a/apps/frontend/src/app/accounts/staff.ts b/apps/frontend/src/app/accounts/staff.ts new file mode 100644 index 00000000..67c7f479 --- /dev/null +++ b/apps/frontend/src/app/accounts/staff.ts @@ -0,0 +1,24 @@ +import { User } from '@/types'; + +// Placeholder roster. Lives beside the page, not in it: Next.js rejects unknown +// exports from a page.tsx at build time (jest does not). +const mockUsers: User[] = [ + { user_id: 1, name: 'Mehana Nagarur', email: 'nagarur.m@northeastern.edu', is_admin: true }, + { user_id: 2, name: 'Alex Rivera', email: 'rivera.a@northeastern.edu', is_admin: true }, + { user_id: 3, name: 'Jordan Lee', email: 'lee.j@northeastern.edu', is_admin: true }, + { user_id: 4, name: 'Priya Sharma', email: 'sharma.p@northeastern.edu', is_admin: true }, + { user_id: 5, name: 'Chris Nguyen', email: 'nguyen.c@northeastern.edu', is_admin: true }, + { user_id: 6, name: 'Taylor Brooks', email: 'brooks.t@northeastern.edu', is_admin: false }, + { user_id: 7, name: 'Sam Patel', email: 'patel.s@northeastern.edu', is_admin: false }, + { user_id: 8, name: 'Morgan Clarke', email: 'clarke.m@northeastern.edu', is_admin: false }, + { user_id: 9, name: 'Jamie Wu', email: 'wu.j@northeastern.edu', is_admin: false }, + { user_id: 10, name: 'Riley Thompson', email: 'thompson.r@northeastern.edu',is_admin: false }, + { user_id: 11, name: 'Avery Johnson', email: 'johnson.a@northeastern.edu', is_admin: false }, + { user_id: 12, name: 'Casey Martinez', email: 'martinez.c@northeastern.edu',is_admin: false }, + { user_id: 13, name: 'Drew Hassan', email: 'hassan.d@northeastern.edu', is_admin: false }, + { user_id: 14, name: 'Quinn Okafor', email: 'okafor.q@northeastern.edu', is_admin: false }, + { user_id: 15, name: 'Blake Fernandez', email: 'fernandez.b@northeastern.edu',is_admin: false }, +]; + +export const facilitationTeam = mockUsers.filter(u => u.is_admin); +export const teamMembers = mockUsers.filter(u => !u.is_admin); diff --git a/apps/frontend/src/app/components/AuthGate.tsx b/apps/frontend/src/app/components/AuthGate.tsx index e1c9a917..63d89e42 100644 --- a/apps/frontend/src/app/components/AuthGate.tsx +++ b/apps/frontend/src/app/components/AuthGate.tsx @@ -4,9 +4,10 @@ import { useEffect } from 'react'; import { usePathname, useRouter } from 'next/navigation'; import { useAuth } from '@/context/AuthContext'; import { + DEFAULT_LANDING_PATH, LOGIN_PATH, - POST_LOGIN_PATH, classifyRoute, + landingPathFor, requiresAdmin, } from '@/lib/routes'; import FullPageSpinner from './FullPageSpinner'; @@ -46,9 +47,9 @@ export default function AuthGate({ children }: { children: React.ReactNode }) { } if (access === 'public' && isAuthenticated) { - router.replace(POST_LOGIN_PATH); + router.replace(landingPathFor(isAdmin)); } - }, [isLoading, isAuthenticated, access, pathname, router]); + }, [isLoading, isAuthenticated, isAdmin, access, pathname, router]); if (isLoading) return ; @@ -91,10 +92,10 @@ function NoAccessPanel() { access, ask an admin to update your account.

- Back to dashboard + Back to projects ); diff --git a/apps/frontend/src/app/components/ExpensesBarChart.tsx b/apps/frontend/src/app/components/ExpensesBarChart.tsx new file mode 100644 index 00000000..73205e79 --- /dev/null +++ b/apps/frontend/src/app/components/ExpensesBarChart.tsx @@ -0,0 +1,182 @@ +import React from 'react'; +import type { DashboardMonthlyExpense } from '@/types/dashboard'; + +/** Stacked monthly expenditure chart. Hand-rolled — no chart library is a dep. */ + +// Fluid height, not the design's fixed px. Everything inside the plot is sized +// in percentages so the columns stay in step with the gridlines. +const PLOT_HEIGHT_CLASS = 'h-[clamp(11rem,30vh,19rem)]'; +const TICK_INTERVALS = 4; + +const MONTH_LABELS = [ + 'Jan', 'Feb', 'March', 'April', 'May', 'June', + 'July', 'Aug', 'Sept', 'Oct', 'Nov', 'Dec', +]; + +// Categories the design names, darkest first. `expenditures.category` is free +// text, so unknown values get a fallback colour rather than being dropped. +const KNOWN_CATEGORIES: { label: string; color: string }[] = [ + { label: 'General', color: 'var(--color-primary-800)' }, + { label: 'Travel', color: 'var(--color-primary-600)' }, + { label: 'Travel Foreign', color: 'var(--color-primary-300)' }, + { label: 'Visitor/Honorarium', color: 'var(--color-primary-200)' }, +]; + +const FALLBACK_COLORS = [ + 'var(--color-primary-700)', + 'var(--color-primary-500)', + 'var(--color-primary-400)', + 'var(--color-primary-100)', +]; + +// Match on this, not the raw string: the expenses form writes +// "Visitor / Honorarium", the design names the band "Visitor/Honorarium". +function categoryKey(category: string): string { + return category.replace(/\s+/g, '').toLowerCase(); +} + +/** Rounds up to a readable axis maximum that divides evenly into ticks. */ +function axisMaxFor(peak: number): number { + if (peak <= 0) return TICK_INTERVALS; + const rawStep = peak / TICK_INTERVALS; + const magnitude = 10 ** Math.floor(Math.log10(rawStep)); + const step = + [1, 2, 2.5, 5, 10] + .map((m) => m * magnitude) + .find((candidate) => candidate >= rawStep) ?? 10 * magnitude; + return step * TICK_INTERVALS; +} + +function categoriesIn(expenses: DashboardMonthlyExpense[]) { + const known = new Set(KNOWN_CATEGORIES.map((c) => categoryKey(c.label))); + const extras = [ + ...new Map( + expenses + .filter((e) => !known.has(categoryKey(e.category))) + .map((e) => [categoryKey(e.category), e.category]), + ).values(), + ].sort(); + + return [ + ...KNOWN_CATEGORIES, + ...extras.map((label, i) => ({ + label, + color: FALLBACK_COLORS[i % FALLBACK_COLORS.length], + })), + ].map((c) => ({ ...c, key: categoryKey(c.label) })); +} + +interface ExpensesBarChartProps { + year: number; + expenses: DashboardMonthlyExpense[]; +} + +export default function ExpensesBarChart({ + year, + expenses, +}: ExpensesBarChartProps) { + const categories = categoriesIn(expenses); + + // The API only returns months that had spend; the axis always shows all 12. + const months = MONTH_LABELS.map((label, index) => { + const key = `${year}-${String(index + 1).padStart(2, '0')}`; + const amounts = new Map(); + for (const row of expenses) { + if (row.month !== key) continue; + const bucket = categoryKey(row.category); + amounts.set(bucket, (amounts.get(bucket) ?? 0) + row.amount); + } + return { label, amounts }; + }); + + const peak = Math.max( + 0, + ...months.map((m) => [...m.amounts.values()].reduce((a, b) => a + b, 0)), + ); + const axisMax = axisMaxFor(peak); + const ticks = Array.from( + { length: TICK_INTERVALS + 1 }, + (_, i) => (axisMax / TICK_INTERVALS) * i, + ); + + return ( +
+ {/* Stacks above the plot on narrow screens, sits beside it from lg up. */} +
    + {categories.map((category) => ( +
  • +
    • + ))} +
+ +
+
+ {ticks.map((tick, i) => ( + + {tick.toLocaleString()} + + ))} +
+ +
+ +
+ ); +} diff --git a/apps/frontend/src/app/components/Navbar.tsx b/apps/frontend/src/app/components/Navbar.tsx index b54fd96d..fa3bc373 100644 --- a/apps/frontend/src/app/components/Navbar.tsx +++ b/apps/frontend/src/app/components/Navbar.tsx @@ -20,7 +20,7 @@ interface NavItem { label: string; href?: string; action?: "logout"; roles?: Use // keying the special case on `action` means a future /logout page couldn't // silently turn the button back into a dead link. const NAV_ITEMS: NavItem[] = [ - { label: "Dashboard", href: "/dashboard" }, + { label: "Dashboard", href: "/dashboard", roles: ["admin"] }, { label: "Projects", href: "/projects" }, { label: "Donors", href: "/donors" }, { label: "Donations", href: "/donations" }, diff --git a/apps/frontend/src/app/components/ProjectCard.tsx b/apps/frontend/src/app/components/ProjectCard.tsx index c27adbbe..c360ed61 100644 --- a/apps/frontend/src/app/components/ProjectCard.tsx +++ b/apps/frontend/src/app/components/ProjectCard.tsx @@ -20,11 +20,17 @@ type ArchiveProps = { end_date: string; }; -type ProjectCardProps = ActiveProps | ArchiveProps; +// `fullWidth` hands sizing to the parent: the responsive widths below are tuned +// for the projects list and would shrink inside a grid cell. +type ProjectCardProps = (ActiveProps | ArchiveProps) & { fullWidth?: boolean }; export default function ProjectCard(props: ProjectCardProps) { + const widthClasses = props.fullWidth + ? 'w-full' + : 'w-full sm:w-[50%] md:w-[35%] lg:w-[25%]'; + return ( -
+

{props.name}

@@ -48,15 +54,24 @@ export default function ProjectCard(props: ProjectCardProps) {
{props.variant === 'active' ? ( -
-
-
-
-

{Math.round((props.budget_used / props.total_budget) * 100)}%

-
+ // A project with no budget set divides by zero; show 0% rather + // than "NaN%" and a bar of unset width. + (() => { + const percentUsed = props.total_budget > 0 + ? Math.round((props.budget_used / props.total_budget) * 100) + : 0; + return ( +
+
+
+
+

{percentUsed}%

+
+ ); + })() ) : (
diff --git a/apps/frontend/src/app/components/SummaryStatCard.tsx b/apps/frontend/src/app/components/SummaryStatCard.tsx new file mode 100644 index 00000000..99dad607 --- /dev/null +++ b/apps/frontend/src/app/components/SummaryStatCard.tsx @@ -0,0 +1,28 @@ +import React from 'react'; + +interface SummaryStatCardProps { + label: string; + value: string; + caption: string; +} + +/** One of the four figures across the top of the admin dashboard. */ +export default function SummaryStatCard({ + label, + value, + caption, +}: SummaryStatCardProps) { + return ( + // `!` on both border width and colour — Chakra's reset outranks plain + // Tailwind border utilities. +
+

{label}

+ {/* Not an

— the headings on this page mark its sections. Wraps + instead of truncating: a fluid grid can't guarantee width. */} +

+ {value} +

+ {caption} +

+ ); +} diff --git a/apps/frontend/src/app/dashboard/page.tsx b/apps/frontend/src/app/dashboard/page.tsx index f5dfd992..dfe5e999 100644 --- a/apps/frontend/src/app/dashboard/page.tsx +++ b/apps/frontend/src/app/dashboard/page.tsx @@ -2,37 +2,39 @@ import { useCallback, useEffect, useState } from 'react'; import Link from 'next/link'; +import { LuChevronRight } from 'react-icons/lu'; import NavBar from '../components/Navbar'; import Header from '../components/Header'; import ProjectCard from '../components/ProjectCard'; +import SummaryStatCard from '../components/SummaryStatCard'; +import ExpensesBarChart from '../components/ExpensesBarChart'; import { useApi } from '@/hooks/useApi'; -import { useAuth } from '@/context/AuthContext'; +import type { DashboardResponse } from '@/types/dashboard'; /** - * Landing page for a signed-in user, and the target the login flow redirects to. - * - * The Navbar has always linked here; the route simply never existed. + * Admin-only overview of spend and projects. Gated twice: `/dashboard` is in + * `ADMIN_PREFIXES`, and `GET /projects/dashboard` checks isAdmin server side. */ -interface ProjectRow { - project_id: number; - name: string; - total_budget: number | string | null; +/** How many project cards the design shows before "View All". */ +const PROJECT_PREVIEW_COUNT = 3; + +function formatMoney(amount: number): string { + return `$${Math.round(amount).toLocaleString()}`; } export default function DashboardPage() { const api = useApi(); - const { user } = useAuth(); - const [projects, setProjects] = useState([]); + const [data, setData] = useState(null); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(true); const load = useCallback(async () => { try { setError(null); - setProjects(await api.get('/projects')); + setData(await api.get('/projects/dashboard')); } catch (err) { - setError(err instanceof Error ? err.message : 'Could not load projects'); + setError(err instanceof Error ? err.message : 'Could not load dashboard'); } finally { setIsLoading(false); } @@ -42,54 +44,94 @@ export default function DashboardPage() { void load(); }, [load]); - const firstName = user?.name?.split(' ')[0]; + const summary = data?.summary; + const topCategory = summary?.topExpenseCategory; return (
-
+
-
-

- {firstName ? `Welcome back, ${firstName}` : 'Dashboard'} -

+
+ {isLoading &&

Loading dashboard…

} + {error &&

{error}

} - {isLoading &&

Loading projects…

} - {error &&

{error}

} - {!isLoading && !error && projects.length === 0 && ( -

You are not a member of any projects yet.

- )} + {!isLoading && !error && data && summary && ( + <> +
+ + + + +
+ +
+
+

Projects

+ +
-
- {projects.map((project) => ( - - No projects yet.

+ ) : ( +
+ {data.projects + .slice(0, PROJECT_PREVIEW_COUNT) + .map((project) => ( + + + + ))} +
+ )} +
+ +
+

Total Expenses

+ - - ))} -
+ + + )}
diff --git a/apps/frontend/src/app/expenses/page.tsx b/apps/frontend/src/app/expenses/page.tsx index e6432961..2c074398 100644 --- a/apps/frontend/src/app/expenses/page.tsx +++ b/apps/frontend/src/app/expenses/page.tsx @@ -26,7 +26,8 @@ const MONTHS = [ const SORT_OPTIONS = ['Amount', 'Date']; const ROWS_PER_PAGE = 10; -export const EXPENSE_CATEGORIES = [ +// Not exported: Next.js rejects unknown exports from a page module. +const EXPENSE_CATEGORIES = [ 'General', 'Travel', 'Travel Foreign', diff --git a/apps/frontend/src/app/login/page.tsx b/apps/frontend/src/app/login/page.tsx index a996fa24..cd3e8338 100644 --- a/apps/frontend/src/app/login/page.tsx +++ b/apps/frontend/src/app/login/page.tsx @@ -20,7 +20,9 @@ function LoginPageContent() { // Where to land after signing in. AuthGate sets ?next= when it bounces an // unauthenticated user off a protected page; safeNextPath rejects anything // that isn't a same-origin path, so a crafted link can't redirect offsite. - const next = safeNextPath(searchParams.get('next')); + // With no ?next= we hand off to "/" rather than naming a page: the landing + // route depends on isAdmin, which only arrives with GET /auth/me. + const next = safeNextPath(searchParams.get('next'), '/'); const [email, setEmail] = useState(''); const [password, setPassword] = useState(''); diff --git a/apps/frontend/src/app/page.tsx b/apps/frontend/src/app/page.tsx index c5eb464e..39ef3958 100644 --- a/apps/frontend/src/app/page.tsx +++ b/apps/frontend/src/app/page.tsx @@ -3,7 +3,12 @@ import { useEffect, useState } from 'react'; import { useRouter } from 'next/navigation'; import { useAuth } from '@/context/AuthContext'; -import { LOGIN_PATH, POST_LOGIN_PATH, normalizePath } from '@/lib/routes'; +import { + DEFAULT_LANDING_PATH, + LOGIN_PATH, + landingPathFor, + normalizePath, +} from '@/lib/routes'; import FullPageSpinner from './components/FullPageSpinner'; /** @@ -26,7 +31,7 @@ import FullPageSpinner from './components/FullPageSpinner'; const SPA_FALLBACK_KEY = 'branch_spa_fallback_path'; export default function RootPage() { - const { isAuthenticated, isLoading } = useAuth(); + const { isAuthenticated, isAdmin, isLoading } = useAuth(); const router = useRouter(); const [notFound, setNotFound] = useState(false); @@ -58,8 +63,8 @@ export default function RootPage() { // A genuine visit to "/". Route by session, but only once it is known. if (isLoading) return; - router.replace(isAuthenticated ? POST_LOGIN_PATH : LOGIN_PATH); - }, [isLoading, isAuthenticated, router]); + router.replace(isAuthenticated ? landingPathFor(isAdmin) : LOGIN_PATH); + }, [isLoading, isAuthenticated, isAdmin, router]); if (notFound) return ; return ; @@ -87,10 +92,10 @@ function NotFoundPanel() { That link doesn't point anywhere in BRANCH.

- Back to dashboard + Back to projects
); diff --git a/apps/frontend/src/lib/routes.ts b/apps/frontend/src/lib/routes.ts index 610a3003..a98290f6 100644 --- a/apps/frontend/src/lib/routes.ts +++ b/apps/frontend/src/lib/routes.ts @@ -10,7 +10,18 @@ export type RouteAccess = 'public' | 'protected' | 'bootstrap'; export const LOGIN_PATH = '/login'; -export const POST_LOGIN_PATH = '/dashboard'; + +/** + * Landing route when nothing more specific applies. Must stay reachable by every + * role — it was `/dashboard`, which dropped non-admins on the no-access panel + * once that page became admin-only. Prefer `landingPathFor()` when the role is known. + */ +export const DEFAULT_LANDING_PATH = '/projects'; +export const ADMIN_LANDING_PATH = '/dashboard'; + +export function landingPathFor(isAdmin: boolean): string { + return isAdmin ? ADMIN_LANDING_PATH : DEFAULT_LANDING_PATH; +} /** Reachable without a session. Authenticated users get bounced off these. */ const PUBLIC_PREFIXES = [ @@ -20,7 +31,12 @@ const PUBLIC_PREFIXES = [ ] as const; /** Require `isAdmin` on top of authentication. */ -const ADMIN_PREFIXES = ['/expenses', '/reports', '/accounts'] as const; +const ADMIN_PREFIXES = [ + '/dashboard', + '/expenses', + '/reports', + '/accounts', +] as const; /** * Strips the trailing slash and lowercases. @@ -64,7 +80,7 @@ export function requiresAdmin(pathname: string): boolean { */ export function safeNextPath( raw: string | null | undefined, - fallback = POST_LOGIN_PATH, + fallback = DEFAULT_LANDING_PATH, ): string { if (!raw) return fallback; if (!raw.startsWith('/')) return fallback; diff --git a/apps/frontend/src/types/dashboard.ts b/apps/frontend/src/types/dashboard.ts new file mode 100644 index 00000000..e20798ab --- /dev/null +++ b/apps/frontend/src/types/dashboard.ts @@ -0,0 +1,40 @@ +/** Shape of `GET /projects/dashboard` (admin only). */ + +export interface DashboardTopCategory { + category: string; + amount: number; + percentage: number; +} + +export interface DashboardSummary { + /** Null when nothing has been spent in `year`. */ + topExpenseCategory: DashboardTopCategory | null; + totalSpent: number; + totalProjects: number; + averageSpendPerProject: number; +} + +export interface DashboardProject { + project_id: number; + name: string; + total_budget: number | null; + currency: string | null; + spent: number; + staff_count: number; + spent_percentage: number; +} + +export interface DashboardMonthlyExpense { + /** `YYYY-MM`. Only months with expenditures are present. */ + month: string; + category: string; + amount: number; +} + +export interface DashboardResponse { + /** Calendar year the spend aggregates cover. */ + year: number; + summary: DashboardSummary; + projects: DashboardProject[]; + expensesByMonth: DashboardMonthlyExpense[]; +} diff --git a/apps/frontend/src/types/index.ts b/apps/frontend/src/types/index.ts index 231818ef..cf1a028d 100644 --- a/apps/frontend/src/types/index.ts +++ b/apps/frontend/src/types/index.ts @@ -1,3 +1,4 @@ +export * from './dashboard'; export * from './project'; export * from './expenditure'; export * from './user'; diff --git a/apps/frontend/test/app/RootPage.test.tsx b/apps/frontend/test/app/RootPage.test.tsx index ac2ac9e4..68bcbfec 100644 --- a/apps/frontend/test/app/RootPage.test.tsx +++ b/apps/frontend/test/app/RootPage.test.tsx @@ -49,12 +49,20 @@ describe('RootPage', () => { await waitFor(() => expect(mockReplace).toHaveBeenCalledWith('/login')); }); - it('sends an authenticated user to /dashboard', async () => { - authState = { isAuthenticated: true, isAdmin: false, isLoading: false }; + it('sends an authenticated admin to /dashboard', async () => { + authState = { isAuthenticated: true, isAdmin: true, isLoading: false }; render(); await waitFor(() => expect(mockReplace).toHaveBeenCalledWith('/dashboard')); }); + it('sends an authenticated non-admin somewhere they can load', async () => { + // /dashboard is admin-only, so routing every session there would land a + // non-admin on the no-access panel straight off the root route. + authState = { isAuthenticated: true, isAdmin: false, isLoading: false }; + render(); + await waitFor(() => expect(mockReplace).toHaveBeenCalledWith('/projects')); + }); + it('waits for the session before routing', () => { authState = { isAuthenticated: false, isAdmin: false, isLoading: true }; render(); diff --git a/apps/frontend/test/components/AccountsPage.test.tsx b/apps/frontend/test/components/AccountsPage.test.tsx index 1f9e6c90..89f80e43 100644 --- a/apps/frontend/test/components/AccountsPage.test.tsx +++ b/apps/frontend/test/components/AccountsPage.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from '../utils'; -import AccountsPage, { facilitationTeam, teamMembers } from '@/app/accounts/page'; +import AccountsPage from '@/app/accounts/page'; +import { facilitationTeam, teamMembers } from '@/app/accounts/staff'; describe('AccountsPage', () => { it('renders the headings', () => { diff --git a/apps/frontend/test/components/AuthGate.test.tsx b/apps/frontend/test/components/AuthGate.test.tsx index 9dd4cbbf..55d7e106 100644 --- a/apps/frontend/test/components/AuthGate.test.tsx +++ b/apps/frontend/test/components/AuthGate.test.tsx @@ -5,7 +5,7 @@ import AuthGate from '@/app/components/AuthGate'; // jest.setup.ts returns a fresh router spy on every call, so redirects need a // local mock with stable spies and a mutable pathname. const mockReplace = jest.fn(); -let currentPath = '/dashboard'; +let currentPath = '/projects'; // Stable object: the real useRouter returns a stable reference, and returning a // fresh one here would re-run effects that list `router` as a dependency. @@ -34,7 +34,7 @@ function renderGate() { beforeEach(() => { jest.clearAllMocks(); - currentPath = '/dashboard'; + currentPath = '/projects'; authState = { isAuthenticated: false, isAdmin: false, isLoading: false }; }); @@ -59,6 +59,15 @@ describe('AuthGate', () => { expect(mockReplace).toHaveBeenCalledWith('/login?next=%2Fexpenses'); }); + it('keeps the dashboard behind the admin flag', () => { + currentPath = '/dashboard'; + authState = { isAuthenticated: true, isAdmin: false, isLoading: false }; + renderGate(); + + expect(screen.getByText(/don't have access to this page/i)).toBeInTheDocument(); + expect(screen.queryByTestId('protected-content')).not.toBeInTheDocument(); + }); + it('does not render protected children to an anonymous visitor', () => { // The redirect is asynchronous; without render suppression the page would // mount and start fetching in the meantime. @@ -77,11 +86,19 @@ describe('AuthGate', () => { }); describe('public routes', () => { - it('bounces an authenticated user off /login', () => { + it('bounces an authenticated non-admin off /login to a page they can load', () => { currentPath = '/login'; authState = { isAuthenticated: true, isAdmin: false, isLoading: false }; renderGate(); + expect(mockReplace).toHaveBeenCalledWith('/projects'); + }); + + it('bounces an authenticated admin off /login to the dashboard', () => { + currentPath = '/login'; + authState = { isAuthenticated: true, isAdmin: true, isLoading: false }; + renderGate(); + expect(mockReplace).toHaveBeenCalledWith('/dashboard'); }); @@ -134,7 +151,7 @@ describe('AuthGate', () => { it('classifies production-style paths the same as dev ones', () => { // next.config.ts sets trailingSlash: true, so production emits "/login/". currentPath = '/login/'; - authState = { isAuthenticated: true, isAdmin: false, isLoading: false }; + authState = { isAuthenticated: true, isAdmin: true, isLoading: false }; renderGate(); expect(mockReplace).toHaveBeenCalledWith('/dashboard'); diff --git a/apps/frontend/test/components/DashboardPage.test.tsx b/apps/frontend/test/components/DashboardPage.test.tsx new file mode 100644 index 00000000..dc189ba5 --- /dev/null +++ b/apps/frontend/test/components/DashboardPage.test.tsx @@ -0,0 +1,129 @@ +import { render, screen, waitFor } from '../utils'; +import DashboardPage from '@/app/dashboard/page'; +import type { DashboardResponse } from '@/types/dashboard'; + +const mockApiFetch = jest.fn(); +jest.mock('../../src/lib/authClient', () => ({ + ...jest.requireActual('../../src/lib/authClient'), + authedFetch: (...args: Parameters) => mockApiFetch(...args), +})); + +jest.mock('next/navigation', () => ({ + useRouter: jest.fn(() => ({ push: jest.fn(), replace: jest.fn() })), + usePathname: jest.fn(() => '/dashboard'), + useSearchParams: jest.fn(() => new URLSearchParams()), +})); + +const response: DashboardResponse = { + year: 2026, + summary: { + topExpenseCategory: { + category: 'Visitor/Honorarium', + amount: 90000, + percentage: 30, + }, + totalSpent: 300000, + totalProjects: 6, + averageSpendPerProject: 12000, + }, + projects: [ + { project_id: 1, name: 'Alpha', total_budget: 100000, currency: 'USD', spent: 30000, staff_count: 3, spent_percentage: 30 }, + { project_id: 2, name: 'Beta', total_budget: 100000, currency: 'USD', spent: 30000, staff_count: 3, spent_percentage: 30 }, + { project_id: 3, name: 'Gamma', total_budget: 100000, currency: 'USD', spent: 30000, staff_count: 3, spent_percentage: 30 }, + { project_id: 4, name: 'Delta', total_budget: 100000, currency: 'USD', spent: 10000, staff_count: 1, spent_percentage: 10 }, + ], + expensesByMonth: [ + { month: '2026-01', category: 'General', amount: 4000 }, + { month: '2026-01', category: 'Travel', amount: 2000 }, + { month: '2026-05', category: 'Visitor/Honorarium', amount: 1000 }, + ], +}; + +beforeEach(() => { + jest.clearAllMocks(); + mockApiFetch.mockResolvedValue(response); +}); + +describe('Dashboard Page', () => { + it('reads the admin dashboard aggregate rather than the plain project list', async () => { + render(); + await waitFor(() => + expect(mockApiFetch).toHaveBeenCalledWith( + '/projects/dashboard', + expect.anything(), + ), + ); + }); + + it('renders the four summary figures with their captions', async () => { + render(); + + await waitFor(() => + expect(screen.getByText('TOP EXPENSE CATEGORY')).toBeInTheDocument(), + ); + // Also the chart legend's lightest band, hence getAllByText. + expect(screen.getAllByText('Visitor/Honorarium').length).toBeGreaterThan(0); + expect(screen.getByText('30% of expenses')).toBeInTheDocument(); + + expect(screen.getByText('TOTAL SPENT')).toBeInTheDocument(); + expect(screen.getByText('$300,000')).toBeInTheDocument(); + expect(screen.getByText('this year')).toBeInTheDocument(); + + expect(screen.getByText('TOTAL PROJECTS')).toBeInTheDocument(); + expect(screen.getByText('6')).toBeInTheDocument(); + expect(screen.getByText('active projects')).toBeInTheDocument(); + + expect(screen.getByText('AVG SPEND/PROJECT')).toBeInTheDocument(); + expect(screen.getByText('$12,000')).toBeInTheDocument(); + expect(screen.getByText('per project')).toBeInTheDocument(); + }); + + it('previews three projects and links the rest behind View All', async () => { + render(); + + await waitFor(() => expect(screen.getByText('Alpha')).toBeInTheDocument()); + expect(screen.getByText('Beta')).toBeInTheDocument(); + expect(screen.getByText('Gamma')).toBeInTheDocument(); + expect(screen.queryByText('Delta')).not.toBeInTheDocument(); + + expect(screen.getByRole('link', { name: /View All/ })).toHaveAttribute( + 'href', + '/projects', + ); + }); + + it('renders the expenses chart with every category in the legend', async () => { + render(); + + await waitFor(() => + expect( + screen.getByRole('heading', { name: 'Total Expenses' }), + ).toBeInTheDocument(), + ); + for (const label of ['General', 'Travel', 'Travel Foreign']) { + expect(screen.getByText(label)).toBeInTheDocument(); + } + }); + + it('falls back to a placeholder when nothing has been spent', async () => { + mockApiFetch.mockResolvedValue({ + ...response, + summary: { ...response.summary, topExpenseCategory: null }, + }); + render(); + + await waitFor(() => + expect(screen.getByText('no expenses yet')).toBeInTheDocument(), + ); + }); + + it('surfaces a load failure instead of rendering empty cards', async () => { + mockApiFetch.mockRejectedValue(new Error('Admin access required')); + render(); + + await waitFor(() => + expect(screen.getByText('Admin access required')).toBeInTheDocument(), + ); + expect(screen.queryByText('TOTAL SPENT')).not.toBeInTheDocument(); + }); +}); diff --git a/apps/frontend/test/components/ExpensesBarChart.test.tsx b/apps/frontend/test/components/ExpensesBarChart.test.tsx new file mode 100644 index 00000000..0c829fbe --- /dev/null +++ b/apps/frontend/test/components/ExpensesBarChart.test.tsx @@ -0,0 +1,90 @@ +import { render, screen } from '../utils'; +import ExpensesBarChart from '@/app/components/ExpensesBarChart'; + +describe('ExpensesBarChart', () => { + it('labels every other month, as the design does', () => { + render(); + + for (const shown of ['Jan', 'March', 'May', 'July', 'Sept', 'Nov']) { + expect(screen.getByText(shown)).toBeInTheDocument(); + } + for (const hidden of ['Feb', 'April', 'June', 'Dec']) { + expect(screen.queryByText(hidden)).not.toBeInTheDocument(); + } + }); + + it('rounds the axis up to readable ticks above the tallest column', () => { + render( + , + ); + + for (const tick of ['0', '2,000', '4,000', '6,000', '8,000']) { + expect(screen.getByText(tick)).toBeInTheDocument(); + } + }); + + it('always lists the four designed categories, even with no data', () => { + render(); + + for (const label of [ + 'General', + 'Travel', + 'Travel Foreign', + 'Visitor/Honorarium', + ]) { + expect(screen.getByText(label)).toBeInTheDocument(); + } + }); + + it('folds a spacing variant onto the category the design named', () => { + // The expenses form writes "Visitor / Honorarium"; the design's legend says + // "Visitor/Honorarium". Treating them as two categories would double-count + // the band and colour half of it with a fallback. + render( + , + ); + + expect(screen.getByText('Visitor/Honorarium')).toBeInTheDocument(); + expect(screen.queryByText('Visitor / Honorarium')).not.toBeInTheDocument(); + }); + + it('keeps categories the database holds but the design never named', () => { + // expenditures.category is free text, so dropping unknown values would make + // the columns disagree with the Total Spent figure. + render( + , + ); + + expect(screen.getByText('Equipment')).toBeInTheDocument(); + }); + + it('ignores months belonging to another year', () => { + const { container } = render( + , + ); + + // Nothing from 2025 should size the axis. + expect(screen.getByText('4')).toBeInTheDocument(); + expect(container.querySelectorAll('[style*="background-color"]').length).toBe( + // legend swatches only, no column segments + 4, + ); + }); +}); diff --git a/apps/frontend/test/components/LoginPage.test.tsx b/apps/frontend/test/components/LoginPage.test.tsx index f214e5df..2258f520 100644 --- a/apps/frontend/test/components/LoginPage.test.tsx +++ b/apps/frontend/test/components/LoginPage.test.tsx @@ -76,7 +76,9 @@ describe('Login Page Component', () => { }); describe('submitting', () => { - it('signs in and redirects to the dashboard', async () => { + it('signs in and hands off to the root route, which routes by role', async () => { + // The landing page depends on isAdmin, which only arrives with + // GET /auth/me — so this page names "/" rather than guessing. mockLogin.mockResolvedValue({ status: 'authenticated' }); render(); @@ -86,7 +88,7 @@ describe('Login Page Component', () => { await waitFor(() => expect(mockLogin).toHaveBeenCalledWith('jane@example.com', 'Password123!'), ); - expect(mockReplace).toHaveBeenCalledWith('/dashboard'); + expect(mockReplace).toHaveBeenCalledWith('/'); }); it('honours a ?next= target', async () => { @@ -108,7 +110,7 @@ describe('Login Page Component', () => { await fillCredentials(); await submit(); - await waitFor(() => expect(mockReplace).toHaveBeenCalledWith('/dashboard')); + await waitFor(() => expect(mockReplace).toHaveBeenCalledWith('/')); }); it('does not call login when validation fails', async () => { @@ -216,7 +218,7 @@ describe('Login Page Component', () => { }), ), ); - expect(mockReplace).toHaveBeenCalledWith('/dashboard'); + expect(mockReplace).toHaveBeenCalledWith('/'); }); it('explains that an MFA challenge is not supported yet', async () => { diff --git a/apps/frontend/test/components/Navbar.test.tsx b/apps/frontend/test/components/Navbar.test.tsx index 26a54275..d2930fc2 100644 --- a/apps/frontend/test/components/Navbar.test.tsx +++ b/apps/frontend/test/components/Navbar.test.tsx @@ -52,21 +52,23 @@ describe("NavBar", () => { // ── Role-based visibility ───────────────────────────────────────────────── it("hides admin-only items for standard role", () => { - render(); + render(); + expect(screen.queryByText("Dashboard")).not.toBeInTheDocument(); expect(screen.queryByText("Expenses")).not.toBeInTheDocument(); expect(screen.queryByText("Reports")).not.toBeInTheDocument(); expect(screen.queryByText("Accounts")).not.toBeInTheDocument(); }); it("hides admin-only items for limited role", () => { - render(); + render(); + expect(screen.queryByText("Dashboard")).not.toBeInTheDocument(); expect(screen.queryByText("Expenses")).not.toBeInTheDocument(); expect(screen.queryByText("Reports")).not.toBeInTheDocument(); expect(screen.queryByText("Accounts")).not.toBeInTheDocument(); }); it("shows shared items for all roles", () => { - const sharedItems = ["Dashboard", "Projects", "Donors", "Donations", "Log Out"]; + const sharedItems = ["Projects", "Donors", "Donations", "Log Out"]; const roles: UserRole[] = ["admin", "standard", "limited"]; roles.forEach((role) => { diff --git a/apps/frontend/test/lib/routes.test.ts b/apps/frontend/test/lib/routes.test.ts index 29e2ab2a..2988c5e1 100644 --- a/apps/frontend/test/lib/routes.test.ts +++ b/apps/frontend/test/lib/routes.test.ts @@ -1,7 +1,9 @@ import { + ADMIN_LANDING_PATH, + DEFAULT_LANDING_PATH, LOGIN_PATH, - POST_LOGIN_PATH, classifyRoute, + landingPathFor, normalizePath, requiresAdmin, safeNextPath, @@ -51,14 +53,18 @@ describe('classifyRoute', () => { }); describe('requiresAdmin', () => { - it.each(['/expenses', '/reports/', '/accounts', '/expenses/123'])( - 'requires admin for %s', - (path) => { - expect(requiresAdmin(path)).toBe(true); - }, - ); + it.each([ + '/expenses', + '/reports/', + '/accounts', + '/expenses/123', + '/dashboard', + '/dashboard/', + ])('requires admin for %s', (path) => { + expect(requiresAdmin(path)).toBe(true); + }); - it.each(['/dashboard', '/donors', '/projects/7', '/reports-archive'])( + it.each(['/donors', '/projects/7', '/reports-archive'])( 'does not require admin for %s', (path) => { expect(requiresAdmin(path)).toBe(false); @@ -66,6 +72,20 @@ describe('requiresAdmin', () => { ); }); +describe('landingPathFor', () => { + it('sends an admin to the dashboard', () => { + expect(landingPathFor(true)).toBe(ADMIN_LANDING_PATH); + }); + + it('sends everyone else somewhere they can actually load', () => { + // Regression guard: the landing route was /dashboard for every role, so + // making the dashboard admin-only dropped non-admins on the no-access panel + // the instant they signed in. + expect(landingPathFor(false)).toBe(DEFAULT_LANDING_PATH); + expect(requiresAdmin(DEFAULT_LANDING_PATH)).toBe(false); + }); +}); + describe('safeNextPath', () => { it('accepts a same-origin path with a query string', () => { expect(safeNextPath('/expenses?page=2')).toBe('/expenses?page=2'); @@ -82,9 +102,12 @@ describe('safeNextPath', () => { ['', 'empty value'], ]; - it.each(unsafe)('rejects %p (%s) and falls back to the dashboard', (raw) => { - expect(safeNextPath(raw)).toBe(POST_LOGIN_PATH); - }); + it.each(unsafe)( + 'rejects %p (%s) and falls back to the default landing route', + (raw) => { + expect(safeNextPath(raw)).toBe(DEFAULT_LANDING_PATH); + }, + ); it('honours an explicit fallback', () => { expect(safeNextPath(null, LOGIN_PATH)).toBe(LOGIN_PATH); From 7d8a5efc6ef296fb216ae7e2dc29c18f8a1e8f5e Mon Sep 17 00:00:00 2001 From: nourshoreibah Date: Tue, 11 Aug 2026 22:37:15 -0400 Subject: [PATCH 2/6] fix(projects): align dashboard e2e seed with year-scoped aggregates The dashboard handler now filters summary cards to the current calendar year and active projects, but e2e tests still used unmodified 2025 seed dates. Shift fixture dates in the dashboard suite so CI assertions stay stable. Co-authored-by: Cursor --- .../lambdas/projects/test/projects.e2e.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/apps/backend/lambdas/projects/test/projects.e2e.test.ts b/apps/backend/lambdas/projects/test/projects.e2e.test.ts index 565e64bb..d7c5b0b9 100644 --- a/apps/backend/lambdas/projects/test/projects.e2e.test.ts +++ b/apps/backend/lambdas/projects/test/projects.e2e.test.ts @@ -317,6 +317,22 @@ describe('GET /dashboard (e2e)', () => { const client = await pool.connect(); try { await resetData(client); + // Dashboard cards scope spend to the current calendar year and count + // only active projects. Seed rows use 2025 spend dates and early-2026 + // end dates, so shift them forward for deterministic e2e assertions. + await client.query(` + UPDATE branch.projects + SET end_date = '2099-12-31' + WHERE end_date IS NOT NULL + `); + await client.query(` + UPDATE branch.expenditures + SET spent_on = make_date( + EXTRACT(YEAR FROM CURRENT_DATE)::int, + EXTRACT(MONTH FROM spent_on)::int, + EXTRACT(DAY FROM spent_on)::int + ) + `); } finally { client.release(); } From 5fa7acdf9c5d22d453d4ed265777d74b59e76f55 Mon Sep 17 00:00:00 2001 From: nourshoreibah Date: Tue, 11 Aug 2026 23:22:02 -0400 Subject: [PATCH 3/6] fix(preview): never cancel an in-flight preview terraform apply The deploy job used cancel-in-progress, so a commit pushed moments after the test-environment label killed the run mid `terraform apply`. The runner dies before Terraform can persist state or release its DynamoDB lock, so the workspace stays locked and everything already created is orphaned. On PR #318 that left a REST API with no `prod` stage, which 403s every request without CORS headers -- surfacing in the browser as a CORS error on login. Queue superseded runs instead of killing them; GitHub still cancels all but the newest pending run, so rapid pushes collapse to one survivor as before. Also check that the `prod` stage exists on the update path. Only the API's existence was verified, so the run right after the cancelled one posted a green "updated in place" on a stack that could not serve a single request. Co-authored-by: Cursor --- .github/workflows/preview-env.yml | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index 62958368..69e53e61 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -20,11 +20,13 @@ on: types: [labeled, synchronize, unlabeled, closed] # Concurrency is set PER JOB, not workflow-wide, on purpose: -# - deploy uses a `preview-deploy-` group with cancel-in-progress so -# back-to-back commits cancel the older run (the survivor rebuilds from the -# full base...HEAD diff, so the final state matches the latest commit). -# - teardown uses a SEPARATE `preview-teardown-` group with -# cancel-in-progress: false so a teardown is NEVER cancelled by a deploy. +# - deploy uses a `preview-deploy-` group, so back-to-back commits queue +# behind the running deploy; GitHub cancels every pending run but the newest, +# so the survivor still rebuilds from the full base...HEAD diff. +# - teardown uses a SEPARATE `preview-teardown-` group so a teardown is +# NEVER cancelled by a deploy. +# Neither group cancels in progress: both run terraform, and terraform killed +# mid-apply is unrecoverable (see `deploy`). # A single shared group would let a push (e.g. right after removing the label) # cancel an in-flight teardown, leaking the stack. Separate groups + the # DynamoDB state lock (which serializes any overlapping terraform) keep cleanup @@ -46,9 +48,15 @@ jobs: (github.event.action == 'synchronize' && contains(github.event.pull_request.labels.*.name, 'test-environment')) runs-on: ubuntu-latest concurrency: - # Back-to-back commits: newer deploy cancels the older one for this PR. + # Back-to-back commits: the newest run supersedes older PENDING runs but never + # kills one already in flight. This job runs `terraform apply`, and cancelling + # that mid-apply cannot be recovered from: the runner is killed before + # Terraform persists state or releases its DynamoDB lock, so everything it had + # created so far is orphaned and the workspace stays locked. The wreckage is + # a REST API with no `prod` stage, which answers every request with a bare 403 + # carrying no CORS headers -- reaching the browser as an opaque CORS error. group: preview-deploy-${{ github.event.pull_request.number }} - cancel-in-progress: true + cancel-in-progress: false environment: preview permissions: id-token: write @@ -156,6 +164,7 @@ jobs: id: stack working-directory: infrastructure/preview run: | + set -euo pipefail if [ "${{ steps.mode.outputs.create }}" = "true" ]; then API_URL=$(terraform output -raw api_gateway_url) else @@ -165,6 +174,14 @@ jobs: echo "::error::No preview API for PR #${PR}. Re-add the ${LABEL} label to (re)create it." exit 1 fi + # The API existing is not enough. A stack whose apply died partway has + # the resources but no stage, and every call to it 403s without CORS + # headers, so the browser blames CORS. Fail here rather than posting a + # green "updated in place" on an environment that cannot serve a request. + if ! aws apigateway get-stage --rest-api-id "$API_ID" --stage-name prod >/dev/null 2>&1; then + echo "::error::Preview API ${API_ID} for PR #${PR} has no 'prod' stage — the stack is incomplete. Remove and re-add the ${LABEL} label to rebuild it." + exit 1 + fi API_URL="https://${API_ID}.execute-api.${AWS_REGION}.amazonaws.com/prod" fi echo "api_url=$API_URL" >> "$GITHUB_OUTPUT" From 073596e7394da1f0180d8b255de20794185e912d Mon Sep 17 00:00:00 2001 From: nourshoreibah Date: Tue, 11 Aug 2026 23:44:30 -0400 Subject: [PATCH 4/6] perf(dashboard): aggregate the dashboard rollups in SQL, not in the lambda The endpoint pulled one row per expenditure into the lambda to produce at most 12 x categories of monthly buckets, and fetched every project with selectAll -- including the unbounded description column -- next to two full-table GROUP BYs that were then stitched together with JS Maps. Bytes transferred grew with the expenditures table on every admin page load, though the output never did. Postgres now does the bucketing (date_trunc + GROUP BY) and the per-project join. The join goes through pre-aggregated subqueries rather than joining the raw tables onto projects, which would multiply each expenditure by the membership count and inflate `spent`. Seven queries become six. Verified by diffing old and new responses over the same seeded database: year, projects, expensesByMonth, totalSpent, totalProjects and topExpenseCategory come back identical, including with an ended project in the fixture. Doing the month bucketing in SQL also drops a latent bug -- the old code read a DATE through the runtime's local timezone and only landed on the right month because lambda runs in UTC. Also fixes averageSpendPerProject, which divided all-projects spend by the active-project count, inflating it whenever a project ended mid-year. Both sides of the divide are now the same set of active projects. Co-authored-by: Cursor --- apps/backend/lambdas/projects/handler.ts | 109 ++++++++++++------ .../projects/test/dashboard.unit.test.ts | 74 ++++++++---- 2 files changed, 122 insertions(+), 61 deletions(-) diff --git a/apps/backend/lambdas/projects/handler.ts b/apps/backend/lambdas/projects/handler.ts index 7a9c4d17..3aea3d3b 100644 --- a/apps/backend/lambdas/projects/handler.ts +++ b/apps/backend/lambdas/projects/handler.ts @@ -1,4 +1,5 @@ import { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; +import { sql } from 'kysely'; import db from './db'; import { ProjectValidationUtils } from './validation-utils'; import { @@ -55,13 +56,24 @@ export const handler = async (event: any): Promise => { const yearEnd = `${year}-12-31`; const today = now.toISOString().slice(0, 10); + // A project is active until its end_date passes; a null end_date never + // ends. Shared by the count and by the spend feeding the average so the + // two can never drift out of agreement. + const isActive = (column: any) => (eb: any) => + eb.or([eb(column, 'is', null), eb(column, '>=', today as any)]); + + // Postgres does the month bucketing. Selecting raw rows and grouping them + // in JS moved one row per expenditure into the lambda to produce at most + // 12 x categories of them, and read a DATE through the runtime's local + // timezone, which only lands on the right month because lambda runs UTC. + const monthExpr = sql`to_char(date_trunc('month', spent_on), 'YYYY-MM')`; + const [ totalSpentRow, totalProjectsRow, topCategoryRow, + activeSpentRow, projectRows, - spentByProject, - staffByProject, monthRows, ] = await Promise.all([ db.selectFrom('branch.expenditures') @@ -71,12 +83,7 @@ export const handler = async (event: any): Promise => { .executeTakeFirst(), db.selectFrom('branch.projects') .select(db.fn.count('project_id').as('count')) - .where((eb) => - eb.or([ - eb('end_date', 'is', null), - eb('end_date', '>=', today as any), - ]), - ) + .where(isActive('end_date')) .executeTakeFirst(), db.selectFrom('branch.expenditures') .select(['category', db.fn.sum('amount').as('total')]) @@ -87,36 +94,72 @@ export const handler = async (event: any): Promise => { .orderBy(db.fn.sum('amount'), 'desc') .limit(1) .executeTakeFirst(), - db.selectFrom('branch.projects') - .selectAll() - .orderBy('project_id', 'asc') - .execute(), - db.selectFrom('branch.expenditures') - .select(['project_id', db.fn.sum('amount').as('total')]) - .groupBy('project_id') - .execute(), - db.selectFrom('branch.project_memberships') - .select(['project_id', db.fn.count('user_id').as('count')]) - .groupBy('project_id') + // Numerator for the average: this year's spend on the very projects the + // denominator counts. expenditures.project_id is NOT NULL against a FK, + // so the join can never drop a row. + 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.spent_on', '>=', yearStart as any) + .where('e.spent_on', '<=', yearEnd as any) + .where(isActive('p.end_date')) + .executeTakeFirst(), + // Spend and headcount arrive as pre-aggregated subqueries. Joining the + // raw tables onto projects instead would multiply every expenditure by + // the membership count and silently inflate `spent`. + db.selectFrom('branch.projects as p') + .leftJoin( + (eb) => + eb.selectFrom('branch.expenditures') + .select('project_id') + .select((sub) => sub.fn.sum('amount').as('total')) + .groupBy('project_id') + .as('spend'), + (join) => join.onRef('spend.project_id', '=', 'p.project_id'), + ) + .leftJoin( + (eb) => + eb.selectFrom('branch.project_memberships') + .select('project_id') + .select((sub) => sub.fn.count('user_id').as('count')) + .groupBy('project_id') + .as('staff'), + (join) => join.onRef('staff.project_id', '=', 'p.project_id'), + ) + .select([ + 'p.project_id', + 'p.name', + 'p.total_budget', + 'p.currency', + 'spend.total as spent', + 'staff.count as staff_count', + ]) + .orderBy('p.project_id', 'asc') .execute(), db.selectFrom('branch.expenditures') - .select(['spent_on', 'category', 'amount']) + .select([monthExpr.as('month'), 'category', db.fn.sum('amount').as('total')]) .where('category', 'is not', null) .where('spent_on', '>=', yearStart as any) .where('spent_on', '<=', yearEnd as any) + .groupBy([monthExpr, 'category']) + .orderBy(monthExpr) + .orderBy('category') .execute(), ]); const totalSpent = Number(totalSpentRow?.total ?? 0); const totalProjects = Number(totalProjectsRow?.count ?? 0); - const averageSpendPerProject = totalProjects > 0 ? totalSpent / totalProjects : 0; - const spentMap = new Map(spentByProject.map((r) => [r.project_id, Number(r.total)])); - const staffMap = new Map(staffByProject.map((r) => [r.project_id, Number(r.count)])); + // A true aggregate over active projects: this year's spend on active + // projects divided by how many there are. Dividing the all-projects total + // by the active count inflated the figure whenever a project ended + // mid-year, since its spend stayed in the numerator. + const activeSpent = Number(activeSpentRow?.total ?? 0); + const averageSpendPerProject = totalProjects > 0 ? activeSpent / totalProjects : 0; const projects = projectRows.map((p) => { const budget = p.total_budget !== null ? Number(p.total_budget) : null; - const spent = spentMap.get(p.project_id) ?? 0; + const spent = Number(p.spent ?? 0); const spentPercentage = budget && budget > 0 ? (spent / budget) * 100 : 0; return { project_id: p.project_id, @@ -124,22 +167,16 @@ export const handler = async (event: any): Promise => { total_budget: budget, currency: p.currency, spent, - staff_count: staffMap.get(p.project_id) ?? 0, + staff_count: Number(p.staff_count ?? 0), spent_percentage: Number(spentPercentage.toFixed(2)), }; }); - const monthMap = new Map(); - for (const row of monthRows) { - const d = new Date(row.spent_on as any); - const month = `${d.getUTCFullYear()}-${String(d.getUTCMonth() + 1).padStart(2, '0')}`; - const category = row.category as string; - const key = `${month}|${category}`; - const prev = monthMap.get(key); - if (prev) prev.amount += Number(row.amount); - else monthMap.set(key, { month, category, amount: Number(row.amount) }); - } - const expensesByMonth = [...monthMap.values()].sort((a, b) => a.month.localeCompare(b.month)); + const expensesByMonth = monthRows.map((r) => ({ + month: r.month, + category: r.category as string, + amount: Number(r.total), + })); // Computed here, not client-side: totalSpent is the divisor and may be 0. const topCategoryAmount = Number(topCategoryRow?.total ?? 0); diff --git a/apps/backend/lambdas/projects/test/dashboard.unit.test.ts b/apps/backend/lambdas/projects/test/dashboard.unit.test.ts index 683bc6bf..d02cb277 100644 --- a/apps/backend/lambdas/projects/test/dashboard.unit.test.ts +++ b/apps/backend/lambdas/projects/test/dashboard.unit.test.ts @@ -81,33 +81,26 @@ describe('GET /dashboard unit tests', () => { describe('Response shape', () => { beforeEach(() => { mockDb.selectFrom = jest.fn(); - // 1) totalSpent + // 1) totalSpent (every project, this year) mockDb.selectFrom.mockReturnValueOnce(chain({ total: '18000.00' })); - // 2) totalProjects + // 2) totalProjects (active only) mockDb.selectFrom.mockReturnValueOnce(chain({ count: '4' })); // 3) topCategory mockDb.selectFrom.mockReturnValueOnce(chain({ category: 'Travel', total: '6800.00' })); - // 4) projectRows - mockDb.selectFrom.mockReturnValueOnce(chain([ - { project_id: 1, name: 'P1', total_budget: '500000.00', currency: 'USD' }, - { project_id: 2, name: 'P2', total_budget: '300000.00', currency: 'USD' }, - { project_id: 3, name: 'P3', total_budget: null, currency: 'USD' }, - ])); - // 5) spentByProject - mockDb.selectFrom.mockReturnValueOnce(chain([ - { project_id: 1, total: '9200.00' }, - { project_id: 2, total: '4500.00' }, - { project_id: 3, total: '4300.00' }, - ])); - // 6) staffByProject + // 4) activeSpent — numerator of the average. Every project is active here, + // so it matches totalSpent and the average stays 18000/4. + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '18000.00' })); + // 5) projectRows, with spend/headcount already joined by the database. + // P3 carries the LEFT JOIN misses as nulls. mockDb.selectFrom.mockReturnValueOnce(chain([ - { project_id: 1, count: '2' }, - { project_id: 2, count: '1' }, + { project_id: 1, name: 'P1', total_budget: '500000.00', currency: 'USD', spent: '9200.00', staff_count: '2' }, + { project_id: 2, name: 'P2', total_budget: '300000.00', currency: 'USD', spent: '4500.00', staff_count: '1' }, + { project_id: 3, name: 'P3', total_budget: null, currency: 'USD', spent: '4300.00', staff_count: null }, ])); - // 7) raw expenditure rows (handler buckets by YYYY-MM in JS) + // 6) expenses already grouped into YYYY-MM x category by the database mockDb.selectFrom.mockReturnValueOnce(chain([ - { spent_on: new Date('2025-02-10'), category: 'Travel', amount: '5000.00' }, - { spent_on: new Date('2025-03-22'), category: 'Travel Foreign', amount: '4200.00' }, + { month: '2025-02', category: 'Travel', total: '5000.00' }, + { month: '2025-03', category: 'Travel Foreign', total: '4200.00' }, ])); }); @@ -151,7 +144,7 @@ describe('GET /dashboard unit tests', () => { expect(body.projects[2].spent_percentage).toBe(0); }); - test('200: expensesByMonth buckets raw rows into YYYY-MM', async () => { + test('200: expensesByMonth passes through the database buckets', async () => { const res = await handler(getEvent()); const body = JSON.parse(res.body); expect(body.expensesByMonth).toEqual([ @@ -173,8 +166,7 @@ describe('GET /dashboard unit tests', () => { mockDb.selectFrom.mockReturnValueOnce(chain({ total: null })); mockDb.selectFrom.mockReturnValueOnce(chain({ count: '0' })); mockDb.selectFrom.mockReturnValueOnce(chain(undefined)); - mockDb.selectFrom.mockReturnValueOnce(chain([])); - mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain({ total: null })); mockDb.selectFrom.mockReturnValueOnce(chain([])); mockDb.selectFrom.mockReturnValueOnce(chain([])); @@ -194,8 +186,7 @@ describe('GET /dashboard unit tests', () => { mockDb.selectFrom.mockReturnValueOnce(chain({ total: '0' })); mockDb.selectFrom.mockReturnValueOnce(chain({ count: '2' })); mockDb.selectFrom.mockReturnValueOnce(chain({ category: 'Travel', total: '0' })); - mockDb.selectFrom.mockReturnValueOnce(chain([])); - mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '0' })); mockDb.selectFrom.mockReturnValueOnce(chain([])); mockDb.selectFrom.mockReturnValueOnce(chain([])); @@ -204,6 +195,39 @@ describe('GET /dashboard unit tests', () => { expect(body.summary.topExpenseCategory.percentage).toBe(0); }); + test('200: average aggregates active projects only, on both sides of the divide', async () => { + mockDb.selectFrom = jest.fn(); + // 18000 spent this year across every project... + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '18000.00' })); + // ...but only 4 projects are still active... + mockDb.selectFrom.mockReturnValueOnce(chain({ count: '4' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ category: 'Travel', total: '6800.00' })); + // ...and only 12000 of that spend belongs to them. + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '12000.00' })); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + + const body = JSON.parse((await handler(getEvent())).body); + // 12000/4, not 18000/4: the 6000 belonging to projects that have already + // ended is out of the numerator, matching the denominator. + expect(body.summary.averageSpendPerProject).toBe(3000); + // The headline total still reports every project's spend. + expect(body.summary.totalSpent).toBe(18000); + }); + + test('200: average is 0 when active projects exist but none of them spent', async () => { + mockDb.selectFrom = jest.fn(); + mockDb.selectFrom.mockReturnValueOnce(chain({ total: '5000.00' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ count: '3' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ category: 'Travel', total: '5000.00' })); + mockDb.selectFrom.mockReturnValueOnce(chain({ total: null })); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + mockDb.selectFrom.mockReturnValueOnce(chain([])); + + const body = JSON.parse((await handler(getEvent())).body); + expect(body.summary.averageSpendPerProject).toBe(0); + }); + test('500: db failure surfaces as 500', async () => { mockDb.selectFrom = jest.fn().mockImplementation(() => { throw new Error('boom'); From 14a223cb5ef03d9e093deb2c42aa286da732f207 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 12 Aug 2026 03:45:13 +0000 Subject: [PATCH 5/6] chore: regenerate lambda READMEs --- apps/backend/lambdas/projects/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/backend/lambdas/projects/README.md b/apps/backend/lambdas/projects/README.md index 3d5a7ee9..9e127d7f 100644 --- a/apps/backend/lambdas/projects/README.md +++ b/apps/backend/lambdas/projects/README.md @@ -10,6 +10,8 @@ Lambda for managing projects. |--------|------|-------------| | GET | /health | Health check | | GET | /dashboard | | +| GET | /project | | +| GET | /true | | | GET | /projects/{id}/members | | | GET | /projects | | | GET | /projects/{id}/donors | | From 196c3925f9403ee0d0cbefe30f98fd2a5af7dd98 Mon Sep 17 00:00:00 2001 From: nourshoreibah Date: Tue, 11 Aug 2026 23:50:18 -0400 Subject: [PATCH 6/6] docs(projects): stop two code comments registering as fake routes generate-readme scans every comment between the ROUTES markers with /\/\/\s*([A-Z]+)\s+([\/\w\{\}\-]+)/, so a comment opening with a lone capital and a space parses as a route. "A project is active..." and "A true aggregate..." became `GET /project` and `GET /true` in the endpoint table when the readme bot regenerated it. Reword both openings and regenerate. Co-authored-by: Cursor --- apps/backend/lambdas/projects/README.md | 2 -- apps/backend/lambdas/projects/handler.ts | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/backend/lambdas/projects/README.md b/apps/backend/lambdas/projects/README.md index 9e127d7f..3d5a7ee9 100644 --- a/apps/backend/lambdas/projects/README.md +++ b/apps/backend/lambdas/projects/README.md @@ -10,8 +10,6 @@ Lambda for managing projects. |--------|------|-------------| | GET | /health | Health check | | GET | /dashboard | | -| GET | /project | | -| GET | /true | | | GET | /projects/{id}/members | | | GET | /projects | | | GET | /projects/{id}/donors | | diff --git a/apps/backend/lambdas/projects/handler.ts b/apps/backend/lambdas/projects/handler.ts index 3aea3d3b..2bca9a14 100644 --- a/apps/backend/lambdas/projects/handler.ts +++ b/apps/backend/lambdas/projects/handler.ts @@ -56,9 +56,9 @@ export const handler = async (event: any): Promise => { const yearEnd = `${year}-12-31`; const today = now.toISOString().slice(0, 10); - // A project is active until its end_date passes; a null end_date never - // ends. Shared by the count and by the spend feeding the average so the - // two can never drift out of agreement. + // Projects stay active until their end_date passes; a null end_date + // never ends. Shared by the count and by the spend feeding the average + // so the two can never drift out of agreement. const isActive = (column: any) => (eb: any) => eb.or([eb(column, 'is', null), eb(column, '>=', today as any)]); @@ -150,7 +150,7 @@ export const handler = async (event: any): Promise => { const totalSpent = Number(totalSpentRow?.total ?? 0); const totalProjects = Number(totalProjectsRow?.count ?? 0); - // A true aggregate over active projects: this year's spend on active + // True aggregate over active projects: this year's spend on active // projects divided by how many there are. Dividing the all-projects total // by the active count inflated the figure whenever a project ended // mid-year, since its spend stayed in the numerator.