Merge pull request 'Promote dev→uat: GRO-2033 services_pkey seed fix (fc6c6ef7)' (#149) from dev into uat

UAT_PLAYBOOK.md

@@ -192,6 +192,33 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the
 | TC-API-5.4 | Update service | PATCH /api/services/{id} with updated fields | 200 OK, service updated |
 | TC-API-5.5 | Delete service | DELETE /api/services/{id} | 200 OK, service deleted |
 
+#### 4.5.1 Seed/Reset idempotency (GRO-2064)
+
+Services seeding is now keyed on the deterministic `services.id` (not `name`) and
+the reset path now `TRUNCATE`s `services` alongside the other dynamic tables.
+This means:
+
+- Running the seed Job twice in a row (no reset in between) converges to the
+  same catalogue — no `services_pkey` collision.
+- A `pnpm reset` followed by `pnpm seed` (or a CronJob reset fire) leaves the
+  catalogue exactly matching `servicesDef` (10 rows, ids `b0000001-…-001` …
+  `…-00a`), regardless of any stale rows that were present beforehand.
+- Mixed `seedKnownUsers` + full `seed()` invocations are safe — the
+  `demoSvcs` subset (Bath & Brush, Full Groom Small/Medium, Nail Trim) is
+  keyed on ids `…-001`, `…-002`, `…-003`, `…-005` and the upsert target
+  is `services.id`, so the same-id / different-name collision that broke
+  GRO-2033 (id `…-004` = "Nail Trim" vs servicesDef `…-004` =
+  "Full Groom — Large") cannot recur.
+
+**UAT regression** (verify after a new image is rolled out):
+
+| # | Scenario | Steps | Expected |
+|---|----------|-------|----------|
+| TC-SEED-1 | Reset → seed converges | `kubectl -n groombook exec deploy/api -- pnpm reset && pnpm seed` | Seed completes 1/1, `services` count = 10, all ids match `servicesDef` |
+| TC-SEED-2 | Idempotent re-seed | Re-run `pnpm seed` without reset | Seed completes 1/1, no `services_pkey` errors, `services` count still 10 |
+| TC-SEED-3 | Catalogue matches servicesDef | `psql -c "SELECT id, name FROM services ORDER BY id"` | Rows `…-001`…`…-00a` with names "Bath & Brush"…"Sanitary Trim" exactly as in `servicesDef` |
+| TC-SEED-4 | Demo subset coexists | Run `seedKnownUsers` then full `seed` | No collision, demo subset (4 services) ends up with the same rows the full seed would write |
+
 ### 4.6 Staff Management
 
 | # | Scenario | Steps | Expected |

apps/api/src/db/seed.ts

@@ -636,21 +636,28 @@ async function seedKnownUsers() {
     }
   }
 
-  // ── Services: idempotent upsert using name as unique key ─────────────────────
+  // ── Services: idempotent upsert keyed on `id` ─────────────────────────────
-  // UNIQUE constraint on services.name (migration 0020) must exist first.
+  // GRO-2064: previously keyed on `services.name` while writing a
-  // Uses b0000001-... IDs to match main seed servicesDef for same-named services.
+  // deterministic `id`. If a stale row existed with the same `id` but a
+  // different `name`, PostgreSQL raised `services_pkey` (id collision)
+  // before the name-targeted ON CONFLICT could fire. Switch the conflict
+  // target to `services.id` so deterministic ids always win; pair with
+  // `TRUNCATE services … CASCADE` above so each reset rebuilds the
+  // catalogue from `servicesDef` cleanly. GRO-2033 close-out.
+  // Id↔name map MUST stay in sync with `servicesDef` (the canonical source
+  // of truth in the main `seed()` function).
   const demoSvcs = [
     { id: "b0000001-0000-0000-0000-000000000001", name: "Bath & Brush", description: "Full bath, blow-dry, brush out, and ear cleaning", basePriceCents: 4500, durationMinutes: 45 },
     { id: "b0000001-0000-0000-0000-000000000002", name: "Full Groom — Small", description: "Complete grooming for dogs under 25 lbs", basePriceCents: 6500, durationMinutes: 60 },
     { id: "b0000001-0000-0000-0000-000000000003", name: "Full Groom — Medium", description: "Complete grooming for dogs 25-50 lbs", basePriceCents: 8000, durationMinutes: 75 },
-    { id: "b0000001-0000-0000-0000-000000000004", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
+    { id: "b0000001-0000-0000-0000-000000000005", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
   ];
   for (const svc of demoSvcs) {
     await db.insert(schema.services)
       .values({ ...svc, active: true })
       .onConflictDoUpdate({
-        target: schema.services.name,
+        target: schema.services.id,
-        set: { description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
+        set: { name: svc.name, description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
       });
   }
   console.log(`✓ Seeded ${demoSvcs.length} services`);
@@ -757,7 +764,13 @@ async function seed() {
     ({ id: uuid(), name: `Bather ${i + 1}`, email: `bather${i + 1}@groombook.dev`, role: "groomer" as const, isSuperUser: false })
   );
 
-  await db.execute(sql`TRUNCATE impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
+  // GRO-2064: also TRUNCATE `services` so each reset rebuilds the catalogue
+  // from `servicesDef` (deterministic IDs + UNIQUE(name)). Stale service rows
+  // (e.g. a prior `seedKnownUsers` run that wrote a different `name` for the
+  // same `id`) would otherwise cause the deterministic upsert to PK-collide
+  // on `services.id` — see CTO review on infra PR #605 (rev #4230). TRUNCATE
+  // CASCADE handles appointments/invoices FKs to services.id.
+  await db.execute(sql`TRUNCATE services, impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
 
   const allStaff = [...managerStaff, ...receptionistStaff, ...groomers, ...bathers];
   for (const s of allStaff) {
@@ -828,9 +841,11 @@ async function seed() {
   }
 
   // ── Services ──
-  // Upsert services using name as unique key. With deterministic IDs in
+  // GRO-2064: key the upsert on `services.id` (not `name`) so deterministic
-  // servicesDef and TRUNCATE clearing downstream tables first, this is
+  // ids always win, and rely on the TRUNCATE above to clear stale rows before
-  // idempotent: first run inserts, subsequent runs update existing rows.
+  // the catalogue is rebuilt. The previous name-targeted upsert failed with
+  // `services_pkey` when a prior run had left a row with the same id but a
+  // different name (CTO review on infra PR #605, rev #4230).
   const serviceIds: string[] = [];
   for (const s of servicesDef) {
     serviceIds.push(s.id);
@@ -844,8 +859,8 @@ async function seed() {
         active: true,
       })
       .onConflictDoUpdate({
-        target: schema.services.name,
+        target: schema.services.id,
-        set: { description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
+        set: { name: s.name, description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
       });
   }
   console.log(`✓ Created ${servicesDef.length} services`);

packages/db/src/seed.ts

@@ -747,21 +747,28 @@ async function seedKnownUsers() {
   // and the full seed() UAT branch.
   await seedUatStaffAccounts(db);
 
-  // ── Services: idempotent upsert using name as unique key ─────────────────────
+  // ── Services: idempotent upsert keyed on `id` ─────────────────────────────
-  // UNIQUE constraint on services.name (migration 0020) must exist first.
+  // GRO-2064: previously keyed on `services.name` while writing a
-  // Uses b0000001-... IDs to match main seed servicesDef for same-named services.
+  // deterministic `id`. If a stale row existed with the same `id` but a
+  // different `name`, PostgreSQL raised `services_pkey` (id collision)
+  // before the name-targeted ON CONFLICT could fire. Switch the conflict
+  // target to `services.id` so deterministic ids always win; pair with
+  // `TRUNCATE services … CASCADE` above so each reset rebuilds the
+  // catalogue from `servicesDef` cleanly. GRO-2033 close-out.
+  // Id↔name map MUST stay in sync with `servicesDef` (the canonical source
+  // of truth in the main `seed()` function).
   const demoSvcs = [
     { id: "b0000001-0000-0000-0000-000000000001", name: "Bath & Brush", description: "Full bath, blow-dry, brush out, and ear cleaning", basePriceCents: 4500, durationMinutes: 45 },
     { id: "b0000001-0000-0000-0000-000000000002", name: "Full Groom — Small", description: "Complete grooming for dogs under 25 lbs", basePriceCents: 6500, durationMinutes: 60 },
     { id: "b0000001-0000-0000-0000-000000000003", name: "Full Groom — Medium", description: "Complete grooming for dogs 25-50 lbs", basePriceCents: 8000, durationMinutes: 75 },
-    { id: "b0000001-0000-0000-0000-000000000004", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
+    { id: "b0000001-0000-0000-0000-000000000005", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 },
   ];
   for (const svc of demoSvcs) {
     await db.insert(schema.services)
       .values({ ...svc, active: true })
       .onConflictDoUpdate({
-        target: schema.services.name,
+        target: schema.services.id,
-        set: { description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
+        set: { name: svc.name, description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true },
       });
   }
   console.log(`✓ Seeded ${demoSvcs.length} services`);
@@ -868,7 +875,13 @@ async function seed() {
     ({ id: uuid(), name: `Bather ${i + 1}`, email: `bather${i + 1}@groombook.dev`, role: "groomer" as const, isSuperUser: false })
   );
 
-  await db.execute(sql`TRUNCATE impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
+  // GRO-2064: also TRUNCATE `services` so each reset rebuilds the catalogue
+  // from `servicesDef` (deterministic IDs + UNIQUE(name)). Stale service rows
+  // (e.g. a prior `seedKnownUsers` run that wrote a different `name` for the
+  // same `id`) would otherwise cause the deterministic upsert to PK-collide
+  // on `services.id` — see CTO review on infra PR #605 (rev #4230). TRUNCATE
+  // CASCADE handles appointments/invoices FKs to services.id.
+  await db.execute(sql`TRUNCATE services, impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`);
 
   const allStaff = [...managerStaff, ...receptionistStaff, ...groomers, ...bathers];
   for (const s of allStaff) {
@@ -919,9 +932,11 @@ async function seed() {
   await seedUatStaffAccounts(db);
 
   // ── Services ──
-  // Upsert services using name as unique key. With deterministic IDs in
+  // GRO-2064: key the upsert on `services.id` (not `name`) so deterministic
-  // servicesDef and TRUNCATE clearing downstream tables first, this is
+  // ids always win, and rely on the TRUNCATE above to clear stale rows before
-  // idempotent: first run inserts, subsequent runs update existing rows.
+  // the catalogue is rebuilt. The previous name-targeted upsert failed with
+  // `services_pkey` when a prior run had left a row with the same id but a
+  // different name (CTO review on infra PR #605, rev #4230).
   const serviceIds: string[] = [];
   for (const s of servicesDef) {
     serviceIds.push(s.id);
@@ -935,8 +950,8 @@ async function seed() {
         active: true,
       })
       .onConflictDoUpdate({
-        target: schema.services.name,
+        target: schema.services.id,
-        set: { description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
+        set: { name: s.name, description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true },
       });
   }
   console.log(`✓ Created ${servicesDef.length} services`);