Merge pull request #218 from Sea-Haven-Industries/fix/ab/sh-375-edit-error-copy

SH-375: keep backend validation text out of work-order edit errors

src/domain/work-orders/errors/board-create-errors.ts

@@ -1,12 +1,12 @@
 import { isHTTPError, isNetworkError, isTimeoutError } from "ky";
 import { mapHttpStatusToMessage } from "@/api/api-error";
-import { DISPATCH_REQUIRED_MESSAGE } from "@/domain/work-orders/errors/board-patch-errors";
+import {
+  DISPATCH_REQUIRED_MESSAGE,
+  DUPLICATE_WO_NUMBER_MESSAGE,
+} from "@/domain/work-orders/errors/board-patch-errors";
 
 export const BOARD_CREATE_FAILED_MESSAGE = "Unable to create the work order. Please try again.";
 
-const DUPLICATE_WO_NUMBER_MESSAGE =
-  "This work order number is already assigned to another work order.";
-
 /** Statuses whose mapped copy is fixed product text, never the response body. */
 const FIXED_COPY_STATUSES = new Set([401, 403, 404, 500]);
 

src/domain/work-orders/errors/board-patch-errors.ts

@@ -1,3 +1,5 @@
+import { isNetworkError, isTimeoutError } from "ky";
+import { ApiError, mapHttpStatusToMessage } from "@/api/api-error";
 import type { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row";
 
 export class BoardPatchConflictError extends Error {
@@ -24,10 +26,64 @@ export class BoardPatchValidationError extends Error {
 
 export const DISPATCH_REQUIRED_MESSAGE = "Select a vendor company before saving.";
 
-/** why: backend field names must never be exposed through user-facing board toasts. */
+export const BOARD_PATCH_FAILED_MESSAGE = "Unable to save the work order. Please try again.";
+
+export const NO_PERSISTABLE_FIELDS_MESSAGE =
+  "These changes cannot be saved on the board. Refresh and edit a supported field.";
+
+export const DUPLICATE_WO_NUMBER_MESSAGE =
+  "This work order number is already assigned to another work order.";
+
+const KNOWN_VALIDATION_COPY: ReadonlyMap<string, string> = new Map([
+  ["DispatchRequired", DISPATCH_REQUIRED_MESSAGE],
+  ["NoPersistableFields", NO_PERSISTABLE_FIELDS_MESSAGE],
+  // why: editing a WO number to one that already exists returns 422 DuplicateWoNumber;
+  // show the same product copy as create, not the generic save-failed fallback.
+  ["DuplicateWoNumber", DUPLICATE_WO_NUMBER_MESSAGE],
+]);
+
+/**
+ * why: board PATCH validation text ("vendorId is required…", object ids, backend codes)
+ * must never reach the screen. Unknown codes fall back to generic copy, not `error.message`.
+ */
 export function boardPatchValidationMessage(error: BoardPatchValidationError): string {
-  if (error.code === "DispatchRequired") {
-    return DISPATCH_REQUIRED_MESSAGE;
-  }
-  return error.message;
+  return KNOWN_VALIDATION_COPY.get(error.code) ?? BOARD_PATCH_FAILED_MESSAGE;
+}
+
+export const BOARD_PATCH_CONFLICT_MESSAGE =
+  "This work order was changed by someone else. Refresh to see the latest before saving again.";
+
+export const PARTIAL_SAVE_PREFIX = "Some changes were saved, but the rest were not.";
+
+/** Statuses whose mapped copy is fixed product text, never the response body. */
+const FIXED_COPY_STATUSES = new Set([401, 403, 404, 500]);
+
+/** why: any board edit failure toast must use product copy, never backend text or field keys. */
+export function boardPatchErrorMessage(error: unknown): string {
+  if (error instanceof BoardPatchConflictError) {
+    return BOARD_PATCH_CONFLICT_MESSAGE;
+  }
+  if (error instanceof BoardPatchValidationError) {
+    return boardPatchValidationMessage(error);
+  }
+  if (error instanceof ApiError && FIXED_COPY_STATUSES.has(error.status)) {
+    return mapHttpStatusToMessage(error.status);
+  }
+  if (isNetworkError(error) || isTimeoutError(error)) {
+    return error.message;
+  }
+  return BOARD_PATCH_FAILED_MESSAGE;
+}
+
+/** Thrown when a multi-field board edit fails after some fields already persisted. */
+export class BoardPatchPartialSaveError extends Error {
+  readonly partialRow: WorkOrderTableRow;
+  readonly failure: unknown;
+
+  constructor(partialRow: WorkOrderTableRow, failure: unknown) {
+    super(`${PARTIAL_SAVE_PREFIX} ${boardPatchErrorMessage(failure)}`);
+    this.name = "BoardPatchPartialSaveError";
+    this.partialRow = partialRow;
+    this.failure = failure;
+  }
 }

src/domain/work-orders/use-cases/use-work-order-table-mutations.ts

@@ -3,9 +3,11 @@ import { useMutation, useQueryClient } from "@tanstack/react-query";
 import { toast } from "react-toastify";
 import { workOrdersApi } from "@/domain/work-orders/api/work-orders-api";
 import {
-  boardPatchValidationMessage,
+  boardPatchErrorMessage,
   BoardPatchConflictError,
+  BoardPatchPartialSaveError,
   BoardPatchValidationError,
+  NO_PERSISTABLE_FIELDS_MESSAGE,
 } from "@/domain/work-orders/errors/board-patch-errors";
 import { mapApiStatusToWizard } from "@/domain/work-orders/mappers/wizard-draft-mapper";
 import {
@@ -58,10 +60,7 @@ async function applyBoardFieldOps(
 ): Promise<WorkOrderTableRow> {
   const ops = expandTablePatchToBoardFieldOps(patch);
   if (ops.length === 0) {
-    throw new BoardPatchValidationError(
-      "NoPersistableFields",
-      "These changes cannot be saved on the board. Refresh and edit a supported field.",
-    );
+    throw new BoardPatchValidationError("NoPersistableFields", NO_PERSISTABLE_FIELDS_MESSAGE);
   }
 
   const current = getRow?.(id);
@@ -77,14 +76,7 @@ async function applyBoardFieldOps(
       completedOps += 1;
     } catch (error) {
       if (completedOps > 0) {
-        const field = op.field;
-        const partial = new Error(
-          `Schedule update partially saved (${completedOps}/${ops.length} fields). Failed on "${field}": ${
-            error instanceof Error ? error.message : "Unknown error"
-          }`,
-        );
-        (partial as Error & { partialRow?: WorkOrderTableRow }).partialRow = latest;
-        throw partial;
+        throw new BoardPatchPartialSaveError(latest, error);
       }
       throw error;
     }
@@ -155,9 +147,8 @@ export function useWorkOrderTableMutations(
       invalidate();
     },
     onError: (error: Error, { id }) => {
-      const partialRow = (error as Error & { partialRow?: WorkOrderTableRow }).partialRow;
-      if (partialRow) {
-        onPatch(id, partialRow);
+      if (error instanceof BoardPatchPartialSaveError) {
+        onPatch(id, error.partialRow);
         toast.error(error.message);
         invalidate();
         return;
@@ -165,13 +156,10 @@ export function useWorkOrderTableMutations(
 
       if (error instanceof BoardPatchConflictError && error.currentState) {
         onPatch(id, error.currentState);
-        toast.error(error.message || "Conflict — refreshed with latest data");
-      } else if (error instanceof BoardPatchValidationError) {
-        clearPatch(id);
-        toast.error(boardPatchValidationMessage(error));
+        toast.error(boardPatchErrorMessage(error));
       } else {
         clearPatch(id);
-        toast.error(error.message || "Failed to update work order");
+        toast.error(boardPatchErrorMessage(error));
       }
       invalidate();
     },

src/test/domain/work-orders/api/work-orders-api.test.ts

@@ -634,6 +634,28 @@ describe("workOrdersApi.patchBoardField", () => {
       }),
     ).rejects.toBeInstanceOf(BoardPatchConflictError);
   });
+
+  it("throws BoardPatchValidationError with DuplicateWoNumber code on 422", async () => {
+    apiPatchFn.mockResolvedValue({
+      ok: false,
+      status: 422,
+      json: async () => ({
+        code: "DuplicateWoNumber",
+        message: "WO number already exists.",
+      }),
+    });
+
+    await expect(
+      workOrdersApi.patchBoardField(10, {
+        field: "woNumber",
+        value: "12345678901",
+        workOrderVersion: "AAAAAAA=",
+      }),
+    ).rejects.toMatchObject({
+      name: "BoardPatchValidationError",
+      code: "DuplicateWoNumber",
+    });
+  });
 });
 
 describe("workOrdersApi.createComment", () => {

src/test/domain/work-orders/errors/board-patch-errors.test.ts

@@ -0,0 +1,29 @@
+import { describe, expect, it } from "vitest";
+import {
+  BOARD_PATCH_CONFLICT_MESSAGE,
+  BOARD_PATCH_FAILED_MESSAGE,
+  BoardPatchConflictError,
+  BoardPatchValidationError,
+  DUPLICATE_WO_NUMBER_MESSAGE,
+  boardPatchErrorMessage,
+} from "@/domain/work-orders/errors/board-patch-errors";
+
+describe("boardPatchErrorMessage", () => {
+  it("maps a DuplicateWoNumber validation error to the duplicate-WO product copy", () => {
+    const error = new BoardPatchValidationError("DuplicateWoNumber", "WO number already exists.");
+
+    expect(boardPatchErrorMessage(error)).toBe(DUPLICATE_WO_NUMBER_MESSAGE);
+  });
+
+  it("never leaks backend validation text for unknown codes", () => {
+    const error = new BoardPatchValidationError("SomethingElse", "vendorId is required.");
+
+    expect(boardPatchErrorMessage(error)).toBe(BOARD_PATCH_FAILED_MESSAGE);
+  });
+
+  it("maps a conflict error to the concurrent-edit copy", () => {
+    const error = new BoardPatchConflictError("stale", null);
+
+    expect(boardPatchErrorMessage(error)).toBe(BOARD_PATCH_CONFLICT_MESSAGE);
+  });
+});

src/test/domain/work-orders/use-cases/use-work-order-table-mutations-errors.test.tsx

@@ -0,0 +1,171 @@
+import { QueryClientProvider } from "@tanstack/react-query";
+import { act, renderHook, waitFor } from "@testing-library/react";
+import type { ReactNode } from "react";
+import { toast } from "react-toastify";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import {
+  BOARD_PATCH_CONFLICT_MESSAGE,
+  BOARD_PATCH_FAILED_MESSAGE,
+  DISPATCH_REQUIRED_MESSAGE,
+  PARTIAL_SAVE_PREFIX,
+} from "@/domain/work-orders/errors/board-patch-errors";
+import type {
+  WorkOrderTablePatch,
+  WorkOrderTableRow,
+} from "@/domain/work-orders/types/work-order-table-row";
+import { useWorkOrderTableMutations } from "@/domain/work-orders/use-cases/use-work-order-table-mutations";
+import { createTestQueryClient } from "@/test/test-utils";
+
+vi.mock("react-toastify", () => ({
+  toast: { error: vi.fn(), success: vi.fn(), warning: vi.fn() },
+}));
+
+const ROW = { id: 7, rowVersion: "v1" } as WorkOrderTableRow;
+
+function wrapper({ children }: { children: ReactNode }) {
+  return <QueryClientProvider client={createTestQueryClient()}>{children}</QueryClientProvider>;
+}
+
+type FakeResponse = { status: number; body: Record<string, unknown> };
+
+/** Board PATCH goes through the real ky client; only the network responses are faked. */
+function respondWith(...responses: FakeResponse[]) {
+  const queue = [...responses];
+  globalThis.fetch = vi.fn(async () => {
+    const next = queue.shift() ?? responses[responses.length - 1];
+    return new Response(JSON.stringify(next.body), {
+      status: next.status,
+      headers: { "Content-Type": "application/json" },
+    });
+  }) as typeof fetch;
+}
+
+async function patchAndGetToast(
+  patch: WorkOrderTablePatch = { woNumber: "WO-100" },
+  expectedCalls = 1,
+  onPatch: (id: string | number, patch: WorkOrderTablePatch) => void = () => undefined,
+): Promise<unknown> {
+  const { result } = renderHook(
+    () =>
+      useWorkOrderTableMutations({
+        onPatch,
+        clearPatch: () => undefined,
+        getRow: () => ROW,
+      }),
+    { wrapper },
+  );
+  act(() => {
+    result.current.patchField(7, patch);
+  });
+  await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1));
+  expect(globalThis.fetch).toHaveBeenCalledTimes(expectedCalls);
+  return vi.mocked(toast.error).mock.calls[0]?.[0];
+}
+
+describe("useWorkOrderTableMutations edit error copy", () => {
+  const originalFetch = globalThis.fetch;
+
+  beforeEach(() => {
+    vi.mocked(toast.error).mockClear();
+  });
+
+  afterEach(() => {
+    globalThis.fetch = originalFetch;
+  });
+
+  it("shows generic copy instead of backend text for an unknown 422 code", async () => {
+    respondWith({
+      status: 422,
+      body: {
+        code: "InvalidLifecycleStatus",
+        message: "lifecycleStatus cannot move from Completed for workOrderId 7.",
+      },
+    });
+
+    const message = await patchAndGetToast();
+
+    expect(message).toBe(BOARD_PATCH_FAILED_MESSAGE);
+    expect(message).not.toMatch(/lifecycleStatus|workOrderId|InvalidLifecycleStatus/);
+  });
+
+  it("keeps specific copy for DispatchRequired", async () => {
+    respondWith({
+      status: 422,
+      body: {
+        code: "DispatchRequired",
+        message: "vendorId is required when setting appointment fields.",
+      },
+    });
+
+    const message = await patchAndGetToast();
+
+    expect(message).toBe(DISPATCH_REQUIRED_MESSAGE);
+  });
+
+  it("shows generic copy instead of backend text for a non-422 failure", async () => {
+    respondWith({ status: 400, body: { message: "siteCode is required for accountId 42." } });
+
+    const message = await patchAndGetToast();
+
+    expect(message).toBe(BOARD_PATCH_FAILED_MESSAGE);
+  });
+
+  it("reports a partial save in plain language without backend field keys", async () => {
+    const onPatch = vi.fn();
+    respondWith(
+      { status: 200, body: { id: 7, workOrderNumber: "WO-100", rowVersion: "v2" } },
+      {
+        status: 422,
+        body: {
+          code: "SeverityInvalid",
+          message: "severity 3 is not allowed for workOrderType Emergency.",
+        },
+      },
+    );
+
+    const message = await patchAndGetToast({ woNumber: "WO-100", severity: 3 }, 2, onPatch);
+
+    expect(message).toBe(`${PARTIAL_SAVE_PREFIX} ${BOARD_PATCH_FAILED_MESSAGE}`);
+    expect(message).not.toMatch(/severity|woNumber|workOrderType|SeverityInvalid|\d\/\d/);
+    expect(onPatch).toHaveBeenCalledWith(7, expect.objectContaining({ rowVersion: "v2" }));
+  });
+
+  it("keeps DispatchRequired copy inside a partial save", async () => {
+    respondWith(
+      { status: 200, body: { id: 7, workOrderNumber: "WO-100", rowVersion: "v2" } },
+      {
+        status: 422,
+        body: { code: "DispatchRequired", message: "vendorId is required." },
+      },
+    );
+
+    const message = await patchAndGetToast({ woNumber: "WO-100", severity: 2 }, 2);
+
+    expect(message).toBe(`${PARTIAL_SAVE_PREFIX} ${DISPATCH_REQUIRED_MESSAGE}`);
+  });
+
+  it("shows fixed conflict copy and still applies the latest row on a 409", async () => {
+    const onPatch = vi.fn();
+    respondWith({
+      status: 409,
+      body: {
+        message: "RowVersion mismatch for workOrderId 7 (expected v1, found v3).",
+        currentState: { id: 7, workOrderNumber: "WO-099", rowVersion: "v3" },
+      },
+    });
+
+    const message = await patchAndGetToast({ woNumber: "WO-100" }, 1, onPatch);
+
+    expect(message).toBe(BOARD_PATCH_CONFLICT_MESSAGE);
+    expect(message).not.toMatch(/RowVersion|workOrderId|v3/);
+    expect(onPatch).toHaveBeenLastCalledWith(7, expect.objectContaining({ rowVersion: "v3" }));
+  });
+
+  it("shows fixed conflict copy when a 409 carries no latest row", async () => {
+    respondWith({ status: 409, body: { message: "RowVersion mismatch for workOrderId 7." } });
+
+    const message = await patchAndGetToast();
+
+    expect(message).toBe(BOARD_PATCH_CONFLICT_MESSAGE);
+  });
+});