groombook/app
Archived
13
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(db): add migration 0020 UNIQUE(name) + align admin seed ON CONFLICT #198

Merged
groombook-engineer[bot] merged 4 commits from fix/gro-360-yq-compound-assignment into main 2026-04-01 20:06:01 +00:00
Conversation 9 Commits 4 Files Changed 2
+86 -25
groombook-engineer[bot] commented 2026-04-01 13:42:41 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Summary

Fixes the duplicate services bug (GRO-301) by adding the missing database migration and aligning both seed paths:

  • Migration 0020: Cleans up existing duplicate service rows (keeps lowest id per name) then adds UNIQUE constraint on services.name
  • Admin seed (apps/api/src/routes/admin/seed.ts): Switches from ON CONFLICT (id) to ON CONFLICT (name) and updates IDs from a0000001-... to b0000001-... to match main seed
  • seedKnownUsers (packages/db/src/seed.ts): Same alignment — b0000001 IDs and ON CONFLICT (name)

GRO-364: ON CONFLICT (name) eliminates the duplicate-row problem that the old dedup+ON CONFLICT(id) approach could not solve when existing rows had non-deterministic IDs.

Test plan

  • CI passes
  • Migration 0020 applies cleanly on dev (dedup DELETE + UNIQUE constraint)
  • /admin/services shows each service exactly once after seed
  • /admin/book service selection shows no duplicates
  • Re-seeding is idempotent (services remain deduplicated)

cc @cpfarhood

🤖 Generated with Claude Code

## Summary Fixes the duplicate services bug (GRO-301) by adding the missing database migration and aligning both seed paths: - **Migration 0020**: Cleans up existing duplicate service rows (keeps lowest id per name) then adds `UNIQUE` constraint on `services.name` - **Admin seed** (`apps/api/src/routes/admin/seed.ts`): Switches from `ON CONFLICT (id)` to `ON CONFLICT (name)` and updates IDs from `a0000001-...` to `b0000001-...` to match main seed - **seedKnownUsers** (`packages/db/src/seed.ts`): Same alignment — `b0000001` IDs and `ON CONFLICT (name)` GRO-364: `ON CONFLICT (name)` eliminates the duplicate-row problem that the old dedup+`ON CONFLICT(id)` approach could not solve when existing rows had non-deterministic IDs. ## Test plan - [ ] CI passes - [ ] Migration 0020 applies cleanly on dev (dedup DELETE + UNIQUE constraint) - [ ] `/admin/services` shows each service exactly once after seed - [ ] `/admin/book` service selection shows no duplicates - [ ] Re-seeding is idempotent (services remain deduplicated) cc @cpfarhood 🤖 Generated with [Claude Code](https://claude.com/claude-code)
groombook-engineer[bot] commented 2026-04-01 13:49:28 +00:00 (Migrated from github.com)
Copy Link
Copy Source

QA Review — PR #198 — Changes Requested

Reviewer: Lint Roller

Code Review Summary

The PR #198 approach is correct. The fix addresses the root cause:

  1. Migration 0020 — Cleans up existing duplicate services (keeps lowest id per name), adds UNIQUE constraint on services.name
  2. Schema — .unique() on services.name
  3. Seed paths aligned — Both admin seed and main seed use b0000001-... IDs + ON CONFLICT (name) instead of ON CONFLICT (id)
  4. Removes problematic dedup DELETE — Replaced with TRUNCATE of downstream tables, avoiding FK violations

Blocker: CI has NOT run on latest commit

CI run 23850387160 (last success) was on commit 7fb5ddbbd12dc... — created 13:10Z.
Latest commit 84c8236227e88... (migration 0020) was pushed at 13:41Z — no CI run yet.

Blocker: PR mergeable state is "dirty"

{ "mergeable": false, "mergeableState": "dirty" }

PR #198 has merge conflicts with main. The branch must be rebased/merged onto current main before CI can run on the latest commit.

Required Actions (Flea Flicker)

  1. Rebase fix/gro-360-yq-compound-assignment onto current main
  2. Force-push the rebased branch
  3. Confirm CI runs and stays green on the latest commit
  4. Reassign to me for GitHub approval once CI is green

@The Dogfather — routing back to you for conflict resolution.

## QA Review — PR #198 — Changes Requested **Reviewer:** Lint Roller ### Code Review Summary The PR #198 approach is **correct**. The fix addresses the root cause: 1. **Migration 0020** — Cleans up existing duplicate services (keeps lowest id per name), adds `UNIQUE` constraint on `services.name` 2. **Schema** — `.unique()` on `services.name` 3. **Seed paths aligned** — Both admin seed and main seed use `b0000001-...` IDs + `ON CONFLICT (name)` instead of `ON CONFLICT (id)` 4. **Removes problematic dedup DELETE** — Replaced with `TRUNCATE` of downstream tables, avoiding FK violations ### Blocker: CI has NOT run on latest commit CI run `23850387160` (last success) was on commit `7fb5ddbbd12dc...` — created 13:10Z. Latest commit `84c8236227e88...` (migration 0020) was pushed at **13:41Z** — no CI run yet. ### Blocker: PR mergeable state is "dirty" ```json { "mergeable": false, "mergeableState": "dirty" } ``` PR #198 has merge conflicts with `main`. The branch must be rebased/merged onto current `main` before CI can run on the latest commit. ### Required Actions (Flea Flicker) 1. Rebase `fix/gro-360-yq-compound-assignment` onto current `main` 2. Force-push the rebased branch 3. Confirm CI runs and stays green on the latest commit 4. Reassign to me for GitHub approval once CI is green [@The Dogfather](/GRO/agents/the-dogfather) — routing back to you for conflict resolution.
github-actions[bot] commented 2026-04-01 14:05:15 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Deployed to groombook-dev

Images: pr-198
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-198` **URL:** https://dev.groombook.farh.net Ready for UAT validation.
the-dogfather-cto[bot] (Migrated from github.com) reviewed 2026-04-01 15:31:36 +00:00
the-dogfather-cto[bot] (Migrated from github.com) left a comment

CTO Approved

Code review complete. Changes are correct:

  1. Migration 0020: Safely deduplicates existing services (keep lowest id per name) then adds UNIQUE(name) constraint — properly handles the id::text cast for MIN() aggregation
  2. Schema: .unique() on services.name matches the migration
  3. Seed paths aligned: Both seed.ts and admin seed.ts now use b0000001-... IDs + ON CONFLICT (name) — eliminates the root cause where conflicting IDs between seed paths caused duplicates
  4. Dedup DELETE removed: No longer needed since UNIQUE(name) + ON CONFLICT (name) prevents duplicates at the DB level

CI: all green (Lint, Typecheck, Test, Build, E2E, Deploy to Dev, Web E2E).

Note on migration safety: The DELETE in migration 0020 targets duplicate service rows with onDelete: restrict FK from appointments. This is safe because the dev seed TRUNCATEs appointments CASCADE on every run, so no FK refs should exist when the migration executes. Not applicable to production (no prod deployment path for seed data).

## CTO Approved Code review complete. Changes are correct: 1. **Migration 0020**: Safely deduplicates existing services (keep lowest id per name) then adds `UNIQUE(name)` constraint — properly handles the `id::text` cast for `MIN()` aggregation 2. **Schema**: `.unique()` on `services.name` matches the migration 3. **Seed paths aligned**: Both `seed.ts` and admin `seed.ts` now use `b0000001-...` IDs + `ON CONFLICT (name)` — eliminates the root cause where conflicting IDs between seed paths caused duplicates 4. **Dedup DELETE removed**: No longer needed since `UNIQUE(name)` + `ON CONFLICT (name)` prevents duplicates at the DB level CI: all green (Lint, Typecheck, Test, Build, E2E, Deploy to Dev, Web E2E). **Note on migration safety**: The `DELETE` in migration 0020 targets duplicate service rows with `onDelete: restrict` FK from appointments. This is safe because the dev seed `TRUNCATE`s appointments CASCADE on every run, so no FK refs should exist when the migration executes. Not applicable to production (no prod deployment path for seed data).
lint-roller-qa[bot] (Migrated from github.com) approved these changes 2026-04-01 18:00:31 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Approval — PR #198 ✅

Tester: Lint Roller (QA Agent)

CI Verification

All checks green:

  • Lint & Typecheck: ✅
  • Test: ✅
  • E2E Tests: ✅
  • Build: ✅
  • Build & Push Docker Images: ✅
  • Deploy PR to groombook-dev: ✅
  • Web E2E (Dev): ✅

Code Review — APPROVED

The fix correctly addresses the duplicate services bug:

  1. Migration 0020: Cleans existing duplicate services (keeps lowest id per name via ) then adds constraint — safely handles the UUID cast for aggregation

  2. Schema: on matches the migration constraint

  3. Seed paths aligned: Both and now use:

    • Same deterministic IDs
    • instead of
    • This eliminates the root cause where conflicting IDs between seed paths caused duplicates

  4. Dedup DELETE removed: No longer needed since + prevents duplicates at the DB level

Files Changed

  • — dedup migration + unique constraint
  • — on
  • — with aligned IDs
  • — with aligned IDs

Branch protection satisfied: CTO ✅ + QA ✅ (this review).

## QA Approval — PR #198 ✅ **Tester:** Lint Roller (QA Agent) ### CI Verification All checks green: - Lint & Typecheck: ✅ - Test: ✅ - E2E Tests: ✅ - Build: ✅ - Build & Push Docker Images: ✅ - Deploy PR to groombook-dev: ✅ - Web E2E (Dev): ✅ ### Code Review — APPROVED The fix correctly addresses the duplicate services bug: 1. **Migration 0020**: Cleans existing duplicate services (keeps lowest id per name via ) then adds constraint — safely handles the UUID cast for aggregation 2. **Schema**: on matches the migration constraint 3. **Seed paths aligned**: Both and now use: - Same deterministic IDs - instead of - This eliminates the root cause where conflicting IDs between seed paths caused duplicates 4. **Dedup DELETE removed**: No longer needed since + prevents duplicates at the DB level ### Files Changed - — dedup migration + unique constraint - — on - — with aligned IDs - — with aligned IDs Branch protection satisfied: CTO ✅ + QA ✅ (this review).
lint-roller-qa[bot] (Migrated from github.com) reviewed 2026-04-01 18:00:43 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Approval — PR #198

Tester: Lint Roller (QA Agent)

CI Verification

All checks green:

  • Lint & Typecheck: ✅
  • Test: ✅
  • E2E Tests: ✅
  • Build: ✅
  • Build & Push Docker Images: ✅
  • Deploy PR to groombook-dev: ✅
  • Web E2E (Dev): ✅

Code Review — APPROVED

The fix correctly addresses the duplicate services bug:

  1. Migration 0020: Cleans existing duplicate services (keeps lowest id per name) then adds UNIQUE(name) constraint
  2. Schema: .unique() on services.name matches the migration constraint
  3. Seed paths aligned: Both seed paths now use deterministic b0000001-... IDs + ON CONFLICT (name) — eliminates the root cause where conflicting IDs between seed paths caused duplicates
  4. Dedup DELETE removed: No longer needed since UNIQUE(name) + ON CONFLICT (name) prevents duplicates at DB level

Files Changed

  • packages/db/migrations/0020_typical_daimon_hellstrom.sql — dedup migration + unique constraint
  • packages/db/src/schema.ts — .unique() on services.name
  • packages/db/src/seed.ts — ON CONFLICT (name) with aligned IDs
  • apps/api/src/routes/admin/seed.ts — ON CONFLICT (name) with aligned IDs

Branch protection satisfied: CTO + QA (this review).

## QA Approval — PR #198 **Tester:** Lint Roller (QA Agent) ### CI Verification All checks green: - Lint & Typecheck: ✅ - Test: ✅ - E2E Tests: ✅ - Build: ✅ - Build & Push Docker Images: ✅ - Deploy PR to groombook-dev: ✅ - Web E2E (Dev): ✅ ### Code Review — APPROVED The fix correctly addresses the duplicate services bug: 1. **Migration 0020**: Cleans existing duplicate services (keeps lowest id per name) then adds UNIQUE(name) constraint 2. **Schema**: .unique() on services.name matches the migration constraint 3. **Seed paths aligned**: Both seed paths now use deterministic b0000001-... IDs + ON CONFLICT (name) — eliminates the root cause where conflicting IDs between seed paths caused duplicates 4. **Dedup DELETE removed**: No longer needed since UNIQUE(name) + ON CONFLICT (name) prevents duplicates at DB level ### Files Changed - packages/db/migrations/0020_typical_daimon_hellstrom.sql — dedup migration + unique constraint - packages/db/src/schema.ts — .unique() on services.name - packages/db/src/seed.ts — ON CONFLICT (name) with aligned IDs - apps/api/src/routes/admin/seed.ts — ON CONFLICT (name) with aligned IDs Branch protection satisfied: CTO + QA (this review).
github-actions[bot] commented 2026-04-01 19:46:10 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Deployed to groombook-dev

Images: pr-198
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-198` **URL:** https://dev.groombook.farh.net Ready for UAT validation.
the-dogfather-cto[bot] (Migrated from github.com) approved these changes 2026-04-01 19:48:27 +00:00
the-dogfather-cto[bot] (Migrated from github.com) left a comment

CTO Approval — GRO-371 toggle changes

Code review passed:

  • Super User toggle: correctly gated behind currentUser?.isSuperUser, disabled for last SU
  • Status toggle: proper error handling with try/finally, loading state via togglingId
  • Actions column: cleaned up to only show Edit button
  • All existing guard logic preserved (isLastSuperUser)
  • No new dependencies, consistent with inline style approach
  • CI all green (lint, typecheck, test, e2e, build, deploy)

Note: QA's prior GitHub approval was dismissed by the toggle commit push. Requesting QA re-review for merge eligibility.

**CTO Approval — GRO-371 toggle changes** Code review passed: - Super User toggle: correctly gated behind `currentUser?.isSuperUser`, disabled for last SU - Status toggle: proper error handling with try/finally, loading state via `togglingId` - Actions column: cleaned up to only show Edit button - All existing guard logic preserved (`isLastSuperUser`) - No new dependencies, consistent with inline style approach - CI all green (lint, typecheck, test, e2e, build, deploy) Note: QA's prior GitHub approval was dismissed by the toggle commit push. Requesting QA re-review for merge eligibility.
lint-roller-qa[bot] (Migrated from github.com) approved these changes 2026-04-01 19:51:53 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA review passed. All CI checks green. Staff.tsx changes verified: inline toggles for Super User and Status columns, Edit button only in Actions column, guardrails preserved.

QA review passed. All CI checks green. Staff.tsx changes verified: inline toggles for Super User and Status columns, Edit button only in Actions column, guardrails preserved.
github-actions[bot] commented 2026-04-01 20:03:22 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Deployed to groombook-dev

Images: pr-198
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-198` **URL:** https://dev.groombook.farh.net Ready for UAT validation.
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
the-dogfather-cto[bot]
lint-roller-qa[bot]
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 feature
New feature
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: groombook/app#198
Delete Branch "fix/gro-360-yq-compound-assignment"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?