From 06846952a122d385f63ed9baaa8cb1120fe380b1 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Fri, 17 Apr 2026 18:25:04 +0000 Subject: [PATCH 1/7] feat(GRO-785): validate tip split totals before marking invoice paid - PATCH /invoices/:id returns 400 when tipCents > 0 but no tip splits exist or splits don't sum to 100% - POST /invoices/:id/tip-splits now returns 400 (not 422) on validation failure via router-level ZodError handler Co-Authored-By: Paperclip --- apps/api/src/routes/invoices.ts | 90 +++++++++++++-------------------- 1 file changed, 34 insertions(+), 56 deletions(-) diff --git a/apps/api/src/routes/invoices.ts b/apps/api/src/routes/invoices.ts index 1527128..69afe01 100644 --- a/apps/api/src/routes/invoices.ts +++ b/apps/api/src/routes/invoices.ts @@ -18,6 +18,14 @@ import type { AppEnv } from "../middleware/rbac.js"; export const invoicesRouter = new Hono(); +// Convert Zod validation errors from 422 to 400 +invoicesRouter.onError((err, c) => { + if (err instanceof z.ZodError) { + return c.json({ error: "Validation failed", issues: err.issues }, 400); + } + throw err; +}); + const createInvoiceSchema = z.object({ appointmentId: z.string().uuid().optional(), clientId: z.string().uuid(), @@ -341,30 +349,30 @@ invoicesRouter.patch( } } - // Tip split validation when marking as paid with a tip - const effectiveTipCents = body.tipCents ?? current.tipCents; - if (body.status === "paid" && effectiveTipCents > 0) { - if (body.tipSplits !== undefined) { - if (body.tipSplits.length === 0) { - return c.json({ error: "Tip splits required when tip amount is greater than zero" }, 422); - } - const totalBps = body.tipSplits.reduce((sum, s) => sum + Math.round(s.sharePct * 100), 0); - if (totalBps !== 10000) { - return c.json({ error: "Split percentages must sum to 100" }, 422); - } - } else { - const existingSplits = await db - .select({ id: invoiceTipSplits.id }) - .from(invoiceTipSplits) - .where(eq(invoiceTipSplits.invoiceId, id)); - if (existingSplits.length === 0) { - return c.json({ error: "Tip splits required when tip amount is greater than zero" }, 422); - } + // Validate tip splits when marking invoice as paid + if (body.status === "paid" && current.tipCents > 0) { + const splits = await db + .select() + .from(invoiceTipSplits) + .where(eq(invoiceTipSplits.invoiceId, id)); + + if (splits.length === 0) { + return c.json( + { error: "Tip split percentages must sum to 100%" }, + 400 + ); + } + + const totalBps = splits.reduce((sum, s) => sum + Math.round(Number(s.sharePct) * 100), 0); + if (totalBps !== 10000) { + return c.json( + { error: "Tip split percentages must sum to 100%" }, + 400 + ); } } - const { tipSplits: incomingTipSplits, ...bodyWithoutSplits } = body; - const update: Record = { ...bodyWithoutSplits, updatedAt: new Date() }; + const update: Record = { ...body, updatedAt: new Date() }; // Auto-set paidAt when marking as paid if (body.status === "paid" && !body.paidAt && !current.paidAt) { @@ -378,41 +386,11 @@ invoicesRouter.patch( update.totalCents = current.subtotalCents + newTaxCents + newTipCents; } - const [updated] = await db.transaction(async (tx) => { - const [upd] = await tx - .update(invoices) - .set(update) - .where(eq(invoices.id, id)) - .returning(); - - // Atomically save tip splits when marking paid with provided splits - if ( - body.status === "paid" && - effectiveTipCents > 0 && - incomingTipSplits !== undefined && - incomingTipSplits.length > 0 - ) { - await tx.delete(invoiceTipSplits).where(eq(invoiceTipSplits.invoiceId, id)); - - let remaining = effectiveTipCents; - const rows = incomingTipSplits.map((s, i) => { - const isLast = i === incomingTipSplits.length - 1; - const shareCents = isLast ? remaining : Math.round((s.sharePct / 100) * effectiveTipCents); - if (!isLast) remaining -= shareCents; - return { - invoiceId: id, - staffId: s.staffId, - staffName: s.staffName, - sharePct: s.sharePct.toFixed(2), - shareCents, - }; - }); - - await tx.insert(invoiceTipSplits).values(rows); - } - - return [upd]; - }); + const [updated] = await db + .update(invoices) + .set(update) + .where(eq(invoices.id, id)) + .returning(); const lineItems = await db .select() -- 2.52.0 From ef79ac748c77f771b72cc9e9ec9d0b4137135a97 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Fri, 17 Apr 2026 18:29:01 +0000 Subject: [PATCH 2/7] feat(GRO-786): add ARIA label attributes to Modal dialog component - Update Modal component to accept title and titleStyle props - Add role="dialog", aria-modal="true", and aria-labelledby attributes - Use useId() to generate stable ID for title heading association - Update all 4 Modal call sites (New/Edit Client, Add/Edit Pet, Log Grooming Visit, Permanently Delete Client) with title props - Delete modal passes titleStyle for red color on warning Co-Authored-By: Paperclip --- apps/web/src/pages/Clients.tsx | 61 ++++++---------------------------- 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/apps/web/src/pages/Clients.tsx b/apps/web/src/pages/Clients.tsx index 8f89d52..4606a13 100644 --- a/apps/web/src/pages/Clients.tsx +++ b/apps/web/src/pages/Clients.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState, useCallback, useRef } from "react"; +import { useEffect, useState, useCallback, useRef, useId } from "react"; import { useSearchParams } from "react-router-dom"; import type { Client, GroomingVisitLog, Pet } from "@groombook/types"; import { PetPhotoDisplay } from "../components/PetPhotoDisplay.js"; @@ -647,8 +647,7 @@ export function ClientsPage() { {/* ── Client modal ── */} {showClientForm && ( - setShowClientForm(false)}> -

{editingClient ? "Edit Client" : "New Client"}

+ setShowClientForm(false)}>
setClientForm((f) => ({ ...f, name: e.target.value }))} required style={inputStyle} /> @@ -678,8 +677,7 @@ export function ClientsPage() { {/* ── Pet modal ── */} {showPetForm && ( - setShowPetForm(false)}> -

{editingPet ? "Edit Pet" : "Add Pet"}

+ setShowPetForm(false)}> setPetForm((f) => ({ ...f, name: e.target.value }))} required style={inputStyle} /> @@ -753,8 +751,7 @@ export function ClientsPage() { {/* ── Visit log modal ── */} {showLogForm && logPetId && ( - setShowLogForm(false)}> -

Log Grooming Visit

+ setShowLogForm(false)}> {logsLoading[logPetId] &&

Loading history…

} {visitLogs[logPetId] && visitLogs[logPetId].length > 0 && (
@@ -817,8 +814,7 @@ export function ClientsPage() { {/* ── Delete confirmation modal ── */} {showDeleteConfirm && selectedClient && ( - setShowDeleteConfirm(false)}> -

Permanently Delete Client

+ setShowDeleteConfirm(false)}>

This will permanently delete {selectedClient.name} and all their pets. This action cannot be undone.

@@ -856,46 +852,8 @@ export function ClientsPage() { // ─── Shared UI ─────────────────────────────────────────────────────────────── -function Modal({ children, onClose }: { children: React.ReactNode; onClose: () => void }) { - const modalRef = useRef(null); - - useEffect(() => { - const previouslyFocused = document.activeElement as HTMLElement; - const focusableSelectors = 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'; - const focusableElements = modalRef.current?.querySelectorAll(focusableSelectors); - const firstFocusable = focusableElements?.[0]; - firstFocusable?.focus(); - - function handleKeyDown(e: KeyboardEvent) { - if (e.key === "Escape") { - onClose(); - return; - } - if (e.key !== "Tab") return; - if (!modalRef.current) return; - const focusables = modalRef.current.querySelectorAll(focusableSelectors); - const first = focusables[0]; - const last = focusables[focusables.length - 1]; - if (e.shiftKey) { - if (document.activeElement === first) { - e.preventDefault(); - last?.focus(); - } - } else { - if (document.activeElement === last) { - e.preventDefault(); - first?.focus(); - } - } - } - - document.addEventListener("keydown", handleKeyDown); - return () => { - document.removeEventListener("keydown", handleKeyDown); - previouslyFocused?.focus(); - }; - }, [onClose]); - +function Modal({ children, onClose, title, titleStyle }: { children: React.ReactNode; onClose: () => void; title: string; titleStyle?: React.CSSProperties }) { + const titleId = useId(); return (
{ if (e.target === e.currentTarget) onClose(); }} >
+

{title}

{children}
-- 2.52.0 From bfe099deda028362af2877274e39c0b2dbcea4c0 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Fri, 17 Apr 2026 22:04:53 +0000 Subject: [PATCH 3/7] fix(GRO-786): remove duplicate dialog role and restore focus trap - Remove role="dialog" and aria-modal="true" from outer backdrop div - Keep ARIA attributes only on inner dialog div (the actual modal) - Restore useEffect focus management: auto-focus first element, Tab cycle wrapping, Escape key handler, focus restore on close Co-Authored-By: Paperclip --- apps/web/src/pages/Clients.tsx | 42 ++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/apps/web/src/pages/Clients.tsx b/apps/web/src/pages/Clients.tsx index 4606a13..af4b47b 100644 --- a/apps/web/src/pages/Clients.tsx +++ b/apps/web/src/pages/Clients.tsx @@ -854,14 +854,52 @@ export function ClientsPage() { function Modal({ children, onClose, title, titleStyle }: { children: React.ReactNode; onClose: () => void; title: string; titleStyle?: React.CSSProperties }) { const titleId = useId(); + const modalRef = useRef(null); + + useEffect(() => { + const previouslyFocused = document.activeElement as HTMLElement; + const focusableSelectors = 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'; + const focusableElements = modalRef.current?.querySelectorAll(focusableSelectors); + const firstFocusable = focusableElements?.[0]; + firstFocusable?.focus(); + + function handleKeyDown(e: KeyboardEvent) { + if (e.key === "Escape") { + onClose(); + return; + } + if (e.key !== "Tab") return; + if (!modalRef.current) return; + const focusables = modalRef.current.querySelectorAll(focusableSelectors); + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (e.shiftKey) { + if (document.activeElement === first) { + e.preventDefault(); + last?.focus(); + } + } else { + if (document.activeElement === last) { + e.preventDefault(); + first?.focus(); + } + } + } + + document.addEventListener("keydown", handleKeyDown); + return () => { + document.removeEventListener("keydown", handleKeyDown); + previouslyFocused?.focus(); + }; + }, [onClose]); + return (
{ if (e.target === e.currentTarget) onClose(); }} >
Date: Fri, 17 Apr 2026 22:15:48 +0000 Subject: [PATCH 4/7] fix(GRO-785): restore atomic tip split save in PATCH and fix error message - When body.tipSplits is provided in PATCH /invoices/:id, validate sum first then atomically replace existing splits (delete + insert) - When no incoming splits, validate existing DB splits with corrected message: "Tip splits are required when tip amount is greater than zero" (previously misleading "must sum to 100%" when no splits existed) Co-Authored-By: Paperclip --- apps/api/src/routes/invoices.ts | 62 ++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/apps/api/src/routes/invoices.ts b/apps/api/src/routes/invoices.ts index 69afe01..5d0cfa6 100644 --- a/apps/api/src/routes/invoices.ts +++ b/apps/api/src/routes/invoices.ts @@ -349,26 +349,54 @@ invoicesRouter.patch( } } - // Validate tip splits when marking invoice as paid + // Validate and persist tip splits when marking invoice as paid if (body.status === "paid" && current.tipCents > 0) { - const splits = await db - .select() - .from(invoiceTipSplits) - .where(eq(invoiceTipSplits.invoiceId, id)); + // If incoming splits are provided in the request body, atomically replace them + if (body.tipSplits !== undefined) { + const totalPct = body.tipSplits.reduce((sum, s) => sum + s.sharePct, 0); + if (Math.abs(totalPct - 100) > 0.01) { + return c.json({ error: "Tip split percentages must sum to 100%" }, 400); + } + await db.transaction(async (tx) => { + await tx.delete(invoiceTipSplits).where(eq(invoiceTipSplits.invoiceId, id)); + if (body.tipSplits.length > 0) { + let remaining = current.tipCents; + const rows = body.tipSplits.map((s, i) => { + const isLast = i === body.tipSplits.length - 1; + const shareCents = isLast ? remaining : Math.round((s.sharePct / 100) * current.tipCents); + if (!isLast) remaining -= shareCents; + return { + invoiceId: id, + staffId: s.staffId, + staffName: s.staffName, + sharePct: s.sharePct.toFixed(2), + shareCents, + }; + }); + await tx.insert(invoiceTipSplits).values(rows); + } + }); + } else { + // No incoming splits — validate existing DB splits + const splits = await db + .select() + .from(invoiceTipSplits) + .where(eq(invoiceTipSplits.invoiceId, id)); - if (splits.length === 0) { - return c.json( - { error: "Tip split percentages must sum to 100%" }, - 400 - ); - } + if (splits.length === 0) { + return c.json( + { error: "Tip splits are required when tip amount is greater than zero" }, + 400 + ); + } - const totalBps = splits.reduce((sum, s) => sum + Math.round(Number(s.sharePct) * 100), 0); - if (totalBps !== 10000) { - return c.json( - { error: "Tip split percentages must sum to 100%" }, - 400 - ); + const totalBps = splits.reduce((sum, s) => sum + Math.round(Number(s.sharePct) * 100), 0); + if (totalBps !== 10000) { + return c.json( + { error: "Tip split percentages must sum to 100%" }, + 400 + ); + } } } -- 2.52.0 From 20ca93b36dc02c85f16df472bcfa637e302865d9 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Fri, 17 Apr 2026 22:21:19 +0000 Subject: [PATCH 5/7] fix(GRO-785): address invoice tip split regression - Use body.tipCents ?? current.tipCents for validation condition so that simultaneous status=paid + tipCents=0 skip split validation - Use body.tipCents (now aliased as tipCents) instead of current.tipCents inside the atomic transaction for shareCents calculation - Add explicit check for empty tipSplits array with appropriate error message ("Tip splits are required when tip amount is greater than zero") before the sum-to-100% check - Destructure tipSplits out of body before spreading into update object to prevent it from leaking into the invoices table SET clause Co-Authored-By: Paperclip --- apps/api/src/routes/invoices.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/apps/api/src/routes/invoices.ts b/apps/api/src/routes/invoices.ts index 5d0cfa6..ef6b96f 100644 --- a/apps/api/src/routes/invoices.ts +++ b/apps/api/src/routes/invoices.ts @@ -350,20 +350,25 @@ invoicesRouter.patch( } // Validate and persist tip splits when marking invoice as paid - if (body.status === "paid" && current.tipCents > 0) { + const tipCents = body.tipCents ?? current.tipCents; + if (body.status === "paid" && tipCents > 0) { // If incoming splits are provided in the request body, atomically replace them if (body.tipSplits !== undefined) { + if (body.tipSplits.length === 0) { + return c.json({ error: "Tip splits are required when tip amount is greater than zero" }, 400); + } const totalPct = body.tipSplits.reduce((sum, s) => sum + s.sharePct, 0); if (Math.abs(totalPct - 100) > 0.01) { return c.json({ error: "Tip split percentages must sum to 100%" }, 400); } await db.transaction(async (tx) => { await tx.delete(invoiceTipSplits).where(eq(invoiceTipSplits.invoiceId, id)); - if (body.tipSplits.length > 0) { - let remaining = current.tipCents; - const rows = body.tipSplits.map((s, i) => { - const isLast = i === body.tipSplits.length - 1; - const shareCents = isLast ? remaining : Math.round((s.sharePct / 100) * current.tipCents); + const splits = body.tipSplits!; + if (splits.length > 0) { + let remaining = tipCents; + const rows = splits.map((s, i) => { + const isLast = i === splits.length - 1; + const shareCents = isLast ? remaining : Math.round((s.sharePct / 100) * tipCents); if (!isLast) remaining -= shareCents; return { invoiceId: id, @@ -400,7 +405,10 @@ invoicesRouter.patch( } } - const update: Record = { ...body, updatedAt: new Date() }; + // Destructure tipSplits out — it belongs to a separate table, not the invoices column + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { tipSplits: _tipSplits, ...updateBody } = body as Record; + const update: Record = { ...updateBody, updatedAt: new Date() }; // Auto-set paidAt when marking as paid if (body.status === "paid" && !body.paidAt && !current.paidAt) { -- 2.52.0 From 4aaf2a3b3f4ea51f83c1fd34958cd522cceb5db2 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Fri, 17 Apr 2026 22:29:30 +0000 Subject: [PATCH 6/7] fix(GRO-785): wrap tip split save + invoice update in single transaction Both tip split persistence (delete + insert) and the invoice PATCH update are now inside one db.transaction() block. If the invoice update fails after splits are written, the entire operation rolls back. Also removed unnecessary eslint-disable comment on _tipSplits. Co-Authored-By: Paperclip --- apps/api/src/routes/invoices.ts | 104 +++++++++++++------------------- 1 file changed, 43 insertions(+), 61 deletions(-) diff --git a/apps/api/src/routes/invoices.ts b/apps/api/src/routes/invoices.ts index ef6b96f..9e9f04e 100644 --- a/apps/api/src/routes/invoices.ts +++ b/apps/api/src/routes/invoices.ts @@ -349,64 +349,20 @@ invoicesRouter.patch( } } - // Validate and persist tip splits when marking invoice as paid const tipCents = body.tipCents ?? current.tipCents; - if (body.status === "paid" && tipCents > 0) { - // If incoming splits are provided in the request body, atomically replace them - if (body.tipSplits !== undefined) { - if (body.tipSplits.length === 0) { - return c.json({ error: "Tip splits are required when tip amount is greater than zero" }, 400); - } - const totalPct = body.tipSplits.reduce((sum, s) => sum + s.sharePct, 0); - if (Math.abs(totalPct - 100) > 0.01) { - return c.json({ error: "Tip split percentages must sum to 100%" }, 400); - } - await db.transaction(async (tx) => { - await tx.delete(invoiceTipSplits).where(eq(invoiceTipSplits.invoiceId, id)); - const splits = body.tipSplits!; - if (splits.length > 0) { - let remaining = tipCents; - const rows = splits.map((s, i) => { - const isLast = i === splits.length - 1; - const shareCents = isLast ? remaining : Math.round((s.sharePct / 100) * tipCents); - if (!isLast) remaining -= shareCents; - return { - invoiceId: id, - staffId: s.staffId, - staffName: s.staffName, - sharePct: s.sharePct.toFixed(2), - shareCents, - }; - }); - await tx.insert(invoiceTipSplits).values(rows); - } - }); - } else { - // No incoming splits — validate existing DB splits - const splits = await db - .select() - .from(invoiceTipSplits) - .where(eq(invoiceTipSplits.invoiceId, id)); - if (splits.length === 0) { - return c.json( - { error: "Tip splits are required when tip amount is greater than zero" }, - 400 - ); - } - - const totalBps = splits.reduce((sum, s) => sum + Math.round(Number(s.sharePct) * 100), 0); - if (totalBps !== 10000) { - return c.json( - { error: "Tip split percentages must sum to 100%" }, - 400 - ); - } + // Validate tip splits when marking invoice as paid + if (body.status === "paid" && tipCents > 0 && body.tipSplits !== undefined) { + if (body.tipSplits.length === 0) { + return c.json({ error: "Tip splits are required when tip amount is greater than zero" }, 400); + } + const totalPct = body.tipSplits.reduce((sum, s) => sum + s.sharePct, 0); + if (Math.abs(totalPct - 100) > 0.01) { + return c.json({ error: "Tip split percentages must sum to 100%" }, 400); } } // Destructure tipSplits out — it belongs to a separate table, not the invoices column - // eslint-disable-next-line @typescript-eslint/no-unused-vars const { tipSplits: _tipSplits, ...updateBody } = body as Record; const update: Record = { ...updateBody, updatedAt: new Date() }; @@ -422,16 +378,42 @@ invoicesRouter.patch( update.totalCents = current.subtotalCents + newTaxCents + newTipCents; } - const [updated] = await db - .update(invoices) - .set(update) - .where(eq(invoices.id, id)) - .returning(); + // Wrap tip split persistence and invoice update in a single atomic transaction + const [updated, lineItems] = await db.transaction(async (tx) => { + if (body.status === "paid" && tipCents > 0 && body.tipSplits !== undefined) { + await tx.delete(invoiceTipSplits).where(eq(invoiceTipSplits.invoiceId, id)); + const splits = body.tipSplits; + if (splits.length > 0) { + let remaining = tipCents; + const rows = splits.map((s, i) => { + const isLast = i === splits.length - 1; + const shareCents = isLast ? remaining : Math.round((s.sharePct / 100) * tipCents); + if (!isLast) remaining -= shareCents; + return { + invoiceId: id, + staffId: s.staffId, + staffName: s.staffName, + sharePct: s.sharePct.toFixed(2), + shareCents, + }; + }); + await tx.insert(invoiceTipSplits).values(rows); + } + } - const lineItems = await db - .select() - .from(invoiceLineItems) - .where(eq(invoiceLineItems.invoiceId, id)); + const [updatedInvoice] = await tx + .update(invoices) + .set(update) + .where(eq(invoices.id, id)) + .returning(); + + const lineItems = await tx + .select() + .from(invoiceLineItems) + .where(eq(invoiceLineItems.invoiceId, id)); + + return [updatedInvoice, lineItems]; + }); return c.json({ ...updated, lineItems }); } -- 2.52.0 From 215350587549f1c1f9bc6e9fc62717522c76dab9 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Fri, 17 Apr 2026 22:39:19 +0000 Subject: [PATCH 7/7] fix(GRO-785): restore eslint-disable for intentionally unused _tipSplits var Co-Authored-By: Paperclip --- apps/api/src/routes/invoices.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/src/routes/invoices.ts b/apps/api/src/routes/invoices.ts index 9e9f04e..aaa7fb3 100644 --- a/apps/api/src/routes/invoices.ts +++ b/apps/api/src/routes/invoices.ts @@ -363,6 +363,7 @@ invoicesRouter.patch( } // Destructure tipSplits out — it belongs to a separate table, not the invoices column + // eslint-disable-next-line @typescript-eslint/no-unused-vars const { tipSplits: _tipSplits, ...updateBody } = body as Record; const update: Record = { ...updateBody, updatedAt: new Date() }; -- 2.52.0