fix(document-viewer): harden manual-tag editor UX (validation, delete confirm, 401)

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<DocumentLabelType>("topic");
   const [busyAction, setBusyAction] = useState<string | null>(null);
+  const [confirmingDeleteId, setConfirmingDeleteId] = useState<string | null>(null);
   const [error, setError] = useState<string | null>(null);
 
   async function submitManualTag(method: "POST" | "PATCH" | "DELETE", body: Record<string, unknown>, 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({
         </select>
         <button
           type="submit"
-          disabled={!canManage || busyAction !== null || !draftLabel.trim()}
+          disabled={!canManage || busyAction !== null || draftLabel.trim().length < manualLabelMinLength}
           className={cn(primaryButton, "min-h-tap px-3 text-xs")}
         >
           {busyAction === "add" ? (
@@ -176,6 +190,7 @@ export function DocumentManualTagEditor({
                     <input
                       value={editingLabel}
                       onChange={(event) => setEditingLabel(event.target.value)}
+                      maxLength={manualLabelMaxLength}
                       className={fieldControl}
                       aria-label="Manual tag label"
                     />
@@ -204,7 +219,7 @@ export function DocumentManualTagEditor({
                       <button
                         type="button"
                         onClick={() => saveManualTag(label)}
-                        disabled={!editingLabel.trim() || busyAction !== null}
+                        disabled={editingLabel.trim().length < manualLabelMinLength || busyAction !== null}
                         className={cn(primaryButton, "sm:min-h-9 px-2 text-xs")}
                         aria-label={`Save ${label.label}`}
                       >
@@ -224,11 +239,55 @@ export function DocumentManualTagEditor({
                         <X aria-hidden="true" className="h-4 w-4" />
                       </button>
                     </>
+                  ) : 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. */}
+                      <span
+                        key="delete-confirm-prompt"
+                        className="inline-flex min-h-tap items-center px-1 text-xs font-semibold text-[color:var(--text-muted)] sm:min-h-9"
+                      >
+                        Remove this tag?
+                      </span>
+                      <button
+                        key="delete-confirm"
+                        type="button"
+                        onClick={() => deleteManualTag(label)}
+                        disabled={!canManage || busyAction !== null}
+                        className={cn(
+                          secondaryButton,
+                          "sm:min-h-9 px-2 text-xs border-[color:var(--danger)]/40 text-[color:var(--danger)]",
+                        )}
+                        aria-label={`Confirm remove ${label.label}`}
+                      >
+                        {busyAction === `delete:${label.id}` ? (
+                          <Loader2 aria-hidden="true" className="h-4 w-4 animate-spin" />
+                        ) : (
+                          <Trash2 aria-hidden="true" className="h-4 w-4" />
+                        )}
+                        Confirm remove
+                      </button>
+                      <button
+                        key="delete-cancel"
+                        type="button"
+                        onClick={() => setConfirmingDeleteId(null)}
+                        disabled={busyAction !== null}
+                        className={cn(secondaryButton, "sm:min-h-9 px-2 text-xs")}
+                        aria-label={`Cancel removing ${label.label}`}
+                      >
+                        <X aria-hidden="true" className="h-4 w-4" />
+                      </button>
+                    </>
                   ) : (
                     <>
                       <button
+                        key="rename"
                         type="button"
                         onClick={() => {
+                          setConfirmingDeleteId(null);
                           setEditingId(label.id);
                           setEditingLabel(label.label);
                           setEditingType(label.label_type);
@@ -240,17 +299,14 @@ export function DocumentManualTagEditor({
                         <Pencil aria-hidden="true" className="h-4 w-4" />
                       </button>
                       <button
+                        key="remove"
                         type="button"
-                        onClick={() => deleteManualTag(label)}
+                        onClick={() => setConfirmingDeleteId(label.id)}
                         disabled={!canManage || busyAction !== null}
                         className={cn(secondaryButton, "sm:min-h-9 px-2 text-xs text-[color:var(--danger)]")}
                         aria-label={`Remove ${label.label}`}
                       >
-                        {busyAction === `delete:${label.id}` ? (
-                          <Loader2 aria-hidden="true" className="h-4 w-4 animate-spin" />
-                        ) : (
-                          <Trash2 aria-hidden="true" className="h-4 w-4" />
-                        )}
+                        <Trash2 aria-hidden="true" className="h-4 w-4" />
                       </button>
                     </>
                   )}

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> = {}): 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<string, string>,
+  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(<DocumentManualTagEditor document={makeDocument()} {...baseProps} />);
+    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(<DocumentManualTagEditor document={makeDocument([manualLabel()])} {...baseProps} />);
+
+    // 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(<DocumentManualTagEditor document={makeDocument([manualLabel()])} {...baseProps} />);
+
+    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(<DocumentManualTagEditor document={makeDocument()} {...baseProps} onUnauthorized={onUnauthorized} />);
+    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();
+  });
+});