diff --git a/src/components/document-viewer/manual-tag-editor.tsx b/src/components/document-viewer/manual-tag-editor.tsx index 36833f3bb..a7ecac514 100644 --- a/src/components/document-viewer/manual-tag-editor.tsx +++ b/src/components/document-viewer/manual-tag-editor.tsx @@ -9,6 +9,12 @@ import type { ClinicalDocument, DocumentLabel, DocumentLabelType } from "@/lib/t const primaryButton = primaryControl; const secondaryButton = floatingControl; +// Mirrors the server contract (`manualLabelSchema` in +// src/app/api/documents/[id]/labels/route.ts: z.string().trim().min(2).max(64)) so a too-short +// or too-long tag is caught in the field instead of round-tripping to a generic 400. +const manualLabelMinLength = 2; +const manualLabelMaxLength = 64; + const manualLabelTypeOptions: Array<{ value: DocumentLabelType; label: string }> = [ { value: "site", label: "Site" }, { value: "topic", label: "Topic" }, @@ -52,6 +58,7 @@ export function DocumentManualTagEditor({ const [editingLabel, setEditingLabel] = useState(""); const [editingType, setEditingType] = useState("topic"); const [busyAction, setBusyAction] = useState(null); + const [confirmingDeleteId, setConfirmingDeleteId] = useState(null); const [error, setError] = useState(null); async function submitManualTag(method: "POST" | "PATCH" | "DELETE", body: Record, action: string) { @@ -67,7 +74,12 @@ export function DocumentManualTagEditor({ body: JSON.stringify(body), }); const payload = await response.json().catch(() => ({})); - if (response.status === 401) onUnauthorized(); + if (response.status === 401) { + // Hand off to the parent's unauthorized UI and stop here — falling through + // to the generic throw would also raise an inline error banner underneath it. + onUnauthorized(); + return false; + } if (!response.ok) throw new Error(typeof payload?.error === "string" ? payload.error : "Tag update failed."); if (Array.isArray(payload.labels)) onLabelsUpdated(payload.labels as DocumentLabel[]); return true; @@ -101,6 +113,7 @@ export function DocumentManualTagEditor({ } async function deleteManualTag(label: DocumentLabel) { + setConfirmingDeleteId(null); await submitManualTag("DELETE", { labelId: label.id }, `delete:${label.id}`); } @@ -126,6 +139,7 @@ export function DocumentManualTagEditor({ onChange={(event) => setDraftLabel(event.target.value)} placeholder="Add clean manual tag" disabled={!canManage || busyAction !== null} + maxLength={manualLabelMaxLength} className={fieldControl} aria-label="Manual tag" /> @@ -144,7 +158,7 @@ export function DocumentManualTagEditor({ + ) : confirmingDeleteId === label.id ? ( + <> + {/* Distinct keys keep React from reusing the Remove button's DOM node for + the destructive confirm (so the control the user pressed is never turned + into "delete" in place), and the leading prompt occupies the position the + Remove button was clicked from — together a rapid double-click on the old + Remove target can't reach the confirm. */} + + Remove this tag? + + + + ) : ( <> )} diff --git a/tests/manual-tag-editor.dom.test.tsx b/tests/manual-tag-editor.dom.test.tsx new file mode 100644 index 000000000..78496d238 --- /dev/null +++ b/tests/manual-tag-editor.dom.test.tsx @@ -0,0 +1,116 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { DocumentManualTagEditor } from "@/components/document-viewer/manual-tag-editor"; +import type { ClinicalDocument, DocumentLabel } from "@/lib/types"; + +function manualLabel(overrides: Partial = {}): DocumentLabel { + return { + id: "label-1", + document_id: "doc-1", + label: "Existing tag", + label_type: "topic", + source: "manual", + confidence: 1, + ...overrides, + }; +} + +// The editor only reads `id` and `labels`; a minimal cast keeps the fixture focused. +function makeDocument(labels: DocumentLabel[] = []): ClinicalDocument { + return { id: "doc-1", labels } as unknown as ClinicalDocument; +} + +const baseProps = { + canManage: true, + clientDemoMode: false, + authorizationHeader: {} as Record, + onLabelsUpdated: () => {}, + onUnauthorized: () => {}, +}; + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("DocumentManualTagEditor", () => { + it("keeps Add disabled until the tag meets the server's 2-char minimum and caps length at 64", () => { + render(); + const input = screen.getByLabelText("Manual tag") as HTMLInputElement; + const addButton = screen.getByRole("button", { name: "Add" }); + + expect(addButton).toBeDisabled(); + fireEvent.change(input, { target: { value: " a " } }); // 1 non-space char + expect(addButton).toBeDisabled(); + fireEvent.change(input, { target: { value: "ab" } }); + expect(addButton).toBeEnabled(); + expect(input.maxLength).toBe(64); + }); + + it("requires an explicit second click to confirm a delete before issuing the request", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ labels: [] }), + }); + vi.stubGlobal("fetch", fetchMock); + + render(); + + // First click only asks for confirmation — no destructive request yet. + fireEvent.click(screen.getByRole("button", { name: "Remove Existing tag" })); + expect(fetchMock).not.toHaveBeenCalled(); + + // Cancelling backs out cleanly, still no request. + fireEvent.click(screen.getByRole("button", { name: "Cancel removing Existing tag" })); + expect(fetchMock).not.toHaveBeenCalled(); + + // Confirming fires the DELETE. + fireEvent.click(screen.getByRole("button", { name: "Remove Existing tag" })); + fireEvent.click(screen.getByRole("button", { name: "Confirm remove Existing tag" })); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expect(fetchMock.mock.calls[0][1]).toMatchObject({ method: "DELETE" }); + }); + + it("keeps a double-click on the initial Remove control from reaching the destructive confirm", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ labels: [] }), + }); + vi.stubGlobal("fetch", fetchMock); + + render(); + + const removeButton = screen.getByRole("button", { name: "Remove Existing tag" }); + fireEvent.click(removeButton); + + // The original Remove hit target is replaced by a non-action prompt, and the destructive + // control is a separately-labelled button rendered after it — so the second click of a + // rapid double-click on the old Remove position cannot confirm the delete. + expect(screen.queryByRole("button", { name: "Remove Existing tag" })).toBeNull(); + expect(screen.getByText("Remove this tag?")).toBeTruthy(); + fireEvent.click(removeButton); // stale/detached node — must not issue a request + await Promise.resolve(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("routes a 401 to onUnauthorized without also raising an inline error banner", async () => { + const onUnauthorized = vi.fn(); + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + status: 401, + json: async () => ({ error: "Unauthorized" }), + }); + vi.stubGlobal("fetch", fetchMock); + + render(); + fireEvent.change(screen.getByLabelText("Manual tag"), { target: { value: "new tag" } }); + fireEvent.click(screen.getByRole("button", { name: "Add" })); + + await waitFor(() => expect(onUnauthorized).toHaveBeenCalledTimes(1)); + // The generic error banner must not also render the server message. + expect(screen.queryByText("Unauthorized")).toBeNull(); + }); +});