From 0ff6a2a3a6d3e45189c895a043b35700df8bcd52 Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 6 Nov 2026 08:46:21 +0200 Subject: [PATCH] fix(repairs): pin the transition guard's wiring, test the office pages, and clear the review findings --- src/app/(app)/repairs/[id]/page.test.tsx | 166 ++++++++++++++++++ src/app/(app)/repairs/[id]/page.tsx | 2 +- src/app/(app)/repairs/page.test.tsx | 149 ++++++++++++++++ .../v1/repairs/[id]/files/[field]/route.ts | 6 +- src/lib/server/repairs-service.test.ts | 50 ++++++ src/lib/server/repairs-service.ts | 5 +- 6 files changed, 372 insertions(+), 6 deletions(-) create mode 100644 src/app/(app)/repairs/[id]/page.test.tsx create mode 100644 src/app/(app)/repairs/page.test.tsx diff --git a/src/app/(app)/repairs/[id]/page.test.tsx b/src/app/(app)/repairs/[id]/page.test.tsx new file mode 100644 index 0000000..0636aae --- /dev/null +++ b/src/app/(app)/repairs/[id]/page.test.tsx @@ -0,0 +1,166 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ + push: vi.fn(), + replace: vi.fn(), + back: vi.fn(), + }), +})); + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + }, +})); + +import Page from "./page"; + +/* --------------------------------------------------------------------------- + * Test data + * ------------------------------------------------------------------------ */ + +const REPAIR = { + id: "rep1", + repair_number: "REP-2026-11-001", + customer: "cust1", + company: "co1", + order: "", + supplier: "sup1", + status: "assessing", + item_description: "Welding machine", + fault_description: "Will not strike", + supplier_scope: "Strip and quote the strike mechanism", + notes: "", + serial_no: "SN-12345", + quote_ref: "", + quote_amount: 0, + quote_date: "", + purchase_order_ref: "", + signed_request: "", + photos: [], + quote: [], + purchase_order: [], + repair_request: [], + created: "2026-11-05 09:00:00.000Z", + updated: "2026-11-05 09:00:00.000Z", + expand: { + customer: { id: "cust1", customer: "Acme Mining" }, + supplier: { id: "sup1", suppliername: "Alpha Bearings" }, + order: null, + }, +}; + +/* --------------------------------------------------------------------------- + * Fetch helpers + * ------------------------------------------------------------------------ */ + +function jsonResponse(body: unknown): Response { + return { + ok: true, + status: 200, + json: async () => body, + } as Response; +} + +function fetchUrl(input: RequestInfo | URL): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.toString(); + return input.url; +} + +function buildFetchSpy() { + return vi.spyOn(window, "fetch").mockImplementation(async (input) => { + const url = fetchUrl(input); + if (url === "/api/v1/repairs/form-data") { + return jsonResponse({ customers: [], suppliers: [], orders: [] }); + } + if (url === "/api/v1/repairs/rep1") { + return jsonResponse(REPAIR); + } + return jsonResponse({}); + }); +} + +/* --------------------------------------------------------------------------- + * Tests + * ------------------------------------------------------------------------ */ + +describe("RepairDetailPage", () => { + beforeEach(() => { + class ResizeObserverMock { + observe() {} + unobserve() {} + disconnect() {} + } + + vi.stubGlobal("ResizeObserver", ResizeObserverMock); + window.HTMLElement.prototype.scrollIntoView = vi.fn(); + buildFetchSpy(); + }); + + it("shows the shared status label, not the raw status", async () => { + render(); + + await waitFor(() => + expect(screen.getByText("REP-2026-11-001")).toBeInTheDocument(), + ); + + // "assessing" is the raw PocketBase value — the office reads + // "With supplier", the label from @/lib/repair-status. + expect(screen.getByText("With supplier")).toBeInTheDocument(); + expect(screen.queryAllByText("assessing")).toHaveLength(0); + }); + + it("labels the supplier scope the same way in read mode and edit mode", async () => { + render(); + + await waitFor(() => + expect(screen.getByText("REP-2026-11-001")).toBeInTheDocument(), + ); + + expect( + screen.getByText("What the supplier must look at"), + ).toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: "Edit" })); + + await waitFor(() => + expect(screen.getByText("What the supplier must look at")).toBeInTheDocument(), + ); + }); + + it("offers every status with the shared label", async () => { + render(); + + await waitFor(() => + expect(screen.getByText("REP-2026-11-001")).toBeInTheDocument(), + ); + + await userEvent.click(screen.getByRole("button", { name: "Edit" })); + + await waitFor(() => + expect(screen.getByRole("combobox")).toBeInTheDocument(), + ); + + await userEvent.click(screen.getByRole("combobox")); + + await waitFor(() => + expect(screen.getAllByRole("option").length).toBeGreaterThan(0), + ); + + const labels = screen.getAllByRole("option").map((o) => o.textContent); + expect(labels).toEqual([ + "Booked in", + "With supplier", + "Quote in", + "Approved", + "On the bench", + "Done", + "Cancelled", + ]); + }); +}); diff --git a/src/app/(app)/repairs/[id]/page.tsx b/src/app/(app)/repairs/[id]/page.tsx index 8345fe0..5cfc58d 100644 --- a/src/app/(app)/repairs/[id]/page.tsx +++ b/src/app/(app)/repairs/[id]/page.tsx @@ -762,7 +762,7 @@ export default function RepairDetailPage({
diff --git a/src/app/(app)/repairs/page.test.tsx b/src/app/(app)/repairs/page.test.tsx new file mode 100644 index 0000000..5d175d6 --- /dev/null +++ b/src/app/(app)/repairs/page.test.tsx @@ -0,0 +1,149 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { render, screen, waitFor, within } from "@testing-library/react"; + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ + push: vi.fn(), + replace: vi.fn(), + back: vi.fn(), + }), +})); + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + }, +})); + +import Page from "./page"; + +/* --------------------------------------------------------------------------- + * Test data + * ------------------------------------------------------------------------ */ + +const REPAIR = { + id: "rep1", + repair_number: "REP-2026-11-001", + customer: "cust1", + company: "co1", + order: "", + supplier: "sup1", + status: "assessing", + item_description: "Welding machine", + fault_description: "Will not strike", + notes: "", + serial_no: "SN-12345", + photos: [], + quote: [], + purchase_order: [], + repair_request: [], + expand: { + customer: { id: "cust1", customer: "Acme Mining" }, + supplier: { id: "sup1", suppliername: "Alpha Bearings" }, + }, +}; + +/* --------------------------------------------------------------------------- + * Fetch helpers + * ------------------------------------------------------------------------ */ + +function jsonResponse(body: unknown): Response { + return { + ok: true, + status: 200, + json: async () => body, + } as Response; +} + +function fetchUrl(input: RequestInfo | URL): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.toString(); + return input.url; +} + +function buildFetchSpy(items: unknown[] = [REPAIR]) { + return vi.spyOn(window, "fetch").mockImplementation(async (input) => { + const url = fetchUrl(input); + if (url === "/api/v1/repairs/form-data") { + return jsonResponse({ customers: [], suppliers: [] }); + } + if (url.startsWith("/api/v1/repairs?")) { + return jsonResponse({ + items, + totalItems: items.length, + totalPages: 1, + page: 1, + perPage: 20, + }); + } + return jsonResponse({}); + }); +} + +/* --------------------------------------------------------------------------- + * Tests + * ------------------------------------------------------------------------ */ + +describe("RepairsPage", () => { + beforeEach(() => { + buildFetchSpy(); + }); + + it("shows the shared status label, not the raw status", async () => { + render(); + + await waitFor(() => + expect(screen.getByRole("table")).toBeInTheDocument(), + ); + + const table = within(screen.getByRole("table")); + // "assessing" is the raw PocketBase value — the office reads + // "With supplier", the label from @/lib/repair-status. + expect(table.getByText("With supplier")).toBeInTheDocument(); + expect(screen.queryAllByText("assessing")).toHaveLength(0); + }); + + it("labels every status with the shared vocabulary", async () => { + buildFetchSpy([ + { ...REPAIR, id: "rep1", status: "new" }, + { ...REPAIR, id: "rep2", status: "assessing" }, + { ...REPAIR, id: "rep3", status: "quoted" }, + { ...REPAIR, id: "rep4", status: "approved" }, + { ...REPAIR, id: "rep5", status: "in_progress" }, + { ...REPAIR, id: "rep6", status: "completed" }, + { ...REPAIR, id: "rep7", status: "cancelled" }, + ]); + render(); + + await waitFor(() => + expect(screen.getByRole("table")).toBeInTheDocument(), + ); + + const table = within(screen.getByRole("table")); + for (const label of [ + "Booked in", + "With supplier", + "Quote in", + "Approved", + "On the bench", + "Done", + "Cancelled", + ]) { + expect(table.getByText(label)).toBeInTheDocument(); + } + + // None of the raw status values leak onto the page. + for (const raw of [ + "new", + "assessing", + "quoted", + "approved", + "in_progress", + "completed", + "cancelled", + ]) { + expect(screen.queryAllByText(raw)).toHaveLength(0); + } + }); +}); diff --git a/src/app/api/v1/repairs/[id]/files/[field]/route.ts b/src/app/api/v1/repairs/[id]/files/[field]/route.ts index b3266ba..4f97205 100644 --- a/src/app/api/v1/repairs/[id]/files/[field]/route.ts +++ b/src/app/api/v1/repairs/[id]/files/[field]/route.ts @@ -24,9 +24,9 @@ function isAllowedField(value: string): value is RepairFileField { * POST /api/v1/repairs/[id]/files/[field] * * Multipart upload: field name = the multi-file repair field - * (photos / quote / purchase_order / delivery_note). The file is - * appended to the existing list (re-PATCHes the row with the old - * filenames + the new one). + * (photos / quote / purchase_order / repair_request / signed_request). + * The file is appended to the existing list (re-PATCHes the row with + * the old filenames + the new one). * * For the `quote` field, the repair status is auto-bumped to * `quoted` if it was `new` or `assessing`. diff --git a/src/lib/server/repairs-service.test.ts b/src/lib/server/repairs-service.test.ts index d817986..cd6155d 100644 --- a/src/lib/server/repairs-service.test.ts +++ b/src/lib/server/repairs-service.test.ts @@ -388,6 +388,56 @@ describe("supplier_scope and signed_request", () => { }); }); +describe("updateRepair status guard", () => { + beforeEach(() => vi.clearAllMocks()); + + it("rejects an illegal transition and writes nothing", async () => { + const pb = makePb({ ...baseRepair, status: "new" }); + mockAdminPb(pb); + + // "new" may only go to assessing / quoted / cancelled — the guard + // inside updateRepair is what stops the illegal move. + await expect( + updateRepair("rep1", { status: "completed" }), + ).rejects.toThrow(/Cannot transition repair/); + + // The guard runs BEFORE the write, so nothing was patched. + expect(pb._update).not.toHaveBeenCalled(); + }); + + it("rejects a move out of a terminal status", async () => { + const pb = makePb({ ...baseRepair, status: "completed" }); + mockAdminPb(pb); + + await expect(updateRepair("rep1", { status: "new" })).rejects.toThrow( + /Cannot transition repair/, + ); + + expect(pb._update).not.toHaveBeenCalled(); + }); + + it("allows a legal transition and writes the new status", async () => { + const pb = makePb({ ...baseRepair, status: "new" }); + mockAdminPb(pb); + + const updated = await updateRepair("rep1", { status: "assessing" }); + + expect(pb._update).toHaveBeenCalledWith("rep1", { status: "assessing" }); + expect(updated.status).toBe("assessing"); + }); + + it("does not guard a status-free update", async () => { + const pb = makePb(baseRepair); + mockAdminPb(pb); + + await updateRepair("rep1", { notes: "called the customer" }); + + expect(pb._update).toHaveBeenCalledWith("rep1", { + notes: "called the customer", + }); + }); +}); + describe("getRepairStatusCounts", () => { beforeEach(() => vi.clearAllMocks()); diff --git a/src/lib/server/repairs-service.ts b/src/lib/server/repairs-service.ts index 5bafb89..a80c19b 100644 --- a/src/lib/server/repairs-service.ts +++ b/src/lib/server/repairs-service.ts @@ -58,7 +58,6 @@ import { } from "./quotes-service"; import { REPAIR_STATUS, - REPAIR_TRANSITIONS, assertRepairTransition, type RepairStatus, } from "@/lib/repair-status"; @@ -631,7 +630,9 @@ export async function linkRepairToOrder( ): Promise<{ order: Order; repair: Repair }> { const pb = await getAdminPocketBase(); await pb.collection(ORDERS).getOne(orderId); - const repair = await pb.collection(REPAIRS).getOne(repairId); + // The getOne IS the repair-existence check — it throws a 404 when + // the repair does not exist, before either write lands. + await pb.collection(REPAIRS).getOne(repairId); const order = await pb.collection(ORDERS).update(orderId, { repair: repairId,