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" diff --git a/apps/backend/lambdas/projects/handler.ts b/apps/backend/lambdas/projects/handler.ts index a5d5231a..2bca9a14 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 { @@ -46,56 +47,119 @@ 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); + + // 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)]); + + // 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') .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(isActive('end_date')) .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) .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)])); + // 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, @@ -103,28 +167,34 @@ 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); + 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..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' }, ])); }); @@ -119,7 +112,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 () => { @@ -142,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([ @@ -164,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([])); @@ -180,6 +181,53 @@ 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({ total: '0' })); + 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('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'); diff --git a/apps/backend/lambdas/projects/test/projects.e2e.test.ts b/apps/backend/lambdas/projects/test/projects.e2e.test.ts index afcf39c1..4cbcea64 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(); } diff --git a/apps/frontend/src/app/accounts/page.tsx b/apps/frontend/src/app/accounts/page.tsx index 5157953a..010a80d3 100644 --- a/apps/frontend/src/app/accounts/page.tsx +++ b/apps/frontend/src/app/accounts/page.tsx @@ -4,7 +4,6 @@ import React from 'react'; import StaffCard from '../components/StaffCard'; import { facilitationTeam, teamMembers } from './mockUsers'; - export default function AccountsPage() { return (
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 331cac6b..cd8bd450 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/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 a6addeb1..64f563f6 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 = [ @@ -27,7 +38,7 @@ const PUBLIC_PREFIXES = [ * inside the review modal are admin-gated, and the backend already lets any * authenticated user list expenditures. */ -const ADMIN_PREFIXES = ['/reports', '/accounts'] as const; +const ADMIN_PREFIXES = ['/dashboard', '/reports', '/accounts'] as const; /** * Strips the trailing slash and lowercases. @@ -71,7 +82,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/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 b0677b1c..73af58c6 100644 --- a/apps/frontend/test/components/Navbar.test.tsx +++ b/apps/frontend/test/components/Navbar.test.tsx @@ -52,24 +52,26 @@ describe("NavBar", () => { // ── Role-based visibility ───────────────────────────────────────────────── it("hides admin-only items for standard role", () => { - render(); + render(); + expect(screen.queryByText("Dashboard")).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("Reports")).not.toBeInTheDocument(); expect(screen.queryByText("Accounts")).not.toBeInTheDocument(); }); it("shows shared items for all roles", () => { // Expenses is shared: non-admins submit expenses there. - const sharedItems = ["Dashboard", "Projects", "Donors", "Donations", "Expenses", "Log Out"]; + const sharedItems = ["Projects", "Donors", "Donations", "Expenses", "Log Out"]; const roles: UserRole[] = ["admin", "standard", "limited"]; roles.forEach((role) => { - const { unmount } = render(); + const { unmount } = render(); sharedItems.forEach((label) => { expect(screen.getAllByText(label).length).toBeGreaterThan(0); }); diff --git a/apps/frontend/test/lib/routes.test.ts b/apps/frontend/test/lib/routes.test.ts index 90586390..f719897b 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,7 +53,7 @@ describe('classifyRoute', () => { }); describe('requiresAdmin', () => { - it.each(['/reports/', '/accounts'])( + it.each(['/dashboard', '/dashboard/', '/reports/', '/accounts'])( 'requires admin for %s', (path) => { expect(requiresAdmin(path)).toBe(true); @@ -60,7 +62,7 @@ describe('requiresAdmin', () => { // Non-admins submit expenses, so the page itself is not admin-gated; only the // approve/deny controls inside the review modal are. - it.each(['/dashboard', '/donors', '/projects/7', '/reports-archive', '/expenses', '/expenses/123'])( + it.each(['/donors', '/projects/7', '/reports-archive', '/expenses', '/expenses/123'])( 'does not require admin for %s', (path) => { expect(requiresAdmin(path)).toBe(false); @@ -68,6 +70,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'); @@ -84,9 +100,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);