fix(GRO-638): appointment scheduling correctness and client deletion integrity
fix(GRO-638): appointment scheduling correctness and client deletion integrity
This commit was merged in pull request #279.
This commit is contained in:
@@ -27,12 +27,14 @@ const DISABLED_CLIENT = {
|
|||||||
// ─── Queue-based mock DB ──────────────────────────────────────────────────────
|
// ─── Queue-based mock DB ──────────────────────────────────────────────────────
|
||||||
|
|
||||||
let selectRows: Record<string, unknown>[] = [];
|
let selectRows: Record<string, unknown>[] = [];
|
||||||
|
let appointmentRows: Record<string, unknown>[] = [];
|
||||||
let insertedValues: Record<string, unknown>[] = [];
|
let insertedValues: Record<string, unknown>[] = [];
|
||||||
let updatedValues: Record<string, unknown>[] = [];
|
let updatedValues: Record<string, unknown>[] = [];
|
||||||
let deletedId: string | null = null;
|
let deletedId: string | null = null;
|
||||||
|
|
||||||
function resetMock() {
|
function resetMock() {
|
||||||
selectRows = [];
|
selectRows = [];
|
||||||
|
appointmentRows = [];
|
||||||
insertedValues = [];
|
insertedValues = [];
|
||||||
updatedValues = [];
|
updatedValues = [];
|
||||||
deletedId = null;
|
deletedId = null;
|
||||||
@@ -58,10 +60,19 @@ vi.mock("@groombook/db", () => {
|
|||||||
{ get: (t, p) => (p === "_name" ? "clients" : { table: "clients", column: p }) }
|
{ 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 {
|
return {
|
||||||
getDb: () => ({
|
getDb: () => ({
|
||||||
select: () => ({
|
select: () => ({
|
||||||
from: () => makeChainable(selectRows),
|
from: (table: unknown) => {
|
||||||
|
const tableName = (table as { _name?: string })._name;
|
||||||
|
const rows = tableName === "appointments" ? appointmentRows : selectRows;
|
||||||
|
return makeChainable(rows);
|
||||||
|
},
|
||||||
}),
|
}),
|
||||||
insert: () => ({
|
insert: () => ({
|
||||||
values: (vals: Record<string, unknown>) => {
|
values: (vals: Record<string, unknown>) => {
|
||||||
@@ -95,8 +106,10 @@ vi.mock("@groombook/db", () => {
|
|||||||
}),
|
}),
|
||||||
}),
|
}),
|
||||||
clients,
|
clients,
|
||||||
|
appointments,
|
||||||
eq: vi.fn(),
|
eq: vi.fn(),
|
||||||
and: 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 { notifyWaitlistForAppointment } from "../services/waitlistNotify.js";
|
||||||
import type { AppEnv } from "../middleware/rbac.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>();
|
export const appointmentsRouter = new Hono<AppEnv>();
|
||||||
|
|
||||||
const createAppointmentSchema = z.object({
|
const createAppointmentSchema = z.object({
|
||||||
@@ -212,11 +233,54 @@ appointmentsRouter.post(
|
|||||||
recurrence.frequencyWeeks * 7 * 24 * 60 * 60 * 1000;
|
recurrence.frequencyWeeks * 7 * 24 * 60 * 60 * 1000;
|
||||||
|
|
||||||
let first: typeof appointments.$inferSelect | undefined;
|
let first: typeof appointments.$inferSelect | undefined;
|
||||||
|
const conflictingInstances: number[] = [];
|
||||||
for (let i = 0; i < recurrence.count; i++) {
|
for (let i = 0; i < recurrence.count; i++) {
|
||||||
const instanceStart = new Date(start.getTime() + i * intervalMs);
|
const instanceStart = new Date(start.getTime() + i * intervalMs);
|
||||||
const instanceEnd = new Date(
|
const instanceEnd = new Date(
|
||||||
instanceStart.getTime() + durationMs
|
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
|
const [inserted] = await tx
|
||||||
.insert(appointments)
|
.insert(appointments)
|
||||||
.values({
|
.values({
|
||||||
@@ -227,9 +291,19 @@ appointmentsRouter.post(
|
|||||||
seriesIndex: i,
|
seriesIndex: i,
|
||||||
})
|
})
|
||||||
.returning();
|
.returning();
|
||||||
|
if (!inserted) throw new Error(`Insert failed for occurrence ${i}`);
|
||||||
if (i === 0) first = inserted;
|
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");
|
if (!first) throw new Error("No appointments created");
|
||||||
return first;
|
return first;
|
||||||
});
|
});
|
||||||
@@ -247,9 +321,12 @@ appointmentsRouter.post(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Send confirmation email (fire-and-forget — never fails the request)
|
// Send confirmation email (fire-and-forget — never fails the request)
|
||||||
sendConfirmationEmail(db, firstRow).catch((err) => {
|
withRetry(
|
||||||
console.error("[appointments] Failed to send confirmation email:", err);
|
() => sendConfirmationEmail(db, firstRow),
|
||||||
});
|
2,
|
||||||
|
1000,
|
||||||
|
`Failed to send confirmation email for appointment ${firstRow.id}`
|
||||||
|
);
|
||||||
|
|
||||||
return c.json(firstRow, 201);
|
return c.json(firstRow, 201);
|
||||||
}
|
}
|
||||||
@@ -378,6 +455,76 @@ appointmentsRouter.patch(
|
|||||||
|
|
||||||
let firstUpdated: typeof appointments.$inferSelect | undefined;
|
let firstUpdated: typeof appointments.$inferSelect | undefined;
|
||||||
for (const appt of affected) {
|
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> = {
|
const apptUpdate: Record<string, unknown> = {
|
||||||
updatedAt: new Date(),
|
updatedAt: new Date(),
|
||||||
};
|
};
|
||||||
@@ -413,6 +560,13 @@ appointmentsRouter.patch(
|
|||||||
if (statusCode === 404) return c.json({ error: "Not found" }, 404);
|
if (statusCode === 404) return c.json({ error: "Not found" }, 404);
|
||||||
if (statusCode === 422)
|
if (statusCode === 422)
|
||||||
return c.json({ error: "endTime must be after startTime" }, 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;
|
throw err;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -590,9 +744,12 @@ appointmentsRouter.delete("/:id", async (c) => {
|
|||||||
|
|
||||||
const apptDate = current.startTime.toISOString().slice(0, 10);
|
const apptDate = current.startTime.toISOString().slice(0, 10);
|
||||||
const apptTime = current.startTime.toLocaleTimeString("en-US", { hour: "2-digit", minute: "2-digit", hour12: true });
|
const apptTime = current.startTime.toLocaleTimeString("en-US", { hour: "2-digit", minute: "2-digit", hour12: true });
|
||||||
notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId).catch((err) => {
|
withRetry(
|
||||||
console.error("[appointments] Failed to notify waitlist:", err);
|
() => notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId),
|
||||||
});
|
2,
|
||||||
|
1000,
|
||||||
|
`Failed to notify waitlist for appointment ${id}`
|
||||||
|
);
|
||||||
|
|
||||||
return c.json({ ok: true });
|
return c.json({ ok: true });
|
||||||
}
|
}
|
||||||
@@ -615,9 +772,12 @@ appointmentsRouter.delete("/:id", async (c) => {
|
|||||||
.returning();
|
.returning();
|
||||||
if (!row) return c.json({ error: "Not found" }, 404);
|
if (!row) return c.json({ error: "Not found" }, 404);
|
||||||
|
|
||||||
notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId).catch((err) => {
|
withRetry(
|
||||||
console.error("[appointments] Failed to notify waitlist:", err);
|
() => notifyWaitlistForAppointment(id, apptDate, apptTime, current.serviceId),
|
||||||
});
|
2,
|
||||||
|
1000,
|
||||||
|
`Failed to notify waitlist for appointment ${id}`
|
||||||
|
);
|
||||||
|
|
||||||
return c.json({ ok: true });
|
return c.json({ ok: true });
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -135,9 +135,24 @@ clientsRouter.delete("/:id", async (c) => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const db = getDb();
|
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
|
const [row] = await db
|
||||||
.delete(clients)
|
.delete(clients)
|
||||||
.where(eq(clients.id, c.req.param("id")))
|
.where(eq(clients.id, clientId))
|
||||||
.returning();
|
.returning();
|
||||||
if (!row) return c.json({ error: "Not found" }, 404);
|
if (!row) return c.json({ error: "Not found" }, 404);
|
||||||
return c.json({ ok: true });
|
return c.json({ ok: true });
|
||||||
|
|||||||
Reference in New Issue
Block a user