From 13d887eae8411c2f3d3f583735ff99e5f59d49c2 Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 6 Nov 2026 17:32:33 +0200 Subject: [PATCH] fix(repairs-app): stop the book-in button covering the last card, and cover the session header --- .../repairs-app/repair-board.test.tsx | 170 ++++++++++++++++-- .../repairs-app/repair-board.tsx | 167 +++++++++++------ 2 files changed, 268 insertions(+), 69 deletions(-) diff --git a/src/app/(repairs-app)/repairs-app/repair-board.test.tsx b/src/app/(repairs-app)/repairs-app/repair-board.test.tsx index f1c329a..24d4fbf 100644 --- a/src/app/(repairs-app)/repairs-app/repair-board.test.tsx +++ b/src/app/(repairs-app)/repairs-app/repair-board.test.tsx @@ -1,5 +1,11 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; -import { render, screen, within } from "@testing-library/react"; +import { + fireEvent, + render, + screen, + waitFor, + within, +} from "@testing-library/react"; import { RepairBoard } from "./repair-board"; // The browser session lives in the PocketBase auth store, not on the @@ -8,6 +14,15 @@ vi.mock("@/lib/pocketbase", () => ({ getPocketBase: () => ({ authStore: { token: "t" } }), })); +/** + * A price-shaped figure. + * + * "R 1200.50", "1200.00" and "1 200.00" are money. "HR 200 Press" and + * "HP-2211" are a machine and a serial number, and must not trip it. + */ +const MONEY = /\bR ?[0-9.,]+|\d[\d\s]*[.,]\d{2}/; + +/** The shape the list API actually returns — relations expanded. */ const REPAIRS = [ { id: "rep1", @@ -16,8 +31,11 @@ const REPAIRS = [ item_description: "Hydraulic press, 40T", fault_description: "Will not build pressure", serial_no: "HP-2211", - customer: { name: "Borer Plant" }, - supplier: { suppliername: "Bolt & Nut" }, + customer: "cust1", + expand: { + customer: { id: "cust1", customer: "Borer Plant" }, + supplier: { id: "sup1", suppliername: "Bolt & Nut" }, + }, photos: ["a.jpg", "b.jpg"], }, { @@ -27,21 +45,39 @@ const REPAIRS = [ item_description: "Welding machine", fault_description: "Will not strike", serial_no: "WM-9", - // The list API expands the relations — this is the shape the - // board actually gets from the server. customer: "cust2", expand: { customer: { id: "cust2", customer: "Acme" } }, supplier: null, photos: [], }, + { + id: "rep3", + repair_number: "R-0009", + status: "quoted", + item_description: "HR 200 Press", + fault_description: "Loses pressure", + serial_no: "HR-200", + customer: "cust3", + expand: { customer: { id: "cust3", customer: "Vulcan Plant" } }, + photos: [], + }, ]; -function mockFetch() { +type Call = { url: string; headers: Record }; + +let calls: Call[] = []; + +function mockFetch(options: { listFail?: boolean; countsFail?: boolean } = {}) { + calls = []; vi.stubGlobal( "fetch", - vi.fn(async (input: RequestInfo | URL) => { + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { const url = String(input); + const headers = { ...((init?.headers ?? {}) as Record) }; + calls.push({ url, headers }); + if (url.includes("/status-counts")) { + if (options.countsFail) return new Response("{}", { status: 500 }); return new Response( JSON.stringify({ total: 12, @@ -58,13 +94,14 @@ function mockFetch() { { status: 200, headers: { "content-type": "application/json" } }, ); } + if (options.listFail) return new Response("{}", { status: 500 }); return new Response( JSON.stringify({ items: REPAIRS, totalItems: 12, totalPages: 1, page: 1, - perPage: 20, + perPage: 100, }), { status: 200, headers: { "content-type": "application/json" } }, ); @@ -72,11 +109,24 @@ function mockFetch() { ); } +function listCalls(): Call[] { + return calls.filter((c) => c.url.startsWith("/api/v1/repairs?")); +} + +function countsCalls(): Call[] { + return calls.filter((c) => c.url.includes("/api/v1/repairs/status-counts")); +} + +/** The filter chips — scoped, because the cards repeat the same words. */ +function chips() { + return within(screen.getByRole("group", { name: "Filter by status" })); +} + /** * Render the board and wait for it to load. * * The status chips are on screen before the fetch resolves (a status with - * no repairs still shows a zero), so waiting on a chip would not prove + * no repairs still shows a chip), so waiting on a chip would not prove * the board loaded. Waiting on the first card does. */ async function renderBoard() { @@ -92,9 +142,7 @@ describe("RepairBoard", () => { it("shows every status filter with its count", async () => { await renderBoard(); - const filters = within( - screen.getByRole("group", { name: "Filter by status" }), - ); + const filters = chips(); expect(filters.getByText(/Booked in/)).toBeTruthy(); expect(filters.getByText(/With supplier/)).toBeTruthy(); expect(filters.getByText(/Quote in/)).toBeTruthy(); @@ -106,9 +154,7 @@ describe("RepairBoard", () => { it("the seven status counts add up to the All count", async () => { await renderBoard(); - const filters = within( - screen.getByRole("group", { name: "Filter by status" }), - ); + const filters = chips(); const expected: Array<[string, number]> = [ ["Booked in", 3], @@ -130,24 +176,112 @@ describe("RepairBoard", () => { expect(shown.reduce((sum, n) => sum + n, 0)).toBe(total); }); + it("sends the browser session on both calls", async () => { + // Task 1 made both endpoints session-gated. Without this header the + // board answers 401 on every request. + await renderBoard(); + expect(listCalls()[0]?.headers.Authorization).toBe("Bearer t"); + expect(countsCalls()[0]?.headers.Authorization).toBe("Bearer t"); + }); + it("shows the status as text, not as colour alone", async () => { await renderBoard(); - expect(screen.getAllByText(/With supplier/).length).toBeGreaterThan(0); + // The chip AND the badge on the card — one without the other is not + // enough, and the unscoped count must catch a missing badge. + expect(screen.getAllByText(/With supplier/).length).toBeGreaterThanOrEqual( + 2, + ); + expect(chips().getByText(/With supplier/)).toBeTruthy(); }); it("never renders a price, a cost or a margin", async () => { await renderBoard(); - expect(screen.queryByText(/R ?[0-9.,]+/)).toBeNull(); + expect(screen.queryByText(MONEY)).toBeNull(); expect(screen.queryByText(/cost/i)).toBeNull(); expect(screen.queryByText(/margin/i)).toBeNull(); }); + it("the money guard catches a figure but not a machine name", () => { + expect(MONEY.test("1200.00")).toBe(true); + expect(MONEY.test("R 1200.50")).toBe(true); + expect(MONEY.test("1 200.00")).toBe(true); + expect(MONEY.test("HR 200 Press")).toBe(false); + expect(MONEY.test("HP-2211")).toBe(false); + expect(MONEY.test("Hydraulic press, 40T")).toBe(false); + }); + it("shows the machine, the customer and the serial number on each card", async () => { await renderBoard(); expect(screen.getByText(/Hydraulic press, 40T/)).toBeTruthy(); expect(screen.getByText(/Borer Plant/)).toBeTruthy(); expect(screen.getByText(/HP-2211/)).toBeTruthy(); - // The expanded customer the list API actually sends. expect(screen.getByText(/Acme/)).toBeTruthy(); }); + + it("says so when the counts fail instead of showing zeros", async () => { + mockFetch({ countsFail: true }); + render(); + await screen.findByText(/Hydraulic press, 40T/); + + expect(screen.getByText(/Could not load the counts/)).toBeTruthy(); + // No fake zero above a list of real repairs. + expect(chips().queryByText("All 0")).toBeNull(); + expect(chips().getByText(/All/)).toBeTruthy(); + // The list itself still renders. + expect(screen.getByText(/Borer Plant/)).toBeTruthy(); + }); + + it("retries the counts when the error line is pressed", async () => { + mockFetch({ countsFail: true }); + render(); + await screen.findByText(/Could not load the counts/); + + const before = countsCalls().length; + fireEvent.click(screen.getByText(/Could not load the counts/)); + await waitFor(() => expect(countsCalls().length).toBe(before + 1), { + timeout: 2000, + }); + }); + + it("keeps the counts when the list fails", async () => { + mockFetch({ listFail: true }); + render(); + await screen.findByText(/Could not load the repairs/); + expect(chips().getByText("All 12")).toBeTruthy(); + }); + + it("waits for a pause in typing before searching", async () => { + await renderBoard(); + const before = listCalls().length; + const input = screen.getByLabelText("Search repairs"); + + fireEvent.change(input, { target: { value: "H" } }); + fireEvent.change(input, { target: { value: "HP" } }); + fireEvent.change(input, { target: { value: "HP-2" } }); + + // Nothing goes out while the finger is still on the keyboard. + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(listCalls().length).toBe(before); + + // Three keystrokes, then ONE request — with the search term. + await waitFor(() => expect(listCalls().length).toBe(before + 1), { + timeout: 2000, + }); + expect(listCalls()[before].url).toContain("search=HP-2"); + }); + + it("does not refetch the counts when only the search text changes", async () => { + await renderBoard(); + const countsBefore = countsCalls().length; + + fireEvent.change(screen.getByLabelText("Search repairs"), { + target: { value: "HP" }, + }); + + await waitFor(() => expect(listCalls().length).toBeGreaterThan(1), { + timeout: 2000, + }); + // The counts are global — a new search cannot change them. + expect(countsCalls().length).toBe(countsBefore); + }); }); diff --git a/src/app/(repairs-app)/repairs-app/repair-board.tsx b/src/app/(repairs-app)/repairs-app/repair-board.tsx index 360618e..cb57fcd 100644 --- a/src/app/(repairs-app)/repairs-app/repair-board.tsx +++ b/src/app/(repairs-app)/repairs-app/repair-board.tsx @@ -27,35 +27,44 @@ const EMPTY_COUNTS: Counts = { }, }; -/** - * A row as the list API returns it. - * - * The repairs route expands `customer` and `supplier`, so a row carries - * the relation id AND the expanded record. The board shows the NAME, so - * the expanded record is read first and a plain nested object is the - * fallback. - */ -type BoardRepair = Omit & { - customer?: string | { name?: string; customer?: string } | null; - supplier?: string | { suppliername?: string } | null; -}; +/** How long to wait after the last keystroke before searching. */ +const SEARCH_DEBOUNCE_MS = 300; /** * The board — the screen the repairs person opens first. * - * Two rules drive everything here: + * Three rules drive everything here: * - every status shows its COUNT, and the counts add up to the total; + * - a count that could not be loaded is shown as unknown, never as a + * zero that would contradict the list below it; * - no money, ever. A repair row carries no prices, and this screen * must not invent one. */ export function RepairBoard() { - const [repairs, setRepairs] = useState([]); + const [repairs, setRepairs] = useState([]); const [counts, setCounts] = useState(EMPTY_COUNTS); + /** The counts have never loaded — the chips say "unknown", not "0". */ + const [countsLoaded, setCountsLoaded] = useState(false); + const [countsError, setCountsError] = useState(null); const [status, setStatus] = useState(""); const [search, setSearch] = useState(""); + /** What the list is actually filtered by — the search, after a pause. */ + const [appliedSearch, setAppliedSearch] = useState(""); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); + const [countsRetry, setCountsRetry] = useState(0); + // Typing fires a request per character otherwise — two per keystroke on + // a phone. Wait for a pause before searching. + useEffect(() => { + const handle = window.setTimeout( + () => setAppliedSearch(search), + SEARCH_DEBOUNCE_MS, + ); + return () => window.clearTimeout(handle); + }, [search]); + + // The list. Only the status filter and the paused search change it. useEffect(() => { let cancelled = false; async function load() { @@ -63,23 +72,17 @@ export function RepairBoard() { setError(null); try { const params = new URLSearchParams(); - params.set("perPage", "50"); + params.set("perPage", "100"); params.set("sort", "-created"); if (status) params.set("status", status); - if (search) params.set("search", search); - const headers = authHeaders(); - const [listRes, countRes] = await Promise.all([ - fetch("/api/v1/repairs?" + params.toString(), { headers }), - fetch("/api/v1/repairs/status-counts", { headers }), - ]); - if (!listRes.ok) throw new Error("Could not load the repairs."); - const list = (await listRes.json()) as { items: BoardRepair[] }; - const c = countRes.ok - ? ((await countRes.json()) as Counts) - : EMPTY_COUNTS; + if (appliedSearch) params.set("search", appliedSearch); + const res = await fetch("/api/v1/repairs?" + params.toString(), { + headers: authHeaders(), + }); + if (!res.ok) throw new Error("Could not load the repairs."); + const list = (await res.json()) as { items: Repair[] }; if (cancelled) return; setRepairs(list.items ?? []); - setCounts(c); } catch (err: unknown) { if (!cancelled) { setError( @@ -90,11 +93,55 @@ export function RepairBoard() { if (!cancelled) setLoading(false); } } - load(); + void load(); return () => { cancelled = true; }; - }, [status, search]); + }, [status, appliedSearch]); + + // The counts are GLOBAL — they take no query params, so a new search + // cannot change them and re-fetching them for every keystroke is pure + // waste (each one costs seven queries). They are re-fetched when the + // status filter changes, and when the phone comes back to this screen + // so the numbers stay true. + useEffect(() => { + let cancelled = false; + + async function loadCounts() { + try { + const res = await fetch("/api/v1/repairs/status-counts", { + headers: authHeaders(), + }); + if (!res.ok) throw new Error("Could not load the counts."); + const c = (await res.json()) as Counts; + if (cancelled) return; + setCounts(c); + setCountsLoaded(true); + setCountsError(null); + } catch (err: unknown) { + if (cancelled) return; + // An unknown count is shown as unknown — a zero here would + // contradict the list of repairs right underneath it. + setCountsLoaded(false); + setCountsError( + err instanceof Error ? err.message : "Could not load the counts.", + ); + } + } + + void loadCounts(); + + const refresh = () => { + if (document.visibilityState === "visible") void loadCounts(); + }; + window.addEventListener("focus", refresh); + document.addEventListener("visibilitychange", refresh); + return () => { + cancelled = true; + window.removeEventListener("focus", refresh); + document.removeEventListener("visibilitychange", refresh); + }; + }, [status, countsRetry]); const chips = useMemo( () => [ @@ -108,6 +155,8 @@ export function RepairBoard() { [counts], ); + const countsKnown = countsLoaded && !countsError; + return (

Repairs

@@ -139,12 +188,22 @@ export function RepairBoard() { (active ? "border-primary bg-primary/10 font-medium" : "") } > - {chip.label} {chip.count} + {chip.label} {countsKnown ? chip.count : "\u2014"} ); })}
+ {countsError ? ( + + ) : null} + {error ? (

{error} @@ -152,7 +211,7 @@ export function RepairBoard() { ) : null} {loading ? ( -

Loading repairs…

+

Loading repairs\u2026

) : null} {!loading && !error && repairs.length === 0 ? ( @@ -161,7 +220,12 @@ export function RepairBoard() {

) : null} -
    + {/* + The list carries its own bottom padding so the LAST card clears + the floating "Book in a machine" button, which sits 56px above + the bottom of the shell's content box. + */} +
      {repairs.map((repair) => (
    • - {repair.repair_number} +
      + {repair.repair_number} + {repair.serial_no ? ( +

      + S/N: {repair.serial_no} +

      + ) : null} +
      @@ -177,17 +248,18 @@ export function RepairBoard() {

      {repair.item_description}

      -

      - {customerName(repair)} - {repair.serial_no ? " · S/N " + repair.serial_no : ""} -

      + {customerName(repair) ? ( +

      + {customerName(repair)} +

      + ) : null}

      {photoCount(repair) > 0 ? photoCount(repair) + " photo" + (photoCount(repair) === 1 ? "" : "s") : "No photos"} - {supplierName(repair) ? " · " + supplierName(repair) : ""} + {supplierName(repair) ? " \u00b7 " + supplierName(repair) : ""}

      • @@ -222,22 +294,15 @@ function authHeaders(): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } -function customerName(repair: BoardRepair): string { - const expanded = repair.expand?.customer; - if (expanded?.customer) return expanded.customer; - const raw = repair.customer; - if (raw && typeof raw === "object") return raw.name ?? raw.customer ?? ""; - return ""; +/** The expanded customer record — the relation id carries no name. */ +function customerName(repair: Repair): string { + return repair.expand?.customer?.customer ?? ""; } -function supplierName(repair: BoardRepair): string { - const expanded = repair.expand?.supplier; - if (expanded?.suppliername) return expanded.suppliername; - const raw = repair.supplier; - if (raw && typeof raw === "object") return raw.suppliername ?? ""; - return ""; +function supplierName(repair: Repair): string { + return repair.expand?.supplier?.suppliername ?? ""; } -function photoCount(repair: BoardRepair): number { +function photoCount(repair: Repair): number { return Array.isArray(repair.photos) ? repair.photos.length : 0; }