Promote dev → uat: ARIA modal fix + tip split atomicity (#335)
* 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 <noreply@paperclip.ing> * 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 <noreply@paperclip.ing> * 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 <noreply@paperclip.ing> * 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 <noreply@paperclip.ing> * 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 <noreply@paperclip.ing> * 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 <noreply@paperclip.ing> * fix(GRO-785): restore eslint-disable for intentionally unused _tipSplits var Co-Authored-By: Paperclip <noreply@paperclip.ing> --------- Co-authored-by: Flea Flicker <fleaflicker@groombook.farh.net> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: the-dogfather-cto[bot] <269737991+the-dogfather-cto[bot]@users.noreply.github.com>
This commit was merged in pull request #335.
This commit is contained in:
committed by
GitHub
parent
1cc6d53546
commit
abee344ca4
@@ -18,6 +18,14 @@ import type { AppEnv } from "../middleware/rbac.js";
|
||||
|
||||
export const invoicesRouter = new Hono<AppEnv>();
|
||||
|
||||
// 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,23 @@ 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);
|
||||
}
|
||||
const tipCents = body.tipCents ?? current.tipCents;
|
||||
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
const { tipSplits: incomingTipSplits, ...bodyWithoutSplits } = body;
|
||||
const update: Record<string, unknown> = { ...bodyWithoutSplits, 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<string, unknown>;
|
||||
const update: Record<string, unknown> = { ...updateBody, updatedAt: new Date() };
|
||||
|
||||
// Auto-set paidAt when marking as paid
|
||||
if (body.status === "paid" && !body.paidAt && !current.paidAt) {
|
||||
@@ -378,47 +379,43 @@ invoicesRouter.patch(
|
||||
update.totalCents = current.subtotalCents + newTaxCents + newTipCents;
|
||||
}
|
||||
|
||||
const [updated] = await db.transaction(async (tx) => {
|
||||
const [upd] = await tx
|
||||
// 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 [updatedInvoice] = 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));
|
||||
const lineItems = await tx
|
||||
.select()
|
||||
.from(invoiceLineItems)
|
||||
.where(eq(invoiceLineItems.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];
|
||||
return [updatedInvoice, lineItems];
|
||||
});
|
||||
|
||||
const lineItems = await db
|
||||
.select()
|
||||
.from(invoiceLineItems)
|
||||
.where(eq(invoiceLineItems.invoiceId, id));
|
||||
|
||||
return c.json({ ...updated, lineItems });
|
||||
}
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user