From 185fce8e17f39ce91574d98709558007bc4fa252 Mon Sep 17 00:00:00 2001 From: Chris Farhood <3+cpfarhood@noreply.git.farh.net> Date: Sun, 24 May 2026 18:14:57 +0000 Subject: [PATCH 01/12] Add .mcp.json --- .mcp.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .mcp.json diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 0000000..6efc1ca --- /dev/null +++ b/.mcp.json @@ -0,0 +1,11 @@ +{ + "mcpServers": { + "gitea": { + "type": "http", + "url": "https://git-mcp.farh.net/mcp", + "headers": { + "Authorization": "Bearer ${GITEA_TOKEN}" + } + } + } +} -- 2.52.0 From dd83f2973640b9c68d343f5a2e66b363809dfec5 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Mon, 25 May 2026 23:22:04 +0000 Subject: [PATCH 02/12] chore: trigger CI from uat for GRO-1754 --- trigger-uat-1779751324.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 trigger-uat-1779751324.txt diff --git a/trigger-uat-1779751324.txt b/trigger-uat-1779751324.txt new file mode 100644 index 0000000..e69de29 -- 2.52.0 From 152abfc4d55d167782efa89d11cad3b9264b56fe Mon Sep 17 00:00:00 2001 From: The Dogfather <20+gb_dogfather@noreply.git.farh.net> Date: Tue, 26 May 2026 01:26:05 +0000 Subject: [PATCH 03/12] fix(ci): remove duplicate provenance keys causing YAML parse error Duplicate 'provenance: false' in each docker/build-push-action step caused Gitea to reject the workflow file, breaking push CI and workflow_dispatch. Co-Authored-By: Paperclip --- .gitea/workflows/ci.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index b08c640..b37a76a 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -96,7 +96,6 @@ jobs: file: Dockerfile target: runner push: true - provenance: false tags: | git.farh.net/groombook/api:${{ steps.version.outputs.tag }} ${{ github.ref == 'refs/heads/main' && 'git.farh.net/groombook/api:latest' || '' }} @@ -111,7 +110,6 @@ jobs: file: Dockerfile target: migrate push: true - provenance: false tags: | git.farh.net/groombook/migrate:${{ steps.version.outputs.tag }} ${{ github.ref == 'refs/heads/main' && 'git.farh.net/groombook/migrate:latest' || '' }} @@ -126,7 +124,6 @@ jobs: file: Dockerfile target: seed push: true - provenance: false tags: | git.farh.net/groombook/seed:${{ steps.version.outputs.tag }} ${{ github.ref == 'refs/heads/main' && 'git.farh.net/groombook/seed:latest' || '' }} @@ -141,7 +138,6 @@ jobs: file: Dockerfile target: reset push: true - provenance: false tags: | git.farh.net/groombook/reset:${{ steps.version.outputs.tag }} ${{ github.ref == 'refs/heads/main' && 'git.farh.net/groombook/reset:latest' || '' }} -- 2.52.0 From 23484dc90a10da308757f6f167adc6354c72a280 Mon Sep 17 00:00:00 2001 From: The Dogfather <20+gb_dogfather@noreply.git.farh.net> Date: Mon, 1 Jun 2026 18:27:42 +0000 Subject: [PATCH 04/12] =?UTF-8?q?promote(uat):=20GRO-2014=20profile-summar?= =?UTF-8?q?y=20error-handling=20fix=20(dev=E2=86=92uat)=20(#138)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- UAT_PLAYBOOK.md | 3 + src/__tests__/petProfileSummary.test.ts | 285 ++++++++++++++++++++++++ src/routes/pets.ts | 35 ++- 3 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/petProfileSummary.test.ts diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index f0e1037..5860fbc 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -125,6 +125,9 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \ | TC-API-3.17 | Get pet profile summary — groomer restricted | GET /api/pets/{id}/profile-summary as groomer with no pet linkage | 403 Forbidden | | TC-API-3.18 | Get pet profile summary — visitCount returns full count | GET /api/pets/{id}/profile-summary with 2+ completed appointments | visitCount >= 2 (not capped at 1) | | TC-API-3.19 | Get pet profile summary — upcomingAppointment excludes past | GET /api/pets/{id}/profile-summary with a past confirmed/scheduled appointment | upcomingAppointment is null (past appointments filtered by startTime >= now) | +| TC-API-3.29 | Get pet profile summary — unknown UUID returns 404 (GRO-2014) | GET /api/pets/00000000-0000-0000-0000-000000000001/profile-summary while authenticated (any role) | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014) | +| TC-API-3.30 | Get pet profile summary — malformed UUID returns 404 (GRO-2014) | GET /api/pets/not-a-uuid/profile-summary while authenticated | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014 — Postgres uuid cast failure) | +| TC-API-3.31 | Get pet profile summary — never empty-body 500 (GRO-2014) | GET /api/pets/{anyId}/profile-summary across the test sweep | No response has status 500 with an empty body. Any 500 must include a JSON body `{"error":"Internal Server Error"}` | #### Seed Data Verification (GRO-1898) diff --git a/src/__tests__/petProfileSummary.test.ts b/src/__tests__/petProfileSummary.test.ts new file mode 100644 index 0000000..8a17d43 --- /dev/null +++ b/src/__tests__/petProfileSummary.test.ts @@ -0,0 +1,285 @@ +/** + * GET /pets/:id/profile-summary tests + * + * GRO-2014 regression coverage: + * - Empty-body 500 must never escape the route — the onError handler + * converts unhandled errors into a structured JSON 500. + * - Malformed UUIDs must return 404 (not 500 via a Postgres uuid cast). + * - Missing staff context must return 401 (not TypeError on staffRow.id). + * - Pet not found must return 404. + * - Groomer with no appointment linkage must return 403. + * - Manager and groomer with linkage must receive the summary body. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; +import type { AppEnv, StaffRow } from "../middleware/rbac.js"; + +// ─── Fixtures ──────────────────────────────────────────────────────────────── + +const MANAGER: StaffRow = { + id: "00000000-0000-0000-0000-0000000000aa", + oidcSub: "oidc-manager-sub", + userId: null, + role: "manager", + isSuperUser: true, + name: "Manager McManager", + email: "manager@example.com", + active: true, + icalToken: null, + createdAt: new Date(), + updatedAt: new Date(), +}; + +const GROOMER: StaffRow = { + ...MANAGER, + id: "00000000-0000-0000-0000-0000000000bb", + oidcSub: "oidc-groomer-sub", + role: "groomer", + isSuperUser: false, + name: "Groomer Gary", + email: "groomer@example.com", +}; + +const PET_UUID = "11111111-1111-1111-1111-111111111111"; +const CLIENT_UUID = "22222222-2222-2222-2222-222222222222"; +const UNKNOWN_PET_UUID = "00000000-0000-0000-0000-000000000001"; + +const PET_ROW = { + id: PET_UUID, + clientId: CLIENT_UUID, + name: "Biscuit", + species: "dog", + breed: "Beagle", + coatType: "short", + petSizeCategory: "medium", + weightKg: "12.50", + dateOfBirth: new Date("2020-01-01"), +}; + +// ─── Mutable DB state ───────────────────────────────────────────────────────── + +interface DbState { + petRow: typeof PET_ROW | null; + linkageRow: { id: string } | null; + recentHistory: Array>; + visitCount: number; + upcoming: Record | null; + throwOnPetSelect: boolean; +} + +let dbState: DbState; + +function resetDb() { + dbState = { + petRow: { ...PET_ROW }, + linkageRow: { id: "appt-link" }, + recentHistory: [], + visitCount: 0, + upcoming: null, + throwOnPetSelect: false, + }; +} + +// ─── @groombook/db mock ────────────────────────────────────────────────────── +// +// Each select chain needs to know which table it's targeting and which columns +// it's projecting so we can return the right mocked rows. We thread that state +// through a per-call object whose chain methods all return `this`. The chain +// is also `then`-able so any `await` position resolves to the rows. + +vi.mock("@groombook/db", () => { + const namedTable = (name: string) => + new Proxy( + { _name: name }, + { + get(_t, p) { + if (p === "_name") return name; + return { table: name, column: p }; + }, + } + ); + + const pets = namedTable("pets"); + const appointments = namedTable("appointments"); + const services = namedTable("services"); + const staff = namedTable("staff"); + + // The full chain interface is intentionally loose — only `then` is exposed + // with a typed signature so vitest's await resolves to the right shape. + interface ChainLike { + from: (table: { _name: string }) => ChainLike; + where: (...args: unknown[]) => ChainLike; + innerJoin: (...args: unknown[]) => ChainLike; + leftJoin: (...args: unknown[]) => ChainLike; + orderBy: (...args: unknown[]) => ChainLike; + limit: (...args: unknown[]) => ChainLike; + then: ( + onfulfilled?: ((value: unknown[]) => T | PromiseLike) | null + ) => Promise; + } + + function buildSelect(projection?: Record): ChainLike { + let targetTable = ""; + + const resolveRows = (): unknown[] => { + if (targetTable === "pets") { + if (dbState.throwOnPetSelect) { + throw new Error("simulated postgres uuid cast failure"); + } + return dbState.petRow ? [dbState.petRow] : []; + } + if (targetTable === "appointments") { + const keys = projection ? Object.keys(projection) : []; + if (projection && keys.length === 1 && keys[0] === "id") { + return dbState.linkageRow ? [dbState.linkageRow] : []; + } + if (projection && keys.includes("count")) { + return [{ count: dbState.visitCount }]; + } + if (projection && keys.includes("confirmationStatus")) { + return dbState.upcoming ? [dbState.upcoming] : []; + } + return dbState.recentHistory; + } + return []; + }; + + const chain: ChainLike = { + from(table) { + targetTable = table._name; + return chain; + }, + where() { + return chain; + }, + innerJoin() { + return chain; + }, + leftJoin() { + return chain; + }, + orderBy() { + return chain; + }, + limit() { + return chain; + }, + then(onfulfilled) { + return Promise.resolve(resolveRows()).then(onfulfilled ?? undefined); + }, + }; + + return chain; + } + + return { + getDb: () => ({ + select: (projection?: Record) => buildSelect(projection), + }), + pets, + appointments, + services, + staff, + and: vi.fn(() => ({ _op: "and" })), + or: vi.fn(() => ({ _op: "or" })), + eq: vi.fn(() => ({ _op: "eq" })), + desc: vi.fn((arg: unknown) => arg), + exists: vi.fn((arg: unknown) => arg), + sql: Object.assign( + () => ({ _op: "sql" }), + { [Symbol.toPrimitive]: () => "sql" } + ), + }; +}); + +vi.mock("../lib/s3.js", () => ({ + getPresignedUploadUrl: vi.fn().mockResolvedValue("https://example.com/put"), + getPresignedGetUrl: vi.fn().mockResolvedValue("https://example.com/get"), + deleteObject: vi.fn().mockResolvedValue(undefined), +})); + +const { petsRouter } = await import("../routes/pets.js"); + +// ─── App builder ───────────────────────────────────────────────────────────── + +function buildApp(staffRow: StaffRow | null) { + const app = new Hono(); + app.use("*", async (c, next) => { + if (staffRow) c.set("staff", staffRow); + await next(); + }); + app.route("/pets", petsRouter); + return app; +} + +beforeEach(() => { + resetDb(); + vi.clearAllMocks(); +}); + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +describe("GET /pets/:id/profile-summary — GRO-2014 error handling", () => { + it("returns 404 (not 500) for a malformed UUID path param", async () => { + const app = buildApp(MANAGER); + const res = await app.request("/pets/not-a-uuid/profile-summary"); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Not found"); + }); + + it("returns 401 when staff context is missing (defense in depth)", async () => { + const app = buildApp(null); + const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`); + expect(res.status).toBe(401); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Unauthorized"); + }); + + it("returns 404 when authenticated and pet does not exist", async () => { + dbState.petRow = null; + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Not found"); + }); + + it("returns 403 when groomer has no appointment linkage to the pet's client", async () => { + dbState.linkageRow = null; + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(403); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Forbidden"); + }); + + it("returns 200 with summary for a manager (no groomer linkage check)", async () => { + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body.id).toBe(PET_UUID); + expect(body.name).toBe("Biscuit"); + expect(body.visitCount).toBe(0); + expect(body.upcomingAppointment).toBeNull(); + expect(body.recentGroomingHistory).toEqual([]); + }); + + it("returns 200 with summary for a groomer with linkage", async () => { + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body.id).toBe(PET_UUID); + }); + + it("returns a JSON envelope (not empty body) when a downstream query throws", async () => { + dbState.throwOnPetSelect = true; + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(500); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Internal Server Error"); + }); +}); diff --git a/src/routes/pets.ts b/src/routes/pets.ts index ffe494c..9695af7 100644 --- a/src/routes/pets.ts +++ b/src/routes/pets.ts @@ -23,6 +23,23 @@ import { export const petsRouter = new Hono(); +// Convert Zod validation errors from 422 to 400 and ensure any thrown error +// returns a structured JSON body rather than Hono's default empty-body 500. +// GRO-2014: profile-summary previously bubbled unhandled errors and produced +// an empty-body 500. Mirror the onError pattern already used in invoices.ts +// and reports.ts so every error has a JSON envelope. +petsRouter.onError((err, c) => { + if (err instanceof z.ZodError) { + return c.json({ error: "Validation failed", issues: err.issues }, 400); + } + console.error("[pets] unhandled error", err); + return c.json({ error: "Internal Server Error" }, 500); +}); + +// UUID format used by all pet routes — guards path params against malformed +// values before they hit Drizzle / Postgres uuid columns (which would throw). +const uuidSchema = z.string().uuid(); + const createPetSchema = z.object({ clientId: z.string().uuid(), name: z.string().min(1).max(200), @@ -112,8 +129,24 @@ petsRouter.get("/:id", async (c) => { petsRouter.get("/:id/profile-summary", async (c) => { const db = getDb(); const petId = c.req.param("id"); + + // GRO-2014: validate UUID format before hitting Postgres. Passing a non-UUID + // string to a uuid column makes the driver throw, which previously surfaced + // as an empty-body 500 to clients. + const parsedId = uuidSchema.safeParse(petId); + if (!parsedId.success) { + return c.json({ error: "Not found" }, 404); + } + + // Defense in depth: resolveStaffMiddleware should always populate `staff` + // for protected routes (or short-circuit with 401/403 of its own). Guard + // anyway so a misconfigured route mount can't trigger a TypeError on + // staffRow.id when the linkage check runs. const staffRow = c.get("staff"); - const isGroomer = staffRow?.role === "groomer"; + if (!staffRow) { + return c.json({ error: "Unauthorized" }, 401); + } + const isGroomer = staffRow.role === "groomer"; // Fetch the pet const [pet] = await db.select().from(pets).where(eq(pets.id, petId)); -- 2.52.0 From bd384bdf5c55730d9bec4036f7e7de91e8dcbfae Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 2 Jun 2026 18:24:40 +0000 Subject: [PATCH 05/12] docs(UAT_PLAYBOOK): add TC-UAT-2/3 for uat-groomer linked/unlinked pet profile-summary (GRO-2100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint Roller review on PR #152 flagged that the GRO-2100 seed change produces new observable UAT API behavior that the playbook must reflect. Add two deterministic rows pinning the contract GRO-1987 TC-UAT-2/3 will exercise: - TC-UAT-2: uat-groomer + linked pet c0000001-...-002 (UAT Pup Alpha) → 200 - TC-UAT-3: uat-groomer + unlinked pet c0000001-...-003 (UAT Pup Beta) → 403 The 403-vs-404 note in TC-UAT-3 mirrors the verification note in the GRO-2100 issue body so the QA runner knows where to file if the API returns 404 (a separate RBAC defect, not against the seed). --- UAT_PLAYBOOK.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index b8949de..ed52ecd 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -147,6 +147,8 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the | TC-API-3.19b | Get pet profile summary — customer cross-tenant blocked (GRO-2013) | Sign in as `uat-customer@groombook.dev`; reuse the customer's sessionId from TC-API-3.19a; `GET /api/pets/{otherClientPetId}/profile-summary` for a pet owned by a different client (`c0000002-...` or any non-customer pet) | 403 Forbidden (owner-bypass requires session.clientId === pet.clientId) | | TC-API-3.19c | Get pet profile summary — customer without portal session header | Same as TC-API-3.19a but omit the `X-Impersonation-Session-Id` header | 403 Forbidden (no owner-bypass without valid portal session) | | TC-API-3.19d | Get pet profile summary — owner-bypass writes audit row (GRO-2063) | Same setup as TC-API-3.19a (sign in as `uat-customer@groombook.dev`, establish a portal session for the customer's own clientId, call `GET /api/pets/{ownPetId}/profile-summary` with `X-Impersonation-Session-Id: {sessionId}` and a 200 OK response). Then call `GET /api/impersonation/sessions/{sessionId}/audit-log` and confirm there is exactly one entry with `action === "read_profile_summary"`, `pageVisited` matching the profile-summary path, and `metadata` containing `petId` and `actorStaffId` for the customer. Repeat TC-API-3.19b (cross-tenant attempt) and confirm NO new `read_profile_summary` row was written for the cross-tenant attempt. | 200 OK on the profile-summary call AND an audit log entry is present with the correct shape (defense-in-depth audit row; bypass attempts against other clients must NOT log) | +| TC-UAT-2 | Groomer accesses linked pet profile summary (GRO-2100) | Sign in as `uat-groomer@groombook.dev`; `GET /api/pets/c0000001-0000-0000-0000-000000000002/profile-summary` (UAT Pup Alpha — linked via deterministic completed appointment `a0000001-0000-0000-0000-000000000001`, service `b0000001-…-0001` "Bath & Brush", `startTime` ~7 days ago) | 200 OK, `recentGroomingHistory[]` non-empty (>=1 entry), `visitCount >= 1`, `upcomingAppointment` null (the seeded appointment is in the past) | +| TC-UAT-3 | Groomer blocked from unlinked pet profile summary (GRO-2100) | Sign in as `uat-groomer@groombook.dev`; `GET /api/pets/c0000001-0000-0000-0000-000000000003/profile-summary` (UAT Pup Beta — intentionally UNLINKED; no appointment row references this pet's clientId+groomerId combo) | 403 Forbidden (RBAC `groomer` role lacks the appointment-linkage grant for this pet). NOTE: if 404 is returned instead of 403, file a separate RBAC defect (not against the seed) — see GRO-2100 verification note | | TC-API-3.29 | Get pet profile summary — unknown UUID returns 404 (GRO-2014) | GET /api/pets/00000000-0000-0000-0000-000000000001/profile-summary while authenticated (any role) | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014) | | TC-API-3.30 | Get pet profile summary — malformed UUID returns 404 (GRO-2014) | GET /api/pets/not-a-uuid/profile-summary while authenticated | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014 — Postgres uuid cast failure) | | TC-API-3.31 | Get pet profile summary — never empty-body 500 (GRO-2014) | GET /api/pets/{anyId}/profile-summary across the test sweep | No response has status 500 with an empty body. Any 500 must include a JSON body `{"error":"Internal Server Error"}` | -- 2.52.0 From d4a4ddce37ba17fccdf4d3e91308f00848d3e936 Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 2 Jun 2026 18:28:17 +0000 Subject: [PATCH 06/12] =?UTF-8?q?ci:=20retrigger=20GRO-2100=20PR=20#152=20?= =?UTF-8?q?Build=20&=20Push=20Docker=20Images=20(Reset=20image=20build=20f?= =?UTF-8?q?ailed=20=E2=80=94=20docker=20registry=20flake)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -- 2.52.0 From e639cc82d17c3baa7ef51cd8493474b860b77c81 Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Tue, 2 Jun 2026 20:23:54 +0000 Subject: [PATCH 07/12] chore(uat): GRO-2100 promote uat-groomer seed-linkage ordering fix to uat (#154) Co-authored-by: Flea Flicker Co-committed-by: Flea Flicker --- packages/db/src/seed.ts | 48 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/packages/db/src/seed.ts b/packages/db/src/seed.ts index 13cf103..8e9d376 100644 --- a/packages/db/src/seed.ts +++ b/packages/db/src/seed.ts @@ -401,7 +401,9 @@ const servicesDef = [ * * In seedKnownUsers() this replaces the inline UAT-staff block. */ -async function seedUatStaffAccounts(db: ReturnType) { +async function seedUatStaffAccounts( + db: ReturnType, +): Promise { // ── Staff: UAT Super User (oidcSub from SEED_UAT_SUPER_OIDC_SUB env var) ── const uatSuperOidcSub = process.env.SEED_UAT_SUPER_OIDC_SUB; if (uatSuperOidcSub) { @@ -677,7 +679,12 @@ async function seedUatStaffAccounts(db: ReturnType) { // We deterministically link the UAT groomer to the UAT customer's first pet // ("UAT Pup Alpha") and leave the second pet ("UAT Pup Beta") UNLINKED so // TC-UAT-2 (200) and TC-UAT-3 (403) can both hardcode the stable petIds. - await seedUatGroomerLinkage(db, uatCustomerClientId); + // + // The linkage call itself is performed by the caller AFTER the `services` + // catalogue has been seeded (this helper runs before services exist, + // which previously caused the linkage to be silently skipped on every + // reset). GRO-2100 follow-up. + return uatCustomerClientId; } /** @@ -692,12 +699,18 @@ async function seedUatStaffAccounts(db: ReturnType) { */ async function seedUatGroomerLinkage( db: ReturnType, - customerClientId: string, + customerClientId: string | null, ): Promise { const uatGroomerEmail = "uat-groomer@groombook.dev"; const LINKED_PET_ID = "c0000001-0000-0000-0000-000000000002"; // UAT Pup Alpha const APPT_ID = "a0000001-0000-0000-0000-000000000001"; + // Skip silently if the UAT Customer client wasn't created (non-UAT seed + // profile, e.g. seedKnownUsers() in an env without the UAT personas). + if (!customerClientId) { + return; + } + // Only run if the UAT groomer staff record actually exists — dev/test seeds // that don't set SEED_UAT_STAFF_OIDC_SUB should not crash. const [uatGroomerStaff] = await db @@ -720,6 +733,19 @@ async function seedUatGroomerLinkage( return; } + // Skip if the linked pet hasn't been seeded yet (defensive: caller should + // ensure pets exist; if the helper is re-ordered later we don't want to + // crash here). + const [linkedPet] = await db + .select({ id: schema.pets.id }) + .from(schema.pets) + .where(eq(schema.pets.id, LINKED_PET_ID)) + .limit(1); + if (!linkedPet) { + console.warn(`⚠ GRO-2100: UAT Pup Alpha (${LINKED_PET_ID}) not found — skipping uat-groomer linkage`); + return; + } + // The "Bath & Brush" service id is stable across the reset; falls back to // any active service if it has not been seeded yet (e.g. seedKnownUsers // runs in isolation). @@ -847,7 +873,7 @@ async function seedKnownUsers() { // ── UAT staff accounts + Better Auth credentials (shared impl) ────────────── // Extracted into seedUatStaffAccounts() so it runs in both seedKnownUsers() // and the full seed() UAT branch. - await seedUatStaffAccounts(db); + const uatCustomerClientId = await seedUatStaffAccounts(db); // ── Services: idempotent upsert keyed on `id` ───────────────────────────── // GRO-2064: previously keyed on `services.name` while writing a @@ -875,6 +901,12 @@ async function seedKnownUsers() { } console.log(`✓ Seeded ${demoSvcs.length} services`); + // GRO-2100: deterministic uat-groomer ↔ UAT Pup Alpha linkage. Must run + // AFTER services are seeded (this helper looks up an active service id + // to attach to the appointment; on a fresh reset there are none yet at + // the time seedUatStaffAccounts() returns). + await seedUatGroomerLinkage(db, uatCustomerClientId); + // ── Client: Demo Client ── const [existingClient] = await db .select() @@ -1031,7 +1063,7 @@ async function seed() { // ── UAT staff accounts + Better Auth credentials (shared impl) ────────────── // Seeds deterministic UAT staff with numeric OIDC subs and Better Auth credentials. // Must run AFTER random staff are created so upserts land correctly. - await seedUatStaffAccounts(db); + const uatCustomerClientId = await seedUatStaffAccounts(db); // ── Services ── // GRO-2064: key the upsert on `services.id` (not `name`) so deterministic @@ -1058,6 +1090,12 @@ async function seed() { } console.log(`✓ Created ${servicesDef.length} services`); + // GRO-2100: deterministic uat-groomer ↔ UAT Pup Alpha linkage. Must run + // AFTER services are seeded (this helper looks up an active service id + // to attach to the appointment; on a fresh reset there are none yet at + // the time seedUatStaffAccounts() returns). + await seedUatGroomerLinkage(db, uatCustomerClientId); + // ── Clients & Pets ── const now = new Date(); const appointmentsBackDate = new Date(now); -- 2.52.0 From 8721f0b63cacb467be1a4dc7c96672faf814aabd Mon Sep 17 00:00:00 2001 From: Flea Flicker <22+gb_flea@noreply.git.farh.net> Date: Mon, 8 Jun 2026 12:06:43 +0000 Subject: [PATCH 08/12] =?UTF-8?q?dev=20=E2=86=92=20uat:=20GRO-2154=20geoco?= =?UTF-8?q?ding=20endpoints=20(Phase=201.3)=20(#171)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitea/workflows/ci.yml | 6 +- UAT_PLAYBOOK.md | 18 ++ src/__tests__/clientGeocoding.test.ts | 192 +++++++++++++++++++++ src/__tests__/petProfileSummary.test.ts | 16 -- src/index.ts | 9 + src/routes/clients.ts | 124 +++++++++++++- src/services/clientGeocoding.ts | 212 ++++++++++++++++++++++++ 7 files changed, 555 insertions(+), 22 deletions(-) create mode 100644 src/__tests__/clientGeocoding.test.ts create mode 100644 src/services/clientGeocoding.ts diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index d848d3b..1529ed5 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -33,11 +33,11 @@ jobs: - name: Typecheck run: | - pnpm --filter @groombook/api typecheck + pnpm run typecheck pnpm --filter @groombook/db typecheck - name: Lint - run: pnpm --filter @groombook/api lint + run: pnpm run lint test: name: Test @@ -58,7 +58,7 @@ jobs: run: pnpm install --frozen-lockfile - name: Run tests - run: pnpm --filter @groombook/api test + run: pnpm run test docker: name: Build & Push Docker Images diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index 6653db5..47a84bd 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -120,6 +120,24 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the | TC-API-2.5 | Disable client | PATCH /api/clients/{id} with status: "disabled" | 200 OK, client marked as disabled | | TC-API-2.6 | Delete client | DELETE /api/clients/{id}?confirm=true | 200 OK, client deleted (if no appointments) | +#### Client Geocoding — Route Optimization (GRO-2154, Phase 1.3) + +Geocoding turns a client's street address into `latitude`/`longitude` + `geocodedAt`. Provider is driven by `businessSettings.routeOptimizationProvider` (default Nominatim/OpenStreetMap, 1 req/sec; optional Google fallback). All explicit geocode endpoints are **manager-only**. + +| # | Scenario | Steps | Expected | +|---|----------|-------|----------| +| TC-API-2.7 | Geocode single client (success) | As **manager**, `POST /api/clients/{id}/geocode` for a client with a valid, real address (e.g. a seed client) | 200 OK; body `{ status: "geocoded", latitude, longitude, geocodedAt, formattedAddress, provider }`. Subsequent `GET /api/clients/{id}` shows the same non-null `latitude`/`longitude`/`geocodedAt` persisted | +| TC-API-2.8 | Geocode single client — no address | As manager, `POST /api/clients/{id}/geocode` for a client whose `address` is null/blank | 422; `{ status: "no_address", message: "...no address on file..." }` (clear, actionable) | +| TC-API-2.9 | Geocode single client — unresolvable/ambiguous address | As manager, set a nonsense address (e.g. `"asdkjhqweoui 99999"`) then `POST /api/clients/{id}/geocode` | 422; `{ status: "unresolved", message: "Address could not be resolved..." }` so groomers/managers know to correct it | +| TC-API-2.10 | Geocode single client — not found | As manager, `POST /api/clients/00000000-0000-0000-0000-000000000000/geocode` | 404 `{ error: "Not found" }` | +| TC-API-2.11 | Geocode endpoint is manager-only | As **groomer** or **receptionist**, `POST /api/clients/{id}/geocode` | 403 Forbidden (role not permitted) | +| TC-API-2.12 | Batch geocode un-geocoded clients | As manager, `POST /api/clients/geocode-batch?limit=10` on a DB with un-geocoded clients | 200 OK; body `{ provider, processed, geocoded, unresolved, errors, remaining, outcomes[] }`. `processed` ≤ 10; `remaining` reflects un-geocoded clients beyond this batch. Re-run while `remaining > 0` to finish (throttled to provider rate limit) | +| TC-API-2.13 | Batch geocode — invalid limit | As manager, `POST /api/clients/geocode-batch?limit=0` (or non-numeric) | 400 `{ error: "limit must be a positive integer" }` | +| TC-API-2.14 | Batch geocode — manager-only | As groomer/receptionist, `POST /api/clients/geocode-batch` | 403 Forbidden | +| TC-API-2.15 | Auto-geocode on create | As manager/receptionist, `POST /api/clients` with a valid `address` | 201 Created; response includes a `geocoding` object (`status: "geocoded"` for a resolvable address) and the persisted client carries `latitude`/`longitude`/`geocodedAt`. Creating without an address succeeds with no `geocoding` field | +| TC-API-2.16 | Auto-geocode on address update | As manager/receptionist, `PATCH /api/clients/{id}` changing `address` to a new valid value | 200 OK; response includes a `geocoding` object and refreshed coordinates. Patching unrelated fields (e.g. `name`) does NOT re-geocode (no `geocoding` field) | +| TC-API-2.17 | Clearing address drops coordinates | As manager/receptionist, `PATCH /api/clients/{id}` with `address: ""` | 200 OK; `latitude`/`longitude`/`geocodedAt` reset to null (no stale pin) | + ### 4.3 Pet Management | # | Scenario | Steps | Expected | diff --git a/src/__tests__/clientGeocoding.test.ts b/src/__tests__/clientGeocoding.test.ts new file mode 100644 index 0000000..675bc1e --- /dev/null +++ b/src/__tests__/clientGeocoding.test.ts @@ -0,0 +1,192 @@ +import { describe, it, expect, vi } from "vitest"; +import { + geocodeClient, + geocodeUngeocodedClients, + resolveClientGeocodingProvider, +} from "../services/clientGeocoding.js"; +import { + NominatimGeocodingProvider, + type GeocodeResult, + type GeocodingProvider, +} from "../services/geocoding.js"; + +// ─── Fakes ────────────────────────────────────────────────────────────────── + +/** Fake provider with a scripted geocode behaviour and a call log. */ +function fakeProvider( + impl: (address: string) => Promise +): GeocodingProvider & { calls: string[] } { + const calls: string[] = []; + return { + name: "nominatim", + minRequestIntervalMs: 0, + calls, + geocode: (address: string) => { + calls.push(address); + return impl(address); + }, + }; +} + +const okResult = (lat: number, lng: number): GeocodeResult => ({ + latitude: lat, + longitude: lng, + formattedAddress: "1 Main St, Anytown", + provider: "nominatim", +}); + +/** + * Minimal db double recording update() set-values. `select()` chains return the + * preloaded `selectQueue` shift()ed per call so different statements get + * different rows (used by geocodeUngeocodedClients: count, then rows). + */ +function fakeDb(selectQueue: unknown[][]) { + const updates: Record[] = []; + const queue = [...selectQueue]; + const chain = () => { + const rows = queue.shift() ?? []; + const proxy: Record = {}; + for (const k of ["from", "where", "orderBy", "limit"]) { + proxy[k] = () => proxy; + } + // Make the chain awaitable / iterable as the resolved rows. + (proxy as { then: unknown }).then = (resolve: (v: unknown) => void) => + resolve(rows); + (proxy as { [Symbol.iterator]: unknown })[Symbol.iterator] = () => + (rows as unknown[])[Symbol.iterator](); + return proxy; + }; + const db = { + select: () => chain(), + update: () => ({ + set: (vals: Record) => ({ + where: () => { + updates.push(vals); + return { returning: async () => [] }; + }, + }), + }), + updates, + }; + return db as unknown as Parameters[0] & { + updates: Record[]; + }; +} + +const clientRow = (over: Record = {}) => + ({ + id: "client-1", + name: "Alice", + email: "a@example.com", + address: "1 Main St", + latitude: null, + longitude: null, + geocodedAt: null, + ...over, + }) as unknown as Parameters[1]; + +// ─── geocodeClient ──────────────────────────────────────────────────────────── + +describe("geocodeClient", () => { + it("persists coordinates and returns a geocoded outcome", async () => { + const db = fakeDb([]); + const provider = fakeProvider(async () => okResult(40.1, -74.2)); + const outcome = await geocodeClient(db, clientRow(), provider); + + expect(outcome.status).toBe("geocoded"); + expect(outcome.latitude).toBe(40.1); + expect(outcome.longitude).toBe(-74.2); + expect(outcome.geocodedAt).toBeTruthy(); + expect(db.updates).toHaveLength(1); + expect(db.updates[0]!.latitude).toBe(40.1); + expect(db.updates[0]!.longitude).toBe(-74.2); + expect(db.updates[0]!.geocodedAt).toBeInstanceOf(Date); + }); + + it("returns no_address and does not persist when address is blank", async () => { + const db = fakeDb([]); + const provider = fakeProvider(async () => okResult(0, 0)); + const outcome = await geocodeClient(db, clientRow({ address: " " }), provider); + + expect(outcome.status).toBe("no_address"); + expect(provider.calls).toHaveLength(0); + expect(db.updates).toHaveLength(0); + }); + + it("returns unresolved when the provider finds no match", async () => { + const db = fakeDb([]); + const provider = fakeProvider(async () => null); + const outcome = await geocodeClient(db, clientRow(), provider); + + expect(outcome.status).toBe("unresolved"); + expect(outcome.message).toMatch(/could not be resolved/i); + expect(db.updates).toHaveLength(0); + }); + + it("returns error (without throwing) when the provider fails", async () => { + const db = fakeDb([]); + const provider = fakeProvider(async () => { + throw new Error("quota exceeded"); + }); + const outcome = await geocodeClient(db, clientRow(), provider); + + expect(outcome.status).toBe("error"); + expect(outcome.message).toMatch(/quota exceeded/); + expect(db.updates).toHaveLength(0); + }); +}); + +// ─── geocodeUngeocodedClients ───────────────────────────────────────────────── + +describe("geocodeUngeocodedClients", () => { + it("geocodes candidates, tallies outcomes, and reports remaining", async () => { + // First select() = count query, second select() = candidate rows. + const db = fakeDb([ + [{ count: 5 }], + [ + clientRow({ id: "c1", address: "1 Main St" }), + clientRow({ id: "c2", address: "2 Oak Ave" }), + clientRow({ id: "c3", address: "" }), // no_address + ], + ]); + const provider = fakeProvider(async (addr) => + addr === "2 Oak Ave" ? null : okResult(1, 2) + ); + + const summary = await geocodeUngeocodedClients(db, 50, provider); + + expect(summary.processed).toBe(3); + expect(summary.geocoded).toBe(1); + expect(summary.unresolved).toBe(1); // "2 Oak Ave" + expect(summary.remaining).toBe(2); // 5 total - 3 processed + expect(summary.provider).toBe("nominatim"); + expect(db.updates).toHaveLength(1); // only the successful one persisted + }); + + it("clamps the limit to the 1..500 range", async () => { + const db = fakeDb([[{ count: 0 }], []]); + const provider = fakeProvider(async () => okResult(1, 2)); + const summary = await geocodeUngeocodedClients(db, 0, provider); + expect(summary.processed).toBe(0); + expect(summary.remaining).toBe(0); + }); +}); + +// ─── resolveClientGeocodingProvider ─────────────────────────────────────────── + +describe("resolveClientGeocodingProvider", () => { + it("defaults to Nominatim when no settings row exists", async () => { + const db = fakeDb([[]]); // businessSettings select -> empty + const provider = await resolveClientGeocodingProvider(db); + expect(provider).toBeInstanceOf(NominatimGeocodingProvider); + expect(provider.name).toBe("nominatim"); + }); + + it("defaults to Nominatim when provider is unset on settings", async () => { + const db = fakeDb([[{ routeOptimizationProvider: null, googleMapsApiKey: null }]]); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const provider = await resolveClientGeocodingProvider(db); + expect(provider.name).toBe("nominatim"); + warn.mockRestore(); + }); +}); diff --git a/src/__tests__/petProfileSummary.test.ts b/src/__tests__/petProfileSummary.test.ts index d18d2da..0ea0f35 100644 --- a/src/__tests__/petProfileSummary.test.ts +++ b/src/__tests__/petProfileSummary.test.ts @@ -131,20 +131,6 @@ function makeAppointment(overrides: Record = {}) { }; } -function makeService(overrides: Record = {}) { - return { - id: "service-1", - name: "Full Groom", - description: null, - basePriceCents: 6000, - durationMinutes: 120, - active: true, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - }; -} - function makeSession(overrides: Record = {}) { return { id: "sess-owner", @@ -164,7 +150,6 @@ function makeSession(overrides: Record = {}) { let petsTable: Record[]; let appointmentsTable: Record[]; -let servicesTable: Record[]; let sessionsTable: Record[]; // selectQueue: queries resolve in FIFO order. Each .from(table) result @@ -198,7 +183,6 @@ function enqueueThrow(table: string, message: string) { function resetMock() { petsTable = [makePet()]; appointmentsTable = [makeAppointment()]; - servicesTable = [makeService()]; sessionsTable = [makeSession()]; selectQueue = []; insertCapture = []; diff --git a/src/index.ts b/src/index.ts index 3dd8921..2845b14 100644 --- a/src/index.ts +++ b/src/index.ts @@ -235,6 +235,15 @@ api.on( requireRole("manager", "receptionist", "groomer") ); +// Route-optimization geocoding endpoints are manager-only (GRO-2154), stricter +// than the general client write guard below. Registered FIRST so receptionists +// are rejected here before the manager+receptionist guard can admit them. +api.on( + ["POST"], + ["/clients/geocode-batch", "/clients/:clientId/geocode"], + requireRole("manager") +); + // Clients, appointments: all roles may read; only manager + receptionist may write api.on( ["POST", "PUT", "PATCH", "DELETE"], diff --git a/src/routes/clients.ts b/src/routes/clients.ts index 38104ec..e7ac65c 100644 --- a/src/routes/clients.ts +++ b/src/routes/clients.ts @@ -3,9 +3,61 @@ import { zValidator } from "@hono/zod-validator"; import { z } from "zod/v3"; import { and, eq, exists, getDb, or, clients, appointments } from "@groombook/db"; import type { AppEnv } from "../middleware/rbac.js"; +import { + geocodeClient, + geocodeUngeocodedClients, + resolveClientGeocodingProvider, + type ClientGeocodeOutcome, +} from "../services/clientGeocoding.js"; export const clientsRouter = new Hono(); +type ClientRow = typeof clients.$inferSelect; + +/** + * Best-effort auto-geocode of a freshly created/updated client (GRO-2154). + * Never throws: a flaky geocoding backend must not break client mutations. + * Returns the (possibly coordinate-enriched) row plus a structured outcome the + * caller surfaces under a `geocoding` field so ambiguous addresses are visible. + */ +async function autoGeocodeClient( + db: ReturnType, + row: ClientRow +): Promise<{ row: ClientRow; outcome: ClientGeocodeOutcome }> { + try { + const provider = await resolveClientGeocodingProvider(db); + const outcome = await geocodeClient(db, row, provider); + const enriched = + outcome.status === "geocoded" + ? { + ...row, + latitude: outcome.latitude, + longitude: outcome.longitude, + geocodedAt: outcome.geocodedAt + ? new Date(outcome.geocodedAt) + : row.geocodedAt, + } + : row; + return { row: enriched, outcome }; + } catch (err) { + return { + row, + outcome: { + clientId: row.id, + status: "error", + message: `Auto-geocode failed: ${ + err instanceof Error ? err.message : String(err) + }`, + latitude: null, + longitude: null, + geocodedAt: null, + formattedAddress: null, + provider: null, + }, + }; + } +} + const createClientSchema = z.object({ name: z.string().min(1).max(200), email: z.string().email(), @@ -91,9 +143,59 @@ clientsRouter.post("/", zValidator("json", createClientSchema), async (c) => { const db = getDb(); const body = c.req.valid("json"); const [row] = await db.insert(clients).values(body).returning(); + if (!row) return c.json({ error: "Failed to create client" }, 500); + + // Auto-geocode on create when an address is supplied (GRO-2154). Best-effort: + // the client is created regardless; the `geocoding` field surfaces failures. + if (body.address && body.address.trim()) { + const { row: enriched, outcome } = await autoGeocodeClient(db, row); + return c.json({ ...enriched, geocoding: outcome }, 201); + } return c.json(row, 201); }); +// Geocode a single client's address and persist coordinates (manager-only; +// enforced by the route guard in index.ts). +clientsRouter.post("/:clientId/geocode", async (c) => { + const db = getDb(); + const clientId = c.req.param("clientId"); + const [client] = await db + .select() + .from(clients) + .where(eq(clients.id, clientId)); + if (!client) return c.json({ error: "Not found" }, 404); + + const provider = await resolveClientGeocodingProvider(db); + const outcome = await geocodeClient(db, client, provider); + + // Map outcome to an HTTP status so the result is unambiguous to the caller: + // geocoded -> 200, provider error -> 502, no_address/unresolved -> 422. + const status = + outcome.status === "geocoded" + ? 200 + : outcome.status === "error" + ? 502 + : 422; + return c.json(outcome, status); +}); + +// Batch-geocode un-geocoded clients with provider-rate-limited throttling +// (manager-only). Processes up to ?limit clients (default 50, max 500) per call; +// re-invoke while `remaining` > 0 to finish large datasets. +clientsRouter.post("/geocode-batch", async (c) => { + const db = getDb(); + const limitRaw = c.req.query("limit"); + let limit = 50; + if (limitRaw !== undefined) { + limit = Number(limitRaw); + if (!Number.isFinite(limit) || limit <= 0) { + return c.json({ error: "limit must be a positive integer" }, 400); + } + } + const summary = await geocodeUngeocodedClients(db, limit); + return c.json(summary); +}); + // Update a client (including status changes) const patchClientSchema = createClientSchema.partial().extend({ status: z.enum(["active", "disabled"]).optional(), @@ -123,13 +225,29 @@ clientsRouter.patch( } delete setValues.smsOptOut; - const [row] = await db + // Auto-geocode on address change (GRO-2154). If the address was cleared, + // drop any stale coordinates so a disabled/blank address never keeps a pin. + const addressProvided = Object.prototype.hasOwnProperty.call(body, "address"); + const trimmedAddress = + typeof body.address === "string" ? body.address.trim() : undefined; + if (addressProvided && !trimmedAddress) { + setValues.latitude = null; + setValues.longitude = null; + setValues.geocodedAt = null; + } + + const [updated] = await db .update(clients) .set(setValues) .where(eq(clients.id, c.req.param("id"))) .returning(); - if (!row) return c.json({ error: "Not found" }, 404); - return c.json(row); + if (!updated) return c.json({ error: "Not found" }, 404); + + if (addressProvided && trimmedAddress) { + const { row: enriched, outcome } = await autoGeocodeClient(db, updated); + return c.json({ ...enriched, geocoding: outcome }); + } + return c.json(updated); } ); diff --git a/src/services/clientGeocoding.ts b/src/services/clientGeocoding.ts new file mode 100644 index 0000000..eb16b29 --- /dev/null +++ b/src/services/clientGeocoding.ts @@ -0,0 +1,212 @@ +import { + getDb, + businessSettings, + clients, + and, + eq, + isNull, + sql, +} from "@groombook/db"; +import { + resolveGeocodingProvider, + type GeocodingProvider, + type GeocodeResult, +} from "./geocoding.js"; + +/** + * Client geocoding orchestration (GRO-2154, Phase 1.3 of Route Optimization). + * + * Bridges the provider-agnostic {@link GeocodingProvider} layer (GRO-2153) and + * the `clients` table: resolves the configured provider from `businessSettings`, + * geocodes a client's address, and persists `latitude`/`longitude`/`geocodedAt`. + * + * Outcomes are returned as structured {@link ClientGeocodeOutcome} values so that + * callers (the geocode endpoints and the auto-geocode create/update hook) can + * surface clear, actionable feedback — groomers need to know when an address is + * ambiguous or unresolvable, not just that "something failed". + */ + +type Db = ReturnType; +type ClientRow = typeof clients.$inferSelect; + +/** Status of a single client geocode attempt. */ +export type ClientGeocodeStatus = + /** Coordinates resolved and persisted. */ + | "geocoded" + /** Client has no (non-blank) address on file — nothing to geocode. */ + | "no_address" + /** Provider returned no match; the address is ambiguous or unrecognized. */ + | "unresolved" + /** Provider call failed (transport, quota, bad key). Coordinates unchanged. */ + | "error"; + +/** Structured, UI-surfaceable result of geocoding one client. */ +export interface ClientGeocodeOutcome { + clientId: string; + status: ClientGeocodeStatus; + /** Human-readable explanation, safe to show to managers/groomers. */ + message: string; + latitude: number | null; + longitude: number | null; + geocodedAt: string | null; + /** Provider-normalized address, when a match was found. */ + formattedAddress: string | null; + /** Provider that produced (or attempted) this result. */ + provider: string | null; +} + +/** + * Builds the geocoding provider for the current business settings. A single + * provider instance should be reused across a batch so its internal rate limiter + * throttles the whole run (e.g. Nominatim's 1 req/sec policy). + */ +export async function resolveClientGeocodingProvider( + db: Db +): Promise { + const [settings] = await db.select().from(businessSettings).limit(1); + return resolveGeocodingProvider(settings ?? null); +} + +/** + * Geocodes a single client row through the given provider and persists the + * result on success. Never throws on provider failure — transport/quota errors + * are captured as an `"error"` outcome so callers (especially the create/update + * auto-geocode hook) are not broken by a flaky geocoding backend. + */ +export async function geocodeClient( + db: Db, + client: ClientRow, + provider: GeocodingProvider +): Promise { + const base = { + clientId: client.id, + latitude: null as number | null, + longitude: null as number | null, + geocodedAt: null as string | null, + formattedAddress: null as string | null, + provider: provider.name as string | null, + }; + + const address = client.address?.trim(); + if (!address) { + return { + ...base, + status: "no_address", + message: "Client has no address on file, so it cannot be geocoded.", + }; + } + + let result: GeocodeResult | null; + try { + result = await provider.geocode(address); + } catch (err) { + return { + ...base, + status: "error", + message: `Geocoding provider (${provider.name}) failed: ${ + err instanceof Error ? err.message : String(err) + }`, + }; + } + + if (!result) { + return { + ...base, + status: "unresolved", + message: `Address could not be resolved to a location: "${address}". Please verify or correct the address.`, + }; + } + + const geocodedAt = new Date(); + await db + .update(clients) + .set({ + latitude: result.latitude, + longitude: result.longitude, + geocodedAt, + updatedAt: geocodedAt, + }) + .where(eq(clients.id, client.id)); + + return { + clientId: client.id, + status: "geocoded", + message: `Geocoded via ${result.provider} to ${result.latitude}, ${result.longitude}.`, + latitude: result.latitude, + longitude: result.longitude, + geocodedAt: geocodedAt.toISOString(), + formattedAddress: result.formattedAddress, + provider: result.provider, + }; +} + +/** Summary returned by {@link geocodeUngeocodedClients}. */ +export interface BatchGeocodeSummary { + provider: string; + /** Number of clients processed in this invocation. */ + processed: number; + geocoded: number; + unresolved: number; + errors: number; + /** Un-geocoded clients with an address that were NOT processed (over `limit`). */ + remaining: number; + /** Per-client outcomes for everything processed this invocation. */ + outcomes: ClientGeocodeOutcome[]; +} + +/** + * Batch-geocodes clients that have an address but no `geocodedAt` yet, throttled + * by the active provider's rate limiter. + * + * Because Nominatim allows only ~1 req/sec, geocoding every un-geocoded client in + * a single HTTP request would risk timeouts on large datasets. Each invocation + * therefore processes at most `limit` clients (default 50, clamped 1..500) and + * reports `remaining`; managers re-run until `remaining` is 0. + */ +export async function geocodeUngeocodedClients( + db: Db, + limit = 50, + injectedProvider?: GeocodingProvider +): Promise { + const effectiveLimit = Math.min(Math.max(Math.trunc(limit) || 0, 1), 500); + const provider = + injectedProvider ?? (await resolveClientGeocodingProvider(db)); + + // Un-geocoded = geocodedAt IS NULL with a non-blank address. + const candidateFilter = and( + isNull(clients.geocodedAt), + sql`${clients.address} IS NOT NULL AND length(trim(${clients.address})) > 0` + ); + + const countRows = await db + .select({ count: sql`count(*)::int` }) + .from(clients) + .where(candidateFilter); + const totalRemaining = countRows[0]?.count ?? 0; + + const rows = await db + .select() + .from(clients) + .where(candidateFilter) + .orderBy(clients.createdAt) + .limit(effectiveLimit); + + const outcomes: ClientGeocodeOutcome[] = []; + for (const row of rows) { + outcomes.push(await geocodeClient(db, row, provider)); + } + + const geocoded = outcomes.filter((o) => o.status === "geocoded").length; + const unresolved = outcomes.filter((o) => o.status === "unresolved").length; + const errors = outcomes.filter((o) => o.status === "error").length; + + return { + provider: provider.name, + processed: outcomes.length, + geocoded, + unresolved, + errors, + remaining: Math.max(totalRemaining - outcomes.length, 0), + outcomes, + }; +} -- 2.52.0 From 587fd4ec9516c5fb2aa9f407128d755880c95ed7 Mon Sep 17 00:00:00 2001 From: Flea Flicker <22+gb_flea@noreply.git.farh.net> Date: Mon, 8 Jun 2026 16:45:44 +0000 Subject: [PATCH 09/12] =?UTF-8?q?dev=20=E2=86=92=20uat:=20GRO-2155=20route?= =?UTF-8?q?=20optimization=20endpoints=20(carries=20GRO-2163)=20(#176)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- UAT_PLAYBOOK.md | 18 ++ packages/db/package.json | 7 +- packages/db/scripts/wait-for-db.mjs | 104 ++++++ src/__tests__/routeOptimization.test.ts | 184 +++++++++++ src/index.ts | 6 + src/routes/routes.ts | 284 ++++++++++++++++ src/services/routeOptimization.ts | 413 ++++++++++++++++++++++++ 7 files changed, 1013 insertions(+), 3 deletions(-) create mode 100644 packages/db/scripts/wait-for-db.mjs create mode 100644 src/__tests__/routeOptimization.test.ts create mode 100644 src/routes/routes.ts create mode 100644 src/services/routeOptimization.ts diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index 47a84bd..dace647 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -360,6 +360,24 @@ This means: | TC-API-15.6 | Reject missing required fields | POST /api/admin/buffer-rules with service only | 400 Bad Request, species and sizeCategory required | | TC-API-15.7 | Booking uses buffer | Book appointment for pet with sizeCategory; verify duration reflects buffer | 201 Created, appointment duration includes buffer time | +### 4.16 Route Optimization — Route CRUD + Optimize (GRO-2155, Phase 2.1) + +A groomer's daily route is one row per `(staffId, routeDate)` in `groomer_routes`, with ordered `route_stops`. `POST /api/routes/optimize` pulls the day's non-cancelled appointments whose client is geocoded (GRO-2154), orders them (Google Directions `optimizeWaypoints` when a key is configured in `businessSettings.googleMapsApiKey`, else an offline nearest-neighbor heuristic), and persists `stopOrder`, `travelMinsFromPrev`, `travelDistanceKmFromPrev` plus route `totalTravelMins`/`totalDistanceKm`/`optimizedAt`. **Auth: manager (any groomer's route) or groomer (own route only); receptionists have no access.** Pre-condition: at least one geocoded client with appointments on the target date for the staff member (use §4.2 geocoding + a seed groomer). + +| # | Scenario | Steps | Expected | +|---|----------|-------|----------| +| TC-API-16.1 | Fetch daily route (auto-create draft) | As **manager**, `GET /api/routes/daily?staffId={groomerId}&date=YYYY-MM-DD` for a date with no existing route | 200 OK; body `{ route, stops }`. `route.status` is `"draft"`, `route.staffId`/`routeDate` match, `stops` is `[]`. Re-calling returns the same route row (no duplicate) | +| TC-API-16.2 | Optimize a multi-stop day | As manager, with ≥2 geocoded appointments for the groomer on the date, `POST /api/routes/optimize` body `{ "staffId": "{groomerId}", "date": "YYYY-MM-DD" }` | 200 OK; `route.status: "optimized"`, `optimizedAt` set, `totalTravelMins`/`totalDistanceKm` populated. `stops` ordered by `stopOrder` (1..N); first stop has `travelMinsFromPrev: null`, the rest positive. `provider` is `"nearest_neighbor"` (no Google key in UAT). Each stop carries `bufferMins` (default 15) | +| TC-API-16.3 | Re-optimize replaces prior order | As manager, run TC-API-16.2 twice | Second call returns 200; stops fully replaced (no duplicate `route_stops`, `stopOrder` still contiguous 1..N), `optimizedAt` refreshed | +| TC-API-16.4 | Skips un-geocoded appointments | As manager, optimize a day where one appointment's client has no coordinates | 200 OK; that appointment is absent from `stops` and listed under `skipped[]` with `reason: "client address is not geocoded"`; a corresponding entry appears in `warnings[]` | +| TC-API-16.5 | Empty / single-stop day | As manager, optimize a date with 0 (or 1) geocoded appointments | 200 OK; `route.status: "optimized"`, `totalTravelMins: 0`, `totalDistanceKm: "0.00"`. For 1 stop, `stops` has one entry with `travelMinsFromPrev: null` | +| TC-API-16.6 | >25 stops chunked with warning | As manager, optimize a day with >25 geocoded appointments | 200 OK; `chunked: true`, `subRouteCount ≥ 2`, a `warnings[]` entry mentions sub-routes; all appointments appear exactly once with contiguous `stopOrder` | +| TC-API-16.7 | Groomer reads own route | As **groomer**, `GET /api/routes/daily?date=YYYY-MM-DD` (omit staffId, or pass own id) | 200 OK; route resolves to the groomer's own `staffId` | +| TC-API-16.8 | Groomer cannot access another's route | As groomer, `GET /api/routes/daily?staffId={otherGroomerId}&date=...` or `POST /api/routes/optimize` with another `staffId` | 403 Forbidden (`groomers may only access their own route`) | +| TC-API-16.9 | Receptionist denied | As **receptionist**, `GET /api/routes/daily?...` or `POST /api/routes/optimize` | 403 Forbidden (role not permitted) | +| TC-API-16.10 | Manager must supply staffId | As manager, `POST /api/routes/optimize` body `{ "date": "YYYY-MM-DD" }` (no staffId) | 400 `{ error: "staffId is required" }` | +| TC-API-16.11 | Invalid date rejected | `GET /api/routes/daily?staffId=...&date=06-08-2026` (wrong format) | 400 validation error (`date must be YYYY-MM-DD`) | + ## Pass/Fail Criteria **Pass:** diff --git a/packages/db/package.json b/packages/db/package.json index 4cdd0d9..7f97370 100644 --- a/packages/db/package.json +++ b/packages/db/package.json @@ -18,9 +18,10 @@ "scripts": { "build": "tsc --project .", "generate": "drizzle-kit generate", - "migrate": "drizzle-kit migrate", - "seed": "tsx src/seed.ts", - "reset": "tsx src/reset.ts && drizzle-kit migrate && tsx src/seed.ts", + "wait-for-db": "node ./scripts/wait-for-db.mjs", + "migrate": "node ./scripts/wait-for-db.mjs && drizzle-kit migrate", + "seed": "node ./scripts/wait-for-db.mjs && tsx src/seed.ts", + "reset": "node ./scripts/wait-for-db.mjs && tsx src/reset.ts && drizzle-kit migrate && tsx src/seed.ts", "studio": "drizzle-kit studio", "typecheck": "tsc --noEmit" }, diff --git a/packages/db/scripts/wait-for-db.mjs b/packages/db/scripts/wait-for-db.mjs new file mode 100644 index 0000000..04d9d9e --- /dev/null +++ b/packages/db/scripts/wait-for-db.mjs @@ -0,0 +1,104 @@ +#!/usr/bin/env node +// wait-for-db.mjs +// +// GRO-2163: wait for / retry DNS resolution of the database hostname derived +// from DATABASE_URL before invoking `drizzle-kit migrate`. The first attempt +// of a fresh migrate-schema pod occasionally hits a transient CoreDNS miss +// (EAI_AGAIN) on `groombook-postgres-rw..svc`; with backoffLimit: 2 the +// retry pod usually wins, but three unlucky attempts in a row trips +// BackoffLimitExceeded. Resolving once here, with backoff, removes the dice +// roll at the source so the first attempt reliably succeeds. +// +// Mirrors the belt-and-braces pattern used in GRO-1985 (no Corepack +// download fallback): we don't try to outsmart CoreDNS, we just don't ask +// drizzle-kit to do the very first DNS lookup of a freshly-scheduled pod. +// +// Configuration (env): +// WAIT_FOR_DB_MAX_ATTEMPTS default 12 (~30s of total wait at default backoff) +// WAIT_FOR_DB_BASE_DELAY_MS default 500 +// WAIT_FOR_DB_MAX_DELAY_MS default 5000 +// WAIT_FOR_DB_SKIP default unset; set to "1" to skip (debug only) +// +// On success: exit 0. On exhaustion: exit 1 so the Job's backoff is +// preserved (we don't want to silently mask a real outage by giving up +// after 30s and letting drizzle-kit fail with a less-actionable error). + +import { setTimeout as delay } from "node:timers/promises"; +import dns from "node:dns/promises"; + +const MAX_ATTEMPTS = Number(process.env.WAIT_FOR_DB_MAX_ATTEMPTS ?? 12); +const BASE_DELAY_MS = Number(process.env.WAIT_FOR_DB_BASE_DELAY_MS ?? 500); +const MAX_DELAY_MS = Number(process.env.WAIT_FOR_DB_MAX_DELAY_MS ?? 5000); + +function parseHost(databaseUrl) { + try { + return new URL(databaseUrl).hostname || null; + } catch { + return null; + } +} + +async function resolveOnce(host) { + const start = Date.now(); + const result = await dns.lookup(host); + return { address: result.address, ms: Date.now() - start }; +} + +async function main() { + if (process.env.WAIT_FOR_DB_SKIP === "1") { + console.log("[wait-for-db] WAIT_FOR_DB_SKIP=1, skipping"); + return; + } + const databaseUrl = process.env.DATABASE_URL; + if (!databaseUrl) { + // Don't gate the migrate on a misconfigured env — let drizzle-kit fail + // loudly with its own clear error. + console.warn("[wait-for-db] DATABASE_URL not set; skipping"); + return; + } + const host = parseHost(databaseUrl); + if (!host) { + console.warn(`[wait-for-db] could not parse hostname from DATABASE_URL; skipping`); + return; + } + console.log( + `[wait-for-db] host=${host} max_attempts=${MAX_ATTEMPTS} ` + + `base_delay_ms=${BASE_DELAY_MS} max_delay_ms=${MAX_DELAY_MS}`, + ); + + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + try { + const { address, ms } = await resolveOnce(host); + console.log(`[wait-for-db] ok attempt=${attempt} host=${host} -> ${address} (${ms}ms)`); + return; + } catch (err) { + const code = err?.code ?? "UNKNOWN"; + const transient = code === "EAI_AGAIN" || code === "ENOTFOUND" || code === "EAI_NODATA"; + if (!transient) { + // Hard error (e.g. invalid hostname): surface and let drizzle-kit fail + // with a real error rather than spinning. + console.error(`[wait-for-db] non-transient DNS error attempt=${attempt} code=${code}: ${err.message}`); + process.exit(1); + } + if (attempt === MAX_ATTEMPTS) { + console.error( + `[wait-for-db] exhausted attempts=${MAX_ATTEMPTS} host=${host} last_code=${code}; exiting 1`, + ); + process.exit(1); + } + const backoff = Math.min( + MAX_DELAY_MS, + BASE_DELAY_MS * 2 ** (attempt - 1) + Math.floor(Math.random() * BASE_DELAY_MS), + ); + console.log( + `[wait-for-db] transient attempt=${attempt} code=${code} retry_in_ms=${backoff}`, + ); + await delay(backoff); + } + } +} + +main().catch((err) => { + console.error(`[wait-for-db] fatal: ${err?.message ?? err}`); + process.exit(1); +}); diff --git a/src/__tests__/routeOptimization.test.ts b/src/__tests__/routeOptimization.test.ts new file mode 100644 index 0000000..49877d4 --- /dev/null +++ b/src/__tests__/routeOptimization.test.ts @@ -0,0 +1,184 @@ +import { describe, it, expect } from "vitest"; +import { + haversineKm, + estimateLeg, + nearestNeighborOrder, + optimizeRoute, + MAX_STOPS_PER_ROUTE, + type RouteStopInput, +} from "../services/routeOptimization.js"; +import type { FetchLike } from "../services/geocoding.js"; + +/** Builds a fake fetch returning a single JSON body, recording called URLs. */ +function fakeFetch( + body: unknown, + init: { ok?: boolean; status?: number; statusText?: string } = {} +): { fetchImpl: FetchLike; calls: string[] } { + const calls: string[] = []; + const fetchImpl: FetchLike = async (url) => { + calls.push(url); + return { + ok: init.ok ?? true, + status: init.status ?? 200, + statusText: init.statusText ?? "OK", + json: async () => body, + }; + }; + return { fetchImpl, calls }; +} + +function stop(appointmentId: string, lat: number, lng: number): RouteStopInput { + return { appointmentId, latitude: lat, longitude: lng }; +} + +describe("haversineKm", () => { + it("is zero for the same point", () => { + expect(haversineKm({ latitude: 40, longitude: -74 }, { latitude: 40, longitude: -74 })).toBe(0); + }); + + it("approximates 1 degree of latitude as ~111km", () => { + const d = haversineKm({ latitude: 0, longitude: 0 }, { latitude: 1, longitude: 0 }); + expect(d).toBeGreaterThan(110); + expect(d).toBeLessThan(112); + }); +}); + +describe("estimateLeg", () => { + it("applies the circuity factor and average speed", () => { + const a = { latitude: 0, longitude: 0 }; + const b = { latitude: 0, longitude: 1 }; + const { distanceKm, mins } = estimateLeg(a, b); + // ~111km straight * 1.3 circuity ≈ 144.6km; at 40km/h ≈ 217 min + expect(distanceKm).toBeGreaterThan(140); + expect(distanceKm).toBeLessThan(150); + expect(mins).toBeGreaterThan(200); + expect(mins).toBeLessThan(230); + expect(Number.isInteger(mins)).toBe(true); + }); +}); + +describe("nearestNeighborOrder", () => { + it("returns trivial order for 0 or 1 points", () => { + expect(nearestNeighborOrder([])).toEqual([]); + expect(nearestNeighborOrder([{ latitude: 1, longitude: 1 }])).toEqual([0]); + }); + + it("greedily visits the nearest unvisited point", () => { + // Points on a line; scrambled input order. + const points = [ + { latitude: 0, longitude: 0 }, // 0 (start) + { latitude: 0, longitude: 5 }, // 1 (far) + { latitude: 0, longitude: 1 }, // 2 + { latitude: 0, longitude: 2 }, // 3 + ]; + expect(nearestNeighborOrder(points, 0)).toEqual([0, 2, 3, 1]); + }); +}); + +describe("optimizeRoute — nearest-neighbor fallback (no API key)", () => { + it("returns an empty route for no stops", async () => { + const r = await optimizeRoute([]); + expect(r.stops).toHaveLength(0); + expect(r.totalTravelMins).toBe(0); + expect(r.totalDistanceKm).toBe(0); + expect(r.provider).toBe("nearest_neighbor"); + expect(r.chunked).toBe(false); + }); + + it("handles a single stop with null travel-from-prev", async () => { + const r = await optimizeRoute([stop("a", 40, -74)]); + expect(r.stops).toHaveLength(1); + expect(r.stops[0]!.travelMinsFromPrev).toBeNull(); + expect(r.stops[0]!.travelDistanceKmFromPrev).toBeNull(); + expect(r.totalTravelMins).toBe(0); + }); + + it("orders multiple stops greedily and sums totals", async () => { + const stops = [ + stop("start", 0, 0), + stop("far", 0, 5), + stop("near1", 0, 1), + stop("near2", 0, 2), + ]; + const r = await optimizeRoute(stops); + expect(r.provider).toBe("nearest_neighbor"); + expect(r.stops.map((s) => s.appointmentId)).toEqual([ + "start", + "near1", + "near2", + "far", + ]); + // First stop has no inbound leg. + expect(r.stops[0]!.travelMinsFromPrev).toBeNull(); + // Remaining stops have positive travel. + for (const s of r.stops.slice(1)) { + expect(s.travelMinsFromPrev!).toBeGreaterThan(0); + expect(s.travelDistanceKmFromPrev!).toBeGreaterThan(0); + } + const summed = r.stops.reduce((acc, s) => acc + (s.travelMinsFromPrev ?? 0), 0); + expect(r.totalTravelMins).toBe(summed); + }); +}); + +describe("optimizeRoute — Google Directions path", () => { + it("uses optimized waypoint order and real leg metrics, dropping the return leg", async () => { + const stops = [stop("A", 0, 0), stop("B", 0, 1), stop("C", 0, 2)]; + // waypoints = [B, C]; optimizer reorders them to [C, B] (waypoint_order [1,0]). + // legs: A->C, C->B, B->A(return). The return leg must be dropped. + const { fetchImpl, calls } = fakeFetch({ + status: "OK", + routes: [ + { + waypoint_order: [1, 0], + legs: [ + { distance: { value: 2000 }, duration: { value: 600 } }, // A->C + { distance: { value: 1000 }, duration: { value: 300 } }, // C->B + { distance: { value: 3000 }, duration: { value: 900 } }, // B->A (return, dropped) + ], + }, + ], + }); + + const r = await optimizeRoute(stops, { googleApiKey: "key", fetchImpl }); + expect(r.provider).toBe("google"); + expect(r.stops.map((s) => s.appointmentId)).toEqual(["A", "C", "B"]); + expect(r.stops[0]!.travelMinsFromPrev).toBeNull(); + expect(r.stops[1]!.travelDistanceKmFromPrev).toBe(2); // 2000m -> 2km + expect(r.stops[1]!.travelMinsFromPrev).toBe(10); // 600s -> 10min + expect(r.stops[2]!.travelDistanceKmFromPrev).toBe(1); // 1000m + expect(r.stops[2]!.travelMinsFromPrev).toBe(5); // 300s + expect(r.totalDistanceKm).toBe(3); + expect(r.totalTravelMins).toBe(15); + expect(decodeURIComponent(calls[0]!)).toContain("optimize:true"); + }); + + it("falls back to the heuristic when Google returns a non-OK status", async () => { + const stops = [stop("A", 0, 0), stop("B", 0, 1), stop("C", 0, 2)]; + const { fetchImpl } = fakeFetch({ status: "REQUEST_DENIED", error_message: "bad key" }); + const r = await optimizeRoute(stops, { googleApiKey: "key", fetchImpl }); + // Provider label reflects the chosen strategy (google requested) but a + // warning records the degradation and stops are still ordered. + expect(r.stops).toHaveLength(3); + expect(r.warnings.some((w) => w.includes("offline heuristic"))).toBe(true); + expect(r.stops[0]!.travelMinsFromPrev).toBeNull(); + }); +}); + +describe("optimizeRoute — >25 stop chunking", () => { + it("splits into sub-routes with a warning and continuous stop ordering", async () => { + const stops: RouteStopInput[] = []; + for (let i = 0; i < MAX_STOPS_PER_ROUTE + 5; i++) { + stops.push(stop(`s${i}`, 0, i * 0.1)); + } + const r = await optimizeRoute(stops); + expect(r.chunked).toBe(true); + expect(r.subRouteCount).toBe(2); + expect(r.warnings.some((w) => w.includes("sub-routes"))).toBe(true); + expect(r.stops).toHaveLength(MAX_STOPS_PER_ROUTE + 5); + // Only the very first stop of the whole route lacks an inbound leg. + expect(r.stops[0]!.travelMinsFromPrev).toBeNull(); + expect(r.stops.slice(1).every((s) => s.travelMinsFromPrev !== null)).toBe(true); + // All appointment ids preserved exactly once. + expect(new Set(r.stops.map((s) => s.appointmentId)).size).toBe(stops.length); + }); +}); diff --git a/src/index.ts b/src/index.ts index 2845b14..681d731 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,6 +20,7 @@ import { settingsRouter } from "./routes/settings.js"; import { authProviderRouter } from "./routes/authProvider.js"; import { searchRouter } from "./routes/search.js"; import { bufferRulesRouter } from "./routes/buffer-rules.js"; +import { routesRouter } from "./routes/routes.js"; import { getObject } from "./lib/s3.js"; import { calendarRouter } from "./routes/calendar.js"; import { setupRouter } from "./routes/setup.js"; @@ -220,6 +221,10 @@ api.use("/reports/*", requireRole("manager")); api.use("/invoices/*", requireRole("manager", "groomer")); api.use("/impersonation/*", requireRole("manager")); +// Route optimization: manager (any groomer's route) or groomer (own route only, +// enforced in-handler). Receptionists have no access. (GRO-2155) +api.use("/routes/*", requireRole("manager", "groomer")); + // Manager + Receptionist only (groomers have no access): appointment-groups, grooming-logs, waitlist api.use("/appointment-groups/*", requireRole("manager", "receptionist")); api.use("/grooming-logs/*", requireRole("manager", "receptionist")); @@ -283,6 +288,7 @@ api.route("/admin/auth-provider", authProviderRouter); api.route("/admin/seed", adminSeedRouter); api.route("/search", searchRouter); api.route("/buffer-rules", bufferRulesRouter); +api.route("/routes", routesRouter); const port = Number(process.env.PORT ?? 3000); await initAuth(); diff --git a/src/routes/routes.ts b/src/routes/routes.ts new file mode 100644 index 0000000..e9a1d41 --- /dev/null +++ b/src/routes/routes.ts @@ -0,0 +1,284 @@ +import { Hono } from "hono"; +import { zValidator } from "@hono/zod-validator"; +import { z } from "zod/v3"; +import { + and, + asc, + eq, + gte, + lt, + ne, + getDb, + appointments, + businessSettings, + clients, + groomerRoutes, + routeStops, +} from "@groombook/db"; +import type { AppEnv, StaffRow } from "../middleware/rbac.js"; +import { + optimizeRoute, + resolveRouteGoogleApiKey, + type RouteStopInput, +} from "../services/routeOptimization.js"; + +export const routesRouter = new Hono(); + +const dailyQuerySchema = z.object({ + staffId: z.string().uuid().optional(), + date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/, "date must be YYYY-MM-DD"), +}); + +const optimizeBodySchema = z.object({ + staffId: z.string().uuid().optional(), + date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/, "date must be YYYY-MM-DD"), +}); + +/** + * Resolves the target staffId for the request and enforces the groomer-own / + * manager authorization rule. Groomers may only act on their own route; if a + * groomer omits staffId it defaults to their own. Returns either the resolved + * id or an error tuple the caller turns into a JSON response. + */ +function resolveTargetStaffId( + staffRow: StaffRow | undefined, + requestedStaffId: string | undefined +): { staffId: string } | { error: string; status: 400 | 403 } { + const isGroomer = staffRow?.role === "groomer"; + + if (isGroomer) { + if (requestedStaffId && requestedStaffId !== staffRow.id) { + return { + error: "Forbidden: groomers may only access their own route", + status: 403, + }; + } + return { staffId: staffRow.id }; + } + + // Manager: staffId is required (no implicit self — managers plan others' days). + if (!requestedStaffId) { + return { error: "staffId is required", status: 400 }; + } + return { staffId: requestedStaffId }; +} + +/** Day window [date 00:00:00Z, nextDay 00:00:00Z) for filtering appointments. */ +function dayBounds(date: string): { start: Date; end: Date } { + const start = new Date(`${date}T00:00:00.000Z`); + const end = new Date(start.getTime() + 24 * 60 * 60 * 1000); + return { start, end }; +} + +/** Loads a route's persisted stops, enriched with appointment + client detail. */ +async function loadRouteStops(db: ReturnType, routeId: string) { + return db + .select({ + id: routeStops.id, + appointmentId: routeStops.appointmentId, + stopOrder: routeStops.stopOrder, + latitude: routeStops.latitude, + longitude: routeStops.longitude, + travelMinsFromPrev: routeStops.travelMinsFromPrev, + travelDistanceKmFromPrev: routeStops.travelDistanceKmFromPrev, + bufferMins: routeStops.bufferMins, + appointmentStartTime: appointments.startTime, + appointmentEndTime: appointments.endTime, + appointmentStatus: appointments.status, + clientId: clients.id, + clientName: clients.name, + clientAddress: clients.address, + }) + .from(routeStops) + .innerJoin(appointments, eq(routeStops.appointmentId, appointments.id)) + .innerJoin(clients, eq(appointments.clientId, clients.id)) + .where(eq(routeStops.routeId, routeId)) + .orderBy(asc(routeStops.stopOrder)); +} + +/** + * GET /api/routes/daily?staffId=&date= + * Fetches (creating a draft if absent) the daily route for a groomer, with all + * persisted stops. Auth: groomer (own) or manager. + */ +routesRouter.get("/daily", zValidator("query", dailyQuerySchema), async (c) => { + const db = getDb(); + const { staffId: requestedStaffId, date } = c.req.valid("query"); + + const resolved = resolveTargetStaffId(c.get("staff"), requestedStaffId); + if ("error" in resolved) { + return c.json({ error: resolved.error }, resolved.status); + } + const staffId = resolved.staffId; + + let [route] = await db + .select() + .from(groomerRoutes) + .where( + and( + eq(groomerRoutes.staffId, staffId), + eq(groomerRoutes.routeDate, date) + ) + ); + + if (!route) { + // Create a draft route so the day is addressable before optimization. + [route] = await db + .insert(groomerRoutes) + .values({ staffId, routeDate: date, status: "draft" }) + .returning(); + } + + const stops = await loadRouteStops(db, route!.id); + return c.json({ route, stops }); +}); + +/** + * POST /api/routes/optimize { staffId, date } + * Generates or re-optimizes the daily route: pulls the day's geocoded + * appointments, optimizes the visiting order (Google Directions when a key is + * configured, else nearest-neighbor), and persists the ordered stops + totals. + * Auth: groomer (own) or manager. + */ +routesRouter.post( + "/optimize", + zValidator("json", optimizeBodySchema), + async (c) => { + const db = getDb(); + const { staffId: requestedStaffId, date } = c.req.valid("json"); + + const resolved = resolveTargetStaffId(c.get("staff"), requestedStaffId); + if ("error" in resolved) { + return c.json({ error: resolved.error }, resolved.status); + } + const staffId = resolved.staffId; + const { start, end } = dayBounds(date); + + // Pull the day's non-cancelled appointments for this groomer, joined to the + // client coordinates. Ordered by start time so the earliest booking anchors + // the route. + const dayAppointments = await db + .select({ + appointmentId: appointments.id, + startTime: appointments.startTime, + clientId: clients.id, + clientName: clients.name, + latitude: clients.latitude, + longitude: clients.longitude, + }) + .from(appointments) + .innerJoin(clients, eq(appointments.clientId, clients.id)) + .where( + and( + eq(appointments.staffId, staffId), + gte(appointments.startTime, start), + lt(appointments.startTime, end), + ne(appointments.status, "cancelled") + ) + ) + .orderBy(asc(appointments.startTime)); + + const stopInputs: RouteStopInput[] = []; + const skipped: Array<{ appointmentId: string; clientName: string; reason: string }> = + []; + for (const appt of dayAppointments) { + if (appt.latitude == null || appt.longitude == null) { + skipped.push({ + appointmentId: appt.appointmentId, + clientName: appt.clientName, + reason: "client address is not geocoded", + }); + continue; + } + stopInputs.push({ + appointmentId: appt.appointmentId, + latitude: appt.latitude, + longitude: appt.longitude, + }); + } + + const [settings] = await db.select().from(businessSettings).limit(1); + const bufferMins = settings?.defaultTravelBufferMins ?? 15; + + const googleApiKey = await resolveRouteGoogleApiKey(db); + const optimized = await optimizeRoute(stopInputs, { googleApiKey }); + + const warnings = [...optimized.warnings]; + if (skipped.length > 0) { + warnings.push( + `${skipped.length} appointment(s) were skipped because the client address is not geocoded.` + ); + } + + const now = new Date(); + const route = await db.transaction(async (tx) => { + // Upsert the route row for (staffId, date). + const [existing] = await tx + .select() + .from(groomerRoutes) + .where( + and( + eq(groomerRoutes.staffId, staffId), + eq(groomerRoutes.routeDate, date) + ) + ); + + const [routeRow] = existing + ? await tx + .update(groomerRoutes) + .set({ + status: "optimized", + totalTravelMins: optimized.totalTravelMins, + totalDistanceKm: optimized.totalDistanceKm.toFixed(2), + optimizedAt: now, + updatedAt: now, + }) + .where(eq(groomerRoutes.id, existing.id)) + .returning() + : await tx + .insert(groomerRoutes) + .values({ + staffId, + routeDate: date, + status: "optimized", + totalTravelMins: optimized.totalTravelMins, + totalDistanceKm: optimized.totalDistanceKm.toFixed(2), + optimizedAt: now, + }) + .returning(); + + // Replace stops: clear prior ordering, insert the freshly optimized one. + await tx.delete(routeStops).where(eq(routeStops.routeId, routeRow!.id)); + if (optimized.stops.length > 0) { + await tx.insert(routeStops).values( + optimized.stops.map((s, i) => ({ + routeId: routeRow!.id, + appointmentId: s.appointmentId, + stopOrder: i + 1, + latitude: s.latitude, + longitude: s.longitude, + travelMinsFromPrev: s.travelMinsFromPrev, + travelDistanceKmFromPrev: + s.travelDistanceKmFromPrev == null + ? null + : s.travelDistanceKmFromPrev.toFixed(2), + bufferMins, + })) + ); + } + + return routeRow!; + }); + + const stops = await loadRouteStops(db, route.id); + return c.json({ + route, + stops, + provider: optimized.provider, + chunked: optimized.chunked, + subRouteCount: optimized.subRouteCount, + skipped, + warnings, + }); + } +); diff --git a/src/services/routeOptimization.ts b/src/services/routeOptimization.ts new file mode 100644 index 0000000..14de121 --- /dev/null +++ b/src/services/routeOptimization.ts @@ -0,0 +1,413 @@ +import { businessSettings, decryptSecret, type Db } from "@groombook/db"; +import type { FetchLike } from "./geocoding.js"; + +/** + * Route optimization service (GRO-2155, Phase 2.1 of Route Optimization). + * + * Given a groomer's geocoded stops for a day, produces an optimized visiting + * order plus per-leg and total travel estimates. Two strategies: + * + * - {@link optimizeWithGoogle}: Google Maps Directions API with + * `optimizeWaypoints: true` (real road durations/distances), used when a + * Google Maps API key is configured. + * - {@link nearestNeighborOrder}: an offline nearest-neighbor TSP heuristic over + * great-circle distance, used as the default free / no-API-key fallback. + * + * Both strategies share the same public {@link optimizeRoute} orchestrator, + * which also handles the >25-stop edge case by chunking into sub-routes (the + * Google Directions waypoint cap) and surfacing a warning. + */ + +/** Google Directions allows origin + destination + up to 23 waypoints = 25 + * points per request. We cap a sub-route at 25 stops and chunk beyond that. */ +export const MAX_STOPS_PER_ROUTE = 25; + +/** Average driving speed (km/h) used to convert distance into travel minutes in + * the offline heuristic. Tuned for mixed urban/suburban mobile-groomer routes. */ +export const AVG_SPEED_KMH = 40; + +/** Multiplier applied to great-circle distance to approximate real road + * distance in the offline heuristic (straight-line underestimates driving). */ +export const ROAD_CIRCUITY_FACTOR = 1.3; + +const EARTH_RADIUS_KM = 6371; + +/** A geocoded stop to be ordered. `appointmentId` ties it back to the schedule. */ +export interface RouteStopInput { + appointmentId: string; + latitude: number; + longitude: number; +} + +/** A single stop in the optimized order, with travel from the previous stop. */ +export interface OptimizedStop { + appointmentId: string; + latitude: number; + longitude: number; + /** Null for the first stop of the whole route. */ + travelMinsFromPrev: number | null; + /** Null for the first stop of the whole route. Kilometres, 2-dp. */ + travelDistanceKmFromPrev: number | null; +} + +export type RouteOptimizationProvider = "google" | "nearest_neighbor"; + +export interface OptimizedRoute { + provider: RouteOptimizationProvider; + stops: OptimizedStop[]; + totalTravelMins: number; + /** Kilometres, rounded to 2 decimal places. */ + totalDistanceKm: number; + /** True when the route was split into multiple sub-routes (>25 stops). */ + chunked: boolean; + subRouteCount: number; + /** Non-fatal advisories for the caller to surface to the user. */ + warnings: string[]; +} + +export interface OptimizeRouteOptions { + /** Google Maps API key. When absent, the nearest-neighbor heuristic is used. */ + googleApiKey?: string | null; + /** Injectable fetch for testing the Google path. Defaults to global fetch. */ + fetchImpl?: FetchLike; +} + +const defaultFetch: FetchLike = (input, init) => + (globalThis.fetch as unknown as FetchLike)(input, init); + +// ─── Geometry helpers ─────────────────────────────────────────────────────── + +function toRadians(deg: number): number { + return (deg * Math.PI) / 180; +} + +/** Great-circle distance between two coordinates, in kilometres. */ +export function haversineKm( + a: { latitude: number; longitude: number }, + b: { latitude: number; longitude: number } +): number { + const dLat = toRadians(b.latitude - a.latitude); + const dLon = toRadians(b.longitude - a.longitude); + const lat1 = toRadians(a.latitude); + const lat2 = toRadians(b.latitude); + const h = + Math.sin(dLat / 2) ** 2 + + Math.cos(lat1) * Math.cos(lat2) * Math.sin(dLon / 2) ** 2; + return 2 * EARTH_RADIUS_KM * Math.asin(Math.min(1, Math.sqrt(h))); +} + +/** Round to 2 decimal places, returning a finite number. */ +function round2(n: number): number { + return Math.round(n * 100) / 100; +} + +/** + * Estimate a road travel leg from the great-circle distance between two points. + * Applies a circuity factor for distance and a fixed average speed for time. + */ +export function estimateLeg( + a: { latitude: number; longitude: number }, + b: { latitude: number; longitude: number } +): { distanceKm: number; mins: number } { + const straight = haversineKm(a, b); + const distanceKm = straight * ROAD_CIRCUITY_FACTOR; + const mins = (distanceKm / AVG_SPEED_KMH) * 60; + return { distanceKm: round2(distanceKm), mins: Math.round(mins) }; +} + +// ─── Nearest-neighbor heuristic ───────────────────────────────────────────── + +/** + * Orders points greedily: start at `startIndex`, then repeatedly visit the + * nearest unvisited point (great-circle distance). Returns indices into the + * input array in visiting order. Deterministic ties broken by lowest index. + */ +export function nearestNeighborOrder( + points: Array<{ latitude: number; longitude: number }>, + startIndex = 0 +): number[] { + const n = points.length; + if (n <= 1) return points.map((_, i) => i); + + const visited = new Array(n).fill(false); + const order: number[] = [startIndex]; + visited[startIndex] = true; + let current = startIndex; + + for (let step = 1; step < n; step++) { + let best = -1; + let bestDist = Infinity; + for (let j = 0; j < n; j++) { + if (visited[j]) continue; + const d = haversineKm(points[current]!, points[j]!); + if (d < bestDist) { + bestDist = d; + best = j; + } + } + visited[best] = true; + order.push(best); + current = best; + } + return order; +} + +/** Orders one chunk (<= MAX_STOPS_PER_ROUTE) via nearest-neighbor. */ +function optimizeChunkNearestNeighbor( + stops: RouteStopInput[] +): RouteStopInput[] { + const order = nearestNeighborOrder(stops, 0); + return order.map((i) => stops[i]!); +} + +// ─── Google Directions ────────────────────────────────────────────────────── + +const GOOGLE_DIRECTIONS_URL = + "https://maps.googleapis.com/maps/api/directions/json"; + +interface GoogleDirectionsResponse { + status: string; + error_message?: string; + routes?: Array<{ + waypoint_order?: number[]; + legs?: Array<{ + duration?: { value?: number }; + distance?: { value?: number }; + }>; + }>; +} + +/** + * Orders one chunk via the Google Directions API with `optimizeWaypoints=true`. + * + * The first stop is fixed as both origin and destination (a closed tour); the + * remaining stops are passed as optimizable waypoints. We keep the optimized + * forward order and drop the final return-to-origin leg, yielding an open route + * whose per-leg durations/distances come from real road data. + */ +async function optimizeChunkGoogle( + stops: RouteStopInput[], + apiKey: string, + fetchImpl: FetchLike +): Promise<{ stops: RouteStopInput[]; legsMeters: number[]; legsSeconds: number[] }> { + if (stops.length <= 1) { + return { stops: [...stops], legsMeters: [], legsSeconds: [] }; + } + + const origin = stops[0]!; + const waypoints = stops.slice(1); + const url = new URL(GOOGLE_DIRECTIONS_URL); + url.searchParams.set("origin", `${origin.latitude},${origin.longitude}`); + url.searchParams.set("destination", `${origin.latitude},${origin.longitude}`); + url.searchParams.set( + "waypoints", + "optimize:true|" + + waypoints.map((w) => `${w.latitude},${w.longitude}`).join("|") + ); + url.searchParams.set("key", apiKey); + + const res = await fetchImpl(url.toString()); + if (!res.ok) { + throw new Error( + `Google Directions request failed: ${res.status} ${res.statusText}` + ); + } + const body = (await res.json()) as GoogleDirectionsResponse; + if (body.status !== "OK" || !body.routes || body.routes.length === 0) { + throw new Error( + `Google Directions returned status ${body.status}${ + body.error_message ? `: ${body.error_message}` : "" + }` + ); + } + + const route = body.routes[0]!; + const waypointOrder = route.waypoint_order ?? waypoints.map((_, i) => i); + const legs = route.legs ?? []; + + // Ordered stops: origin first, then waypoints in the optimized order. + const orderedStops: RouteStopInput[] = [ + origin, + ...waypointOrder.map((i) => waypoints[i]!), + ]; + + // legs[k] is the travel into orderedStops[k+1]. Drop the trailing return leg + // (orderedStops.length-1 legs describe the open route). + const legsMeters: number[] = []; + const legsSeconds: number[] = []; + for (let k = 0; k < orderedStops.length - 1; k++) { + const leg = legs[k]; + legsMeters.push(leg?.distance?.value ?? 0); + legsSeconds.push(leg?.duration?.value ?? 0); + } + + return { stops: orderedStops, legsMeters, legsSeconds }; +} + +// ─── Orchestration ────────────────────────────────────────────────────────── + +function chunk(items: T[], size: number): T[][] { + const out: T[][] = []; + for (let i = 0; i < items.length; i += size) { + out.push(items.slice(i, i + size)); + } + return out; +} + +/** + * Optimizes a full day's stops into a single visiting order with travel + * metrics. Uses Google Directions when `googleApiKey` is provided, otherwise the + * offline nearest-neighbor heuristic. Routes longer than + * {@link MAX_STOPS_PER_ROUTE} stops are split into sub-routes and a warning is + * emitted; sub-routes are stitched end-to-end, with the boundary leg estimated + * from great-circle distance. + */ +export async function optimizeRoute( + inputStops: RouteStopInput[], + options: OptimizeRouteOptions = {} +): Promise { + const fetchImpl = options.fetchImpl ?? defaultFetch; + const useGoogle = Boolean(options.googleApiKey); + const provider: RouteOptimizationProvider = useGoogle + ? "google" + : "nearest_neighbor"; + const warnings: string[] = []; + + if (inputStops.length === 0) { + return { + provider, + stops: [], + totalTravelMins: 0, + totalDistanceKm: 0, + chunked: false, + subRouteCount: 0, + warnings, + }; + } + + const chunks = chunk(inputStops, MAX_STOPS_PER_ROUTE); + const chunked = chunks.length > 1; + if (chunked) { + warnings.push( + `Route has ${inputStops.length} stops, exceeding the ${MAX_STOPS_PER_ROUTE}-stop optimization limit. Split into ${chunks.length} sub-routes; review the order at sub-route boundaries.` + ); + } + + const ordered: OptimizedStop[] = []; + let prev: RouteStopInput | null = null; + + for (const group of chunks) { + let groupStops: RouteStopInput[]; + let legDistanceKm: (i: number) => number; + let legMins: (i: number) => number; + + if (useGoogle) { + try { + const result = await optimizeChunkGoogle( + group, + options.googleApiKey!, + fetchImpl + ); + groupStops = result.stops; + legDistanceKm = (i) => round2(result.legsMeters[i]! / 1000); + legMins = (i) => Math.round(result.legsSeconds[i]! / 60); + } catch (err) { + // Google failed mid-optimization — degrade to the offline heuristic for + // this run rather than failing the whole request. + warnings.push( + `Google Directions unavailable; used offline heuristic: ${ + err instanceof Error ? err.message : String(err) + }` + ); + groupStops = optimizeChunkNearestNeighbor(group); + legDistanceKm = (i) => estimateLeg(groupStops[i]!, groupStops[i + 1]!).distanceKm; + legMins = (i) => estimateLeg(groupStops[i]!, groupStops[i + 1]!).mins; + } + } else { + groupStops = optimizeChunkNearestNeighbor(group); + legDistanceKm = (i) => estimateLeg(groupStops[i]!, groupStops[i + 1]!).distanceKm; + legMins = (i) => estimateLeg(groupStops[i]!, groupStops[i + 1]!).mins; + } + + for (let i = 0; i < groupStops.length; i++) { + const stop = groupStops[i]!; + if (prev === null) { + // Very first stop of the whole route. + ordered.push({ + appointmentId: stop.appointmentId, + latitude: stop.latitude, + longitude: stop.longitude, + travelMinsFromPrev: null, + travelDistanceKmFromPrev: null, + }); + } else if (i === 0) { + // First stop of a non-initial chunk: estimate the boundary leg. + const est = estimateLeg(prev, stop); + ordered.push({ + appointmentId: stop.appointmentId, + latitude: stop.latitude, + longitude: stop.longitude, + travelMinsFromPrev: est.mins, + travelDistanceKmFromPrev: est.distanceKm, + }); + } else { + ordered.push({ + appointmentId: stop.appointmentId, + latitude: stop.latitude, + longitude: stop.longitude, + travelMinsFromPrev: legMins(i - 1), + travelDistanceKmFromPrev: legDistanceKm(i - 1), + }); + } + prev = stop; + } + } + + const totalTravelMins = ordered.reduce( + (sum, s) => sum + (s.travelMinsFromPrev ?? 0), + 0 + ); + const totalDistanceKm = round2( + ordered.reduce((sum, s) => sum + (s.travelDistanceKmFromPrev ?? 0), 0) + ); + + return { + provider, + stops: ordered, + totalTravelMins, + totalDistanceKm, + chunked, + subRouteCount: chunks.length, + warnings, + }; +} + +// ─── Google API key resolution ────────────────────────────────────────────── + +/** + * Resolves the Google Maps API key for route optimization from + * `businessSettings.googleMapsApiKey` (decrypted at rest) or, as a development + * convenience, the `GOOGLE_MAPS_API_KEY` env var. Returns `null` when no usable + * key exists, in which case callers fall back to the offline heuristic. + */ +export async function resolveRouteGoogleApiKey( + db: Db, + decrypt: (ciphertext: string) => string = decryptSecret +): Promise { + const [settings] = await db.select().from(businessSettings).limit(1); + const stored = settings?.googleMapsApiKey?.trim(); + if (stored) { + try { + const decrypted = decrypt(stored).trim(); + if (decrypted) return decrypted; + } catch (err) { + console.warn( + `Failed to decrypt googleMapsApiKey for route optimization; using offline heuristic: ${ + err instanceof Error ? err.message : String(err) + }` + ); + } + } + const fromEnv = process.env.GOOGLE_MAPS_API_KEY?.trim(); + return fromEnv ? fromEnv : null; +} -- 2.52.0 From eb92f99c4a9b48a7865db85ef64b2761df061a87 Mon Sep 17 00:00:00 2001 From: Flea Flicker <22+gb_flea@noreply.git.farh.net> Date: Mon, 8 Jun 2026 17:53:01 +0000 Subject: [PATCH 10/12] =?UTF-8?q?dev=20=E2=86=92=20uat:=20GRO-2203=20porta?= =?UTF-8?q?l=20pet=20PATCH=20malformed-petId=20500=E2=86=92404=20(#178)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- UAT_PLAYBOOK.md | 1 + src/__tests__/portalPets.test.ts | 17 ++++++++ src/__tests__/waitlist.test.ts | 70 ++++++++++++++++++++++++++++++++ src/routes/portal.ts | 36 +++++++++++++--- 4 files changed, 118 insertions(+), 6 deletions(-) diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index dace647..48c5d54 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -283,6 +283,7 @@ This means: | TC-API-8.13 | Portal pet update — owner success + persistence (GRO-2187, fixes [GRO-1480](/GRO/issues/GRO-1480) §5.23) | With a portal session for the pet's owner, `PATCH /api/portal/pets/{petId}` with body `{ "name": "...", "breed": "...", "weightKg": 18.25, "healthAlerts": "...", "coatType": "double", "petSizeCategory": "xlarge", "preferredCuts": ["teddy bear"], "medicalAlerts": [{"type":"allergy","description":"oatmeal","severity":"medium"}] }` | 200 OK; response reflects the update with `petSizeCategory: "extra_large"` (web `xlarge` → DB `extra_large`). A follow-up `GET /api/portal/pets` shows the persisted values | | TC-API-8.14 | Portal pet update — non-owner blocked (GRO-2187) | `PATCH /api/portal/pets/{petId}` for a pet owned by a different client, using another client's portal session | 403 Forbidden (or 404 if pet id is unknown); no mutation persisted | | TC-API-8.15 | Portal pet update — invalid enum rejected (GRO-2187) | `PATCH /api/portal/pets/{petId}` with `coatType: "fluffy"` or `petSizeCategory: "gigantic"` | 422 Unprocessable Entity; pet unchanged | +| TC-API-8.16 | Portal pet update — malformed (non-UUID) petId returns 404 (GRO-2203) | With a valid portal session, `PATCH /api/portal/pets/not-a-uuid` with header `X-Impersonation-Session-Id` and body `{"coatType":"short"}` | 404 Not Found with body `{"error":"Not found"}` (was an unhandled 500 from the Postgres uuid cast in GRO-2203; mirrors the GRO-2014 guard). No mutation persisted | ### 4.9 Waitlist diff --git a/src/__tests__/portalPets.test.ts b/src/__tests__/portalPets.test.ts index c23bca0..a9e41c2 100644 --- a/src/__tests__/portalPets.test.ts +++ b/src/__tests__/portalPets.test.ts @@ -280,6 +280,23 @@ describe("PATCH /portal/pets/:petId", () => { expect(res.status).toBe(404); }); + it("returns 404 for a malformed (non-UUID) petId without hitting the db (GRO-2203)", async () => { + selectSessionRow = ACTIVE_SESSION; + // A non-UUID petId previously reached `where(eq(pets.id, ...))` and made + // Postgres throw "invalid input syntax for type uuid" → unhandled 500. + // It must now short-circuit to 404 before any select/update. + selectPetRow = PET; + + const res = await jsonPatch( + `/portal/pets/not-a-uuid`, + { coatType: "short" }, + { "X-Impersonation-Session-Id": SESSION_ID } + ); + + expect(res.status).toBe(404); + expect(updatedValues).toHaveLength(0); + }); + it("returns 422 for an invalid coatType", async () => { selectSessionRow = ACTIVE_SESSION; selectPetRow = PET; diff --git a/src/__tests__/waitlist.test.ts b/src/__tests__/waitlist.test.ts index 383bc80..5c4d209 100644 --- a/src/__tests__/waitlist.test.ts +++ b/src/__tests__/waitlist.test.ts @@ -184,6 +184,66 @@ describe("POST /portal/waitlist", () => { expect(insertedValues).toHaveLength(1); }); + it("normalizes HH:MM:SS preferredTime and returns 201 (GRO-2211)", async () => { + selectSessionRow = ACTIVE_SESSION; + const res = await jsonRequest("POST", "/portal/waitlist", { + petId: VALID_UUID_3, + serviceId: VALID_UUID_4, + preferredDate: "2026-03-25", + preferredTime: "10:00:00", + }, { "X-Impersonation-Session-Id": VALID_UUID_5 }); + expect(res.status).toBe(201); + expect(insertedValues[0]?.preferredTime).toBe("10:00:00"); + }); + + it("normalizes HH:MM preferredTime to HH:MM:SS before insert (GRO-2211)", async () => { + selectSessionRow = ACTIVE_SESSION; + const res = await jsonRequest("POST", "/portal/waitlist", { + petId: VALID_UUID_3, + serviceId: VALID_UUID_4, + preferredDate: "2026-03-25", + preferredTime: "10:00", + }, { "X-Impersonation-Session-Id": VALID_UUID_5 }); + expect(res.status).toBe(201); + expect(insertedValues[0]?.preferredTime).toBe("10:00:00"); + }); + + it("returns 400 (not 500) for a full ISO datetime preferredTime (GRO-2211)", async () => { + selectSessionRow = ACTIVE_SESSION; + const res = await jsonRequest("POST", "/portal/waitlist", { + petId: VALID_UUID_3, + serviceId: VALID_UUID_4, + preferredDate: "2026-03-25", + preferredTime: "2026-06-09T10:00:00.000Z", + }, { "X-Impersonation-Session-Id": VALID_UUID_5 }); + expect(res.status).toBe(400); + expect(insertedValues).toHaveLength(0); + }); + + it("returns 400 for a malformed preferredDate (GRO-2211)", async () => { + selectSessionRow = ACTIVE_SESSION; + const res = await jsonRequest("POST", "/portal/waitlist", { + petId: VALID_UUID_3, + serviceId: VALID_UUID_4, + preferredDate: "03/25/2026", + preferredTime: "10:00", + }, { "X-Impersonation-Session-Id": VALID_UUID_5 }); + expect(res.status).toBe(400); + expect(insertedValues).toHaveLength(0); + }); + + it("returns 400 for an out-of-range preferredTime (GRO-2211)", async () => { + selectSessionRow = ACTIVE_SESSION; + const res = await jsonRequest("POST", "/portal/waitlist", { + petId: VALID_UUID_3, + serviceId: VALID_UUID_4, + preferredDate: "2026-03-25", + preferredTime: "25:99", + }, { "X-Impersonation-Session-Id": VALID_UUID_5 }); + expect(res.status).toBe(400); + expect(insertedValues).toHaveLength(0); + }); + it("returns 401 without session", async () => { const res = await jsonRequest("POST", "/portal/waitlist", { petId: VALID_UUID_3, @@ -258,6 +318,16 @@ describe("PATCH /portal/waitlist/:id", () => { expect(updatedValues[0]?.status).toBe("cancelled"); }); + it("returns 400 (not 500) for a full ISO datetime preferredTime on update (GRO-2211)", async () => { + selectSessionRow = ACTIVE_SESSION; + selectRows = [WAITLIST_ENTRY]; + const res = await jsonRequest("PATCH", `/portal/waitlist/${VALID_UUID_1}`, { + preferredTime: "2026-06-09T10:00:00.000Z", + }, { "X-Impersonation-Session-Id": VALID_UUID_5 }); + expect(res.status).toBe(400); + expect(updatedValues).toHaveLength(0); + }); + it("returns 401 without session", async () => { const res = await jsonRequest("PATCH", `/portal/waitlist/${VALID_UUID_1}`, { status: "cancelled", diff --git a/src/routes/portal.ts b/src/routes/portal.ts index 0b106fb..aa1593a 100644 --- a/src/routes/portal.ts +++ b/src/routes/portal.ts @@ -296,6 +296,14 @@ portalRouter.patch( const body = c.req.valid("json"); const clientId = c.get("portalClientId"); + // GRO-2203: validate UUID format before hitting Postgres. Passing a non-UUID + // string to a uuid column makes the driver throw ("invalid input syntax for + // type uuid"), which previously surfaced as an unhandled 500. Mirror the + // GRO-2014 fix in pets.ts and treat a malformed id as Not found. + if (!z.string().uuid().safeParse(petId).success) { + return c.json({ error: "Not found" }, 404); + } + const [pet] = await db .select() .from(pets) @@ -551,17 +559,33 @@ portalRouter.post("/appointments/:id/cancel", async (c) => { // ─── Client-facing waitlist routes ──────────────────────────────────────────── +// Postgres `date` / `time` columns reject arbitrary strings (e.g. a full ISO +// datetime), throwing a DateTimeParseError that surfaces as an unhandled 500. +// Constrain client input here so malformed values are rejected with a 400 by +// zValidator before they ever reach the DB (GRO-2211 defense-in-depth). +const preferredDateSchema = z + .string() + .regex(/^\d{4}-\d{2}-\d{2}$/, "preferredDate must be YYYY-MM-DD"); +const preferredTimeSchema = z + .string() + .regex(/^([01]\d|2[0-3]):[0-5]\d(:[0-5]\d)?$/, "preferredTime must be HH:MM or HH:MM:SS"); + +// Normalize HH:MM → HH:MM:SS so it matches the Postgres `time` column format. +function normalizeTime(value: string): string { + return value.length === 5 ? `${value}:00` : value; +} + const createWaitlistEntrySchema = z.object({ petId: z.string().uuid(), serviceId: z.string().uuid(), - preferredDate: z.string(), - preferredTime: z.string(), + preferredDate: preferredDateSchema, + preferredTime: preferredTimeSchema, }); const updateWaitlistEntrySchema = z.object({ status: z.literal("cancelled").optional(), - preferredDate: z.string().optional(), - preferredTime: z.string().optional(), + preferredDate: preferredDateSchema.optional(), + preferredTime: preferredTimeSchema.optional(), }); portalRouter.post( @@ -579,7 +603,7 @@ portalRouter.post( petId: body.petId, serviceId: body.serviceId, preferredDate: body.preferredDate, - preferredTime: body.preferredTime, + preferredTime: normalizeTime(body.preferredTime), }) .returning(); @@ -610,7 +634,7 @@ portalRouter.patch( const updateData: Record = { 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; + if (body.preferredTime !== undefined) updateData.preferredTime = normalizeTime(body.preferredTime); const [updated] = await db .update(waitlistEntries) -- 2.52.0 From e9ad92de01d383aefcd23e75f72cbb188aa6034b Mon Sep 17 00:00:00 2001 From: Flea Flicker <22+gb_flea@noreply.git.farh.net> Date: Tue, 9 Jun 2026 01:23:06 +0000 Subject: [PATCH 11/12] =?UTF-8?q?uat=E2=86=92main=20(PROD):=20GRO-2157=20n?= =?UTF-8?q?av=20export=20+=20GRO-2225/2235=20(frozen=20@4868f18)=20(#192)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit feat: nav export + conflict guard + UAT seed (GRO-2157, GRO-2225, GRO-2235) Squash-merges PR #192: uat→main PROD promotion. Freezes at validated SHA 4868f18 (UAT regression GRO-2261 11/11 PASS). Bundles: GRO-2157 (nav export), GRO-2225 (UAT seed), GRO-2235 (conflict guard). CTO-reviewed and approved (review #4542). Co-authored-by: Flea Flicker <22+gb_flea@noreply.git.farh.net> Co-committed-by: Flea Flicker <22+gb_flea@noreply.git.farh.net> --- UAT_PLAYBOOK.md | 30 ++- packages/db/src/seed.ts | 210 ++++++++++++++++++ src/__tests__/navigationExport.test.ts | 140 ++++++++++++ src/__tests__/portalWaitlistDuplicate.test.ts | 154 +++++++++++++ src/routes/portal.ts | 36 ++- src/routes/routes.ts | 69 +++++- src/services/navigationExport.ts | 155 +++++++++++++ 7 files changed, 782 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/navigationExport.test.ts create mode 100644 src/__tests__/portalWaitlistDuplicate.test.ts create mode 100644 src/services/navigationExport.ts diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index d590556..78b73f3 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -365,7 +365,12 @@ This means: ### 4.16 Route Optimization — Route CRUD + Optimize (GRO-2155, Phase 2.1) -A groomer's daily route is one row per `(staffId, routeDate)` in `groomer_routes`, with ordered `route_stops`. `POST /api/routes/optimize` pulls the day's non-cancelled appointments whose client is geocoded (GRO-2154), orders them (Google Directions `optimizeWaypoints` when a key is configured in `businessSettings.googleMapsApiKey`, else an offline nearest-neighbor heuristic), and persists `stopOrder`, `travelMinsFromPrev`, `travelDistanceKmFromPrev` plus route `totalTravelMins`/`totalDistanceKm`/`optimizedAt`. **Auth: manager (any groomer's route) or groomer (own route only); receptionists have no access.** Pre-condition: at least one geocoded client with appointments on the target date for the staff member (use §4.2 geocoding + a seed groomer). +A groomer's daily route is one row per `(staffId, routeDate)` in `groomer_routes`, with ordered `route_stops`. `POST /api/routes/optimize` pulls the day's non-cancelled appointments whose client is geocoded (GRO-2154), orders them (Google Directions `optimizeWaypoints` when a key is configured in `businessSettings.googleMapsApiKey`, else an offline nearest-neighbor heuristic), and persists `stopOrder`, `travelMinsFromPrev`, `travelDistanceKmFromPrev` plus route `totalTravelMins`/`totalDistanceKm`/`optimizedAt`. **Auth: manager (any groomer's route) or groomer (own route only); receptionists have no access.** + +**Pre-condition (GRO-2225 — zero-touch; no manual PATCH/geocoding needed).** A fresh UAT reset+seed now provisions a deterministic route cohort, so §4.16 runs directly against seed data: +- **Groomer:** `uat-groomer@groombook.dev` (staffId `00000000-0000-0000-0000-000000000004`). Resolve its id via `GET /api/staff` or sign in as the groomer and omit `staffId`. +- **Date:** `2026-09-15` (fixed). On this date the groomer has **12** confirmed appointments: **10 pre-geocoded** clients clustered in the Seattle metro (multi-stop route) + **2 intentionally un-geocoded** clients (exercise the skip-and-surface path, TC-API-16.4). Cohort clients are named `Route Demo — …` (emails `route-client-NN@uat.groombook.dev`). +- **Receptionist (TC-API-16.9 403):** sign in as `uat-receptionist@groombook.dev` (password from the `seed-uat-passwords` secret, key `SEED_UAT_RECEPTIONIST_PASSWORD`) — a standing receptionist login; no hand-built session required. | # | Scenario | Steps | Expected | |---|----------|-------|----------| @@ -403,6 +408,29 @@ Builds on §4.16. After optimization each consecutive leg carries a travel `buff | TC-API-17.7 | Reorder invalid routeId | `PATCH /api/routes/not-a-uuid/reorder` | 400 `{ error: "routeId must be a UUID" }` | | TC-API-17.8 | Groomer cannot reorder another's route | As groomer, reorder a route owned by a different groomer | 403 Forbidden (`groomers may only access their own route`) | +### 4.18 Route Optimization — Navigation Export (GRO-2157, Phase 2.3) + +Builds on §4.16/§4.17. Two read-only endpoints turn an optimized route into a native-navigation deep-link URL the frontend opens on the groomer's phone: + +- `GET /api/routes/:routeId/export/google-maps` → Google Maps URLs API link (`https://www.google.com/maps/dir/?api=1&travelmode=driving&origin=…&destination=…&waypoints=…`) +- `GET /api/routes/:routeId/export/apple-maps` → Apple Maps URL scheme (`maps://?saddr=…&daddr=+to:…&dirflg=d`) + +Both use the stops' stored `latitude`/`longitude` in `stopOrder`: **origin = first stop, destination = last stop, the rest are ordered intermediate waypoints**. Each response body is `{ platform, url, stopCount, waypointCount }` where `waypointCount` = stops minus origin and destination. Waypoint limits are validated per platform: **Google Maps ≤ 9**, **Apple Maps ≤ 15** intermediate waypoints; over-limit routes return 400. **Auth: manager (any route) or groomer (own route only); receptionists have no access.** + +| ID | Scenario | Steps | Expected | +|----|----------|-------|----------| +| TC-API-18.1 | Google Maps export of a multi-stop route | As manager, optimize a multi-stop day (§4.16), then `GET /api/routes/{routeId}/export/google-maps` | 200 OK; `platform:"google-maps"`, `url` starts `https://www.google.com/maps/dir/?api=1`, contains `travelmode=driving`, `origin`/`destination` are the first/last stop coords, `waypoints` lists the middle stops in order (pipe-separated). `stopCount` = total stops, `waypointCount` = `stopCount − 2` | +| TC-API-18.2 | Apple Maps export of a multi-stop route | As manager, `GET /api/routes/{routeId}/export/apple-maps` for the same route | 200 OK; `platform:"apple-maps"`, `url` starts `maps://?saddr=`, `daddr` chains the remaining stops with `+to:`, ends `&dirflg=d`; `stopCount`/`waypointCount` as above | +| TC-API-18.3 | Single-stop route | Export a route (google-maps and apple-maps) that has exactly one stop | 200 OK; `waypointCount:0`. Google url has `destination` and no `waypoints=`; Apple url is `maps://?daddr=&dirflg=d` (no `saddr`) | +| TC-API-18.4 | Empty route rejected | Export a route with no stops (a fresh `draft` route) | 400 `{ error: "route has no stops to export" }` | +| TC-API-18.5 | Google waypoint limit | Export (google-maps) a route with >11 stops (>9 intermediate waypoints) | 400 with an `error` mentioning Google Maps' limit of 9 | +| TC-API-18.6 | Apple waypoint limit | Export (apple-maps) a route with >17 stops (>15 intermediate waypoints) | 400 with an `error` mentioning Apple Maps' limit of 15 | +| TC-API-18.7 | Unknown route | `GET /api/routes/{randomUuid}/export/google-maps` | 404 `{ error: "Route not found" }` | +| TC-API-18.8 | Invalid routeId | `GET /api/routes/not-a-uuid/export/apple-maps` | 400 `{ error: "routeId must be a UUID" }` | +| TC-API-18.9 | Groomer exports own route | As **groomer**, export a route owned by self | 200 OK; deep-link returned | +| TC-API-18.10 | Groomer cannot export another's route | As groomer, export a route owned by a different groomer | 403 Forbidden (`groomers may only access their own route`) | +| TC-API-18.11 | Receptionist denied | As **receptionist**, export any route | 403 Forbidden (role not permitted) | + ## Pass/Fail Criteria **Pass:** diff --git a/packages/db/src/seed.ts b/packages/db/src/seed.ts index 0959be0..55b2ee4 100644 --- a/packages/db/src/seed.ts +++ b/packages/db/src/seed.ts @@ -456,6 +456,36 @@ async function seedUatStaffAccounts( } } + // ── Staff: UAT Receptionist (GRO-2225) ────────────────────────────────────── + // Standing receptionist staff record so the route-optimization 403 path + // (TC-API-16.9: receptionist GET/POST /api/routes → 403) is reproducible + // without a hand-built session. The matching Better-Auth credential is + // provisioned below from SEED_UAT_RECEPTIONIST_PASSWORD. Created here (gated + // on the password env) so the credential loop's staff-link step finds it. + if (process.env.SEED_UAT_RECEPTIONIST_PASSWORD) { + const UAT_RECEPTIONIST_STAFF_ID = "00000000-0000-0000-0000-000000000099"; + const [existingReceptionist] = await db + .select() + .from(schema.staff) + .where(eq(schema.staff.email, "uat-receptionist@groombook.dev")) + .limit(1); + + if (existingReceptionist) { + console.log(`✓ Staff 'UAT Receptionist' already exists — skipping`); + } else { + await db.insert(schema.staff).values({ + id: UAT_RECEPTIONIST_STAFF_ID, + name: "UAT Receptionist", + email: "uat-receptionist@groombook.dev", + oidcSub: "uat-receptionist@groombook.dev", + role: "receptionist", + isSuperUser: false, + active: true, + }); + console.log(`✓ Created staff 'UAT Receptionist' (uat-receptionist@groombook.dev)`); + } + } + // ── Staff: UAT Groomer Personas (SEED_UAT_GROOMER_EMAILS + SEED_UAT_GROOMER_NAMES) ── const groomerEmails = process.env.SEED_UAT_GROOMER_EMAILS?.split(",").map((e) => e.trim()).filter(Boolean) ?? []; const groomerNames = process.env.SEED_UAT_GROOMER_NAMES?.split(",").map((n) => n.trim()).filter(Boolean) ?? []; @@ -495,6 +525,8 @@ async function seedUatStaffAccounts( { email: "uat-groomer@groombook.dev", name: "UAT Staff Groomer", passwordEnv: "SEED_UAT_GROOMER_PASSWORD", staffEmail: "uat-groomer@groombook.dev" }, { email: "uat-customer@groombook.dev", name: "UAT Customer", passwordEnv: "SEED_UAT_CUSTOMER_PASSWORD", staffEmail: null }, { email: "uat-tester@groombook.dev", name: "UAT Tester", passwordEnv: "SEED_UAT_TESTER_PASSWORD", staffEmail: "uat-tester@groombook.dev" }, + // GRO-2225: standing receptionist login for the route-optimization 403 path (TC-API-16.9). + { email: "uat-receptionist@groombook.dev", name: "UAT Receptionist", passwordEnv: "SEED_UAT_RECEPTIONIST_PASSWORD", staffEmail: "uat-receptionist@groombook.dev" }, ]; for (const acct of uatPasswordAccounts) { @@ -798,6 +830,179 @@ async function seedUatGroomerLinkage( ); } +// ── GRO-2225: deterministic route-optimization cohort ──────────────────────── + +/** + * GRO-2225: seed a deterministic, pre-geocoded client cohort + a fixed-date set + * of appointments for the UAT groomer so the route-optimization endpoints + * (`GET /api/routes/daily`, `POST /api/routes/optimize`, UAT §4.16 + * TC-API-16.1…16.11) are exercisable with ZERO manual PATCHing. + * + * Design (no live geocoder — UAT has no Google Maps key, provider is + * nearest_neighbor; coordinates are hand-picked fixtures clustered in the + * Seattle metro): + * - All appointments are on a FIXED calendar date (ROUTE_DATE) and assigned to + * the UAT groomer (`uat-groomer@groombook.dev`). The optimize endpoint pulls + * non-cancelled appointments in [date 00:00Z, +24h) joined to client coords. + * - 10 clients carry deterministic lat/lng → a multi-stop optimized route. + * - 2 clients are intentionally left UN-geocoded so the "skipped + surfaced" + * path (TC-API-16.5) stays reproducible. + * + * Idempotent: clients/pets are upserted by fixed UUID (they are NOT truncated on + * reset); appointments are upserted by fixed UUID too (they ARE truncated on + * reset, but the upsert keeps re-runs safe in non-truncating dev/test paths). + * Skips cleanly when the UAT groomer staff record is absent (e.g. prod/demo or a + * dev seed without the UAT personas). + */ +async function seedUatRouteCohort(db: ReturnType): Promise { + // Fixed calendar date the UAT playbook hardcodes for §4.16. Times are UTC so + // they fall inside the optimize endpoint's [date 00:00Z, +24h) day window. + const ROUTE_DATE = "2026-09-15"; + + const [uatGroomer] = await db + .select({ id: schema.staff.id }) + .from(schema.staff) + .where(eq(schema.staff.email, "uat-groomer@groombook.dev")) + .limit(1); + if (!uatGroomer) { + console.log("✓ GRO-2225: uat-groomer not present — skipping route cohort"); + return; + } + + // Resolve a service for the appointments: prefer Bath & Brush, else any active. + const BATH_AND_BRUSH_ID = "b0000001-0000-0000-0000-000000000001"; + const [bathService] = await db + .select({ id: schema.services.id }) + .from(schema.services) + .where(eq(schema.services.id, BATH_AND_BRUSH_ID)) + .limit(1); + let serviceId: string; + if (bathService) { + serviceId = bathService.id; + } else { + const [fallback] = await db + .select({ id: schema.services.id }) + .from(schema.services) + .where(eq(schema.services.active, true)) + .limit(1); + if (!fallback) { + console.warn("⚠ GRO-2225: no active services found — skipping route cohort"); + return; + } + serviceId = fallback.id; + } + + // Hand-picked fixture coordinates clustered in the Seattle metro. `coords:null` + // marks an intentionally un-geocoded client (skip-and-surface path TC-16.5). + const cohort: Array<{ + n: number; + name: string; + coords: { lat: number; lng: number } | null; + }> = [ + { n: 1, name: "Route Demo — Ada Lovelace", coords: { lat: 47.6097, lng: -122.3331 } }, + { n: 2, name: "Route Demo — Grace Hopper", coords: { lat: 47.6205, lng: -122.3493 } }, + { n: 3, name: "Route Demo — Alan Turing", coords: { lat: 47.5990, lng: -122.3300 } }, + { n: 4, name: "Route Demo — Katherine Johnson", coords: { lat: 47.6150, lng: -122.3200 } }, + { n: 5, name: "Route Demo — Edsger Dijkstra", coords: { lat: 47.6280, lng: -122.3550 } }, + { n: 6, name: "Route Demo — Barbara Liskov", coords: { lat: 47.5920, lng: -122.3150 } }, + { n: 7, name: "Route Demo — Donald Knuth", coords: { lat: 47.6350, lng: -122.3400 } }, + { n: 8, name: "Route Demo — Margaret Hamilton", coords: { lat: 47.6050, lng: -122.3600 } }, + { n: 9, name: "Route Demo — Ken Thompson", coords: { lat: 47.6420, lng: -122.3250 } }, + { n: 10, name: "Route Demo — Radia Perlman", coords: { lat: 47.5880, lng: -122.3450 } }, + // Intentionally un-geocoded — exercises the skip-and-surface path. + { n: 11, name: "Route Demo — Ungeocoded One", coords: null }, + { n: 12, name: "Route Demo — Ungeocoded Two", coords: null }, + ]; + + // Stagger appointments 45 min apart starting 15:00Z on ROUTE_DATE. + const dayStartMs = new Date(`${ROUTE_DATE}T15:00:00.000Z`).getTime(); + const SLOT_MS = 45 * 60 * 1000; + + let geocodedCount = 0; + let ungeocodedCount = 0; + for (const c of cohort) { + const pad = String(c.n).padStart(2, "0"); + const clientId = `d0000000-0000-0000-0000-0000000000${pad}`; + const petId = `d0000000-0000-0000-0000-0000000001${pad}`; + const apptId = `d0000000-0000-0000-0000-0000000002${pad}`; + const geocodedAt = c.coords ? new Date(`${ROUTE_DATE}T00:00:00.000Z`) : null; + + await db.insert(schema.clients) + .values({ + id: clientId, + name: c.name, + email: `route-client-${pad}@uat.groombook.dev`, + phone: `(206) 555-01${pad}`, + address: `${100 + c.n} Pike Street, Seattle, WA 98101`, + status: "active", + latitude: c.coords?.lat ?? null, + longitude: c.coords?.lng ?? null, + geocodedAt, + }) + .onConflictDoUpdate({ + target: schema.clients.id, + set: { + name: c.name, + address: `${100 + c.n} Pike Street, Seattle, WA 98101`, + latitude: c.coords?.lat ?? null, + longitude: c.coords?.lng ?? null, + geocodedAt, + }, + }); + + await db.insert(schema.pets) + .values({ + id: petId, + clientId, + name: `Route Pup ${c.n}`, + species: "Dog", + breed: "Mixed", + weightKg: "18.00", + }) + .onConflictDoUpdate({ + target: schema.pets.id, + set: { clientId, name: `Route Pup ${c.n}`, species: "Dog" }, + }); + + const startTime = new Date(dayStartMs + (c.n - 1) * SLOT_MS); + const endTime = new Date(startTime.getTime() + SLOT_MS); + await db.insert(schema.appointments) + .values({ + id: apptId, + clientId, + petId, + serviceId, + staffId: uatGroomer.id, + batherStaffId: null, + status: "confirmed", + startTime, + endTime, + notes: "GRO-2225: deterministic route-optimization cohort appointment.", + priceCents: null, + confirmationStatus: "confirmed", + }) + .onConflictDoUpdate({ + target: schema.appointments.id, + set: { + clientId, + petId, + serviceId, + staffId: uatGroomer.id, + status: "confirmed", + startTime, + endTime, + }, + }); + + if (c.coords) geocodedCount++; + else ungeocodedCount++; + } + + console.log( + `✓ GRO-2225: seeded route cohort for ${ROUTE_DATE} — ${geocodedCount} geocoded + ${ungeocodedCount} un-geocoded appointment(s) for uat-groomer (${uatGroomer.id})`, + ); +} + // ── Known-users-only seed (prod/demo) ─────────────────────────────────────── /** @@ -1169,6 +1374,11 @@ async function runSeedBody( // the time seedUatStaffAccounts() returns). await seedUatGroomerLinkage(db, uatCustomerClientId); + // GRO-2225: deterministic pre-geocoded route cohort + fixed-date appointments + // for the UAT groomer. Must run AFTER services are seeded (it looks up a + // service id for the appointments). Skips cleanly if uat-groomer is absent. + await seedUatRouteCohort(db); + // ── Clients & Pets ── const now = new Date(); const appointmentsBackDate = new Date(now); diff --git a/src/__tests__/navigationExport.test.ts b/src/__tests__/navigationExport.test.ts new file mode 100644 index 0000000..902cb9d --- /dev/null +++ b/src/__tests__/navigationExport.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect } from "vitest"; +import { + buildGoogleMapsUrl, + buildAppleMapsUrl, + buildNavigationUrl, + intermediateWaypointCount, + GOOGLE_MAPS_MAX_WAYPOINTS, + APPLE_MAPS_MAX_WAYPOINTS, + type NavigationStop, +} from "../services/navigationExport.js"; + +function stops(n: number): NavigationStop[] { + return Array.from({ length: n }, (_, i) => ({ + latitude: 47 + i / 100, + longitude: -122 - i / 100, + label: `Stop ${i + 1}`, + })); +} + +describe("intermediateWaypointCount", () => { + it("excludes origin and destination", () => { + expect(intermediateWaypointCount(0)).toBe(0); + expect(intermediateWaypointCount(1)).toBe(0); + expect(intermediateWaypointCount(2)).toBe(0); + expect(intermediateWaypointCount(5)).toBe(3); + }); +}); + +describe("buildGoogleMapsUrl", () => { + it("rejects an empty route", () => { + const r = buildGoogleMapsUrl([]); + expect(r).toEqual({ error: "route has no stops to export", status: 400 }); + }); + + it("builds a single-stop link (destination only, no waypoints)", () => { + const r = buildGoogleMapsUrl(stops(1)); + if ("error" in r) throw new Error(r.error); + expect(r.platform).toBe("google-maps"); + expect(r.stopCount).toBe(1); + expect(r.waypointCount).toBe(0); + expect(r.url).toContain("https://www.google.com/maps/dir/?"); + expect(r.url).toContain("api=1"); + expect(r.url).toContain("travelmode=driving"); + expect(r.url).toContain("origin=47%2C-122"); + expect(r.url).toContain("destination=47%2C-122"); + expect(r.url).not.toContain("waypoints="); + }); + + it("builds origin/destination only for two stops", () => { + const r = buildGoogleMapsUrl(stops(2)); + if ("error" in r) throw new Error(r.error); + expect(r.waypointCount).toBe(0); + expect(r.url).not.toContain("waypoints="); + expect(r.url).toContain("origin=47%2C-122"); + expect(r.url).toContain("destination=47.01%2C-122.01"); + }); + + it("includes intermediate waypoints in order, pipe-separated", () => { + const r = buildGoogleMapsUrl(stops(4)); + if ("error" in r) throw new Error(r.error); + expect(r.stopCount).toBe(4); + expect(r.waypointCount).toBe(2); + // waypoints param holds stops[1] and stops[2], pipe-joined (encoded %7C) + const url = new URL(r.url); + expect(url.searchParams.get("origin")).toBe("47,-122"); + expect(url.searchParams.get("destination")).toBe("47.03,-122.03"); + expect(url.searchParams.get("waypoints")).toBe( + "47.01,-122.01|47.02,-122.02" + ); + }); + + it("accepts a route at exactly the waypoint limit", () => { + const r = buildGoogleMapsUrl(stops(GOOGLE_MAPS_MAX_WAYPOINTS + 2)); + if ("error" in r) throw new Error(r.error); + expect(r.waypointCount).toBe(GOOGLE_MAPS_MAX_WAYPOINTS); + }); + + it("rejects a route over the waypoint limit", () => { + const r = buildGoogleMapsUrl(stops(GOOGLE_MAPS_MAX_WAYPOINTS + 3)); + expect("error" in r).toBe(true); + if ("error" in r) { + expect(r.status).toBe(400); + expect(r.error).toContain(`${GOOGLE_MAPS_MAX_WAYPOINTS}`); + } + }); +}); + +describe("buildAppleMapsUrl", () => { + it("rejects an empty route", () => { + const r = buildAppleMapsUrl([]); + expect(r).toEqual({ error: "route has no stops to export", status: 400 }); + }); + + it("builds a destination-only link for one stop", () => { + const r = buildAppleMapsUrl(stops(1)); + if ("error" in r) throw new Error(r.error); + expect(r.platform).toBe("apple-maps"); + expect(r.url).toBe("maps://?daddr=47,-122&dirflg=d"); + expect(r.url).not.toContain("saddr="); + }); + + it("chains destinations with +to: for multiple stops", () => { + const r = buildAppleMapsUrl(stops(3)); + if ("error" in r) throw new Error(r.error); + expect(r.stopCount).toBe(3); + expect(r.waypointCount).toBe(1); + expect(r.url).toBe( + "maps://?saddr=47,-122&daddr=47.01,-122.01+to:47.02,-122.02&dirflg=d" + ); + }); + + it("accepts a route at exactly the waypoint limit", () => { + const r = buildAppleMapsUrl(stops(APPLE_MAPS_MAX_WAYPOINTS + 2)); + if ("error" in r) throw new Error(r.error); + expect(r.waypointCount).toBe(APPLE_MAPS_MAX_WAYPOINTS); + }); + + it("rejects a route over the waypoint limit", () => { + const r = buildAppleMapsUrl(stops(APPLE_MAPS_MAX_WAYPOINTS + 3)); + expect("error" in r).toBe(true); + if ("error" in r) { + expect(r.status).toBe(400); + expect(r.error).toContain(`${APPLE_MAPS_MAX_WAYPOINTS}`); + } + }); +}); + +describe("buildNavigationUrl", () => { + it("dispatches to the google-maps builder", () => { + const r = buildNavigationUrl("google-maps", stops(2)); + if ("error" in r) throw new Error(r.error); + expect(r.platform).toBe("google-maps"); + }); + + it("dispatches to the apple-maps builder", () => { + const r = buildNavigationUrl("apple-maps", stops(2)); + if ("error" in r) throw new Error(r.error); + expect(r.platform).toBe("apple-maps"); + }); +}); diff --git a/src/__tests__/portalWaitlistDuplicate.test.ts b/src/__tests__/portalWaitlistDuplicate.test.ts new file mode 100644 index 0000000..c0edbc6 --- /dev/null +++ b/src/__tests__/portalWaitlistDuplicate.test.ts @@ -0,0 +1,154 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; + +// GRO-2235: a duplicate active waitlist entry violates the partial unique index +// idx_waitlist_active_unique. postgres-js surfaces it as SQLSTATE 23505 — the +// handler must return a friendly 409, not a generic 500. The first insert still +// returns 201, and unrelated errors still surface as 500. + +const CLIENT_ID = "550e8400-e29b-41d4-a716-446655440001"; +const SESSION_ID = "770e8400-e29b-41d4-a716-446655440003"; +const PET_ID = "880e8400-e29b-41d4-a716-446655440004"; +const SERVICE_ID = "990e8400-e29b-41d4-a716-446655440005"; + +const futureDate = () => new Date(Date.now() + 30 * 60 * 1000); + +const ACTIVE_SESSION = { + id: SESSION_ID, + clientId: CLIENT_ID, + status: "active" as const, + reason: "manual", + startedAt: new Date(), + expiresAt: futureDate(), + createdAt: new Date(), +}; + +// Behaviour knob for the waitlist insert: "ok" returns a row, "duplicate" throws +// a postgres-js-shaped unique-violation, "other" throws an unrelated error. +let waitlistInsertMode: "ok" | "duplicate" | "other" = "ok"; + +function resetMock() { + waitlistInsertMode = "ok"; +} + +function tableProxy(name: string) { + return new Proxy( + { _name: name }, + { get: (t, p) => (p === "_name" ? name : { table: name, column: p }) } + ); +} + +vi.mock("@groombook/db", () => { + function makeChainable(data: unknown[]): unknown { + const arr = [...data]; + const chain = new Proxy(arr, { + get(target, prop) { + if (prop === "where" || prop === "orderBy" || prop === "limit") { + return () => chain; + } + // @ts-expect-error proxy + return target[prop]; + }, + }); + return chain; + } + + const impersonationSessions = tableProxy("impersonationSessions"); + const waitlistEntries = tableProxy("waitlistEntries"); + const impersonationAuditLogs = tableProxy("impersonationAuditLogs"); + + return { + getDb: () => ({ + select: () => ({ + from: (table: { _name: string }) => { + if (table._name === "impersonationSessions") { + return makeChainable([ACTIVE_SESSION]); + } + return makeChainable([]); + }, + }), + insert: (table: { _name: string }) => ({ + values: (vals: Record) => ({ + returning: () => { + if (table._name === "waitlistEntries") { + if (waitlistInsertMode === "duplicate") { + throw Object.assign(new Error("duplicate key value"), { code: "23505" }); + } + if (waitlistInsertMode === "other") { + throw Object.assign(new Error("not null violation"), { code: "23502" }); + } + return [{ id: "entry-1", ...vals }]; + } + // impersonationAuditLogs and anything else: succeed silently. + return [{ id: "audit-1", ...vals }]; + }, + }), + }), + update: () => ({ + set: () => ({ where: () => Promise.resolve() }), + }), + }), + impersonationSessions, + waitlistEntries, + impersonationAuditLogs, + appointments: tableProxy("appointments"), + clients: tableProxy("clients"), + pets: tableProxy("pets"), + services: tableProxy("services"), + staff: tableProxy("staff"), + invoices: tableProxy("invoices"), + invoiceLineItems: tableProxy("invoiceLineItems"), + eq: vi.fn(), + and: vi.fn(), + inArray: vi.fn(), + }; +}); + +const { portalRouter } = await import("../routes/portal.js"); + +const app = new Hono(); +app.route("/portal", portalRouter); + +function postWaitlist(body: unknown) { + return app.request("/portal/waitlist", { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-Impersonation-Session-Id": SESSION_ID, + }, + body: JSON.stringify(body), + }); +} + +const VALID_BODY = { + petId: PET_ID, + serviceId: SERVICE_ID, + preferredDate: "2026-07-01", + preferredTime: "09:00", +}; + +beforeEach(() => resetMock()); + +describe("POST /portal/waitlist duplicate handling (GRO-2235)", () => { + it("returns 201 for the first insert", async () => { + waitlistInsertMode = "ok"; + const res = await postWaitlist(VALID_BODY); + expect(res.status).toBe(201); + }); + + it("returns 409 with a friendly message for a duplicate (23505)", async () => { + waitlistInsertMode = "duplicate"; + const res = await postWaitlist(VALID_BODY); + expect(res.status).toBe(409); + const json = (await res.json()) as { error: string }; + expect(json.error).toBe( + "You already have a booking for this pet at that date and time." + ); + }); + + it("still surfaces unrelated DB errors as 500", async () => { + waitlistInsertMode = "other"; + const res = await postWaitlist(VALID_BODY); + expect(res.status).toBe(500); + }); +}); diff --git a/src/routes/portal.ts b/src/routes/portal.ts index d614e51..3c7dab9 100644 --- a/src/routes/portal.ts +++ b/src/routes/portal.ts @@ -596,16 +596,32 @@ portalRouter.post( const body = c.req.valid("json"); const clientId = c.get("portalClientId"); - const [entry] = await db - .insert(waitlistEntries) - .values({ - clientId, - petId: body.petId, - serviceId: body.serviceId, - preferredDate: body.preferredDate, - preferredTime: normalizeTime(body.preferredTime), - }) - .returning(); + let entry; + try { + [entry] = await db + .insert(waitlistEntries) + .values({ + clientId, + petId: body.petId, + serviceId: body.serviceId, + preferredDate: body.preferredDate, + preferredTime: normalizeTime(body.preferredTime), + }) + .returning(); + } catch (err) { + // An exact duplicate active waitlist entry violates the partial unique + // index idx_waitlist_active_unique (client_id, pet_id, service_id, + // preferred_date, preferred_time WHERE status='active'). postgres-js + // surfaces this as SQLSTATE 23505 — return a friendly 409 rather than a + // generic 500 (GRO-2235). Unrelated errors still surface as 500. + if ((err as { code?: string })?.code === "23505") { + return c.json( + { error: "You already have a booking for this pet at that date and time." }, + 409 + ); + } + throw err; + } return c.json(entry, 201); } diff --git a/src/routes/routes.ts b/src/routes/routes.ts index 3bf905a..898b16a 100644 --- a/src/routes/routes.ts +++ b/src/routes/routes.ts @@ -1,4 +1,4 @@ -import { Hono } from "hono"; +import { Hono, type Context } from "hono"; import { zValidator } from "@hono/zod-validator"; import { z } from "zod/v3"; import { @@ -24,6 +24,11 @@ import { type RouteStopInput, type StopConflictFlags, } from "../services/routeOptimization.js"; +import { + buildNavigationUrl, + type NavigationPlatform, + type NavigationStop, +} from "../services/navigationExport.js"; export const routesRouter = new Hono(); @@ -460,3 +465,65 @@ routesRouter.patch( }); } ); + +/** + * GET /:routeId/export/:platform — build a native-navigation deep-link URL for an + * optimized route. Origin = first stop, destination = last stop, the rest carried + * as ordered intermediate waypoints. Waypoint count is validated against the + * platform's limit. Auth: manager (any route) or groomer (own route only). + */ +async function handleNavigationExport( + c: Context, + platform: NavigationPlatform +) { + const db = getDb(); + const routeId = c.req.param("routeId"); + if (!routeId || !z.string().uuid().safeParse(routeId).success) { + return c.json({ error: "routeId must be a UUID" }, 400); + } + + const [route] = await db + .select() + .from(groomerRoutes) + .where(eq(groomerRoutes.id, routeId)); + if (!route) { + return c.json({ error: "Route not found" }, 404); + } + + // Reuse the groomer-own / manager authorization rule against the route owner. + const resolved = resolveTargetStaffId(c.get("staff"), route.staffId); + if ("error" in resolved) { + return c.json({ error: resolved.error }, resolved.status); + } + + const stops = await loadRouteStops(db, routeId); + if (stops.length === 0) { + return c.json({ error: "route has no stops to export" }, 400); + } + + const navStops: NavigationStop[] = stops.map((s) => ({ + latitude: s.latitude, + longitude: s.longitude, + label: s.clientName, + })); + + const result = buildNavigationUrl(platform, navStops); + if ("error" in result) { + return c.json({ error: result.error }, result.status); + } + + return c.json({ + platform: result.platform, + url: result.url, + stopCount: result.stopCount, + waypointCount: result.waypointCount, + }); +} + +routesRouter.get("/:routeId/export/google-maps", (c) => + handleNavigationExport(c, "google-maps") +); + +routesRouter.get("/:routeId/export/apple-maps", (c) => + handleNavigationExport(c, "apple-maps") +); diff --git a/src/services/navigationExport.ts b/src/services/navigationExport.ts new file mode 100644 index 0000000..ecac68a --- /dev/null +++ b/src/services/navigationExport.ts @@ -0,0 +1,155 @@ +// Navigation export — turn an optimized groomer route into a deep-link URL that +// opens the device's native navigation app (Google Maps / Apple Maps). +// +// A route is exported as: origin = first stop, destination = last stop, with the +// in-between stops carried as ordered intermediate waypoints. Each platform caps +// how many intermediate waypoints a deep link may carry, so callers must validate +// the route length before handing the URL to the client. + +/** + * Max intermediate waypoints a Google Maps URLs API deep link supports + * (`https://www.google.com/maps/dir/?api=1&...&waypoints=...`). Google documents + * a ceiling of 9 waypoints between origin and destination. + */ +export const GOOGLE_MAPS_MAX_WAYPOINTS = 9; + +/** + * Max intermediate waypoints we allow in an Apple Maps `maps://` deep link. Apple's + * URL scheme chains destinations with `+to:` but does not publish a hard cap; 15 is + * a conservative practical limit that keeps the URL well under length limits. + */ +export const APPLE_MAPS_MAX_WAYPOINTS = 15; + +export type NavigationPlatform = "google-maps" | "apple-maps"; + +/** A single ordered point on the route. `label` is optional, for display only. */ +export interface NavigationStop { + latitude: number; + longitude: number; + label?: string | null; +} + +export interface NavigationExportSuccess { + platform: NavigationPlatform; + url: string; + /** Total stops included (origin + waypoints + destination). */ + stopCount: number; + /** Intermediate waypoints only (excludes origin and destination). */ + waypointCount: number; +} + +export interface NavigationExportError { + error: string; + status: 400; +} + +export type NavigationExportResult = + | NavigationExportSuccess + | NavigationExportError; + +function isError(r: NavigationExportResult): r is NavigationExportError { + return "error" in r; +} + +/** Intermediate waypoints = every stop that is neither origin nor destination. */ +export function intermediateWaypointCount(stopCount: number): number { + return Math.max(0, stopCount - 2); +} + +function coord(stop: NavigationStop): string { + return `${stop.latitude},${stop.longitude}`; +} + +/** + * Builds a Google Maps URLs API driving deep link. On mobile this opens the + * native Google Maps app; on desktop it opens maps.google.com. + */ +export function buildGoogleMapsUrl( + stops: NavigationStop[] +): NavigationExportResult { + if (stops.length === 0) { + return { error: "route has no stops to export", status: 400 }; + } + const waypointCount = intermediateWaypointCount(stops.length); + if (waypointCount > GOOGLE_MAPS_MAX_WAYPOINTS) { + return { + error: `route has ${waypointCount} intermediate waypoints, exceeding Google Maps' limit of ${GOOGLE_MAPS_MAX_WAYPOINTS}`, + status: 400, + }; + } + + const origin = stops[0]!; + const destination = stops[stops.length - 1]!; + const params = new URLSearchParams(); + params.set("api", "1"); + params.set("travelmode", "driving"); + params.set("origin", coord(origin)); + params.set("destination", coord(destination)); + if (stops.length > 2) { + const mids = stops + .slice(1, -1) + .map(coord) + .join("|"); + params.set("waypoints", mids); + } + + return { + platform: "google-maps", + url: `https://www.google.com/maps/dir/?${params.toString()}`, + stopCount: stops.length, + waypointCount, + }; +} + +/** + * Builds an Apple Maps `maps://` driving deep link. The first stop is the source + * (`saddr`); the remaining stops are chained as destinations with `+to:` (`daddr`). + * Built by hand because the `+to:` separators are part of Apple's scheme and must + * not be percent-encoded. + */ +export function buildAppleMapsUrl( + stops: NavigationStop[] +): NavigationExportResult { + if (stops.length === 0) { + return { error: "route has no stops to export", status: 400 }; + } + const waypointCount = intermediateWaypointCount(stops.length); + if (waypointCount > APPLE_MAPS_MAX_WAYPOINTS) { + return { + error: `route has ${waypointCount} intermediate waypoints, exceeding Apple Maps' limit of ${APPLE_MAPS_MAX_WAYPOINTS}`, + status: 400, + }; + } + + const params: string[] = ["dirflg=d"]; + if (stops.length === 1) { + // Single stop: destination only, no source. + params.unshift(`daddr=${coord(stops[0]!)}`); + } else { + const daddr = stops + .slice(1) + .map(coord) + .join("+to:"); + params.unshift(`daddr=${daddr}`); + params.unshift(`saddr=${coord(stops[0]!)}`); + } + + return { + platform: "apple-maps", + url: `maps://?${params.join("&")}`, + stopCount: stops.length, + waypointCount, + }; +} + +/** Dispatches to the correct builder for the requested platform. */ +export function buildNavigationUrl( + platform: NavigationPlatform, + stops: NavigationStop[] +): NavigationExportResult { + return platform === "google-maps" + ? buildGoogleMapsUrl(stops) + : buildAppleMapsUrl(stops); +} + +export { isError as isNavigationExportError }; -- 2.52.0 From c7007051d7ac6d2d439ee40aacbd0124010ae70a Mon Sep 17 00:00:00 2001 From: Flea Flicker Date: Tue, 9 Jun 2026 06:12:42 +0000 Subject: [PATCH 12/12] GRO-2294: Route Optimization security hardening (LOW) Two defense-in-depth fixes from the GRO-2162 feature-level security review: 1. Enforce the documented ?limit cap on POST /api/clients/geocode-batch. The handler now clamps limit to GEOCODE_BATCH_MAX_LIMIT (500) after the positive-integer check, bounding synchronous request duration and per-request external API cost when routeOptimizationProvider = "google". 2. Redact the encrypted googleMapsApiKey from GET /api/admin/settings on both the existing-row and auto-create branches. The ciphertext is never needed client-side and is now stripped via redactSettings(). Adds route-level tests for the limit clamp (default/passthrough/clamp/floor/ reject) and the settings redaction (both branches). Updates UAT_PLAYBOOK.md TC-API-2.13a and TC-API-13.1. Co-Authored-By: Paperclip --- UAT_PLAYBOOK.md | 3 +- src/__tests__/geocodeBatchLimit.test.ts | 89 ++++++++++++++++++++++++ src/__tests__/settings.test.ts | 91 +++++++++++++++++++++++++ src/routes/clients.ts | 11 ++- src/routes/settings.ts | 16 ++++- 5 files changed, 206 insertions(+), 4 deletions(-) create mode 100644 src/__tests__/geocodeBatchLimit.test.ts create mode 100644 src/__tests__/settings.test.ts diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index 78b73f3..48082de 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -133,6 +133,7 @@ Geocoding turns a client's street address into `latitude`/`longitude` + `geocode | TC-API-2.11 | Geocode endpoint is manager-only | As **groomer** or **receptionist**, `POST /api/clients/{id}/geocode` | 403 Forbidden (role not permitted) | | TC-API-2.12 | Batch geocode un-geocoded clients | As manager, `POST /api/clients/geocode-batch?limit=10` on a DB with un-geocoded clients | 200 OK; body `{ provider, processed, geocoded, unresolved, errors, remaining, outcomes[] }`. `processed` ≤ 10; `remaining` reflects un-geocoded clients beyond this batch. Re-run while `remaining > 0` to finish (throttled to provider rate limit) | | TC-API-2.13 | Batch geocode — invalid limit | As manager, `POST /api/clients/geocode-batch?limit=0` (or non-numeric) | 400 `{ error: "limit must be a positive integer" }` | +| TC-API-2.13a | Batch geocode — `?limit` cap enforced (GRO-2294) | As manager, `POST /api/clients/geocode-batch?limit=100000` on a DB with un-geocoded clients | 200 OK; the request is **clamped to the documented max of 500** — `processed` ≤ 500 (never the raw 100000). A fractional `?limit` (e.g. `49.9`) is floored to `49`. Confirms a manager cannot hold one synchronous request open / accrue unbounded Google API cost via an oversized limit | | TC-API-2.14 | Batch geocode — manager-only | As groomer/receptionist, `POST /api/clients/geocode-batch` | 403 Forbidden | | TC-API-2.15 | Auto-geocode on create | As manager/receptionist, `POST /api/clients` with a valid `address` | 201 Created; response includes a `geocoding` object (`status: "geocoded"` for a resolvable address) and the persisted client carries `latitude`/`longitude`/`geocodedAt`. Creating without an address succeeds with no `geocoding` field | | TC-API-2.16 | Auto-geocode on address update | As manager/receptionist, `PATCH /api/clients/{id}` changing `address` to a new valid value | 200 OK; response includes a `geocoding` object and refreshed coordinates. Patching unrelated fields (e.g. `name`) does NOT re-geocode (no `geocoding` field) | @@ -331,7 +332,7 @@ This means: | # | Scenario | Steps | Expected | |---|----------|-------|----------| -| TC-API-13.1 | Get business settings | GET /api/admin/settings | 200 OK, business settings returned | +| TC-API-13.1 | Get business settings | GET /api/admin/settings | 200 OK, business settings returned. Response body **must NOT include `googleMapsApiKey`** — the encrypted secret is redacted from the projection (GRO-2294, defense-in-depth); non-secret fields (`businessName`, colors, `routeOptimizationProvider`, etc.) are still present | | TC-API-13.2 | Update business settings | PATCH /api/admin/settings with updated values | 200 OK, settings updated | | TC-API-13.3 | Upload logo | POST /api/admin/settings/logo/upload with file | 200 OK, logo uploaded and stored | | TC-API-13.4 | View logo | GET /api/admin/settings/logo | 200 OK, logo image returned | diff --git a/src/__tests__/geocodeBatchLimit.test.ts b/src/__tests__/geocodeBatchLimit.test.ts new file mode 100644 index 0000000..8731c02 --- /dev/null +++ b/src/__tests__/geocodeBatchLimit.test.ts @@ -0,0 +1,89 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; + +// ─── Mocks ────────────────────────────────────────────────────────────────── +// GRO-2294: the POST /clients/geocode-batch handler must clamp ?limit to the +// documented maximum (500) before invoking the geocoding service. We mock the +// service to capture the exact limit the route forwards. + +const geocodeUngeocodedClients = vi.fn(async () => ({ + totalRemaining: 0, + processed: 0, + geocoded: 0, + failed: 0, + remaining: 0, +})); + +vi.mock("../services/clientGeocoding.js", () => ({ + geocodeUngeocodedClients, + geocodeClient: vi.fn(), + resolveClientGeocodingProvider: vi.fn(), +})); + +vi.mock("@groombook/db", () => { + const tableProxy = (name: string) => + new Proxy( + { _name: name }, + { get: (_t, p) => (p === "_name" ? name : { table: name, column: p }) } + ); + return { + getDb: () => ({}), + clients: tableProxy("clients"), + appointments: tableProxy("appointments"), + and: vi.fn(), + eq: vi.fn(), + or: vi.fn(), + exists: vi.fn(), + }; +}); + +const { clientsRouter } = await import("../routes/clients.js"); + +const app = new Hono(); +app.route("/clients", clientsRouter); + +function postBatch(query: string) { + return app.request(`/clients/geocode-batch${query}`, { method: "POST" }); +} + +describe("POST /clients/geocode-batch — ?limit cap (GRO-2294)", () => { + beforeEach(() => { + geocodeUngeocodedClients.mockClear(); + }); + + it("defaults to 50 when no ?limit is supplied", async () => { + const res = await postBatch(""); + expect(res.status).toBe(200); + expect(geocodeUngeocodedClients).toHaveBeenCalledWith(expect.anything(), 50); + }); + + it("passes through a value within the cap", async () => { + const res = await postBatch("?limit=120"); + expect(res.status).toBe(200); + expect(geocodeUngeocodedClients).toHaveBeenCalledWith(expect.anything(), 120); + }); + + it("clamps an over-cap value to 500", async () => { + const res = await postBatch("?limit=100000"); + expect(res.status).toBe(200); + expect(geocodeUngeocodedClients).toHaveBeenCalledWith(expect.anything(), 500); + }); + + it("floors a fractional value before clamping", async () => { + const res = await postBatch("?limit=49.9"); + expect(res.status).toBe(200); + expect(geocodeUngeocodedClients).toHaveBeenCalledWith(expect.anything(), 49); + }); + + it("rejects a non-positive limit with 400", async () => { + const res = await postBatch("?limit=0"); + expect(res.status).toBe(400); + expect(geocodeUngeocodedClients).not.toHaveBeenCalled(); + }); + + it("rejects a non-numeric limit with 400", async () => { + const res = await postBatch("?limit=abc"); + expect(res.status).toBe(400); + expect(geocodeUngeocodedClients).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/settings.test.ts b/src/__tests__/settings.test.ts new file mode 100644 index 0000000..c878999 --- /dev/null +++ b/src/__tests__/settings.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; + +// ─── Mocks ────────────────────────────────────────────────────────────────── +// GRO-2294: GET /api/admin/settings must not return the encrypted +// googleMapsApiKey ciphertext, on either the existing-row or auto-create branch. + +let selectRows: Record[] = []; +let insertReturning: Record[] = []; + +function makeChainable(data: unknown[]): unknown { + const arr = [...data]; + const chain = new Proxy(arr, { + get(target, prop) { + if (prop === "where" || prop === "orderBy" || prop === "limit") { + return () => chain; + } + // @ts-expect-error proxy passthrough + return target[prop]; + }, + }); + return chain; +} + +vi.mock("@groombook/db", () => { + const businessSettings = new Proxy( + { _name: "business_settings" }, + { get: (_t, p) => (p === "_name" ? "business_settings" : { column: p }) } + ); + return { + getDb: () => ({ + select: () => ({ from: () => makeChainable(selectRows) }), + insert: () => ({ + values: () => ({ returning: () => insertReturning }), + }), + }), + businessSettings, + eq: vi.fn(), + }; +}); + +vi.mock("../lib/s3.js", () => ({ + getPresignedUploadUrl: vi.fn(), + deleteObject: vi.fn(), + putObject: vi.fn(), + getObject: vi.fn(), +})); + +const { settingsRouter } = await import("../routes/settings.js"); + +const app = new Hono(); +app.route("/settings", settingsRouter); + +const FULL_ROW = { + id: "settings-uuid-1", + businessName: "GroomBook", + primaryColor: "#4f8a6f", + accentColor: "#8b7355", + routeOptimizationProvider: "google", + googleMapsApiKey: "ENCRYPTED::super-secret-ciphertext", + createdAt: new Date(), + updatedAt: new Date(), +}; + +describe("GET /settings — googleMapsApiKey redaction (GRO-2294)", () => { + beforeEach(() => { + selectRows = []; + insertReturning = []; + }); + + it("omits googleMapsApiKey from an existing settings row", async () => { + selectRows = [{ ...FULL_ROW }]; + const res = await app.request("/settings", { method: "GET" }); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body).not.toHaveProperty("googleMapsApiKey"); + // Non-secret fields are still returned. + expect(body.businessName).toBe("GroomBook"); + expect(body.routeOptimizationProvider).toBe("google"); + }); + + it("omits googleMapsApiKey from the auto-create branch", async () => { + selectRows = []; + insertReturning = [{ ...FULL_ROW, id: "settings-uuid-new" }]; + const res = await app.request("/settings", { method: "GET" }); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body).not.toHaveProperty("googleMapsApiKey"); + expect(body.id).toBe("settings-uuid-new"); + }); +}); diff --git a/src/routes/clients.ts b/src/routes/clients.ts index e7ac65c..328ed31 100644 --- a/src/routes/clients.ts +++ b/src/routes/clients.ts @@ -12,6 +12,12 @@ import { export const clientsRouter = new Hono(); +// Batch-geocode bounds (GRO-2294): default 50, hard cap 500. The cap bounds how +// long one synchronous request stays open and the per-request external API cost +// when routeOptimizationProvider = "google". +const GEOCODE_BATCH_DEFAULT_LIMIT = 50; +const GEOCODE_BATCH_MAX_LIMIT = 500; + type ClientRow = typeof clients.$inferSelect; /** @@ -185,12 +191,15 @@ clientsRouter.post("/:clientId/geocode", async (c) => { clientsRouter.post("/geocode-batch", async (c) => { const db = getDb(); const limitRaw = c.req.query("limit"); - let limit = 50; + let limit = GEOCODE_BATCH_DEFAULT_LIMIT; if (limitRaw !== undefined) { limit = Number(limitRaw); if (!Number.isFinite(limit) || limit <= 0) { return c.json({ error: "limit must be a positive integer" }, 400); } + // Clamp to the documented maximum to bound synchronous request duration + // and (for the Google provider) per-request external API cost. + limit = Math.min(Math.floor(limit), GEOCODE_BATCH_MAX_LIMIT); } const summary = await geocodeUngeocodedClients(db, limit); return c.json(summary); diff --git a/src/routes/settings.ts b/src/routes/settings.ts index 3b931db..8529135 100644 --- a/src/routes/settings.ts +++ b/src/routes/settings.ts @@ -7,6 +7,17 @@ import { requireSuperUser } from "../middleware/rbac.js"; export const settingsRouter = new Hono(); +type BusinessSettingsRow = typeof businessSettings.$inferSelect; + +// Strip the encrypted googleMapsApiKey ciphertext from settings responses +// (GRO-2294, defense-in-depth). The secret is never needed client-side; it is +// only written via the dedicated provider-config endpoint. +function redactSettings(row: BusinessSettingsRow) { + const rest: Partial = { ...row }; + delete rest.googleMapsApiKey; + return rest; +} + // GET /api/admin/settings — return current business settings settingsRouter.get("/", async (c) => { const db = getDb(); @@ -14,9 +25,10 @@ settingsRouter.get("/", async (c) => { if (!row) { // Auto-create default settings if none exist const [created] = await db.insert(businessSettings).values({}).returning(); - return c.json(created); + if (!created) throw new Error("Failed to create default settings"); + return c.json(redactSettings(created)); } - return c.json(row); + return c.json(redactSettings(row)); }); const hexColorRegex = /^#[0-9a-fA-F]{6}$/; -- 2.52.0