Merge pull request #304 from groombook/docs/branch-strategy-contributing

docs: add CONTRIBUTING.md with branch strategy (GRO-702)

CONTRIBUTING.md

@@ -0,0 +1,90 @@
+# Contributing to GroomBook
+
+## Branch Strategy
+
+GroomBook uses a three-branch GitOps model:
+
+| Branch | Environment | Purpose |
+|--------|-------------|---------|
+| `dev` | Development | Active development target — all feature/fix PRs target this branch |
+| `uat` | UAT / Staging | Promoted from `dev` by the CTO for acceptance testing |
+| `main` | Production | Promoted from `uat` by the CEO; triggers production deployment |
+
+**Never open a PR directly to `uat` or `main`.** All work flows through `dev` first.
+
+## Developer Workflow
+
+1. **Branch from `dev`** — create a feature or fix branch:
+   ```bash
+   git checkout dev
+   git pull origin dev
+   git checkout -b feat/my-feature
+   ```
+
+2. **Open a PR targeting `dev`** — include the issue identifier in the title and cc @cpfarhood:
+   ```bash
+   gh pr create --base dev --title "feat: description (GRO-NNN)" \
+     --body $'Closes GRO-NNN\n\ncc @cpfarhood'
+   ```
+
+3. **Pipeline gates before merge to `dev`:**
+   - QA (Lint Roller) reviews first — code quality, test coverage, CI pass
+   - CTO (The Dogfather) reviews second — architecture and final approval
+   - Both must approve; 2 approving reviews required by branch protection
+
+## Promotion Flow
+
+### Dev → UAT
+
+After merging to `dev`, the CTO opens a PR from `dev` → `uat`:
+
+```bash
+gh pr create --base uat --head dev \
+  --title "chore: promote dev to uat (YYYY.MM.DD)" \
+  --body $'Promoting dev to UAT for regression and security review.\n\ncc @cpfarhood'
+```
+
+Gates:
+- Shedward Scissorhands runs regression/acceptance tests
+- Barkley Trimsworth performs security review
+- CTO approves and merges (1 approving review required)
+
+### UAT → Main (Production)
+
+After UAT passes, the CTO opens a PR from `uat` → `main` and assigns it to the CEO:
+
+```bash
+gh pr create --base main --head uat \
+  --title "chore: promote uat to main (YYYY.MM.DD)" \
+  --body $'Promoting UAT to production.\n\ncc @cpfarhood'
+```
+
+Gates:
+- CEO (Scrubs McBarkley) reviews for business alignment and merges
+- 1 approving review required; triggers auto-deploy to Production
+
+## Branch Protection Summary
+
+| Branch | Required Approvals | Who approves |
+|--------|--------------------|-------------|
+| `dev` | 2 | QA (Lint Roller) + CTO (The Dogfather) |
+| `uat` | 1 | CTO (The Dogfather) |
+| `main` | 1 | CEO (Scrubs McBarkley) |
+
+Force-pushes and branch deletions are disabled on all three branches.
+
+## Commit Style
+
+Use [Conventional Commits](https://www.conventionalcommits.org/):
+- `feat:` — new feature
+- `fix:` — bug fix
+- `chore:` — maintenance (dependency updates, build config, promotions)
+- `docs:` — documentation only
+- `ci:` — CI/CD changes
+- `refactor:` — code restructure without behaviour change
+
+Reference the Paperclip issue in the commit body: `Refs GRO-NNN`.
+
+## Questions?
+
+Open a Paperclip issue in the GRO project or ask in the team channel.

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

@@ -195,10 +195,11 @@ describe("POST /clients", () => {
     expect(insertedValues[0]!.name).toBe("Charlie");
   });
 
-  it("creates a client with only required name field", async () => {
-    const res = await jsonRequest("POST", "/clients", { name: "Dana" });
+  it("creates a client with name and email", async () => {
+    const res = await jsonRequest("POST", "/clients", { name: "Dana", email: "dana@example.com" });
     expect(res.status).toBe(201);
     expect(insertedValues[0]!.name).toBe("Dana");
+    expect(insertedValues[0]!.email).toBe("dana@example.com");
   });
 
   it("rejects empty name", async () => {

apps/api/src/lib/auth.ts

@@ -204,15 +204,11 @@ export async function initAuth(): Promise<void> {
         const userInfoUrl = discovery.userinfo_endpoint;
         if (authzUrl && tokenUrl && userInfoUrl) {
           const authzUrlObj = new URL(authzUrl);
-          const tokenUrlObj = new URL(tokenUrl);
-          const userInfoUrlObj = new URL(userInfoUrl);
-          if (
-            authzUrlObj.hostname !== issuerHostname ||
-            tokenUrlObj.hostname !== issuerHostname ||
-            userInfoUrlObj.hostname !== issuerHostname
-          ) {
+          // Only validate authorizationUrl hostname against issuer — token/userinfo
+          // may legitimately use internal hostnames (OIDC_INTERNAL_BASE) for server-to-server calls.
+          if (authzUrlObj.hostname !== issuerHostname) {
             throw new Error(
-              `[FATAL] OIDC discovery URL hostname mismatch: expected '${issuerHostname}' but got '${authzUrlObj.hostname}', '${tokenUrlObj.hostname}', or '${userInfoUrlObj.hostname}'. This may indicate a man-in-the-middle attack.`
+              `[FATAL] OIDC discovery URL hostname mismatch: expected '${issuerHostname}' but got '${authzUrlObj.hostname}'. This may indicate a man-in-the-middle attack.`
             );
           }
           oidcConfig = {

apps/api/src/routes/appointments.ts

@@ -338,44 +338,35 @@ async function sendConfirmationEmail(
   db: ReturnType<typeof getDb>,
   appt: typeof appointments.$inferSelect
 ): Promise<void> {
-  const [client] = await db
-    .select({ name: clients.name, email: clients.email, emailOptOut: clients.emailOptOut })
-    .from(clients)
-    .where(eq(clients.id, appt.clientId))
+  const [row] = await db
+    .select({
+      clientName: clients.name,
+      clientEmail: clients.email,
+      clientEmailOptOut: clients.emailOptOut,
+      petName: pets.name,
+      serviceName: services.name,
+      groomerName: staff.name,
+    })
+    .from(appointments)
+    .innerJoin(clients, eq(clients.id, appointments.clientId))
+    .innerJoin(pets, eq(pets.id, appointments.petId))
+    .innerJoin(services, eq(services.id, appointments.serviceId))
+    .leftJoin(staff, eq(staff.id, appointments.staffId))
+    .where(eq(appointments.id, appt.id))
     .limit(1);
 
-  if (!client || !client.email || client.emailOptOut) return;
+  if (!row) return;
+  const { clientName, clientEmail, clientEmailOptOut, petName, serviceName, groomerName } = row;
 
-  const [pet] = await db
-    .select({ name: pets.name })
-    .from(pets)
-    .where(eq(pets.id, appt.petId))
-    .limit(1);
-
-  const [service] = await db
-    .select({ name: services.name })
-    .from(services)
-    .where(eq(services.id, appt.serviceId))
-    .limit(1);
-
-  let groomerName: string | null = null;
-  if (appt.staffId) {
-    const [groomer] = await db
-      .select({ name: staff.name })
-      .from(staff)
-      .where(eq(staff.id, appt.staffId))
-      .limit(1);
-    groomerName = groomer?.name ?? null;
-  }
-
-  if (!pet || !service) return;
+  if (!clientEmail || clientEmailOptOut) return;
+  if (!petName || !serviceName) return;
 
   const sent = await sendEmail(
-    buildConfirmationEmail(client.email, {
-      clientName: client.name,
-      petName: pet.name,
-      serviceName: service.name,
-      groomerName,
+    buildConfirmationEmail(clientEmail, {
+      clientName,
+      petName,
+      serviceName,
+      groomerName: groomerName ?? null,
       startTime: appt.startTime,
     })
   );

apps/api/src/routes/clients.ts

@@ -8,7 +8,7 @@ export const clientsRouter = new Hono<AppEnv>();
 
 const createClientSchema = z.object({
   name: z.string().min(1).max(200),
-  email: z.string().email().optional(),
+  email: z.string().email(),
   phone: z.string().max(50).optional(),
   address: z.string().max(500).optional(),
   notes: z.string().max(2000).optional(),

packages/db/migrations/0028_sms_reminders.sql

@@ -1,12 +1,15 @@
--- SMS opt-in fields for clients
-ALTER TABLE "clients" ADD COLUMN "sms_opt_in" boolean NOT NULL DEFAULT false;
-ALTER TABLE "clients" ADD COLUMN "sms_consent_date" timestamp;
-ALTER TABLE "clients" ADD COLUMN "sms_opt_out_date" timestamp;
-ALTER TABLE "clients" ADD COLUMN "sms_consent_text" text;
+-- SMS opt-in fields for clients (idempotent)
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_opt_in" boolean NOT NULL DEFAULT false;
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_consent_date" timestamp;
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_opt_out_date" timestamp;
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_consent_text" text;
 
--- Add channel column to reminder_logs with default 'email'
-ALTER TABLE "reminder_logs" ADD COLUMN "channel" text NOT NULL DEFAULT 'email';
+-- Add channel column to reminder_logs with default 'email' (idempotent)
+ALTER TABLE "reminder_logs" ADD COLUMN IF NOT EXISTS "channel" text NOT NULL DEFAULT 'email';
 
--- Drop the old unique constraint and recreate with channel
-ALTER TABLE "reminder_logs" DROP CONSTRAINT "reminder_logs_appointment_id_reminder_type_unique";
+-- Drop old unique constraints if they exist (idempotent)
+ALTER TABLE "reminder_logs" DROP CONSTRAINT IF EXISTS "reminder_logs_appointment_id_reminder_type_key";
+ALTER TABLE "reminder_logs" DROP CONSTRAINT IF EXISTS "reminder_logs_appointment_id_reminder_type_unique";
+
+-- Add new unique constraint with channel
 ALTER TABLE "reminder_logs" ADD CONSTRAINT "reminder_logs_appointment_id_reminder_type_channel_unique" UNIQUE ("appointment_id", "reminder_type", "channel");

packages/db/src/seed.ts

@@ -462,6 +462,37 @@ async function seedKnownUsers() {
     }
   }
 
+  // ── 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) ?? [];
+  const groomerCount = Math.min(groomerEmails.length, groomerNames.length);
+  for (let i = 0; i < groomerCount; i++) {
+    const email = groomerEmails[i]!;
+    const name = groomerNames[i]!;
+    // Use deterministic IDs in the 00000000-0000-0000-0000-000000000005+ range
+    const staffId = `00000000-0000-0000-0000-${String(5 + i).padStart(12, "0")}`;
+    const [existingGroomer] = await db
+      .select()
+      .from(schema.staff)
+      .where(eq(schema.staff.email, email))
+      .limit(1);
+
+    if (existingGroomer) {
+      console.log(`✓ Staff groomer '${existingGroomer.name}' already exists — skipping`);
+    } else {
+      await db.insert(schema.staff).values({
+        id: staffId,
+        name,
+        email,
+        oidcSub: email,
+        role: "groomer",
+        isSuperUser: false,
+        active: true,
+      });
+      console.log(`✓ Created staff groomer '${name}' (${email})`);
+    }
+  }
+
   // ── Services: idempotent upsert using name as unique key ─────────────────────
   // UNIQUE constraint on services.name (migration 0020) must exist first.
   // Uses b0000001-... IDs to match main seed servicesDef for same-named services.
@@ -629,6 +660,31 @@ async function seed() {
     console.log(`✓ Upserted admin staff '${adminName}' (${adminEmail})`);
   }
 
+  // ── 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) ?? [];
+  const groomerCount = Math.min(groomerEmails.length, groomerNames.length);
+  for (let i = 0; i < groomerCount; i++) {
+    const email = groomerEmails[i]!;
+    const name = groomerNames[i]!;
+    const staffId = `00000000-0000-0000-0000-${String(5 + i).padStart(12, "0")}`;
+    await db.insert(schema.staff)
+      .values({
+        id: staffId,
+        name,
+        email,
+        oidcSub: email,
+        role: "groomer",
+        isSuperUser: false,
+        active: true,
+      })
+      .onConflictDoUpdate({
+        target: schema.staff.email,
+        set: { id: staffId, name, role: "groomer", isSuperUser: false, active: true },
+      });
+    console.log(`✓ Upserted groomer '${name}' (${email})`);
+  }
+
   // ── Services ──
   // Upsert services using name as unique key. With deterministic IDs in
   // servicesDef and TRUNCATE clearing downstream tables first, this is