fix(GRO-638): appointment scheduling correctness and client deletion integrity #279
@@ -27,12 +27,14 @@ const DISABLED_CLIENT = {
|
||||
// ─── Queue-based mock DB ──────────────────────────────────────────────────────
|
||||
|
||||
let selectRows: Record<string, unknown>[] = [];
|
||||
let appointmentRows: Record<string, unknown>[] = [];
|
||||
let insertedValues: Record<string, unknown>[] = [];
|
||||
let updatedValues: Record<string, unknown>[] = [];
|
||||
let deletedId: string | null = null;
|
||||
|
||||
function resetMock() {
|
||||
selectRows = [];
|
||||
appointmentRows = [];
|
||||
insertedValues = [];
|
||||
updatedValues = [];
|
||||
deletedId = null;
|
||||
@@ -58,10 +60,19 @@ vi.mock("@groombook/db", () => {
|
||||
{ get: (t, p) => (p === "_name" ? "clients" : { table: "clients", column: p }) }
|
||||
);
|
||||
|
||||
const appointments = new Proxy(
|
||||
{ _name: "appointments" },
|
||||
{ get: (t, p) => (p === "_name" ? "appointments" : { table: "appointments", column: p }) }
|
||||
);
|
||||
|
||||
return {
|
||||
getDb: () => ({
|
||||
select: () => ({
|
||||
from: () => makeChainable(selectRows),
|
||||
from: (table: unknown) => {
|
||||
const tableName = (table as { _name?: string })._name;
|
||||
const rows = tableName === "appointments" ? appointmentRows : selectRows;
|
||||
return makeChainable(rows);
|
||||
},
|
||||
}),
|
||||
insert: () => ({
|
||||
values: (vals: Record<string, unknown>) => {
|
||||
@@ -95,8 +106,10 @@ vi.mock("@groombook/db", () => {
|
||||
}),
|
||||
}),
|
||||
clients,
|
||||
appointments,
|
||||
eq: vi.fn(),
|
||||
and: vi.fn(),
|
||||
or: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -23,6 +23,27 @@ import { buildConfirmationEmail, sendEmail } from "../services/email.js";
|
||||
import { notifyWaitlistForAppointment } from "../services/waitlistNotify.js";
|
||||
import type { AppEnv } from "../middleware/rbac.js";
|
||||
|
||||
async function withRetry<T>(
|
||||
fn: () => Promise<T>,
|
||||
maxRetries: number,
|
||||
delayMs: number,
|
||||
context: string
|
||||
): Promise<void> {
|
||||
let lastError: unknown;
|
||||
for (let attempt = 0; attempt <= maxRetries; attempt++) {
|
||||
try {
|
||||
await fn();
|
||||
return;
|
||||
} catch (err) {
|
||||
lastError = err;
|
||||
if (attempt < maxRetries) {
|
||||
await new Promise((resolve) => setTimeout(resolve, delayMs));
|
||||
}
|
||||
}
|
||||
}
|
||||
console.error(`[appointments] ${context}: ${lastError}`);
|
||||
}
|
||||
|
||||
export const appointmentsRouter = new Hono<AppEnv>();
|
||||
|
||||
const createAppointmentSchema = z.object({
|
||||
@@ -212,11 +233,54 @@ appointmentsRouter.post(
|
||||
recurrence.frequencyWeeks * 7 * 24 * 60 * 60 * 1000;
|
||||
|
||||
let first: typeof appointments.$inferSelect | undefined;
|
||||
const conflictingInstances: number[] = [];
|
||||
for (let i = 0; i < recurrence.count; i++) {
|
||||
const instanceStart = new Date(start.getTime() + i * intervalMs);
|
||||
const instanceEnd = new Date(
|
||||
instanceStart.getTime() + durationMs
|
||||
);
|
||||
|
||||
if (apptFields.staffId) {
|
||||
const conflicts = await tx
|
||||
.select({ id: appointments.id })
|
||||
.from(appointments)
|
||||
.where(
|
||||
and(
|
||||
eq(appointments.staffId, apptFields.staffId),
|
||||
lt(appointments.startTime, instanceEnd),
|
||||
gte(appointments.endTime, instanceStart),
|
||||
ne(appointments.status, "cancelled"),
|
||||
ne(appointments.status, "no_show"),
|
||||
)
|
||||
)
|
||||
.limit(1);
|
||||
if (conflicts.length > 0) {
|
||||
conflictingInstances.push(i);
|
||||
}
|
||||
}
|
||||
|
||||
if (apptFields.batherStaffId) {
|
||||
const conflicts = await tx
|
||||
.select({ id: appointments.id })
|
||||
.from(appointments)
|
||||
.where(
|
||||
and(
|
||||
or(
|
||||
eq(appointments.staffId, apptFields.batherStaffId),
|
||||
eq(appointments.batherStaffId, apptFields.batherStaffId)
|
||||
),
|
||||
lt(appointments.startTime, instanceEnd),
|
||||
gte(appointments.endTime, instanceStart),
|
||||
ne(appointments.status, "cancelled"),
|
||||
ne(appointments.status, "no_show"),
|
||||
)
|
||||
)
|
||||
.limit(1);
|
||||
if (conflicts.length > 0) {
|
||||
conflictingInstances.push(i);
|
||||
}
|
||||
}
|
||||
|
||||
const [inserted] = await tx
|
||||
.insert(appointments)
|
||||
.values({
|
||||
@@ -227,9 +291,19 @@ appointmentsRouter.post(
|
||||
seriesIndex: i,
|
||||
})
|
||||
.returning();
|
||||
if (!inserted) throw new Error(`Insert failed for occurrence ${i}`);
|
||||
if (i === 0) first = inserted;
|
||||
}
|
||||
|
||||
if (conflictingInstances.length > 0) {
|
||||
throw Object.assign(
|
||||
new Error(
|
||||
`Conflicts detected at occurrence(s): ${conflictingInstances.join(", ")}`
|
||||
),
|
||||
{ statusCode: 409 }
|
||||
);
|
||||
}
|
||||
|
||||
if (!first) throw new Error("No appointments created");
|
||||
return first;
|
||||
});
|
||||
@@ -247,9 +321,12 @@ appointmentsRouter.post(
|
||||
}
|
||||
|
||||
// Send confirmation email (fire-and-forget — never fails the request)
|
||||
sendConfirmationEmail(db, firstRow).catch((err) => {
|
||||
console.error("[appointments] Failed to send confirmation email:", err);
|
||||
});
|
||||
withRetry(
|
||||
() => sendConfirmationEmail(db, firstRow),
|
||||
2,
|
||||
1000,
|
||||
`Failed to send confirmation email for appointment ${firstRow.id}`
|
||||
);
|
||||
|
||||
return c.json(firstRow, 201);
|
||||
}
|
||||
@@ -378,6 +455,76 @@ appointmentsRouter.patch(
|
||||
|
||||
let firstUpdated: typeof appointments.$inferSelect | undefined;
|
||||
for (const appt of affected) {
|
||||
const newStart =
|
||||
startDeltaMs !== 0
|
||||
? new Date(appt.startTime.getTime() + startDeltaMs)
|
||||
: appt.startTime;
|
||||
const newEnd =
|
||||
endDeltaMs !== 0
|
||||
? new Date(appt.endTime.getTime() + endDeltaMs)
|
||||
: appt.endTime;
|
||||
const newStaffId =
|
||||
updateFields.staffId !== undefined
|
||||
? updateFields.staffId
|
||||
: appt.staffId;
|
||||
const newBatherStaffId =
|
||||
updateFields.batherStaffId !== undefined
|
||||
? updateFields.batherStaffId
|
||||
: appt.batherStaffId;
|
||||
|
||||
if (
|
||||
newStaffId &&
|
||||
(startDeltaMs !== 0 ||
|
||||
endDeltaMs !== 0 ||
|
||||
updateFields.staffId !== undefined)
|
||||
) {
|
||||
const conflicts = await tx
|
||||
.select({ id: appointments.id })
|
||||
.from(appointments)
|
||||
.where(
|
||||
and(
|
||||
eq(appointments.staffId, newStaffId),
|
||||
lt(appointments.startTime, newEnd),
|
||||
gte(appointments.endTime, newStart),
|
||||
ne(appointments.status, "cancelled"),
|
||||
ne(appointments.status, "no_show"),
|
||||
ne(appointments.id, appt.id),
|
||||
)
|
||||
)
|
||||
.limit(1);
|
||||
if (conflicts.length > 0) {
|
||||
throw Object.assign(new Error("conflict"), { statusCode: 409 });
|
||||
}
|
||||
}
|
||||
|
||||
if (
|
||||
newBatherStaffId &&
|
||||
(startDeltaMs !== 0 ||
|
||||
endDeltaMs !== 0 ||
|
||||
updateFields.batherStaffId !== undefined)
|
||||
) {
|
||||
const conflicts = await tx
|
||||
.select({ id: appointments.id })
|
||||
.from(appointments)
|
||||
.where(
|
||||
and(
|
||||
or(
|
||||
eq(appointments.staffId, newBatherStaffId),
|
||||
eq(appointments.batherStaffId, newBatherStaffId)
|
||||
),
|
||||
lt(appointments.startTime, newEnd),
|
||||
gte(appointments.endTime, newStart),
|
||||
ne(appointments.status, "cancelled"),
|
||||
ne(appointments.status, "no_show"),
|
||||
ne(appointments.id, appt.id),
|
||||
)
|
||||
)
|
||||
.limit(1);
|
||||
if (conflicts.length > 0) {
|
||||
throw Object.assign(new Error("conflict"), { statusCode: 409 });
|
||||
}
|
||||
}
|
||||
|
||||
const apptUpdate: Record<string, unknown> = {
|
||||
updatedAt: new Date(),
|
||||
};
|
||||
@@ -413,6 +560,13 @@ appointmentsRouter.patch(
|
||||
if (statusCode === 404) return c.json({ error: "Not found" }, 404);
|
||||
if (statusCode === 422)
|
||||
return c.json({ error: "endTime must be after startTime" }, 422);
|
||||
if (statusCode === 409)
|
||||
return c.json(
|
||||
{
|
||||
error: "Staff member has a conflicting appointment at this time",
|
||||
},
|
||||
409
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -590,9 +744,12 @@ appointmentsRouter.delete("/:id", async (c) => {
|
||||
|
||||
const apptDate = current.startTime.toISOString().slice(0, 10);
|
||||
const apptTime = current.startTime.toLocaleTimeString("en-US", { hour: "2-digit", minute: "2-digit", hour12: true });
|
||||
notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId).catch((err) => {
|
||||
console.error("[appointments] Failed to notify waitlist:", err);
|
||||
});
|
||||
withRetry(
|
||||
() => notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId),
|
||||
2,
|
||||
1000,
|
||||
`Failed to notify waitlist for appointment ${id}`
|
||||
);
|
||||
|
||||
return c.json({ ok: true });
|
||||
}
|
||||
@@ -615,9 +772,12 @@ appointmentsRouter.delete("/:id", async (c) => {
|
||||
.returning();
|
||||
if (!row) return c.json({ error: "Not found" }, 404);
|
||||
|
||||
notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId).catch((err) => {
|
||||
console.error("[appointments] Failed to notify waitlist:", err);
|
||||
});
|
||||
withRetry(
|
||||
() => notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId),
|
||||
2,
|
||||
1000,
|
||||
`Failed to notify waitlist for appointment ${id}`
|
||||
);
|
||||
|
||||
return c.json({ ok: true });
|
||||
});
|
||||
|
||||
@@ -135,9 +135,24 @@ clientsRouter.delete("/:id", async (c) => {
|
||||
}
|
||||
|
||||
const db = getDb();
|
||||
const clientId = c.req.param("id");
|
||||
|
||||
const [existingAppt] = await db
|
||||
.select({ id: appointments.id })
|
||||
.from(appointments)
|
||||
.where(eq(appointments.clientId, clientId))
|
||||
.limit(1);
|
||||
|
||||
if (existingAppt) {
|
||||
return c.json(
|
||||
{ error: "Cannot delete client with existing appointments. Cancel or reassign appointments first." },
|
||||
409
|
||||
);
|
||||
}
|
||||
|
||||
const [row] = await db
|
||||
.delete(clients)
|
||||
.where(eq(clients.id, c.req.param("id")))
|
||||
.where(eq(clients.id, clientId))
|
||||
.returning();
|
||||
if (!row) return c.json({ error: "Not found" }, 404);
|
||||
return c.json({ ok: true });
|
||||
|
||||
Reference in New Issue
Block a user