fix(waitlist): address CTO review on PR #110

- Move client-facing POST/PATCH/DELETE waitlist routes to portalRouter
  so impersonation sessions can reach them (were blocked by requireRole guard)
- Fix portalRouter double-mount: remove from auth-protected api block,
  register publicly at app.route("/api/portal", ...) instead
- Replace N+1 queries in GET /waitlist with a single JOIN across
  clients, pets, and services tables
- Remove dead expiredIds variable in markExpiredEntries; use .some()
  instead of computing an array only for its length
- Fix stray indentation in appointments.ts DELETE handler (line 487)
- Update waitlist tests to exercise routes at new /portal/waitlist paths;
  add leftJoin and lt to chainable mock

Co-Authored-By: Paperclip <noreply@paperclip.ing>

apps/api/src/__tests__/waitlist.test.ts

@@ -54,7 +54,7 @@ vi.mock("@groombook/db", () => {
     const arr = [...data];
     const chain = new Proxy(arr, {
       get(target, prop) {
-        if (prop === "where" || prop === "orderBy" || prop === "limit") {
+        if (prop === "where" || prop === "orderBy" || prop === "limit" || prop === "leftJoin") {
           return () => chain;
         }
         // @ts-expect-error proxy
@@ -89,6 +89,11 @@ vi.mock("@groombook/db", () => {
     { get: (t, p) => (p === "_name" ? "services" : { table: "services", column: p }) }
   );
 
+  const appointments = new Proxy(
+    { _name: "appointments" },
+    { get: (t, p) => (p === "_name" ? "appointments" : { table: "appointments", column: p }) }
+  );
+
   return {
     getDb: () => ({
       select: () => ({
@@ -137,15 +142,19 @@ vi.mock("@groombook/db", () => {
     clients,
     pets,
     services,
+    appointments,
     eq: vi.fn(),
     and: vi.fn(),
+    lt: vi.fn(),
   };
 });
 
 const { waitlistRouter } = await import("../routes/waitlist.js");
+const { portalRouter } = await import("../routes/portal.js");
 
 const app = new Hono();
 app.route("/waitlist", waitlistRouter);
+app.route("/portal", portalRouter);
 
 function jsonRequest(method: string, path: string, body?: unknown, headers?: Record<string, string>) {
   return app.request(path, {
@@ -160,10 +169,10 @@ function jsonRequest(method: string, path: string, body?: unknown, headers?: Rec
 
 beforeEach(() => resetMock());
 
-describe("POST /waitlist", () => {
+describe("POST /portal/waitlist", () => {
   it("creates entry with valid session", async () => {
     selectSessionRow = ACTIVE_SESSION;
-    const res = await jsonRequest("POST", "/waitlist", {
+    const res = await jsonRequest("POST", "/portal/waitlist", {
       petId: VALID_UUID_3,
       serviceId: VALID_UUID_4,
       preferredDate: "2026-03-25",
@@ -176,7 +185,7 @@ describe("POST /waitlist", () => {
   });
 
   it("returns 401 without session", async () => {
-    const res = await jsonRequest("POST", "/waitlist", {
+    const res = await jsonRequest("POST", "/portal/waitlist", {
       petId: VALID_UUID_3,
       serviceId: VALID_UUID_4,
       preferredDate: "2026-03-25",
@@ -187,7 +196,7 @@ describe("POST /waitlist", () => {
 
   it("returns 401 with expired session", async () => {
     selectSessionRow = EXPIRED_SESSION;
-    const res = await jsonRequest("POST", "/waitlist", {
+    const res = await jsonRequest("POST", "/portal/waitlist", {
       petId: VALID_UUID_3,
       serviceId: VALID_UUID_4,
       preferredDate: "2026-03-25",
@@ -197,11 +206,11 @@ describe("POST /waitlist", () => {
   });
 });
 
-describe("DELETE /waitlist/:id", () => {
+describe("DELETE /portal/waitlist/:id", () => {
   it("deletes entry with valid session and correct owner", async () => {
     selectSessionRow = ACTIVE_SESSION;
     selectRows = [WAITLIST_ENTRY];
-    const res = await app.request(`/waitlist/${VALID_UUID_1}`, {
+    const res = await app.request(`/portal/waitlist/${VALID_UUID_1}`, {
       method: "DELETE",
       headers: { "X-Impersonation-Session-Id": VALID_UUID_5 },
     });
@@ -211,7 +220,7 @@ describe("DELETE /waitlist/:id", () => {
   });
 
   it("returns 401 without session", async () => {
-    const res = await app.request(`/waitlist/${VALID_UUID_1}`, {
+    const res = await app.request(`/portal/waitlist/${VALID_UUID_1}`, {
       method: "DELETE",
     });
     expect(res.status).toBe(401);
@@ -220,7 +229,7 @@ describe("DELETE /waitlist/:id", () => {
   it("returns 403 with valid session but wrong owner", async () => {
     selectSessionRow = { ...ACTIVE_SESSION, clientId: "other-client-uuid" };
     selectRows = [WAITLIST_ENTRY];
-    const res = await app.request(`/waitlist/${VALID_UUID_1}`, {
+    const res = await app.request(`/portal/waitlist/${VALID_UUID_1}`, {
       method: "DELETE",
       headers: { "X-Impersonation-Session-Id": VALID_UUID_5 },
     });
@@ -230,7 +239,7 @@ describe("DELETE /waitlist/:id", () => {
   it("returns 404 when entry not found", async () => {
     selectSessionRow = ACTIVE_SESSION;
     selectRows = [];
-    const res = await app.request("/waitlist/nonexistent", {
+    const res = await app.request("/portal/waitlist/nonexistent", {
       method: "DELETE",
       headers: { "X-Impersonation-Session-Id": VALID_UUID_5 },
     });
@@ -238,11 +247,11 @@ describe("DELETE /waitlist/:id", () => {
   });
 });
 
-describe("PATCH /waitlist/:id", () => {
+describe("PATCH /portal/waitlist/:id", () => {
   it("updates entry with valid session and correct owner", async () => {
     selectSessionRow = ACTIVE_SESSION;
     selectRows = [WAITLIST_ENTRY];
-    const res = await jsonRequest("PATCH", `/waitlist/${VALID_UUID_1}`, {
+    const res = await jsonRequest("PATCH", `/portal/waitlist/${VALID_UUID_1}`, {
       status: "notified",
     }, { "X-Impersonation-Session-Id": VALID_UUID_5 });
     expect(res.status).toBe(200);
@@ -250,7 +259,7 @@ describe("PATCH /waitlist/:id", () => {
   });
 
   it("returns 401 without session", async () => {
-    const res = await jsonRequest("PATCH", `/waitlist/${VALID_UUID_1}`, {
+    const res = await jsonRequest("PATCH", `/portal/waitlist/${VALID_UUID_1}`, {
       status: "notified",
     });
     expect(res.status).toBe(401);
@@ -259,7 +268,7 @@ describe("PATCH /waitlist/:id", () => {
   it("returns 403 with valid session but wrong owner", async () => {
     selectSessionRow = { ...ACTIVE_SESSION, clientId: "other-client-uuid" };
     selectRows = [WAITLIST_ENTRY];
-    const res = await jsonRequest("PATCH", `/waitlist/${VALID_UUID_1}`, {
+    const res = await jsonRequest("PATCH", `/portal/waitlist/${VALID_UUID_1}`, {
       status: "notified",
     }, { "X-Impersonation-Session-Id": VALID_UUID_5 });
     expect(res.status).toBe(403);
@@ -268,9 +277,9 @@ describe("PATCH /waitlist/:id", () => {
   it("returns 404 when entry not found", async () => {
     selectSessionRow = ACTIVE_SESSION;
     selectRows = [];
-    const res = await jsonRequest("PATCH", "/waitlist/nonexistent", {
+    const res = await jsonRequest("PATCH", "/portal/waitlist/nonexistent", {
       status: "notified",
     }, { "X-Impersonation-Session-Id": VALID_UUID_5 });
     expect(res.status).toBe(404);
   });
-});
+});

apps/api/src/index.ts

@@ -42,6 +42,9 @@ app.get("/health", (c) => c.json({ status: "ok" }));
 // Public booking routes — no auth required, must be registered before auth middleware
 app.route("/api/book", bookRouter);
 
+// Public portal routes — client-facing, authenticated via impersonation session header
+app.route("/api/portal", portalRouter);
+
 // Dev/demo routes — config is always public, users endpoint is guarded internally
 app.route("/api/dev", devRouter);
 

apps/api/src/routes/appointments.ts

@@ -484,7 +484,7 @@ appointmentsRouter.delete("/:id", async (c) => {
   const id = c.req.param("id");
   const cascade = c.req.query("cascade") ?? "this_only";
 
-    if (cascade === "this_and_future" || cascade === "all") {
+  if (cascade === "this_and_future" || cascade === "all") {
     const [current] = await db
       .select()
       .from(appointments)

apps/api/src/routes/portal.ts

@@ -1,7 +1,7 @@
 import { Hono } from "hono";
 import { zValidator } from "@hono/zod-validator";
 import { z } from "zod";
-import { and, eq, getDb, appointments, impersonationSessions } from "@groombook/db";
+import { and, eq, getDb, appointments, impersonationSessions, waitlistEntries } from "@groombook/db";
 import type { AppEnv } from "../middleware/rbac.js";
 
 export const portalRouter = new Hono<AppEnv>();
@@ -75,3 +75,159 @@ if (!updated) {
     });
   }
 );
+
+// ─── Client-facing waitlist routes ───────────────────────────────────────────
+
+const createWaitlistEntrySchema = z.object({
+  petId: z.string().uuid(),
+  serviceId: z.string().uuid(),
+  preferredDate: z.string(),
+  preferredTime: z.string(),
+});
+
+const updateWaitlistEntrySchema = z.object({
+  status: z.enum(["active", "notified", "expired", "cancelled"]).optional(),
+  preferredDate: z.string().optional(),
+  preferredTime: z.string().optional(),
+});
+
+portalRouter.post(
+  "/waitlist",
+  zValidator("json", createWaitlistEntrySchema),
+  async (c) => {
+    const db = getDb();
+    const body = c.req.valid("json");
+    const sessionId = c.req.header("X-Impersonation-Session-Id");
+
+    let clientId: string | null = null;
+    if (sessionId) {
+      const [session] = await db
+        .select()
+        .from(impersonationSessions)
+        .where(
+          and(
+            eq(impersonationSessions.id, sessionId),
+            eq(impersonationSessions.status, "active")
+          )
+        )
+        .limit(1);
+      if (session && session.expiresAt > new Date()) {
+        clientId = session.clientId;
+      }
+    }
+
+    if (!clientId) {
+      return c.json({ error: "Unauthorized" }, 401);
+    }
+
+    const [entry] = await db
+      .insert(waitlistEntries)
+      .values({
+        clientId,
+        petId: body.petId,
+        serviceId: body.serviceId,
+        preferredDate: body.preferredDate,
+        preferredTime: body.preferredTime,
+      })
+      .returning();
+
+    return c.json(entry, 201);
+  }
+);
+
+portalRouter.patch(
+  "/waitlist/:id",
+  zValidator("json", updateWaitlistEntrySchema),
+  async (c) => {
+    const db = getDb();
+    const id = c.req.param("id");
+    const body = c.req.valid("json");
+    const sessionId = c.req.header("X-Impersonation-Session-Id");
+
+    if (!sessionId) {
+      return c.json({ error: "Unauthorized" }, 401);
+    }
+
+    const [session] = await db
+      .select()
+      .from(impersonationSessions)
+      .where(
+        and(
+          eq(impersonationSessions.id, sessionId),
+          eq(impersonationSessions.status, "active")
+        )
+      )
+      .limit(1);
+
+    if (!session || session.expiresAt <= new Date()) {
+      return c.json({ error: "Unauthorized" }, 401);
+    }
+
+    const [existing] = await db
+      .select()
+      .from(waitlistEntries)
+      .where(eq(waitlistEntries.id, id))
+      .limit(1);
+
+    if (!existing) return c.json({ error: "Not found" }, 404);
+    if (existing.clientId !== session.clientId) {
+      return c.json({ error: "Forbidden" }, 403);
+    }
+
+    const updateData: Record<string, unknown> = { updatedAt: new Date() };
+    if (body.status !== undefined) updateData.status = body.status;
+    if (body.preferredDate !== undefined) updateData.preferredDate = body.preferredDate;
+    if (body.preferredTime !== undefined) updateData.preferredTime = body.preferredTime;
+
+    const [updated] = await db
+      .update(waitlistEntries)
+      .set(updateData)
+      .where(eq(waitlistEntries.id, id))
+      .returning();
+
+    return c.json(updated);
+  }
+);
+
+portalRouter.delete("/waitlist/:id", async (c) => {
+  const db = getDb();
+  const id = c.req.param("id");
+  const sessionId = c.req.header("X-Impersonation-Session-Id");
+
+  if (!sessionId) {
+    return c.json({ error: "Unauthorized" }, 401);
+  }
+
+  const [session] = await db
+    .select()
+    .from(impersonationSessions)
+    .where(
+      and(
+        eq(impersonationSessions.id, sessionId),
+        eq(impersonationSessions.status, "active")
+      )
+    )
+    .limit(1);
+
+  if (!session || session.expiresAt <= new Date()) {
+    return c.json({ error: "Unauthorized" }, 401);
+  }
+
+  const [entry] = await db
+    .select()
+    .from(waitlistEntries)
+    .where(eq(waitlistEntries.id, id))
+    .limit(1);
+
+  if (!entry) return c.json({ error: "Not found" }, 404);
+  if (entry.clientId !== session.clientId) {
+    return c.json({ error: "Forbidden" }, 403);
+  }
+
+  await db
+    .delete(waitlistEntries)
+    .where(eq(waitlistEntries.id, id))
+    .returning();
+
+  return c.json({ ok: true });
+});

apps/api/src/routes/waitlist.ts

@@ -1,6 +1,4 @@
 import { Hono } from "hono";
-import { zValidator } from "@hono/zod-validator";
-import { z } from "zod";
 import {
   and,
   eq,
@@ -10,18 +8,15 @@ import {
   clients,
   pets,
   services,
-  impersonationSessions,
 } from "@groombook/db";
 import type { AppEnv } from "../middleware/rbac.js";
 
 export const waitlistRouter = new Hono<AppEnv>();
 
-async function markExpiredEntries(db: ReturnType<typeof getDb>, rows: typeof waitlistEntries.$inferSelect[]) {
+async function markExpiredEntries(db: ReturnType<typeof getDb>, rows: { status: string; preferredDate: string }[]) {
   const today = new Date().toISOString().slice(0, 10);
-  const expiredIds = rows
+  const hasExpired = rows.some((r) => r.status === "active" && r.preferredDate < today);
-    .filter((r) => r.status === "active" && r.preferredDate < today)
+  if (hasExpired) {
-    .map((r) => r.id);
-  if (expiredIds.length > 0) {
     await db
       .update(waitlistEntries)
       .set({ status: "expired", updatedAt: new Date() })
@@ -29,21 +24,6 @@ async function markExpiredEntries(db: ReturnType<typeof getDb>, rows: typeof wai
   }
 }
 
-const waitlistStatusEnum = z.enum(["active", "notified", "expired", "cancelled"]);
-
-const createWaitlistEntrySchema = z.object({
-  petId: z.string().uuid(),
-  serviceId: z.string().uuid(),
-  preferredDate: z.string(),
-  preferredTime: z.string(),
-});
-
-const updateWaitlistEntrySchema = z.object({
-  status: waitlistStatusEnum.optional(),
-  preferredDate: z.string().optional(),
-  preferredTime: z.string().optional(),
-});
-
 waitlistRouter.get("/", async (c) => {
   const db = getDb();
   const date = c.req.query("date");
@@ -53,50 +33,38 @@ waitlistRouter.get("/", async (c) => {
     conditions.push(eq(waitlistEntries.preferredDate, date));
   }
 
-  const rows =
+  const rows = await db
-    conditions.length > 0
+    .select({
-      ? await db
+      id: waitlistEntries.id,
-          .select()
+      clientId: waitlistEntries.clientId,
-          .from(waitlistEntries)
+      petId: waitlistEntries.petId,
-          .where(and(...conditions))
+      serviceId: waitlistEntries.serviceId,
-          .orderBy(waitlistEntries.createdAt)
+      preferredDate: waitlistEntries.preferredDate,
-      : await db
+      preferredTime: waitlistEntries.preferredTime,
-          .select()
+      status: waitlistEntries.status,
-          .from(waitlistEntries)
+      notifiedAt: waitlistEntries.notifiedAt,
-          .orderBy(waitlistEntries.createdAt);
+      expiresAt: waitlistEntries.expiresAt,
+      createdAt: waitlistEntries.createdAt,
+      updatedAt: waitlistEntries.updatedAt,
+      clientName: clients.name,
+      clientEmail: clients.email,
+      petName: pets.name,
+      serviceName: services.name,
+    })
+    .from(waitlistEntries)
+    .leftJoin(clients, eq(waitlistEntries.clientId, clients.id))
+    .leftJoin(pets, eq(waitlistEntries.petId, pets.id))
+    .leftJoin(services, eq(waitlistEntries.serviceId, services.id))
+    .where(conditions.length > 0 ? and(...conditions) : undefined)
+    .orderBy(waitlistEntries.createdAt);
 
   await markExpiredEntries(db, rows);
 
   const today = new Date().toISOString().slice(0, 10);
-
+  const enriched = rows.map((row) => ({
-  const enriched = await Promise.all(
+    ...row,
-    rows.map(async (entry) => {
+    status: row.status === "active" && row.preferredDate < today ? "expired" : row.status,
-      const [client] = await db
+  }));
-        .select({ name: clients.name, email: clients.email })
-        .from(clients)
-        .where(eq(clients.id, entry.clientId))
-        .limit(1);
-      const [pet] = await db
-        .select({ name: pets.name })
-        .from(pets)
-        .where(eq(pets.id, entry.petId))
-        .limit(1);
-      const [service] = await db
-        .select({ name: services.name })
-        .from(services)
-        .where(eq(services.id, entry.serviceId))
-        .limit(1);
-      const isExpired = entry.status === "active" && entry.preferredDate < today;
-      return {
-        ...entry,
-        status: isExpired ? "expired" : entry.status,
-        clientName: client?.name ?? null,
-        clientEmail: client?.email ?? null,
-        petName: pet?.name ?? null,
-        serviceName: service?.name ?? null,
-      };
-    })
-  );
 
   return c.json(enriched);
 });
@@ -118,144 +86,3 @@ waitlistRouter.get("/:id", async (c) => {
     status: isExpired ? "expired" : row.status,
   });
 });
-
-waitlistRouter.post(
-  "/",
-  zValidator("json", createWaitlistEntrySchema),
-  async (c) => {
-    const db = getDb();
-    const body = c.req.valid("json");
-    const sessionId = c.req.header("X-Impersonation-Session-Id");
-
-    let clientId: string | null = null;
-    if (sessionId) {
-      const [session] = await db
-        .select()
-        .from(impersonationSessions)
-        .where(
-          and(
-            eq(impersonationSessions.id, sessionId),
-            eq(impersonationSessions.status, "active")
-          )
-        )
-        .limit(1);
-      if (session && session.expiresAt > new Date()) {
-        clientId = session.clientId;
-      }
-    }
-
-    if (!clientId) {
-      return c.json({ error: "Unauthorized" }, 401);
-    }
-
-    const [entry] = await db
-      .insert(waitlistEntries)
-      .values({
-        clientId,
-        petId: body.petId,
-        serviceId: body.serviceId,
-        preferredDate: body.preferredDate,
-        preferredTime: body.preferredTime,
-      })
-      .returning();
-
-    return c.json(entry, 201);
-  }
-);
-
-waitlistRouter.patch(
-  "/:id",
-  zValidator("json", updateWaitlistEntrySchema),
-  async (c) => {
-    const db = getDb();
-    const id = c.req.param("id");
-    const body = c.req.valid("json");
-    const sessionId = c.req.header("X-Impersonation-Session-Id");
-
-    if (!sessionId) {
-      return c.json({ error: "Unauthorized" }, 401);
-    }
-
-    const [session] = await db
-      .select()
-      .from(impersonationSessions)
-      .where(
-        and(
-          eq(impersonationSessions.id, sessionId),
-          eq(impersonationSessions.status, "active")
-        )
-      )
-      .limit(1);
-
-    if (!session || session.expiresAt <= new Date()) {
-      return c.json({ error: "Unauthorized" }, 401);
-    }
-
-    const [existing] = await db
-      .select()
-      .from(waitlistEntries)
-      .where(eq(waitlistEntries.id, id))
-      .limit(1);
-
-    if (!existing) return c.json({ error: "Not found" }, 404);
-    if (existing.clientId !== session.clientId) {
-      return c.json({ error: "Forbidden" }, 403);
-    }
-
-    const updateData: Record<string, unknown> = { updatedAt: new Date() };
-    if (body.status !== undefined) updateData.status = body.status;
-    if (body.preferredDate !== undefined) updateData.preferredDate = body.preferredDate;
-    if (body.preferredTime !== undefined) updateData.preferredTime = body.preferredTime;
-
-    const [updated] = await db
-      .update(waitlistEntries)
-      .set(updateData)
-      .where(eq(waitlistEntries.id, id))
-      .returning();
-
-    return c.json(updated);
-  }
-);
-
-waitlistRouter.delete("/:id", async (c) => {
-  const db = getDb();
-  const id = c.req.param("id");
-  const sessionId = c.req.header("X-Impersonation-Session-Id");
-
-  if (!sessionId) {
-    return c.json({ error: "Unauthorized" }, 401);
-  }
-
-  const [session] = await db
-    .select()
-    .from(impersonationSessions)
-    .where(
-      and(
-        eq(impersonationSessions.id, sessionId),
-        eq(impersonationSessions.status, "active")
-      )
-    )
-    .limit(1);
-
-  if (!session || session.expiresAt <= new Date()) {
-    return c.json({ error: "Unauthorized" }, 401);
-  }
-
-  const [entry] = await db
-    .select()
-    .from(waitlistEntries)
-    .where(eq(waitlistEntries.id, id))
-    .limit(1);
-
-  if (!entry) return c.json({ error: "Not found" }, 404);
-  if (entry.clientId !== session.clientId) {
-    return c.json({ error: "Forbidden" }, 403);
-  }
-
-  await db
-    .delete(waitlistEntries)
-    .where(eq(waitlistEntries.id, id))
-    .returning();
-
-  return c.json({ ok: true });
-});