fix: appointment conflict detection, soft-delete, and auth guardrail (#18-22)
Fixes five bugs flagged in CEO code review (GitHub issues #18–22): - #18: Wrap conflict check + insert/update in a DB transaction to prevent double-booking race conditions under concurrent load. - #19: PATCH conflict detection now falls back to the existing appointment's staffId when staffId is omitted from the request body, so rescheduling always checks for conflicts. - #20: DELETE endpoint now soft-deletes (status = 'cancelled') instead of hard-deleting, preserving audit trail and financial records. - #21: Staff DELETE checks for existing non-cancelled appointments before deleting and returns 409 if any are found, preventing orphaned references. - #22: AUTH_DISABLED=true now logs a startup warning in development and calls process.exit(1) in production, preventing accidental auth bypass in deployed environments. Co-authored-by: Groom Book CTO <cto@groombook.app> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit was merged in pull request #24.
This commit is contained in:
committed by
GitHub
parent
0ebc199aea
commit
43e50255ec
@@ -1,7 +1,7 @@
|
||||
import { Hono } from "hono";
|
||||
import { zValidator } from "@hono/zod-validator";
|
||||
import { z } from "zod";
|
||||
import { eq, getDb, staff } from "@groombook/db";
|
||||
import { and, eq, getDb, ne, staff, appointments } from "@groombook/db";
|
||||
|
||||
export const staffRouter = new Hono();
|
||||
|
||||
@@ -55,9 +55,33 @@ staffRouter.patch("/:id", zValidator("json", updateStaffSchema), async (c) => {
|
||||
|
||||
staffRouter.delete("/:id", async (c) => {
|
||||
const db = getDb();
|
||||
const id = c.req.param("id");
|
||||
|
||||
// Prevent deleting staff who have existing non-cancelled appointments (fixes #21).
|
||||
const activeAppointments = await db
|
||||
.select({ id: appointments.id })
|
||||
.from(appointments)
|
||||
.where(
|
||||
and(
|
||||
eq(appointments.staffId, id),
|
||||
ne(appointments.status, "cancelled"),
|
||||
ne(appointments.status, "no_show"),
|
||||
)
|
||||
)
|
||||
.limit(1);
|
||||
if (activeAppointments.length > 0) {
|
||||
return c.json(
|
||||
{
|
||||
error:
|
||||
"Cannot delete staff member with existing appointments. Reassign or cancel their appointments first.",
|
||||
},
|
||||
409
|
||||
);
|
||||
}
|
||||
|
||||
const [row] = await db
|
||||
.delete(staff)
|
||||
.where(eq(staff.id, c.req.param("id")))
|
||||
.where(eq(staff.id, id))
|
||||
.returning();
|
||||
if (!row) return c.json({ error: "Not found" }, 404);
|
||||
return c.json({ ok: true });
|
||||
|
||||
Reference in New Issue
Block a user