From 8f83d7156503b7208df14dc6802932d73df8d2ce Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 14:38:29 +1100 Subject: [PATCH] feat(frontend): review screen for stored uploads (M4b, #108) A Review link on each migrated file opens /review/{uploadId}. It pages through every stored row (100 a page) with column types, marks invalid cells and edited cells (title shows the reason or the original), and lets reviewers and tenant admins exclude or include a row and edit a cell, each with a required reason. A history dialog lists a row's decisions and edits. Other roles can view only. scripts/e2e/m4-review.js drives it in Chromium against a running API and frontend: 8 of 8 checks passed. VCRM: BR-25 and BR-26 verified. vitest 147 passed (4 new); tsc 0; production build OK. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019YPoXzBSj89ZHgw7nfTmbB --- .../src/__tests__/review-table.test.tsx | 122 ++++++ .../src/components/review-table.tsx | 355 ++++++++++++++++++ csv-table-hub-main/src/lib/csv.functions.ts | 78 ++++ csv-table-hub-main/src/routeTree.gen.ts | 31 +- .../src/routes/_authenticated/index.tsx | 2 +- .../_authenticated/review.$uploadId.tsx | 33 ++ docs/VCRM.md | 10 +- scripts/e2e/m4-review.js | 67 ++++ 8 files changed, 689 insertions(+), 9 deletions(-) create mode 100644 csv-table-hub-main/src/__tests__/review-table.test.tsx create mode 100644 csv-table-hub-main/src/components/review-table.tsx create mode 100644 csv-table-hub-main/src/routes/_authenticated/review.$uploadId.tsx create mode 100644 scripts/e2e/m4-review.js diff --git a/csv-table-hub-main/src/__tests__/review-table.test.tsx b/csv-table-hub-main/src/__tests__/review-table.test.tsx new file mode 100644 index 0000000..c208e0e --- /dev/null +++ b/csv-table-hub-main/src/__tests__/review-table.test.tsx @@ -0,0 +1,122 @@ +// M4 (#108): the review screen — rows, validation, decisions and edits. + +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { ReviewTable } from "@/components/review-table"; +import { clearSession } from "@/lib/session"; + +const row = (n: number, cells: string[], extra: object = {}) => ({ + rowNumber: n, + original: cells, + current: cells, + editedColumns: [], + decision: "include", + decisionReason: null, + errors: [], + ...extra, +}); + +const page = { + fileName: "people.csv", + columns: ["n", "t"], + types: ["int8", "text"], + total: 3, + offset: 0, + limit: 100, + excludedRows: 1, + editedRows: 1, + rows: [ + row(1, ["1", "a"], { current: ["10", "a"], editedColumns: [0] }), + row(2, ["x", "b"], { + errors: [{ column: "n", value: "x", reason: '"x" is not a whole number' }], + }), + row(3, ["3", "c"], { decision: "exclude", decisionReason: "duplicate of row 1" }), + ], +}; + +const json = (body: unknown) => + new Response(JSON.stringify(body), { headers: { "content-type": "application/json" } }); + +function show(role: string) { + sessionStorage.setItem( + "te.session", + JSON.stringify({ + token: "t", + expiresAt: "2099-01-01T00:00:00Z", + username: "u", + tenant: "acme", + role, + }), + ); + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => + init?.method === "PUT" + ? json(row(2, ["x", "b"], { decision: "exclude", decisionReason: "bad row" })) + : json(page), + ); + vi.stubGlobal("fetch", fetchMock); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + return fetchMock; +} + +beforeEach(() => clearSession()); +afterEach(() => vi.unstubAllGlobals()); + +it("shows every row with its validation, edits and decision", async () => { + show("reviewer"); + expect(await screen.findByText("people.csv")).toBeInTheDocument(); + expect(screen.getByText(/3 rows · 1 excluded · 1 edited/)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Edit row 1 n" }).closest("td")).toHaveAttribute( + "title", + "Original: 1", + ); + expect(screen.getByRole("button", { name: "Edit row 2 n" }).closest("td")).toHaveAttribute( + "title", + '"x" is not a whole number', + ); + expect(screen.getByText("Invalid")).toBeInTheDocument(); + expect(screen.getByText("Excluded")).toHaveAttribute("title", "duplicate of row 1"); +}); + +it("a reviewer excludes a row with a required reason", async () => { + const fetchMock = show("reviewer"); + const user = userEvent.setup(); + const line = (await screen.findByRole("button", { name: "Edit row 2 n" })).closest("tr")!; + await user.click(within(line as HTMLElement).getByRole("button", { name: "Exclude" })); + const save = screen.getByRole("button", { name: "Save" }); + expect(save).toBeDisabled(); + await user.type(screen.getByLabelText("Reason"), "bad row"); + await user.click(save); + const [url, init] = fetchMock.mock.calls.find(([, i]) => i?.method === "PUT")!; + expect(String(url)).toMatch(/\/api\/csv\/uploads\/7\/rows\/2\/decision$/); + expect(JSON.parse(String(init!.body))).toEqual({ decision: "exclude", reason: "bad row" }); +}); + +it("a reviewer edits a cell as a new value with a reason", async () => { + const fetchMock = show("tenant_admin"); + const user = userEvent.setup(); + await user.click(await screen.findByRole("button", { name: "Edit row 2 n" })); + expect(screen.getByText("Original value: x")).toBeInTheDocument(); + await user.clear(screen.getByLabelText("New value")); + await user.type(screen.getByLabelText("New value"), "2"); + await user.type(screen.getByLabelText("Reason"), "typo"); + await user.click(screen.getByRole("button", { name: "Save" })); + const [url, init] = fetchMock.mock.calls.find(([, i]) => i?.method === "PUT")!; + expect(String(url)).toMatch(/\/rows\/2\/cells\/n$/); + expect(JSON.parse(String(init!.body))).toEqual({ value: "2", reason: "typo" }); +}); + +it("other roles can view but not change", async () => { + show("uploader"); + expect( + await screen.findByText(/Only reviewers and tenant admins can make changes/), + ).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Exclude" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Edit row 1 n" })).toBeNull(); +}); diff --git a/csv-table-hub-main/src/components/review-table.tsx b/csv-table-hub-main/src/components/review-table.tsx new file mode 100644 index 0000000..2e21b15 --- /dev/null +++ b/csv-table-hub-main/src/components/review-table.tsx @@ -0,0 +1,355 @@ +// M4 (#108): review every stored row of an upload: page through it, include or +// exclude rows, edit cells (each edit is a new version) and see a row's history. +// Changes are recorded for review; they don't change the imported table. +import { useState, type FormEvent } from "react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { History, Loader2 } from "lucide-react"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "@/components/ui/table"; +import { + decideRow, + editCell, + getReviewPage, + rowHistory, + type ReviewPage, + type ReviewRow, +} from "@/lib/csv.functions"; +import { getSession } from "@/lib/session"; +import { formatLocalTime } from "@/lib/utils"; + +export const PAGE_SIZE = 100; +const REVIEWERS = new Set(["reviewer", "tenant_admin"]); + +type Change = + | { kind: "decision"; row: ReviewRow; decision: "include" | "exclude" } + | { kind: "edit"; row: ReviewRow; column: number }; + +export function ReviewTable({ uploadId }: { uploadId: string }) { + const [page, setPage] = useState(0); + const [change, setChange] = useState(null); + const [historyRow, setHistoryRow] = useState(null); + const canReview = REVIEWERS.has(getSession()?.role ?? ""); + const queryClient = useQueryClient(); + const key = ["review", uploadId, page]; + const { data, isLoading, error } = useQuery({ + queryKey: key, + queryFn: () => getReviewPage(uploadId, page * PAGE_SIZE, PAGE_SIZE), + }); + + if (isLoading) + return ( +

+ + Loading rows… +

+ ); + if (error || !data) + return

{(error as Error)?.message}

; + + const pages = Math.max(1, Math.ceil(data.total / PAGE_SIZE)); + const saved = (row: ReviewRow) => { + queryClient.setQueryData(key, (old) => + old ? { ...old, rows: old.rows.map((r) => (r.rowNumber === row.rowNumber ? row : r)) } : old, + ); + void queryClient.invalidateQueries({ queryKey: ["review", uploadId] }); + setChange(null); + }; + + return ( +
+

{data.fileName}

+

+ {data.total} row{data.total === 1 ? "" : "s"} · {data.excludedRows} excluded ·{" "} + {data.editedRows} edited. Changes are saved with the upload for review. They don't change + the imported table. + {!canReview && " Only reviewers and tenant admins can make changes."} +

+
+ + + + Row + {data.columns.map((c, i) => ( + + {c} {data.types[i]} + + ))} + Status + + + + + {data.rows.map((row) => ( + setHistoryRow(row.rowNumber)} + /> + ))} + +
+
+
+ + + Page {page + 1} of {pages} + + +
+ {change && ( + setChange(null)} + onSaved={saved} + /> + )} + {historyRow !== null && ( + setHistoryRow(null)} + /> + )} +
+ ); +} + +function ReviewLine({ + row, + columns, + canReview, + onChange, + onHistory, +}: { + row: ReviewRow; + columns: string[]; + canReview: boolean; + onChange: (c: Change) => void; + onHistory: () => void; +}) { + const excluded = row.decision === "exclude"; + const errorFor = (name: string) => row.errors.find((e) => e.column === name); + const extra = errorFor("(beyond header)"); + return ( + + + {row.rowNumber} + + {columns.map((name, i) => { + const err = errorFor(name); + const edited = row.editedColumns.includes(i); + const title = [err?.reason, edited ? `Original: ${row.original[i]}` : ""] + .filter(Boolean) + .join(" · "); + return ( + + {canReview ? ( + + ) : ( + row.current[i] + )} + + ); + })} + + + {excluded ? "Excluded" : row.errors.length ? "Invalid" : "Included"} + + {extra && {extra.reason}} + + + {canReview && ( + + )} + + + + ); +} + +function ChangeDialog({ + uploadId, + change, + columns, + onCancel, + onSaved, +}: { + uploadId: string; + change: Change; + columns: string[]; + onCancel: () => void; + onSaved: (row: ReviewRow) => void; +}) { + const isEdit = change.kind === "edit"; + const [value, setValue] = useState(isEdit ? change.row.current[change.column] : ""); + const [reason, setReason] = useState(""); + const [error, setError] = useState(null); + const [busy, setBusy] = useState(false); + const title = isEdit + ? `Edit row ${change.row.rowNumber}, ${columns[change.column]}` + : `${change.decision === "exclude" ? "Exclude" : "Include"} row ${change.row.rowNumber}`; + + async function submit(e: FormEvent) { + e.preventDefault(); + setBusy(true); + setError(null); + try { + const row = isEdit + ? await editCell(uploadId, change.row.rowNumber, columns[change.column], value, reason) + : await decideRow(uploadId, change.row.rowNumber, change.decision, reason); + onSaved(row); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } finally { + setBusy(false); + } + } + + return ( + !open && onCancel()}> + +
+ + {title} + {isEdit && ( + + Original value: {change.row.original[change.column] || "(empty)"} + + )} + + {isEdit && ( +
+ + setValue(e.target.value)} /> +
+ )} +
+ + setReason(e.target.value)} + /> +
+ {error &&

{error}

} + + + + +
+ + + ); +} + +function HistoryDialog({ + uploadId, + rowNumber, + onClose, +}: { + uploadId: string; + rowNumber: number; + onClose: () => void; +}) { + const { data, isLoading, error } = useQuery({ + queryKey: ["review-history", uploadId, rowNumber], + queryFn: () => rowHistory(uploadId, rowNumber), + }); + return ( + !open && onClose()}> + + + History of row {rowNumber} + + {isLoading &&

Loading…

} + {error &&

{(error as Error).message}

} + {data && data.length === 0 && ( +

No changes yet.

+ )} +
    + {data?.map((h, i) => ( +
  • + + {h.kind === "decision" + ? h.decision === "exclude" + ? "Excluded" + : "Included" + : `${h.column}: "${h.oldValue}" → "${h.newValue}"`} + + + {h.actor ?? "unknown"} · ·{" "} + {h.reason} + +
    • + ))} +
+ + + ); +} diff --git a/csv-table-hub-main/src/lib/csv.functions.ts b/csv-table-hub-main/src/lib/csv.functions.ts index cd6c603..79db888 100644 --- a/csv-table-hub-main/src/lib/csv.functions.ts +++ b/csv-table-hub-main/src/lib/csv.functions.ts @@ -249,3 +249,81 @@ export type UploadDetail = z.infer; export async function getUploadDetail(uploadId: string): Promise { return apiFetch(`/api/csv/uploads/${encodeURIComponent(uploadId)}`, uploadDetailSchema); } + +// M4 (#108): review of a stored upload. Changes are recorded for review; they +// don't change the imported table (applied by the approval step, M6). +const reviewRowSchema = z.object({ + rowNumber: z.number(), + original: z.array(z.string()), + current: z.array(z.string()), + editedColumns: z.array(z.number()), + decision: z.enum(["include", "exclude"]), + decisionReason: z.string().nullable(), + errors: z.array(z.object({ column: z.string(), value: z.string(), reason: z.string() })), +}); +export type ReviewRow = z.infer; + +const reviewPageSchema = z.object({ + fileName: z.string(), + columns: z.array(z.string()), + types: z.array(z.string()), + total: z.number(), + offset: z.number(), + limit: z.number(), + excludedRows: z.number(), + editedRows: z.number(), + rows: z.array(reviewRowSchema), +}); +export type ReviewPage = z.infer; + +const historyEntrySchema = z.object({ + kind: z.enum(["decision", "edit"]), + at: z.string(), + actor: z.string().nullable(), + reason: z.string(), + decision: z.enum(["include", "exclude"]).optional(), + column: z.string().optional(), + oldValue: z.string().optional(), + newValue: z.string().optional(), +}); +export type HistoryEntry = z.infer; + +const uploadPath = (uploadId: string) => `/api/csv/uploads/${encodeURIComponent(uploadId)}`; + +export async function getReviewPage( + uploadId: string, + offset: number, + limit: number, +): Promise { + return apiFetch(`${uploadPath(uploadId)}/rows?offset=${offset}&limit=${limit}`, reviewPageSchema); +} + +export async function decideRow( + uploadId: string, + rowNumber: number, + decision: "include" | "exclude", + reason: string, +): Promise { + return apiFetch(`${uploadPath(uploadId)}/rows/${rowNumber}/decision`, reviewRowSchema, { + method: "PUT", + body: JSON.stringify({ decision, reason }), + }); +} + +export async function editCell( + uploadId: string, + rowNumber: number, + column: string, + value: string, + reason: string, +): Promise { + return apiFetch( + `${uploadPath(uploadId)}/rows/${rowNumber}/cells/${encodeURIComponent(column)}`, + reviewRowSchema, + { method: "PUT", body: JSON.stringify({ value, reason }) }, + ); +} + +export async function rowHistory(uploadId: string, rowNumber: number): Promise { + return apiFetch(`${uploadPath(uploadId)}/rows/${rowNumber}/history`, z.array(historyEntrySchema)); +} diff --git a/csv-table-hub-main/src/routeTree.gen.ts b/csv-table-hub-main/src/routeTree.gen.ts index f2ef5e8..4fbc483 100644 --- a/csv-table-hub-main/src/routeTree.gen.ts +++ b/csv-table-hub-main/src/routeTree.gen.ts @@ -13,6 +13,7 @@ import { Route as AuthenticatedRouteRouteImport } from './routes/_authenticated/ import { Route as AuditRouteImport } from './routes/audit' import { Route as AuthRouteImport } from './routes/auth' import { Route as AuthenticatedIndexRouteImport } from './routes/_authenticated/index' +import { Route as AuthenticatedReviewUploadIdRouteImport } from './routes/_authenticated/review.$uploadId' const AuthenticatedRouteRoute = AuthenticatedRouteRouteImport.update({ id: '/_authenticated', @@ -33,16 +34,24 @@ const AuthenticatedIndexRoute = AuthenticatedIndexRouteImport.update({ path: '/', getParentRoute: () => AuthenticatedRouteRoute, } as any) +const AuthenticatedReviewUploadIdRoute = + AuthenticatedReviewUploadIdRouteImport.update({ + id: '/review/$uploadId', + path: '/review/$uploadId', + getParentRoute: () => AuthenticatedRouteRoute, + } as any) export interface FileRoutesByFullPath { '/': typeof AuthenticatedIndexRoute '/audit': typeof AuditRoute '/auth': typeof AuthRoute + '/review/$uploadId': typeof AuthenticatedReviewUploadIdRoute } export interface FileRoutesByTo { '/audit': typeof AuditRoute '/auth': typeof AuthRoute '/': typeof AuthenticatedIndexRoute + '/review/$uploadId': typeof AuthenticatedReviewUploadIdRoute } export interface FileRoutesById { __root__: typeof rootRouteImport @@ -50,13 +59,20 @@ export interface FileRoutesById { '/audit': typeof AuditRoute '/auth': typeof AuthRoute '/_authenticated/': typeof AuthenticatedIndexRoute + '/_authenticated/review/$uploadId': typeof AuthenticatedReviewUploadIdRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath - fullPaths: '/' | '/audit' | '/auth' + fullPaths: '/' | '/audit' | '/auth' | '/review/$uploadId' fileRoutesByTo: FileRoutesByTo - to: '/audit' | '/auth' | '/' - id: '__root__' | '/_authenticated' | '/audit' | '/auth' | '/_authenticated/' + to: '/audit' | '/auth' | '/' | '/review/$uploadId' + id: + | '__root__' + | '/_authenticated' + | '/audit' + | '/auth' + | '/_authenticated/' + | '/_authenticated/review/$uploadId' fileRoutesById: FileRoutesById } export interface RootRouteChildren { @@ -95,15 +111,24 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AuthenticatedIndexRouteImport parentRoute: typeof AuthenticatedRouteRoute } + '/_authenticated/review/$uploadId': { + id: '/_authenticated/review/$uploadId' + path: '/review/$uploadId' + fullPath: '/review/$uploadId' + preLoaderRoute: typeof AuthenticatedReviewUploadIdRouteImport + parentRoute: typeof AuthenticatedRouteRoute + } } } interface AuthenticatedRouteRouteChildren { AuthenticatedIndexRoute: typeof AuthenticatedIndexRoute + AuthenticatedReviewUploadIdRoute: typeof AuthenticatedReviewUploadIdRoute } const AuthenticatedRouteRouteChildren: AuthenticatedRouteRouteChildren = { AuthenticatedIndexRoute: AuthenticatedIndexRoute, + AuthenticatedReviewUploadIdRoute: AuthenticatedReviewUploadIdRoute, } const AuthenticatedRouteRouteWithChildren = diff --git a/csv-table-hub-main/src/routes/_authenticated/index.tsx b/csv-table-hub-main/src/routes/_authenticated/index.tsx index ae1a1af..d492d06 100644 --- a/csv-table-hub-main/src/routes/_authenticated/index.tsx +++ b/csv-table-hub-main/src/routes/_authenticated/index.tsx @@ -691,7 +691,7 @@ function FilesList({files}:{files:CsvFileSummary[]}){ <>
#FileTableColsRowsUploaded - {visible.map((f,i)=>{startIndex+i+1}{f.file_name}{f.table_name}{f.column_names.length}{f.row_count}{failedRowsNote(f)&&{failedRowsNote(f)}})} + {visible.map((f,i)=>{startIndex+i+1}{f.file_name}{f.table_name}{f.column_names.length}{f.row_count}{failedRowsNote(f)&&{failedRowsNote(f)}}{f.upload_id&&})}
diff --git a/csv-table-hub-main/src/routes/_authenticated/review.$uploadId.tsx b/csv-table-hub-main/src/routes/_authenticated/review.$uploadId.tsx new file mode 100644 index 0000000..1319c09 --- /dev/null +++ b/csv-table-hub-main/src/routes/_authenticated/review.$uploadId.tsx @@ -0,0 +1,33 @@ +// M4 (#108): review screen for one stored upload. +import { createFileRoute, Link } from "@tanstack/react-router"; +import { Toaster } from "sonner"; +import { ReviewTable } from "@/components/review-table"; +import { SessionMenu } from "@/components/session-menu"; + +export const Route = createFileRoute("/_authenticated/review/$uploadId")({ + head: () => ({ meta: [{ title: "Review upload — CSV Migrator" }] }), + component: ReviewScreen, +}); + +function ReviewScreen() { + const { uploadId } = Route.useParams(); + return ( +
+ +
+
+
+ + ← Migrated files + +

Review

+
+ +
+
+
+ +
+
+ ); +} diff --git a/docs/VCRM.md b/docs/VCRM.md index f21a400..7fb465d 100644 --- a/docs/VCRM.md +++ b/docs/VCRM.md @@ -144,8 +144,8 @@ Status as assessed on 2026-10-26 at `main` 032b522. None of these is "Verified" |----|--------|--------|----------------------|-----------| | BR-23 | ✅ | Every upload attempt gets an `uploads` row (M3b, #107): id, original filename, SHA-256, uploader, timestamp and outcome (imported, partly_imported, failed, duplicate, rejected). The tenant is the schema the row lives in (decision 6), not a column. Registry UNIQUE hash and name from M1 (#87). `test_preserved_uploads.py` | Integration + DB constraint tests | M1, M3 | | BR-24 | ⚠️ Partial | Original bytes (`upload_blobs`) and parsed rows (`source_rows`) kept for every upload, including failed ones and files later overwritten or deleted (M3b, #107). Triggers reject UPDATE, DELETE and TRUNCATE (`test_preserved_uploads.py`, 7 tests), and the API's runtime role has no UPDATE, DELETE or TRUNCATE grant on them (M3d, `test_runtime_role.py`). Still to do: show edits kept separate once they exist (M4) | DB negative tests | M3 | -| BR-25 | ⚠️ Partial | API: `GET /api/csv/uploads/{id}/rows` pages through every stored row (up to 500 a page), with column types and each row's validation errors, including rows beyond the 200 errors reported at upload (M4a, #108, `test_review.py`). Still to do: the review screen and its e2e test (M4b) | API paging + e2e | M4 | -| BR-26 | ⚠️ Partial | API: include or exclude a row with a required reason; actor and time recorded; insert-only history, latest wins; audited with before and after (M4a, #108, `test_review.py`). Applied to imports in M6 (decision 1 on #108). Still to do: the review screen and its e2e test (M4b) | API + e2e | M4 | +| BR-25 | ✅ | `GET /api/csv/uploads/{id}/rows` pages through every stored row and column (up to 500 a page), with column types and each row's validation errors, including rows beyond the 200 errors reported at upload (M4a, `test_review.py`). The review screen pages through them with types and invalid cells marked (M4b, `review-table.test.tsx`, e2e `scripts/e2e/m4-review.js`). #108 | API paging + e2e | M4 | +| BR-26 | ✅ | Include or exclude a row with a required reason; actor and time recorded; insert-only history, latest wins; audited with before and after (M4a, `test_review.py`). The review screen does it, shows it after reload and lists the history (M4b, e2e `scripts/e2e/m4-review.js`). Applied to imports in M6 (decision 1 on #108) | API + e2e | M4 | | BR-27 | ✅ | Cell edits are insert-only versions in `cell_edits` with old and new value, actor, time and a required reason; the original in `source_rows` is unchanged; edited values are revalidated; each edit is audited (M4a, #108, `test_review.py`: API and DB tests). The screen to make edits arrives in M4b | API + DB tests | M4 | | BR-28 | ⚠️ Partial | Race guard enforced by UNIQUE constraints (M1: #87, `tests/test_migrations.py`). For signed-in tenants the registry is per tenant, so the same file in two tenants is not a duplicate (M2: #100, `test_duplicate_detection_is_per_tenant`). Requests without a session still share the default registry; M5 finishes the per-tenant model | Concurrency + cross-tenant tests | M1, M5 | | BR-29 | ⚠️ Partial | In-file exact match only | Unit + integration | M5 | @@ -178,12 +178,12 @@ Status as assessed on 2026-10-26 at `main` 032b522. None of these is "Verified" | Status | Count | Requirements | |--------|-------|-------------| -| ✅ Verified | 21 | BR-01, BR-03, BR-04, BR-05, BR-06, BR-07, BR-08, BR-09, BR-10, BR-11, BR-12, BR-13, BR-14, BR-15, BR-16, BR-17, BR-18, BR-19, BR-20, BR-23 (M3b), BR-27 (M4a) | -| ⚠️ Partial | 12 | BR-24, BR-25, BR-26, BR-28, BR-29, BR-30, BR-31, BR-33, BR-34, BR-35, BR-36, BR-37 (ART, added 2026-10-26) | +| ✅ Verified | 23 | BR-01, BR-03, BR-04, BR-05, BR-06, BR-07, BR-08, BR-09, BR-10, BR-11, BR-12, BR-13, BR-14, BR-15, BR-16, BR-17, BR-18, BR-19, BR-20, BR-23 (M3b), BR-25, BR-26, BR-27 (M4) | +| ⚠️ Partial | 10 | BR-24, BR-28, BR-29, BR-30, BR-31, BR-33, BR-34, BR-35, BR-36, BR-37 (ART, added 2026-10-26) | | ❌ Not started | 2 | BR-32 (ART, added 2026-10-26); BR-38 (added 2026-10-27) | | ⏸ Deferred by decision | 3 | BR-02, BR-21, BR-22 | -**Headline (2026-10-28):** with BR-23 verified by M3b (#107) and BR-27 by M4a (#108), **21 of 38 (55 %)** are verified, 3 are deferred by decision, and the other 14 are partial or not started. +**Headline (2026-10-28):** with BR-23 verified by M3b (#107) and BR-25, BR-26 and BR-27 by M4 (#108), **23 of 38 (61 %)** are verified, 3 are deferred by decision, and the other 12 are partial or not started. **Headline (2026-10-26):** with the 15 ART requirements added, **19 of 37 (51 %)** are verified, 3 are deferred by decision, and the other 15 are partial or not started. diff --git a/scripts/e2e/m4-review.js b/scripts/e2e/m4-review.js new file mode 100644 index 0000000..6ea2982 --- /dev/null +++ b/scripts/e2e/m4-review.js @@ -0,0 +1,67 @@ +// M4 end to end (#108): a reviewer reviews an upload in the browser. Excludes a row, edits a cell, +// checks the history, reloads to show it persisted, and confirms the imported table is unchanged. +// +// Needs the API and the frontend running, and a tenant admin or reviewer account: +// E2E_TENANT=dev E2E_USER=admin E2E_PASSWORD=... node scripts/e2e/m4-review.js +// Optional: E2E_API (default http://127.0.0.1:8000), E2E_FRONTEND (default http://127.0.0.1:8080). +// Uses Playwright's Chromium. Prints one PASS or FAIL line per check; exits 1 on any FAIL. +const { chromium } = require('playwright'); +const API = process.env.E2E_API || 'http://127.0.0.1:8000', FE = process.env.E2E_FRONTEND || 'http://127.0.0.1:8080'; +const tenant = process.env.E2E_TENANT, user = process.env.E2E_USER, pw = process.env.E2E_PASSWORD; +if (!tenant || !user || !pw) { console.error('Set E2E_TENANT, E2E_USER and E2E_PASSWORD'); process.exit(2); } +const name = `m4_${Date.now()}.csv`; +const check = (label, ok) => { console.log(`${ok ? 'PASS' : 'FAIL'} ${label}`); if (!ok) process.exitCode = 1; }; + +(async () => { + const login = await (await fetch(API + '/api/auth/login', { method: 'POST', headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ tenant, username: user, password: pw }) })).json(); + const auth = { authorization: 'Bearer ' + login.token, 'content-type': 'application/json' }; + const up = await (await fetch(API + '/api/csv/upload', { method: 'POST', headers: auth, + body: JSON.stringify({ fileName: name, content: `n,t\n1,${name}\nx,b\n3,c\n`, types: ['int8', 'text'] }) })).json(); + console.log('upload', up.status, 'inserted', up.insertedRows, 'failed', up.failedRows, 'uploadId', up.uploadId); + + const browser = await chromium.launch(); + const page = await (await browser.newContext()).newPage(); + const errs = []; + page.on('pageerror', (e) => errs.push(e.message)); + await page.goto(FE + '/auth'); + await page.fill('#tenant', tenant); await page.fill('#username', user); await page.fill('#password', pw); + await page.click('button[type=submit]'); + await page.waitForURL(FE + '/'); + await page.locator('tr', { hasText: name }).getByRole('link', { name: 'Review' }).click(); + await page.waitForURL(`${FE}/review/${up.uploadId}`); + await page.getByRole('heading', { name }).waitFor(); + check('3 rows listed', await page.getByText(/3 rows · 0 excluded · 0 edited/).count() === 1); + check('row 2 flagged invalid', await page.getByText('Invalid').count() === 1); + + const row3 = page.locator('tr', { has: page.getByRole('button', { name: 'Edit row 3 n' }) }); + await row3.getByRole('button', { name: 'Exclude' }).click(); + await page.fill('#review-reason', 'duplicate of row 1'); + await page.getByRole('button', { name: 'Save' }).click(); + await page.getByText(/1 excluded/).waitFor(); + check('row 3 excluded', await row3.getByText('Excluded').count() === 1); + + await page.getByRole('button', { name: 'Edit row 2 n' }).click(); + await page.fill('#review-value', '2'); + await page.fill('#review-reason', 'typo in the source file'); + await page.getByRole('button', { name: 'Save' }).click(); + await page.getByText(/1 edited/).waitFor(); + check('row 2 now valid', await page.getByText('Invalid').count() === 0); + + await page.reload(); + await page.getByText(/3 rows · 1 excluded · 1 edited/).waitFor(); + const cell = page.getByRole('button', { name: 'Edit row 2 n' }).locator('xpath=..'); + check('after reload: edit kept, original shown', (await cell.getAttribute('title')) === 'Original: x'); + + const row2 = page.locator('tr', { has: page.getByRole('button', { name: 'Edit row 2 n' }) }); + await row2.getByRole('button', { name: 'History of row 2' }).click(); + await page.getByText('n: "x" → "2"').waitFor(); + check('history shows the edit with actor and reason', + await page.getByText(new RegExp(`${user} · .* · typo in the source file`)).count() === 1); + await page.keyboard.press('Escape'); + + const files = await (await fetch(API + '/api/csv/files', { headers: auth })).json(); + check('imported table unchanged (decision 1)', files.find((f) => f.upload_id === up.uploadId).row_count === 2); + check('no page errors', errs.length === 0); + await browser.close(); +})().catch((e) => { console.error(e); process.exit(1); });