From 2ba36470dfaa279a1c305f4d1e4d1e6c70c15905 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Tue, 21 Jul 2026 16:39:35 +0000 Subject: [PATCH 01/63] refactor(projects): delete the authz stub, guard via the real lib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `src/app/projects/authz.ts` was a placeholder from the route-shell work (#406): session checks only, with a TODO(#408) to replace its bodies once the real rules landed. #408 has landed, so it goes. The real lib (`src/lib/projects/authz.ts`) is deliberately pure — it takes project facts explicitly and cannot read a session, hit the database, or redirect. So the four route files do not import it directly; they import a thin glue layer, `src/app/projects/guards.ts`, which resolves the session and the facts, delegates every decision to the lib, and turns a refusal into a Next.js navigation. It holds no permission rules of its own. Two behaviour notes: - The redirect-on-no-session behaviour is preserved as-is: no session (or a session whose email has no `user` row) still redirects to `/`, matching `src/middleware.ts`. - Denial of a specific project now calls `notFound()` instead of redirecting to `/projects`, which is what the stub's own TODO asked for. It was redirecting only because `canViewProject` could never return false; now it can, and a 404 is what keeps Project existence from leaking to users outside its organisation. `loadAuthzUserByEmail` joins the repository because the session carries no usable id — `session.user.dbId` is set by the `jwt` callback at initial sign-in only, so sessions issued before that field existed lack it. Email is unique on `user`, so it resolves the same row. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/projects/[projectId]/layout.tsx | 2 +- src/app/projects/[projectId]/page.tsx | 2 +- src/app/projects/authz.ts | 65 -------------- src/app/projects/guards.ts | 88 +++++++++++++++++++ src/app/projects/layout.tsx | 2 +- src/app/projects/page.tsx | 2 +- .../repositories/projects/authzRepository.ts | 22 +++++ 7 files changed, 114 insertions(+), 69 deletions(-) delete mode 100644 src/app/projects/authz.ts create mode 100644 src/app/projects/guards.ts diff --git a/src/app/projects/[projectId]/layout.tsx b/src/app/projects/[projectId]/layout.tsx index 5ebf7d74..4c9c287a 100644 --- a/src/app/projects/[projectId]/layout.tsx +++ b/src/app/projects/[projectId]/layout.tsx @@ -1,6 +1,6 @@ import * as React from "react"; import { ProjectsNav, type ProjectsNavItem } from "../components/ProjectsNav"; -import { requireProjectAccess } from "../authz"; +import { requireProjectAccess } from "../guards"; function projectNav(projectId: string): ProjectsNavItem[] { const base = `/projects/${projectId}`; diff --git a/src/app/projects/[projectId]/page.tsx b/src/app/projects/[projectId]/page.tsx index e84d40da..5f72fcd1 100644 --- a/src/app/projects/[projectId]/page.tsx +++ b/src/app/projects/[projectId]/page.tsx @@ -1,4 +1,4 @@ -import { requireProjectAccess } from "../authz"; +import { requireProjectAccess } from "../guards"; export const metadata = { title: "Project dashboard | Ara", diff --git a/src/app/projects/authz.ts b/src/app/projects/authz.ts deleted file mode 100644 index a9180e85..00000000 --- a/src/app/projects/authz.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { getServerSession } from "next-auth"; -import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; -import { redirect } from "next/navigation"; -import type { Session } from "next-auth"; - -/** - * Temporary authorization seam for the Ara Projects route tree. - * - * TODO(#408): replace the bodies below with calls into - * `src/lib/projects/authz.ts` once that module lands. This file exists only so - * the route shell has a single, obvious place to wire authorization into — - * deliberately *not* a second implementation of the rules. - * - * The visibility rule #408 will enforce: - * - internal users (`@domna.homes`) → every Project - * - client-org members → Projects where - * `project.organisation_id` is their org - * - contractor-org members → Projects where their org is assigned - * to one of the Project's Workstreams - * (`project_workstream_contractor`) - * - * Note that resolving a user to an organisation does not exist yet either — - * that resolution is part of #408, not of this shell. - */ - -/** - * Guards a Projects route: requires a signed-in user and returns the session. - * - * Unauthenticated requests to `/projects/*` are already turned away by - * `src/middleware.ts`; this is the defence-in-depth check for the server - * component itself, matching the `bulk-upload/page.tsx` pattern. - */ -export async function requireProjectsSession(): Promise { - const session = await getServerSession(AuthOptions); - if (!session) redirect("/"); - return session; -} - -/** - * Whether `session`'s user may see the Project `projectId`. - * - * TODO(#408): delegate to the real visibility rule. Until then this only - * asserts a session exists — the shell must not ship its own inlined - * permission logic, so it deliberately does not narrow further. - */ -export async function canViewProject( - session: Session, - projectId: string -): Promise { - void projectId; - return Boolean(session?.user); -} - -/** - * Guards a single-Project route. Throws the caller into a 404 rather than a - * 403 so that Project existence is not leaked to users outside its org. - */ -export async function requireProjectAccess(projectId: string): Promise { - const session = await requireProjectsSession(); - if (!(await canViewProject(session, projectId))) { - // TODO(#408): notFound() once canViewProject can actually return false. - redirect("/projects"); - } - return session; -} diff --git a/src/app/projects/guards.ts b/src/app/projects/guards.ts new file mode 100644 index 00000000..2f6dac3e --- /dev/null +++ b/src/app/projects/guards.ts @@ -0,0 +1,88 @@ +/** + * Route guards for the Ara Projects tree. + * + * This is glue, not policy. It resolves the three things a server component + * cannot get from a pure function — the NextAuth session, the user's + * organisation memberships, and the project's facts — hands them to the real + * guards in `@/lib/projects/authz`, and turns a refusal into a Next.js + * navigation. It contains **no permission rules of its own**; every decision + * below is a call into that module. + * + * Layering: `@/lib/projects/authz` decides (pure, no db), + * `@/app/repositories/projects/authzRepository` loads (Drizzle), this file + * navigates (redirect / notFound). The dependency only ever points that way. + */ +import { getServerSession } from "next-auth"; +import { notFound, redirect } from "next/navigation"; +import type { Session } from "next-auth"; +import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; +import { + loadAuthzUserByEmail, + loadProjectAuthzFacts, +} from "@/app/repositories/projects/authzRepository"; +import { + canViewProject, + type AuthzUser, + type ProjectAuthzFacts, +} from "@/lib/projects/authz"; + +/** + * Requires a signed-in user and returns the session. + * + * Unauthenticated requests to `/projects/*` are already turned away by + * `src/middleware.ts`; this is the defence-in-depth check for the server + * component itself, matching the `bulk-upload/page.tsx` pattern. The redirect + * target matches the middleware's (`/`, the sign-in page). + */ +export async function requireProjectsSession(): Promise { + const session = await getServerSession(AuthOptions); + if (!session?.user?.email) redirect("/"); + return session; +} + +/** + * Requires a signed-in user and resolves them to an `AuthzUser` — the shape + * every guard in `@/lib/projects/authz` takes. + * + * A session whose email has no `user` row is treated as no session at all: it + * cannot be authorized against anything, so it goes back to sign-in rather + * than falling through to an empty-membership user that silently sees nothing. + */ +export async function requireProjectsUser(): Promise { + const session = await requireProjectsSession(); + const user = await loadAuthzUserByEmail(session.user.email!); + if (!user) redirect("/"); + return user; +} + +/** + * Parses a `[projectId]` path segment into the bigint PK it addresses. + * + * Returns null for anything that is not a bare non-negative integer, so a junk + * segment becomes a 404 rather than a Drizzle error. + */ +export function parseProjectId(projectId: string): bigint | null { + return /^\d+$/.test(projectId) ? BigInt(projectId) : null; +} + +/** + * Guards a single-Project route, returning the facts the page will need. + * + * A project the user may not see is a 404, not a 403 — Project existence must + * not leak to users outside its organisation, so "no such project" and "not + * yours" are deliberately indistinguishable. + */ +export async function requireProjectAccess(projectId: string): Promise<{ + user: AuthzUser; + project: ProjectAuthzFacts; +}> { + const user = await requireProjectsUser(); + + const id = parseProjectId(projectId); + if (id === null) notFound(); + + const project = await loadProjectAuthzFacts(id); + if (!project || !canViewProject(user, project)) notFound(); + + return { user, project }; +} diff --git a/src/app/projects/layout.tsx b/src/app/projects/layout.tsx index 3bc5808d..276d4065 100644 --- a/src/app/projects/layout.tsx +++ b/src/app/projects/layout.tsx @@ -1,6 +1,6 @@ import * as React from "react"; import { ProjectsNav, type ProjectsNavItem } from "./components/ProjectsNav"; -import { requireProjectsSession } from "./authz"; +import { requireProjectsSession } from "./guards"; const TOP_LEVEL_NAV: ProjectsNavItem[] = [ { label: "Projects", href: "/projects", icon: "projects", exact: true }, diff --git a/src/app/projects/page.tsx b/src/app/projects/page.tsx index c0da526a..e1a8b423 100644 --- a/src/app/projects/page.tsx +++ b/src/app/projects/page.tsx @@ -1,5 +1,5 @@ import { Squares2X2Icon } from "@heroicons/react/24/outline"; -import { requireProjectsSession } from "./authz"; +import { requireProjectsSession } from "./guards"; export const metadata = { title: "Projects | Ara", diff --git a/src/app/repositories/projects/authzRepository.ts b/src/app/repositories/projects/authzRepository.ts index 418f6e55..0b5d08f7 100644 --- a/src/app/repositories/projects/authzRepository.ts +++ b/src/app/repositories/projects/authzRepository.ts @@ -67,6 +67,28 @@ export async function loadAuthzUser(userId: bigint): Promise { return { id: found.id, email: found.email, organisationIds }; } +/** + * Load a user plus their organisation memberships, keyed by email. + * + * Server components and route handlers hold a NextAuth session, and the only + * identifier that session reliably carries is the email: `session.user.dbId` + * is populated by the `jwt` callback at initial sign-in only, so sessions + * issued before that field existed have no id at all. Email is unique on + * `user`, so this is the same row `loadAuthzUser` would return. + */ +export async function loadAuthzUserByEmail( + email: string, +): Promise { + const [found] = await db + .select({ id: user.id, email: user.email }) + .from(user) + .where(eq(user.email, email)) + .limit(1); + + if (!found) return null; + return { ...found, organisationIds: await getUserOrganisations(found.id) }; +} + /** * Load one project's permission-relevant facts, including every organisation * assigned as contractor to any of its workstreams. Returns null when the From 9cfde2767f580d10b2b142ea55587c96ab32552c Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Tue, 21 Jul 2026 16:43:48 +0000 Subject: [PATCH 02/63] =?UTF-8?q?feat(ara-projects):=20work-order=20import?= =?UTF-8?q?=20step=201=20=E2=80=94=20upload=20and=20parse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `/projects/[projectId]/import`: a CSV/XLSX drop zone that uploads to a route handler, parses server-side with SheetJS (already a dependency — the Bulk tag assignment flow is the in-app parsing precedent, not the FastAPI BulkUpload pipeline), and returns headers plus a first-N preview. Nothing is written to the database at this step. Insert happens at commit (#417). Files: src/lib/projects/import/parse.ts pure, DB-free parser + template src/lib/projects/import/parse.test.ts 23 unit tests, no DB, no fixtures .../api/projects/[projectId]/import/parse/route.ts POST multipart .../api/projects/[projectId]/import/template/route.ts GET template CSV .../projects/[projectId]/import/page.tsx server component .../import/components/ImportUpload.tsx drop zone + preview Persistence choice: hold rows client-side, not S3-and-reparse ------------------------------------------------------------- The ticket asks for a choice between uploading the raw file to S3 via the presigned-URL pattern in `src/lib/bulkUpload/client.ts` and re-parsing it per step, versus holding the parsed rows in the client and POSTing them at commit. Chosen: hold the rows client-side. The parse route therefore returns every row, not only the preview, and the mapping (#415) and validation (#416) steps read them from `ImportUpload`'s mutation state. Reasoning: 1. S3-and-reparse needs somewhere to keep the object key and the import's status between steps, and the Ara Projects schema (PR #404) has no import table. Adding one means designing a second BulkUpload-shaped lifecycle — hard to reverse, and ADR-worthy — for a v1 that is insert-only with no diffing (deferred to #425). Not worth it to carry a file across three steps of one sitting. 2. The scale does not call for it. This import declares which workstreams the properties of a single project take — thousands of rows, not the hundreds-of-thousands the BulkUpload property pipeline handles. The 20k row cap bounds the commit POST to a few MB of JSON. 3. Re-parsing per step parses the same bytes three or four times and lets the steps disagree if the parser ever changes underneath them. Client-held rows are by construction the exact rows the user previewed and will validate. 4. It matches the existing precedent: Bulk tag assignment (ADR-0013) parses a small spreadsheet and POSTs the extracted identifiers at commit, with no S3 round trip. The cost is that a page refresh loses progress and the commit POST carries the payload. Both are acceptable for a single-sitting, insert-only v1; if the import later needs to be resumable or to exceed this scale, S3 staging plus an import table is the upgrade path, and the parser is already isolated behind `parseImportFile` so only the transport changes. Notes ----- - The parser is deliberately header-agnostic: it reports the headers it finds rather than demanding the template's. Mapping arbitrary client headings is #415's job, and rejecting unrecognised headings here would make the mapping step unreachable for exactly the files that need it. - Authorization uses `canManageProject` from `src/lib/projects/authz.ts` with facts from the #408 repository, so importing is internal/client-org only and never a contractor. Unreachable projects 404 rather than 403. The page applies the same check so a contractor never sees the drop zone. - `raw: false` on the SheetJS read keeps leading zeros on numeric-looking property identifiers (covered by a test). - Size is capped at 10MB client- and server-side. The wireframe says 50MB; 10MB is far past what four narrow columns can plausibly reach. - Known limitation, documented in a characterisation test: SheetJS does not throw on loose bytes, so a multi-line non-spreadsheet renamed to .csv parses into nonsense headers rather than being rejected. Nothing is written, the extension allowlist catches the honestly-named case, and the user sees the garbage in the preview. Content sniffing here would be guesswork. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../[projectId]/import/parse/route.ts | 99 ++++++ .../[projectId]/import/template/route.ts | 56 +++ .../import/components/ImportUpload.tsx | 253 +++++++++++++ src/app/projects/[projectId]/import/page.tsx | 93 +++++ src/lib/projects/import/parse.test.ts | 333 ++++++++++++++++++ src/lib/projects/import/parse.ts | 263 ++++++++++++++ 6 files changed, 1097 insertions(+) create mode 100644 src/app/api/projects/[projectId]/import/parse/route.ts create mode 100644 src/app/api/projects/[projectId]/import/template/route.ts create mode 100644 src/app/projects/[projectId]/import/components/ImportUpload.tsx create mode 100644 src/app/projects/[projectId]/import/page.tsx create mode 100644 src/lib/projects/import/parse.test.ts create mode 100644 src/lib/projects/import/parse.ts diff --git a/src/app/api/projects/[projectId]/import/parse/route.ts b/src/app/api/projects/[projectId]/import/parse/route.ts new file mode 100644 index 00000000..0f487888 --- /dev/null +++ b/src/app/api/projects/[projectId]/import/parse/route.ts @@ -0,0 +1,99 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getServerSession } from "next-auth"; +import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; +import { canManageProject } from "@/lib/projects/authz"; +import { + loadAuthzUser, + loadProjectAuthzFacts, +} from "@/app/repositories/projects/authzRepository"; +import { + hasAcceptedImportExtension, + MAX_IMPORT_FILE_BYTES, + parseImportFile, +} from "@/lib/projects/import/parse"; + +type Params = { params: Promise<{ projectId: string }> }; + +/** + * POST — parse an uploaded work-order import file (issue #414). + * + * Takes `multipart/form-data` with a single `file` field and returns the + * headers, every data row, a row count and a first-N preview. It writes + * nothing: the import is not persisted until commit (#417). + * + * The rows come back in full, not just the preview, because the parsed file is + * held client-side across the mapping and validation steps and POSTed once at + * commit — see the commit message for why that beat staging the file in S3. + */ +export async function POST(req: NextRequest, props: Params) { + const { projectId } = await props.params; + + const session = await getServerSession(AuthOptions); + if (!session?.user?.dbId) { + return NextResponse.json({ error: "Unauthorised" }, { status: 401 }); + } + if (!/^\d+$/.test(projectId)) { + return NextResponse.json({ error: "Invalid project id" }, { status: 400 }); + } + + const [user, project] = await Promise.all([ + loadAuthzUser(BigInt(session.user.dbId)), + loadProjectAuthzFacts(BigInt(projectId)), + ]); + // A project the caller cannot see must 404, not 403 — its existence is not + // theirs to learn (see `@/lib/projects/authz`). + if (!user || !project) { + return NextResponse.json({ error: "Project not found" }, { status: 404 }); + } + // Importing is managing: internal and client-org only, never a contractor. + if (!canManageProject(user, project)) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } + + let form: FormData; + try { + form = await req.formData(); + } catch { + return NextResponse.json( + { error: "Expected a file upload" }, + { status: 400 }, + ); + } + + const file = form.get("file"); + if (!(file instanceof File)) { + return NextResponse.json({ error: "No file was uploaded" }, { status: 400 }); + } + if (!hasAcceptedImportExtension(file.name)) { + return NextResponse.json( + { error: "That file type isn't supported — upload a .csv, .xlsx or .xls file." }, + { status: 400 }, + ); + } + // Re-checked here even though the drop zone checks it too: the client-side + // limit is a courtesy, this one is the rule. + if (file.size > MAX_IMPORT_FILE_BYTES) { + return NextResponse.json( + { + error: `That file is larger than ${MAX_IMPORT_FILE_BYTES / (1024 * 1024)}MB — split it into smaller files.`, + }, + { status: 413 }, + ); + } + if (file.size === 0) { + return NextResponse.json({ error: "That file is empty." }, { status: 400 }); + } + + const result = parseImportFile(new Uint8Array(await file.arrayBuffer())); + if (!result.ok) { + return NextResponse.json({ error: result.error }, { status: 400 }); + } + + return NextResponse.json({ + filename: file.name, + headers: result.file.headers, + rows: result.file.rows, + rowCount: result.file.rowCount, + preview: result.file.preview, + }); +} diff --git a/src/app/api/projects/[projectId]/import/template/route.ts b/src/app/api/projects/[projectId]/import/template/route.ts new file mode 100644 index 00000000..3371555a --- /dev/null +++ b/src/app/api/projects/[projectId]/import/template/route.ts @@ -0,0 +1,56 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getServerSession } from "next-auth"; +import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; +import { canManageProject } from "@/lib/projects/authz"; +import { + loadAuthzUser, + loadProjectAuthzFacts, +} from "@/app/repositories/projects/authzRepository"; +import { buildImportTemplateCsv } from "@/lib/projects/import/parse"; + +type Params = { params: Promise<{ projectId: string }> }; + +/** + * GET — the "Download template" artefact: a CSV carrying the expected headings + * plus a worked example (issue #414). + * + * Served from a route handler rather than built in the browser so the template + * and the parser read their column list from the same constant — a heading + * renamed in one place cannot drift from the other. + * + * Gated behind the same permission as the import itself: the template is not + * secret, but there is no reason for it to be the one Projects endpoint a + * contractor can reach. + */ +export async function GET(_req: NextRequest, props: Params) { + const { projectId } = await props.params; + + const session = await getServerSession(AuthOptions); + if (!session?.user?.dbId) { + return NextResponse.json({ error: "Unauthorised" }, { status: 401 }); + } + if (!/^\d+$/.test(projectId)) { + return NextResponse.json({ error: "Invalid project id" }, { status: 400 }); + } + + const [user, project] = await Promise.all([ + loadAuthzUser(BigInt(session.user.dbId)), + loadProjectAuthzFacts(BigInt(projectId)), + ]); + if (!user || !project) { + return NextResponse.json({ error: "Project not found" }, { status: 404 }); + } + if (!canManageProject(user, project)) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } + + // BOM so Excel opens the CSV as UTF-8 rather than guessing a codepage. + return new NextResponse(`${buildImportTemplateCsv()}`, { + headers: { + "Content-Type": "text/csv; charset=utf-8", + "Content-Disposition": + 'attachment; filename="work-order-import-template.csv"', + "Cache-Control": "no-store", + }, + }); +} diff --git a/src/app/projects/[projectId]/import/components/ImportUpload.tsx b/src/app/projects/[projectId]/import/components/ImportUpload.tsx new file mode 100644 index 00000000..e8857d93 --- /dev/null +++ b/src/app/projects/[projectId]/import/components/ImportUpload.tsx @@ -0,0 +1,253 @@ +"use client"; + +import { DragEvent, useRef, useState } from "react"; +import { useMutation } from "@tanstack/react-query"; +import { + ArrowDownTrayIcon, + ArrowPathIcon, + CloudArrowUpIcon, + DocumentTextIcon, + ExclamationTriangleIcon, +} from "@heroicons/react/24/outline"; +import { Button } from "@/app/shadcn_components/ui/button"; +import { + ACCEPTED_IMPORT_EXTENSIONS, + hasAcceptedImportExtension, + MAX_IMPORT_FILE_BYTES, +} from "@/lib/projects/import/parse"; + +/** The parse route's response — the whole file, not just the preview. */ +export interface ParsedImportResponse { + filename: string; + headers: string[]; + rows: string[][]; + rowCount: number; + preview: string[][]; +} + +const MAX_MB = MAX_IMPORT_FILE_BYTES / (1024 * 1024); + +/** + * Work-order import drop zone (issue #414). + * + * Uploads the file to the parse route and renders the returned headers and + * preview. The full `rows` array stays here in `parse.data` — the mapping + * (#415), validation (#416) and commit (#417) steps read it from this + * component's state rather than re-fetching or re-parsing, which is why the + * route hands back every row and not only the preview. + */ +export function ImportUpload({ projectId }: { projectId: string }) { + const fileInputRef = useRef(null); + const [isDragging, setIsDragging] = useState(false); + const [fileName, setFileName] = useState(null); + // Size/extension failures caught before the upload; the route re-checks both. + const [clientError, setClientError] = useState(null); + + const parse = useMutation({ + mutationFn: async (file: File) => { + const form = new FormData(); + form.append("file", file); + const res = await fetch(`/api/projects/${projectId}/import/parse`, { + method: "POST", + body: form, + }); + const body = await res.json(); + if (!res.ok) throw new Error(body.error ?? "Couldn't read that file"); + return body; + }, + }); + + function onFile(file: File) { + setClientError(null); + parse.reset(); + setFileName(file.name); + + if (!hasAcceptedImportExtension(file.name)) { + setClientError( + `That file type isn't supported — upload a ${ACCEPTED_IMPORT_EXTENSIONS.join(", ")} file.`, + ); + return; + } + if (file.size > MAX_IMPORT_FILE_BYTES) { + setClientError( + `That file is larger than ${MAX_MB}MB — split it into smaller files.`, + ); + return; + } + parse.mutate(file); + } + + function handleDragOver(e: DragEvent) { + e.preventDefault(); + setIsDragging(true); + } + + function handleDrop(e: DragEvent) { + e.preventDefault(); + setIsDragging(false); + const dropped = e.dataTransfer.files?.[0]; + if (dropped) onFile(dropped); + } + + function reset() { + setFileName(null); + setClientError(null); + parse.reset(); + if (fileInputRef.current) fileInputRef.current.value = ""; + } + + const error = clientError ?? (parse.isError ? parse.error.message : null); + const parsed = parse.data; + + return ( +
+ + + + + {error && ( +
+ +
+

Couldn't import that file

+

{error}

+ +
+
+ )} + + {parsed && ( +
+
+
+ +
+

+ {parsed.filename} +

+

+ {parsed.rowCount.toLocaleString("en-GB")} row + {parsed.rowCount === 1 ? "" : "s"} ·{" "} + {parsed.headers.length} column + {parsed.headers.length === 1 ? "" : "s"} +

+
+
+ +
+ +
+ + + + {parsed.headers.map((header, i) => ( + + ))} + + + + {parsed.preview.map((row, rowIndex) => ( + + {row.map((cell, cellIndex) => ( + + ))} + + ))} + +
+ {header} +
+ {cell === "" ? ( + — + ) : ( + cell + )} +
+
+ + {parsed.rowCount > parsed.preview.length && ( +

+ Showing the first {parsed.preview.length} of{" "} + {parsed.rowCount.toLocaleString("en-GB")} rows. +

+ )} + +
+ {/* Enabled by the column-mapping step (#415). */} + +
+
+ )} +
+ ); +} diff --git a/src/app/projects/[projectId]/import/page.tsx b/src/app/projects/[projectId]/import/page.tsx new file mode 100644 index 00000000..627f93c4 --- /dev/null +++ b/src/app/projects/[projectId]/import/page.tsx @@ -0,0 +1,93 @@ +import { notFound } from "next/navigation"; +import { requireProjectAccess } from "../../authz"; +import { canManageProject } from "@/lib/projects/authz"; +import { + loadAuthzUser, + loadProjectAuthzFacts, +} from "@/app/repositories/projects/authzRepository"; +import { ImportUpload } from "./components/ImportUpload"; + +export const metadata = { + title: "Import programme data | Ara", +}; + +/** + * Work-order import, step 1 of 3 — upload and parse (issue #414). + * + * Wireframe: `docs/wireframes/ara-projects/1-admin-import/07-import-upload.html`. + * The mapping step (#415), validation (#416) and commit (#417) follow; this + * page ends at a parsed preview and writes nothing. + * + * Only internal and client-org managers may import, so this checks + * `canManageProject` rather than settling for the layout's view-level guard — + * a contractor with legitimate access to the project must not see the drop + * zone at all, not merely have their upload rejected by the route handler. + */ +export default async function ImportPage(props: { + params: Promise<{ projectId: string }>; +}) { + const { projectId } = await props.params; + const session = await requireProjectAccess(projectId); + + if (!/^\d+$/.test(projectId)) notFound(); + + const [user, project] = await Promise.all([ + loadAuthzUser(BigInt(session.user.dbId)), + loadProjectAuthzFacts(BigInt(projectId)), + ]); + // 404 rather than 403 throughout, so project existence stays private. + if (!user || !project || !canManageProject(user, project)) notFound(); + + return ( +
+
+

+ Work-order import · Step 1 of 3 +

+

+ Import programme data +

+

+ Upload a file listing which workstreams each property takes — one row + per property and workstream. A property taking Windows and Doors + appears twice, sharing its identifier. +

+
+ + + +
+ {[ + { + title: "Upload & parse", + body: "We read your file and show you the columns and a sample of the rows.", + }, + { + title: "Column mapping", + body: "Match your headings to the fields we expect.", + }, + { + title: "Validate & commit", + body: "Review what will be created before anything is written.", + }, + ].map((step, i) => ( +
+

+ Step {i + 1} +

+

+ {step.title} +

+

{step.body}

+
+ ))} +
+
+ ); +} diff --git a/src/lib/projects/import/parse.test.ts b/src/lib/projects/import/parse.test.ts new file mode 100644 index 00000000..3ff07100 --- /dev/null +++ b/src/lib/projects/import/parse.test.ts @@ -0,0 +1,333 @@ +import { describe, expect, it } from "vitest"; +import * as XLSX from "xlsx"; +import { + buildImportTemplateCsv, + hasAcceptedImportExtension, + IMPORT_PREVIEW_ROWS, + IMPORT_TEMPLATE_COLUMNS, + MAX_IMPORT_ROWS, + parseImportFile, + parseImportSheet, +} from "./parse"; + +/** + * Build a real workbook buffer in-memory rather than reading a checked-in + * fixture — the repo has no vitest fixture convention, and generating the bytes + * keeps these tests filesystem- and database-free. + */ +function workbookBuffer(rows: unknown[][], bookType: "xlsx" | "csv"): Uint8Array { + const wb = XLSX.utils.book_new(); + XLSX.utils.book_append_sheet(wb, XLSX.utils.aoa_to_sheet(rows), "Sheet1"); + const out = XLSX.write(wb, { type: "array", bookType }); + return new Uint8Array(out); +} + +const HEADERS = [...IMPORT_TEMPLATE_COLUMNS]; + +describe("parseImportSheet", () => { + it("returns headers, rows, count and a preview", () => { + const result = parseImportSheet([ + HEADERS, + ["PROP-0001", "Windows", "WO-1001", "2026-09-30"], + ["PROP-0002", "Roofs", "", ""], + ]); + + expect(result).toEqual({ + ok: true, + file: { + headers: HEADERS, + rows: [ + ["PROP-0001", "Windows", "WO-1001", "2026-09-30"], + ["PROP-0002", "Roofs", "", ""], + ], + rowCount: 2, + preview: [ + ["PROP-0001", "Windows", "WO-1001", "2026-09-30"], + ["PROP-0002", "Roofs", "", ""], + ], + }, + }); + }); + + it("parses a property appearing on multiple rows without complaint", () => { + // One row per (property, workstream) is the decided file format: grouping + // the duplicates is the commit step's job (#417), not the parser's. + const result = parseImportSheet([ + HEADERS, + ["PROP-0001", "Windows", "", ""], + ["PROP-0001", "Doors", "", ""], + ["PROP-0001", "Roofs", "", ""], + ]); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.rowCount).toBe(3); + expect(result.file.rows.map((r) => r[0])).toEqual([ + "PROP-0001", + "PROP-0001", + "PROP-0001", + ]); + expect(result.file.rows.map((r) => r[1])).toEqual([ + "Windows", + "Doors", + "Roofs", + ]); + }); + + it("accepts arbitrary client headings — mapping them is #415's job", () => { + const result = parseImportSheet([ + ["Asset Ref", "Trade", "Contract No"], + ["A-1", "Windows", "C-9"], + ]); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.headers).toEqual(["Asset Ref", "Trade", "Contract No"]); + }); + + it("skips blank lead-in rows and finds the real header row", () => { + const result = parseImportSheet([ + ["", "", "", ""], + ["", "", "", ""], + HEADERS, + ["PROP-0001", "Windows", "", ""], + ]); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.headers).toEqual(HEADERS); + expect(result.file.rowCount).toBe(1); + }); + + it("trims cells, drops blank rows, and pads ragged ones", () => { + const result = parseImportSheet([ + HEADERS, + [" PROP-0001 ", " Windows ", "", ""], + ["", "", "", ""], + ["PROP-0002", "Doors"], + ]); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.rows).toEqual([ + ["PROP-0001", "Windows", "", ""], + ["PROP-0002", "Doors", "", ""], + ]); + }); + + it("truncates rows longer than the header row", () => { + const result = parseImportSheet([ + ["Property identifier", "Workstream"], + ["PROP-0001", "Windows", "stray", "cells"], + ]); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.rows).toEqual([["PROP-0001", "Windows"]]); + }); + + it("drops trailing blank header columns", () => { + const result = parseImportSheet([ + ["Property identifier", "Workstream", "", ""], + ["PROP-0001", "Windows", "", ""], + ]); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.headers).toEqual(["Property identifier", "Workstream"]); + expect(result.file.rows).toEqual([["PROP-0001", "Windows"]]); + }); + + it("caps the preview at IMPORT_PREVIEW_ROWS but keeps every row", () => { + const rows: string[][] = [HEADERS]; + for (let i = 0; i < IMPORT_PREVIEW_ROWS + 5; i++) { + rows.push([`PROP-${i}`, "Windows", "", ""]); + } + + const result = parseImportSheet(rows); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.rowCount).toBe(IMPORT_PREVIEW_ROWS + 5); + expect(result.file.rows).toHaveLength(IMPORT_PREVIEW_ROWS + 5); + expect(result.file.preview).toHaveLength(IMPORT_PREVIEW_ROWS); + expect(result.file.preview[0]).toEqual(["PROP-0", "Windows", "", ""]); + }); + + it("errors on an entirely empty sheet", () => { + expect(parseImportSheet([])).toEqual({ + ok: false, + error: + "That file is empty — it needs a header row and at least one row of data.", + }); + expect(parseImportSheet([["", ""], ["", ""]])).toEqual({ + ok: false, + error: + "That file is empty — it needs a header row and at least one row of data.", + }); + }); + + it("errors when there is a header row but no data", () => { + expect(parseImportSheet([HEADERS])).toEqual({ + ok: false, + error: "The file has a header row but no rows of data.", + }); + }); + + it("errors on a gap between named header columns", () => { + expect( + parseImportSheet([ + ["Property identifier", "", "Workstream"], + ["PROP-0001", "", "Windows"], + ]), + ).toEqual({ + ok: false, + error: + "Column 2 has no heading — give every column a heading, or remove the empty column.", + }); + }); + + it("errors on duplicate headings, ignoring case and spacing", () => { + expect( + parseImportSheet([ + ["Workstream", "Property identifier", " workstream "], + ["Windows", "PROP-0001", "Doors"], + ]), + ).toEqual({ + ok: false, + error: + "The file has more than one 'workstream' column — give each column a unique heading.", + }); + }); + + it("errors when the row cap is exceeded", () => { + const rows: string[][] = [HEADERS]; + for (let i = 0; i <= MAX_IMPORT_ROWS; i++) { + rows.push([`PROP-${i}`, "Windows", "", ""]); + } + + const result = parseImportSheet(rows); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain("split it into smaller files"); + }); +}); + +describe("parseImportFile", () => { + it("parses a real XLSX buffer", () => { + const buf = workbookBuffer( + [HEADERS, ["PROP-0001", "Windows", "WO-1001", "2026-09-30"]], + "xlsx", + ); + + const result = parseImportFile(buf); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.headers).toEqual(HEADERS); + expect(result.file.rows).toEqual([ + ["PROP-0001", "Windows", "WO-1001", "2026-09-30"], + ]); + }); + + it("parses a real CSV buffer", () => { + const buf = workbookBuffer( + [HEADERS, ["PROP-0001", "Windows", "", ""]], + "csv", + ); + + const result = parseImportFile(buf); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.rows).toEqual([["PROP-0001", "Windows", "", ""]]); + }); + + it("keeps the leading zero on a numeric-looking identifier", () => { + // raw:false is what buys this — SheetJS would otherwise hand back the + // number 7510027001 and the identifier would stop matching. + const buf = workbookBuffer( + [["Property identifier", "Workstream"], ["07510027001", "Windows"]], + "xlsx", + ); + + const result = parseImportFile(buf); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.rows[0][0]).toBe("07510027001"); + }); + + it("reads the round-tripped download template", () => { + const result = parseImportFile( + new TextEncoder().encode(buildImportTemplateCsv()), + ); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.headers).toEqual(HEADERS); + // The template demonstrates one property taking two workstreams. + expect(result.file.rows[0][0]).toBe("PROP-0001"); + expect(result.file.rows[1][0]).toBe("PROP-0001"); + expect(result.file.rows[0][1]).toBe("Windows"); + expect(result.file.rows[1][1]).toBe("Doors"); + }); + + it("rejects empty and single-line rubbish with a friendly error", () => { + // SheetJS does not throw on loose bytes — it reads them as CSV — so these + // are turned away by the header/row checks rather than the read. What the + // acceptance criteria care about is the friendly error, not the branch. + for (const bytes of ["", "\x00\x01\x02 not a spreadsheet"]) { + const result = parseImportFile(new TextEncoder().encode(bytes)); + expect(result.ok).toBe(false); + if (result.ok) continue; + expect(result.error.length).toBeGreaterThan(0); + } + }); + + it("known limitation: multi-line text parses as a degenerate sheet", () => { + // A PDF renamed to .csv has more than one line, so SheetJS reads line 1 as + // a header and the rest as data and the parse "succeeds" with nonsense + // headers. Nothing is written at this step, and the extension allowlist on + // the route handler turns away the honestly-named case; the user's real + // signal is the garbage they see in the preview, and the mapping step + // (#415) has no columns to offer. Documented rather than defended against + // — sniffing content types here would be guesswork. + const result = parseImportFile( + new TextEncoder().encode("%PDF-1.4\n%\xe2\xe3\xcf\xd3\n1 0 obj"), + ); + + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.file.headers).toEqual(["%PDF-1.4"]); + }); + + it("returns a friendly error for a corrupt workbook", () => { + // Zip magic bytes followed by rubbish — SheetJS throws on this one. + const result = parseImportFile( + new Uint8Array([0x50, 0x4b, 0x03, 0x04, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]), + ); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toMatch(/valid CSV or Excel file/); + }); +}); + +describe("hasAcceptedImportExtension", () => { + it("accepts csv, xlsx and xls in any case", () => { + expect(hasAcceptedImportExtension("programme.csv")).toBe(true); + expect(hasAcceptedImportExtension("programme.XLSX")).toBe(true); + expect(hasAcceptedImportExtension("Programme.Xls")).toBe(true); + }); + + it("rejects anything else", () => { + expect(hasAcceptedImportExtension("programme.pdf")).toBe(false); + expect(hasAcceptedImportExtension("programme")).toBe(false); + expect(hasAcceptedImportExtension("csv.docx")).toBe(false); + }); +}); + +describe("buildImportTemplateCsv", () => { + it("leads with the template columns in order", () => { + expect(buildImportTemplateCsv().split("\r\n")[0]).toBe( + IMPORT_TEMPLATE_COLUMNS.join(","), + ); + }); +}); diff --git a/src/lib/projects/import/parse.ts b/src/lib/projects/import/parse.ts new file mode 100644 index 00000000..3f31f589 --- /dev/null +++ b/src/lib/projects/import/parse.ts @@ -0,0 +1,263 @@ +/** + * Work-order import — file parsing (issue #414, step 1 of 3). + * + * The "second import": properties already exist in the DB (via BulkUpload or + * postcode search); this file declares **which Workstreams each Property + * gets**. One row per (Property, Workstream) — a Property taking Windows and + * Doors appears twice, sharing an identifier. Grouping those duplicates is the + * commit step's job (#417), not this module's. + * + * Deliberately **pure and DB-free**, mirroring `@/lib/tags/bulkAssign`: it takes + * bytes or a 2D array of cells and returns data or a friendly `error` string. + * That keeps it unit-testable without a connection pool, and keeps the route + * handler down to authorization plus a function call. + * + * Scope note: this module is **header-agnostic on purpose**. It reports the + * headers it found and hands back the rows; it does not insist on the template + * columns and does not validate cell contents. Mapping arbitrary client headings + * onto the template columns is the mapping step (#415), and validating the + * mapped values is #416. Rejecting an unrecognised heading here would make the + * mapping step unreachable for exactly the files that need it most. + */ + +import * as XLSX from "xlsx"; + +/** + * The columns the "Download template" artefact ships with, in order. + * + * `Property identifier` is the landlord property id *or* the UPRN — matching + * decides which at validation time (#416). The last two are optional per row. + */ +export const IMPORT_TEMPLATE_COLUMNS = [ + "Property identifier", + "Workstream", + "Work order reference", + "Forecast end date", +] as const; + +/** + * Upper bound on the upload, enforced client- and server-side. + * + * The wireframe says 50MB; 10MB is well past what this shape of file can + * plausibly reach (four narrow columns, one row per property-workstream), and + * it keeps the whole-file-in-memory parse honest. + */ +export const MAX_IMPORT_FILE_BYTES = 10 * 1024 * 1024; + +/** Upper bound on data rows, so an accidental 500k-row export fails fast. */ +export const MAX_IMPORT_ROWS = 20_000; + +/** How many rows the preview carries back for the mapping step. */ +export const IMPORT_PREVIEW_ROWS = 10; + +/** Extensions offered by the drop zone and re-checked on the server. */ +export const ACCEPTED_IMPORT_EXTENSIONS = [".csv", ".xlsx", ".xls"] as const; + +export interface ParsedImportFile { + /** Header row, trimmed, in file order. Trailing blank columns dropped. */ + headers: string[]; + /** + * Every data row, each padded/truncated to `headers.length`. All-blank rows + * are dropped. Cell values are trimmed strings — never numbers, so a numeric + * identifier keeps its leading zeros. + */ + rows: string[][]; + /** `rows.length`, carried explicitly so callers need not hold `rows` to show it. */ + rowCount: number; + /** The first `IMPORT_PREVIEW_ROWS` of `rows`, for the mapping step's sample table. */ + preview: string[][]; +} + +export type ParseImportResult = + | { ok: true; file: ParsedImportFile } + | { ok: false; error: string }; + +/** What SheetJS hands back per cell before we normalise it. */ +type Cell = string | number | boolean | null | undefined; + +/** + * Read the first sheet of a CSV/XLSX/XLS buffer into rows of cells. + * + * `raw: false` makes SheetJS return each cell's *displayed* text rather than an + * inferred type — so identifier `"07510027001"` survives instead of being + * coerced to the number 7510027001 with its leading zero dropped. `defval: ""` + * keeps ragged rows aligned to their column positions rather than collapsing + * gaps. Both match the conventions in `@/lib/tags/bulkAssign` and + * `@/lib/bulkUpload/addressMatches`. + * + * Throws whatever SheetJS throws on an unreadable file — `parseImportFile` + * turns that into a friendly error. + */ +export function readImportSheet(buf: ArrayBuffer | Uint8Array): Cell[][] { + const wb = XLSX.read(buf, { type: "array" }); + const sheetName = wb.SheetNames[0]; + if (!sheetName) return []; + const sheet = wb.Sheets[sheetName]; + if (!sheet) return []; + return XLSX.utils.sheet_to_json(sheet, { + header: 1, + raw: false, + defval: "", + }) as Cell[][]; +} + +/** A cell as trimmed text. Absent, null and blank all collapse to "". */ +function text(cell: Cell): string { + return String(cell ?? "").trim(); +} + +/** True when every cell in the row is blank — a spacer row, not data. */ +function isBlankRow(row: Cell[]): boolean { + return row.every((cell) => text(cell) === ""); +} + +/** + * Turn raw sheet cells into headers plus rows, or a friendly error. + * + * Exported separately from `parseImportFile` so tests (and any future caller + * that already holds cells) can exercise the row logic without going through + * SheetJS. + */ +export function parseImportSheet(rows: Cell[][]): ParseImportResult { + // Spreadsheets exported from asset-management systems often carry a blank + // lead-in row or two, so find the first row with content rather than + // assuming index 0. + const headerIndex = rows.findIndex((row) => !isBlankRow(row)); + if (headerIndex === -1) { + return { + ok: false, + error: "That file is empty — it needs a header row and at least one row of data.", + }; + } + + // Trailing blank columns are an artefact of how spreadsheets size their used + // range; drop them so the header count reflects real columns. + const rawHeaders = rows[headerIndex].map(text); + let lastUsed = rawHeaders.length - 1; + while (lastUsed >= 0 && rawHeaders[lastUsed] === "") lastUsed--; + const headers = rawHeaders.slice(0, lastUsed + 1); + + if (headers.length === 0) { + return { + ok: false, + error: "Couldn't find a header row — the first row with content is blank.", + }; + } + + // A gap *between* named columns would silently misalign the mapping step. + const blankAt = headers.indexOf(""); + if (blankAt !== -1) { + return { + ok: false, + error: `Column ${blankAt + 1} has no heading — give every column a heading, or remove the empty column.`, + }; + } + + // Two identically-named columns make "which column did I map?" unanswerable. + const duplicate = findDuplicateHeader(headers); + if (duplicate) { + return { + ok: false, + error: `The file has more than one '${duplicate}' column — give each column a unique heading.`, + }; + } + + const dataRows: string[][] = []; + for (const row of rows.slice(headerIndex + 1)) { + if (isBlankRow(row)) continue; + // Pad short rows and truncate long ones so every row lines up with headers. + dataRows.push( + Array.from({ length: headers.length }, (_, i) => text(row[i])), + ); + // Bail as soon as the cap is passed — no point normalising 500k more rows. + if (dataRows.length > MAX_IMPORT_ROWS) { + return { + ok: false, + error: `The file has more than ${MAX_IMPORT_ROWS.toLocaleString("en-GB")} rows — split it into smaller files.`, + }; + } + } + + if (dataRows.length === 0) { + return { + ok: false, + error: "The file has a header row but no rows of data.", + }; + } + + return { + ok: true, + file: { + headers, + rows: dataRows, + rowCount: dataRows.length, + preview: dataRows.slice(0, IMPORT_PREVIEW_ROWS), + }, + }; +} + +/** The first header that appears twice (case- and whitespace-insensitively). */ +function findDuplicateHeader(headers: string[]): string | null { + const seen = new Set(); + for (const header of headers) { + const key = header.toLowerCase().replace(/\s+/g, " "); + if (seen.has(key)) return header; + seen.add(key); + } + return null; +} + +/** + * Read and parse an uploaded file end to end, turning every failure into a + * friendly `error` the route handler can return verbatim. + * + * Note that SheetJS only *throws* on a structurally broken container — a + * corrupt or encrypted XLSX zip. Loose bytes (a PDF, an image, random noise) + * are read happily as a one-line CSV, so they arrive at `parseImportSheet` as a + * degenerate sheet and are turned away by the header/row checks there rather + * than here. The extension allowlist on the route handler is what keeps that + * from being the user's first line of defence. + */ +export function parseImportFile(buf: ArrayBuffer | Uint8Array): ParseImportResult { + let cells: Cell[][]; + try { + cells = readImportSheet(buf); + } catch { + return { + ok: false, + error: "Couldn't read that file — is it a valid CSV or Excel file?", + }; + } + return parseImportSheet(cells); +} + +/** True when the filename carries one of the accepted extensions. */ +export function hasAcceptedImportExtension(filename: string): boolean { + const lower = filename.toLowerCase(); + return ACCEPTED_IMPORT_EXTENSIONS.some((ext) => lower.endsWith(ext)); +} + +/** + * The "Download template" artefact: a CSV carrying the expected headings plus + * one worked example showing a Property that takes two Workstreams — the file + * shape's one non-obvious rule, demonstrated rather than explained. + * + * CSV rather than XLSX so it opens the same everywhere and stays diffable. + */ +export function buildImportTemplateCsv(): string { + const example = [ + ["PROP-0001", "Windows", "WO-1001", "2026-09-30"], + ["PROP-0001", "Doors", "WO-1002", "2026-10-15"], + ["PROP-0002", "Roofs", "", ""], + ]; + return ( + [[...IMPORT_TEMPLATE_COLUMNS], ...example] + .map((row) => row.map(csvCell).join(",")) + .join("\r\n") + "\r\n" + ); +} + +/** Quote a CSV cell only when it needs it. */ +function csvCell(value: string): string { + return /[",\r\n]/.test(value) ? `"${value.replace(/"/g, '""')}"` : value; +} From a7a1f8ee982c88d21473d6a6ff506598ea83a6ca Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Tue, 21 Jul 2026 16:52:53 +0000 Subject: [PATCH 03/63] feat(ara-projects): projects list, empty state and create-project modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setup wizard step 1/5 (#409). The list is a Server Component, so the visibility rule runs before anything reaches the browser and a project the user may not see is never serialised to it. The rule itself is not restated: `listVisibleProjects` loads each project's facts and asks `canViewProject` from `@/lib/projects/authz`. Writing the `domna_admin_access OR owning-org OR contractor-org` disjunction a second time in SQL would have given it two definitions free to drift, and the acceptance criteria (client sees its own, contractor sees what it is assigned to, internal sees all subject to `domna_admin_access`) fall out of the one definition instead of being re-derived. The cost is filtering in the app rather than the database, over two round trips and no N+1. That is the right trade while a project is a works programme and they number in the tens; if the table grows, the fix is a superset prefilter that still lets `canViewProject` decide, not a WHERE clause that re-implements it. Flagged in the function's own docs. Creation reuses the same guard rather than inventing a "can create" rule: `prospectiveProjectFacts` builds the facts the project *would* have, and `canManageProject` judges them. One consequence is worth knowing, and is covered by a test — `domna_admin_access` gates the `internal` role, so a Domna user creating for an organisation they are not a member of must grant that access. Without it they would be creating a project they could not then see, and the guard refuses rather than producing an orphan. The modal offers only organisations the user may pick, but the route handler re-checks the submitted one: a hidden option is not a permission. `createProjectSchema` is shared by the form (as a zodResolver) and the route handler (as the body parser), so the two cannot disagree about what a valid draft is. Empty strings from untouched inputs normalise to undefined rather than reaching the columns as "". Also updates projects-shell.cy.js. Its "renders the per-project dashboard shell" test passed only because the authz stub could never return false; with the real lib wired in, /projects/ is a 404 without standing on that project, so the test now asserts that instead. TESTS: the vitest unit tests pass (50, no database connection). Cypress was NOT run — this environment's DATABASE_URL points at production. The new spec is therefore unverified, and its end-to-end half, which INSERTs, is gated behind CYPRESS_ARA_PROJECTS_E2E_WRITES so it cannot fire by accident; it also needs the #407 seed migration (0274) applied, which has not been run against any database yet either. Co-Authored-By: Claude Opus 4.8 (1M context) --- cypress/e2e/projects/create-project.cy.js | 191 ++++++++++ cypress/e2e/projects/projects-shell.cy.js | 32 +- src/app/api/projects/route.ts | 80 +++++ .../components/CreateProjectDialog.tsx | 333 ++++++++++++++++++ src/app/projects/page.tsx | 207 +++++++++-- .../projects/projectsRepository.ts | 192 ++++++++++ src/lib/projects/createProject.test.ts | 163 +++++++++ src/lib/projects/createProject.ts | 94 +++++ 8 files changed, 1254 insertions(+), 38 deletions(-) create mode 100644 cypress/e2e/projects/create-project.cy.js create mode 100644 src/app/api/projects/route.ts create mode 100644 src/app/projects/components/CreateProjectDialog.tsx create mode 100644 src/app/repositories/projects/projectsRepository.ts create mode 100644 src/lib/projects/createProject.test.ts create mode 100644 src/lib/projects/createProject.ts diff --git a/cypress/e2e/projects/create-project.cy.js b/cypress/e2e/projects/create-project.cy.js new file mode 100644 index 00000000..c868632e --- /dev/null +++ b/cypress/e2e/projects/create-project.cy.js @@ -0,0 +1,191 @@ +/** + * Ara Projects — projects list, empty state and create-project modal (#409) + * + * Two groups, because one of them writes: + * + * 1. The UI group stubs POST /api/projects with cy.intercept. It exercises + * the empty state, the modal, client-side validation and the navigation + * to workstream selection without touching the database, so it is safe to + * run anywhere. + * + * 2. The end-to-end group performs a real create and is the acceptance + * criterion's "create a project end-to-end". It INSERTs, so it is gated + * behind CYPRESS_ARA_PROJECTS_E2E_WRITES and must only ever be pointed at + * a non-production database. It also needs the #407 reference-data + * migration (0274) applied, or the project-type select will be empty and + * the page will not offer the CTA at all. + * + * Like the shell spec, this uses `cy.login` (cypress/support/commands.ts) to + * mint a real next-auth JWE cookie, so NEXTAUTH_JWT_SECRET must be set. + * + * NOT YET RUN. The environment this was written in has DATABASE_URL pointing at + * production, so Cypress was deliberately not executed. Treat the selectors and + * assertions below as unverified until someone runs them against a scratch DB. + */ + +const JWT_SECRET = Cypress.env("NEXTAUTH_JWT_SECRET"); +const E2E_WRITES = Cypress.env("ARA_PROJECTS_E2E_WRITES"); + +const INTERNAL_USER = { + email: "dev@domna.homes", + name: "Internal User", + onboarded: true, + sub: "cypress-internal", +}; + +describe("Ara Projects — create project (stubbed)", function () { + beforeEach(function () { + if (!JWT_SECRET) { + cy.log("NEXTAUTH_JWT_SECRET not set — skipping"); + this.skip(); + } + cy.login(INTERNAL_USER); + }); + + it("offers the Create project CTA from the list", function () { + cy.visit("/projects"); + + // The CTA sits inside the welcome card when there are no projects, and in + // the page header once there are — either way there is exactly one. + cy.get("[data-testid=create-project-trigger]").should("be.visible").click(); + cy.get("[data-testid=create-project-form]").should("be.visible"); + cy.contains("Project details").should("be.visible"); + }); + + it("shows the welcome empty state when the user has no visible projects", function () { + // Only meaningful on an instance with no projects visible to this user; + // where there are some, the list renders instead and this is skipped. + cy.visit("/projects"); + cy.get("body").then(($body) => { + if ($body.find("[data-testid=projects-list-empty]").length === 0) { + cy.log("user has visible projects — empty state not applicable"); + return; + } + cy.contains("Welcome to Ara Projects").should("be.visible"); + cy.contains("How setup works").should("be.visible"); + cy.get("[data-testid=create-project-trigger]").should("be.visible"); + }); + }); + + it("refuses to submit without a name and a project type", function () { + cy.visit("/projects"); + cy.get("[data-testid=create-project-trigger]").click(); + + cy.intercept("POST", "/api/projects", cy.spy().as("createCall")); + cy.get("[data-testid=create-project-submit]").click(); + + cy.contains("Enter a project name").should("be.visible"); + cy.get("@createCall").should("not.have.been.called"); + }); + + it("rejects an end date before the start date", function () { + cy.visit("/projects"); + cy.get("[data-testid=create-project-trigger]").click(); + + cy.get("[data-testid=create-project-name]").type("Backwards dates"); + cy.get("[data-testid=create-project-start-date]").type("2026-09-30"); + cy.get("[data-testid=create-project-end-date]").type("2026-03-01"); + cy.get("[data-testid=create-project-submit]").click(); + + cy.contains("The end date cannot be before the start date").should( + "be.visible", + ); + }); + + it("navigates to workstream selection on success", function () { + cy.intercept("POST", "/api/projects", { + statusCode: 201, + body: { id: "4242" }, + }).as("createProject"); + + cy.visit("/projects"); + cy.get("[data-testid=create-project-trigger]").click(); + + cy.get("[data-testid=create-project-name]").type("Riverside retrofit 2026"); + selectFirstOption("create-project-organisation"); + selectFirstOption("create-project-type"); + cy.get("[data-testid=create-project-domna-admin-access]").click(); + cy.get("[data-testid=create-project-submit]").click(); + + cy.wait("@createProject").then(({ request }) => { + expect(request.body.name).to.eq("Riverside retrofit 2026"); + expect(request.body.domnaAdminAccess).to.eq(true); + }); + + cy.location("pathname").should("eq", "/projects/4242/setup/workstreams"); + }); + + it("surfaces the server's error message on a refusal", function () { + cy.intercept("POST", "/api/projects", { + statusCode: 403, + body: { error: "You cannot create a project for that organisation." }, + }).as("createProject"); + + cy.visit("/projects"); + cy.get("[data-testid=create-project-trigger]").click(); + cy.get("[data-testid=create-project-name]").type("Not mine"); + selectFirstOption("create-project-organisation"); + selectFirstOption("create-project-type"); + cy.get("[data-testid=create-project-submit]").click(); + + cy.wait("@createProject"); + cy.get("[data-testid=create-project-error]") + .should("be.visible") + .and("contain", "cannot create a project for that organisation"); + cy.location("pathname").should("eq", "/projects"); + }); +}); + +describe("Ara Projects — create project (end to end, writes)", function () { + beforeEach(function () { + if (!JWT_SECRET || !E2E_WRITES) { + cy.log( + "ARA_PROJECTS_E2E_WRITES not set — skipping the writing happy path", + ); + this.skip(); + } + cy.login(INTERNAL_USER); + }); + + it("creates a project and lands on workstream selection", function () { + // Unique per run so repeated runs do not collide on a name a human is + // eyeballing later. `project.name` has no unique constraint; this is for + // legibility, not correctness. + const name = `Cypress project ${Date.now()}`; + + cy.visit("/projects"); + cy.get("[data-testid=create-project-trigger]").click(); + + cy.get("[data-testid=create-project-name]").type(name); + selectFirstOption("create-project-organisation"); + selectFirstOption("create-project-type"); + cy.get("[data-testid=create-project-description]").type( + "Created by the Cypress happy path.", + ); + cy.get("[data-testid=create-project-start-date]").type("2026-03-01"); + cy.get("[data-testid=create-project-end-date]").type("2026-09-30"); + // Required for an internal user creating on behalf of an organisation they + // are not a member of — without it the guard refuses, by design. + cy.get("[data-testid=create-project-domna-admin-access]").click(); + + cy.get("[data-testid=create-project-submit]").click(); + + cy.location("pathname").should("match", /^\/projects\/\d+\/setup\/workstreams$/); + + // The new project is now visible in the list, which is the visibility rule + // agreeing with the write that just happened. + cy.visit("/projects"); + cy.get("[data-testid=projects-list]").should("exist"); + cy.contains("[data-testid=projects-list-item]", name).should("be.visible"); + }); +}); + +/** + * Pick the first item in a shadcn/Radix `Select`. The options live in a portal + * outside the trigger, so this opens the trigger and then reaches for the + * listbox at the document root. + */ +function selectFirstOption(testId) { + cy.get(`[data-testid=${testId}]`).click(); + cy.get("[role=option]").first().click(); +} diff --git a/cypress/e2e/projects/projects-shell.cy.js b/cypress/e2e/projects/projects-shell.cy.js index 2676d9da..6e484d68 100644 --- a/cypress/e2e/projects/projects-shell.cy.js +++ b/cypress/e2e/projects/projects-shell.cy.js @@ -76,7 +76,17 @@ describe("Ara Projects route shell", function () { cy.get("[data-testid=projects-shell]").should("exist"); }); - it("renders the per-project dashboard shell and its nav", function () { + // This assertion changed with #409. While the authz stub was in place, + // `canViewProject` could never return false, so /projects/ rendered + // the dashboard shell for any signed-in user — which is what this test used + // to check. The real lib (#408) is now wired in, so a project the user has no + // standing on is a 404, and a project id that does not exist is the same 404 + // (deliberately indistinguishable, so Project existence does not leak). + // + // Rendering the dashboard shell therefore now needs a real, visible project, + // which is covered by the create-project happy path in create-project.cy.js + // rather than by a hardcoded id here. + it("404s on a project the user has no standing on", function () { cy.login({ email: "dev@domna.homes", name: "Internal User", @@ -84,13 +94,21 @@ describe("Ara Projects route shell", function () { sub: "cypress-internal", }); - cy.visit("/projects/1"); + cy.request({ url: "/projects/999999999", failOnStatusCode: false }) + .its("status") + .should("eq", 404); + }); - cy.get("[data-testid=project-shell]").should("exist"); - cy.get("[data-testid=project-dashboard-heading]").should("be.visible"); - - ["dashboard", "workOrders", "import", "settings"].forEach((item) => { - cy.get(`[data-testid=projects-nav-${item}]`).should("be.visible"); + it("404s on a malformed project id", function () { + cy.login({ + email: "dev@domna.homes", + name: "Internal User", + onboarded: true, + sub: "cypress-internal", }); + + cy.request({ url: "/projects/not-an-id", failOnStatusCode: false }) + .its("status") + .should("eq", 404); }); }); diff --git a/src/app/api/projects/route.ts b/src/app/api/projects/route.ts new file mode 100644 index 00000000..8e0c3bd1 --- /dev/null +++ b/src/app/api/projects/route.ts @@ -0,0 +1,80 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getServerSession } from "next-auth"; +import { AuthOptions } from "@/app/api/auth/[...nextauth]/authOptions"; +import { canManageProject } from "@/lib/projects/authz"; +import { + createProjectSchema, + prospectiveProjectFacts, +} from "@/lib/projects/createProject"; +import { loadAuthzUserByEmail } from "@/app/repositories/projects/authzRepository"; +import { + insertProject, + organisationExists, + projectTypeExists, +} from "@/app/repositories/projects/projectsRepository"; + +/** + * POST /api/projects — create an Ara Project (issue #409). + * + * The authorization question "may this user create a project for this + * organisation?" is answered by the *existing* guard: build the facts the + * project would have once created and ask `canManageProject`. That keeps + * creation on the same rule as every other management action instead of + * inventing a parallel one. + * + * The rule has a consequence worth naming: `domna_admin_access` gates the + * `internal` role, so a Domna user creating a project for an organisation they + * are not a member of must grant that access. Without it they would be + * creating a project they could not then see, and the guard refuses rather + * than producing an orphan. + */ +export async function POST(req: NextRequest) { + const session = await getServerSession(AuthOptions); + if (!session?.user?.email) { + return NextResponse.json({ error: "Unauthorised" }, { status: 401 }); + } + + const user = await loadAuthzUserByEmail(session.user.email); + if (!user) { + return NextResponse.json({ error: "Unauthorised" }, { status: 401 }); + } + + const parsed = createProjectSchema.safeParse(await req.json().catch(() => null)); + if (!parsed.success) { + return NextResponse.json( + { error: parsed.error.issues[0]?.message ?? "Invalid body" }, + { status: 400 }, + ); + } + const draft = parsed.data; + + if (!canManageProject(user, prospectiveProjectFacts(draft))) { + return NextResponse.json( + { + error: + "You cannot create a project for that organisation. Granting Domna admin access is required for an organisation you are not a member of.", + }, + { status: 403 }, + ); + } + + // Checked after authorization so an unauthorized caller cannot use the + // difference between 400 and 403 to probe which organisations exist. + const projectTypeId = BigInt(draft.projectTypeId); + const [orgOk, typeOk] = await Promise.all([ + organisationExists(draft.organisationId), + projectTypeExists(projectTypeId), + ]); + if (!orgOk) { + return NextResponse.json({ error: "Unknown organisation" }, { status: 400 }); + } + if (!typeOk) { + return NextResponse.json({ error: "Unknown project type" }, { status: 400 }); + } + + const id = await insertProject({ ...draft, projectTypeId }); + + // `id` is a bigint; JSON cannot carry one, so the client gets a string and + // uses it to build the next wizard step's path. + return NextResponse.json({ id: id.toString() }, { status: 201 }); +} diff --git a/src/app/projects/components/CreateProjectDialog.tsx b/src/app/projects/components/CreateProjectDialog.tsx new file mode 100644 index 00000000..505713a8 --- /dev/null +++ b/src/app/projects/components/CreateProjectDialog.tsx @@ -0,0 +1,333 @@ +"use client"; + +import { useState } from "react"; +import { useForm } from "react-hook-form"; +import { zodResolver } from "@hookform/resolvers/zod"; +import { useMutation } from "@tanstack/react-query"; +import { useRouter } from "next/navigation"; +import { PlusIcon } from "@heroicons/react/24/outline"; +import { + createProjectSchema, + type CreateProjectFormValues, + type SelectOption, +} from "@/lib/projects/createProject"; +import { Button } from "@/app/shadcn_components/ui/button"; +import { Checkbox } from "@/app/shadcn_components/ui/checkbox"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/app/shadcn_components/ui/dialog"; +import { + Form, + FormControl, + FormDescription, + FormField, + FormItem, + FormLabel, + FormMessage, +} from "@/app/shadcn_components/ui/form"; +import { Input } from "@/app/shadcn_components/ui/input"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/app/shadcn_components/ui/select"; + +/** + * The HTTP call, kept outside the component so the component itself contains + * no `fetch` — TanStack Query owns the request lifecycle. + * + * Route handlers in this codebase answer errors as `{ error: string }`, so a + * failure is surfaced with the server's own message rather than a generic one. + */ +async function createProject( + values: CreateProjectFormValues, +): Promise<{ id: string }> { + const response = await fetch("/api/projects", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(values), + }); + + const body = await response.json().catch(() => null); + if (!response.ok) { + throw new Error(body?.error ?? "Could not create the project"); + } + return body; +} + +export interface CreateProjectDialogProps { + organisations: SelectOption[]; + projectTypes: SelectOption[]; + /** Rendered as the trigger's label — "Create project" everywhere so far. */ + label?: string; +} + +/** + * Create-project modal — step 1 of the setup wizard. + * + * On success it navigates to workstream selection (#410) rather than back to + * the list: creating a project is the start of setup, not the end of it. + */ +export function CreateProjectDialog({ + organisations, + projectTypes, + label = "Create project", +}: CreateProjectDialogProps) { + const router = useRouter(); + const [open, setOpen] = useState(false); + + const form = useForm({ + resolver: zodResolver(createProjectSchema), + defaultValues: { + name: "", + // A client user typically belongs to exactly one organisation, so the + // select is pre-filled and effectively fixed. Internal users, offered + // every organisation, start with no choice made. + organisationId: organisations.length === 1 ? organisations[0].id : "", + projectTypeId: "", + description: "", + startDate: "", + endDate: "", + domnaAdminAccess: false, + }, + }); + + const mutation = useMutation(createProject, { + onSuccess: ({ id }) => router.push(`/projects/${id}/setup/workstreams`), + }); + + // Deliberately not reset on close: a user who dismissed the dialog by + // accident gets their typing back. A completed creation navigates away, so + // stale values are never shown against a project that already exists. + function onOpenChange(next: boolean) { + if (!next && mutation.isLoading) return; + setOpen(next); + if (next) mutation.reset(); + } + + return ( + <> + + + + + + Project details + + Step 1 of 5 — name the project and choose who it belongs to. You + will pick its workstreams next. + + + +
+ mutation.mutate(values))} + className="space-y-4" + data-testid="create-project-form" + > + ( + + Project name + + + + + + )} + /> + + ( + + Client + + {organisations.length === 1 && ( + + Projects you create belong to your organisation. + + )} + + + )} + /> + + ( + + Project type + + + + )} + /> + + ( + + Description + +