From 5e048dfd5fe3248840bbc26c449ddd81f0d0e7d5 Mon Sep 17 00:00:00 2001 From: "lorenzo.padovani@padosoft.com" Date: Tue, 19 May 2026 01:49:46 +0200 Subject: [PATCH 1/6] feat(slice-4a): wire Findings kanban status-change to POST /api/findings/:id/status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.7 slice 4a — Findings page actions per audit doc 4a. The Kanban view's terminal-transition confirm modal was previously client-side-only: `doMove` flipped local state and toasted, with the reason textarea unwired. Now: - `doMove` POSTs to `/api/findings/:id/status` (the v1.4 endpoint that was already implemented but unused by the UI) - Body shape: `{status, reason}` (matches server contract; rejected with 400 if either is missing) - Optimistic UI update happens AFTER the server confirms, so a 4xx/5xx leaves the card in its original column with a clear error - The reason textarea is now controlled (`value`/`onChange`) and Confirm is disabled until the reason is non-empty (server requires it; matching client-side prevents the round-trip on empty input) - Inline `` surfaces the server's error message without closing the modal, so the user can fix the reason + retry - Cancel is disabled while a request is in flight to prevent abandoning a mid-flight POST - Non-terminal moves (drag to `draft`) also POST so the change lands in the audit chain, with a default reason indicating drag-origin Adds test ids on cards (`kanban-card-`) and columns (`kanban-col-`) plus a 4-test Playwright e2e suite covering: modal opens on terminal-drop, reason-required disable/enable, happy-path 200 + modal close + correct request body, 4xx keeps modal open and shows server error. Audit doc updated: - line 3837/3840 (kanban card actions) marked **DONE** - slice 4a checklist item marked **SHIPPED** Tests: @aqa/admin 44 Playwright (40 existing + 4 new). Lint + typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/internal/admin-placeholder-audit.md | 4 +- packages/admin/src/app.tsx | 104 +++++++++++++++--- .../admin/test/e2e/findings-status.e2e.ts | 103 +++++++++++++++++ 3 files changed, 196 insertions(+), 15 deletions(-) create mode 100644 packages/admin/test/e2e/findings-status.e2e.ts diff --git a/docs/internal/admin-placeholder-audit.md b/docs/internal/admin-placeholder-audit.md index dba8122..a18fa3b 100644 --- a/docs/internal/admin-placeholder-audit.md +++ b/docs/internal/admin-placeholder-audit.md @@ -29,7 +29,7 @@ The line numbers below were captured against commit `~v1.6.0` (post-merge of PR - `3521`, `3524` — verify / reject row actions - `3609` — open detail -- `3837`, `3840` — kanban card actions +- ~~`3837`, `3840` — kanban card actions~~ **DONE (slice 4a, PR #27):** the kanban confirm-transition modal is now wired to `POST /api/findings/:id/status` with required reason input + error alert + disabled-until-reason submit. Drag-and-drop opens the modal; happy-path closes it and moves the card; 4xx/5xx keeps the modal open with the server's error message. 4 Playwright e2e tests cover the flow. - `3967`, `3970` — cluster expand ### Risks (lines ~4660-4730) @@ -71,7 +71,7 @@ Not yet line-counted — comes after the first 49 fit. Doing all 81 in one PR is unreviewable. Plan: **one PR per page** so each is a manageable review surface. -1. **slice 4a — Findings page actions** (verify, reject, mark-fixed, mark-duplicate row actions). Needs `PATCH /api/findings/:id/status` (already exists in `packages/server` from v1.4). Just wire the UI buttons. +1. ~~**slice 4a — Findings page actions** (verify, reject, mark-fixed, mark-duplicate row actions). Needs `PATCH /api/findings/:id/status` (already exists in `packages/server` from v1.4). Just wire the UI buttons.~~ **SHIPPED (PR #27).** Kanban terminal-transition modal wired to `POST /api/findings/:id/status`. Drag-and-drop, required-reason capture, error-on-fail, optimistic UI only after server confirmation. Remaining row actions in clusters/list views (verify/reject inline buttons) deferred to a follow-up PR if/when those views grow real row actions — today the kanban is the canonical status-change surface. 2. **slice 4b — Packs page** (Import manifest, Install pack). "Install pack" delegates to the wizard from slice 3; "Import manifest" needs a `POST /api/packs/import` route. 3. **slice 4c — Scenarios / Risks / Profiles CRUD** (edit/save/clone/delete). Heaviest slice — needs full CRUD flows on three resources. 4. **slice 4d — Agents page** (install-instructions copy, "Install for X"). Mostly client-side (copy to clipboard, download files). diff --git a/packages/admin/src/app.tsx b/packages/admin/src/app.tsx index 6ac4c10..c08345d 100644 --- a/packages/admin/src/app.tsx +++ b/packages/admin/src/app.tsx @@ -3995,6 +3995,12 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { const [dragId, setDragId] = React.useState(null); const [dropCol, setDropCol] = React.useState(null); const [confirm, setConfirm] = React.useState(null); + // Captured by the confirmation textarea — required for terminal + // transitions because POST /api/findings/:id/status rejects an + // empty reason at the server (see api.test.ts). + const [reason, setReason] = React.useState(''); + const [submitting, setSubmitting] = React.useState(false); + const [submitError, setSubmitError] = React.useState(null); const toast = useToast(); const cols = [ @@ -4025,16 +4031,59 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { return; } if (col.terminal) { + setReason(''); + setSubmitError(null); setConfirm({ finding: f, toCol: col }); } else { - doMove(f.id, col.key); + // Non-terminal moves (only `draft` today) don't require a reason — + // we still POST to the server so the change lands in the audit + // chain, but the body uses a default reason. If the server + // rejects, we surface a toast and revert nothing (the local + // state never actually flipped). + void doMove(f.id, col.key, '(non-terminal move from kanban drag)'); } setDragId(null); }; - const doMove = (id, status) => { - setItems((prev) => prev.map((x) => (x.id === id ? { ...x, status } : x))); - toast.push({ title: 'Status updated', body: `${id} → ${status}`, kind: 'success' }); - }; + // v1.7 slice 4a — wire kanban status transitions to the real + // POST /api/findings/:id/status endpoint. Optimistic local update + // happens AFTER the server confirms, so a 4xx/5xx response leaves + // the card in its original column with a clear error to the user. + async function doMove(id, status, reasonText) { + setSubmitting(true); + setSubmitError(null); + try { + const res = await fetch(`/api/findings/${encodeURIComponent(id)}/status`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ status, reason: reasonText }), + }); + if (!res.ok) { + const text = await res.text(); + let msg = `HTTP ${res.status}`; + try { + const parsed = text ? JSON.parse(text) : null; + if (parsed?.error) msg = parsed.error; + } catch { + // raw text fallback + if (text) msg = text.slice(0, 200); + } + setSubmitError(msg); + toast.push({ kind: 'error', title: 'Status change failed', body: msg }); + return false; + } + setItems((prev) => prev.map((x) => (x.id === id ? { ...x, status } : x))); + toast.push({ title: 'Status updated', body: `${id} → ${status}`, kind: 'success' }); + return true; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + const full = `Could not reach /api/findings/${id}/status (${msg}). The admin is in mock-data mode or the server is down — the kanban change was not recorded to the audit chain.`; + setSubmitError(full); + toast.push({ kind: 'error', title: 'Status change failed', body: full }); + return false; + } finally { + setSubmitting(false); + } + } return ( <> @@ -4044,6 +4093,7 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { key={col.key} className="kanban-col" data-col={col.key} + data-testid={`kanban-col-${col.key}`} onDragOver={onDragOver(col)} onDragLeave={() => setDropCol(null)} onDrop={onDrop(col)} @@ -4064,6 +4114,9 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { className={`kanban-card ${dragId === f.id ? 'dragging' : ''}`} draggable onDragStart={onDragStart(f.id)} + data-testid={`kanban-card-${f.id}`} + data-finding-id={f.id} + data-finding-status={f.status} >
@@ -4103,22 +4156,35 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { setConfirm(null)} + onClose={() => { + if (submitting) return; + setConfirm(null); + }} title={`Confirm transition to ${confirm?.toCol.label}`} sub={`Moving "${confirm?.finding?.title}" to a terminal status. Provide a reason — this will be logged to the audit chain.`} footer={ <> - } @@ -4133,15 +4199,27 @@ function FindingsKanban({ findings: initialFindings, onConfirmTerminal }) { {confirm.finding.id} {confirm.finding.title}
+ {submitError && ( + + {submitError} + + )}
- +