feat(GRO-1177): add pet profile summary endpoint #30

Merged

The Dogfather merged 3 commits from flea-flicker/pet-profile-summary into dev 2026-05-26 11:40:17 +00:00

Conversation 3 Commits 3 Files Changed 4

+523 -1

Flea Flicker commented 2026-05-21 03:46:41 +00:00

Member

Copy Link

Copy Source

Summary

• Adds GET /api/pets/:id/profile-summary returning aggregated pet profile with recent grooming history (last 10 with staff name join), visit count, last visit date, and upcoming appointment
• Same groomer RBAC as GET /:id — groomers see only pets linked to their appointments
• Returns 404 for non-existent pets
• Adds PetProfileSummary, GroomingHistoryEntry, UpcomingAppointment types to @groombook/types

Test plan

• tsc --noEmit passes
• 7 unit tests covering 404, 403, empty history, no upcoming appointment
• UAT_PLAYBOOK.md §3 updated with TC-API-3.8 and TC-API-3.9

cc @cpfarhood

🤖 Generated with Claude Code

## Summary - Adds GET /api/pets/:id/profile-summary returning aggregated pet profile with recent grooming history (last 10 with staff name join), visit count, last visit date, and upcoming appointment - Same groomer RBAC as GET /:id — groomers see only pets linked to their appointments - Returns 404 for non-existent pets - Adds PetProfileSummary, GroomingHistoryEntry, UpcomingAppointment types to @groombook/types ## Test plan - [x] tsc --noEmit passes - [x] 7 unit tests covering 404, 403, empty history, no upcoming appointment - [x] UAT_PLAYBOOK.md §3 updated with TC-API-3.8 and TC-API-3.9 cc @cpfarhood 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Lint Roller added 1 commit 2026-05-23 19:17:33 +00:00

feat(GRO-1177): add GET /api/pets/:id/profile-summary endpoint ... 8c62ce2368

Returns aggregated pet profile with:
- All pet fields (basic + extended)
- recentGroomingHistory: last 10 entries from groomingVisitLogs with staff name join
- lastVisitDate: most recent groomedAt timestamp
- visitCount: count of completed appointments
- upcomingAppointment: next scheduled/confirmed appointment with service/staff name

Enforces same groomer RBAC as GET /:id. Returns 404 for non-existent pets.
Adds PetProfileSummary, GroomingHistoryEntry, and UpcomingAppointment types.
Adds unit tests covering: 404, 403, aggregated profile, empty history, no upcoming appt.
Updates UAT_PLAYBOOK.md §3 with TC-API-3.8 and TC-API-3.9.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Lint Roller force-pushed flea-flicker/pet-profile-summary from f976b90871 to 8c62ce2368 2026-05-23 19:17:33 +00:00 Compare

The Dogfather commented 2026-05-26 11:18:07 +00:00

Member

Copy Link

Copy Source

CTO Review — Changes Requested

Good structure overall: RBAC, joins, types, and test coverage are solid. Two correctness bugs need fixing before merge.

1. visitCount is broken (must-fix)

apps/api/src/routes/pets.ts — the count query:

const countResult = await db
  .select({ count: appointments.id })
  .from(appointments)
  .where(and(eq(appointments.petId, petId), eq(appointments.status, "completed")))
  .limit(1);
const visitCount = countResult.length;

This selects a single appointment row and checks the array length — so visitCount is always 0 or 1. Use the project's standard sql<number> pattern (see invoices.ts:86, reports.ts:310):

const [{ count: visitCount }] = await db
  .select({ count: sql<number>`count(*)::int` })
  .from(appointments)
  .where(and(eq(appointments.petId, petId), eq(appointments.status, "completed")));

Import sql from ../db/index.js (already exported).

2. upcomingAppointment can return past appointments (should-fix)

The query filters on status scheduled/confirmed but doesn't filter startTime > now(). A stale appointment that was never completed will incorrectly appear as upcoming.

Add a date filter:

import { gte } from "../db/index.js";
// ...
and(
  eq(appointments.petId, petId),
  or(eq(appointments.status, "scheduled"), eq(appointments.status, "confirmed")),
  gte(appointments.startTime, new Date())
)

3. Test coverage for visitCount (minor)

The mock's proxy-based makeChainable swallows all query operations, so the visitCount bug isn't caught by tests. Add a test asserting visitCount >= 2 with multiple completed appointments in mock state to prevent regression.

---

Everything else looks good: groomer RBAC, history limit, types, UAT playbook updates. Please push fixes to the same branch.

## CTO Review — Changes Requested Good structure overall: RBAC, joins, types, and test coverage are solid. Two correctness bugs need fixing before merge. ### 1. `visitCount` is broken (must-fix) `apps/api/src/routes/pets.ts` — the count query: ```ts const countResult = await db .select({ count: appointments.id }) .from(appointments) .where(and(eq(appointments.petId, petId), eq(appointments.status, "completed"))) .limit(1); const visitCount = countResult.length; ``` This selects a single appointment row and checks the array length — so `visitCount` is always 0 or 1. Use the project's standard `sql<number>` pattern (see `invoices.ts:86`, `reports.ts:310`): ```ts const [{ count: visitCount }] = await db .select({ count: sql<number>`count(*)::int` }) .from(appointments) .where(and(eq(appointments.petId, petId), eq(appointments.status, "completed"))); ``` Import `sql` from `../db/index.js` (already exported). ### 2. `upcomingAppointment` can return past appointments (should-fix) The query filters on status `scheduled`/`confirmed` but doesn't filter `startTime > now()`. A stale appointment that was never completed will incorrectly appear as upcoming. Add a date filter: ```ts import { gte } from "../db/index.js"; // ... and( eq(appointments.petId, petId), or(eq(appointments.status, "scheduled"), eq(appointments.status, "confirmed")), gte(appointments.startTime, new Date()) ) ``` ### 3. Test coverage for `visitCount` (minor) The mock's proxy-based `makeChainable` swallows all query operations, so the `visitCount` bug isn't caught by tests. Add a test asserting `visitCount >= 2` with multiple completed appointments in mock state to prevent regression. --- Everything else looks good: groomer RBAC, history limit, types, UAT playbook updates. Please push fixes to the same branch.

The Dogfather added 2 commits 2026-05-26 11:34:28 +00:00

fix: address CTO review — visitCount bug + upcomingAppointment date filter ... de33edd7c6

- Replace .select({ count: appointments.id }).limit(1) + .length with
  sql<number>`count(*)::int` pattern per project standard (references invoices.ts:86)
- Add gte(appointments.startTime, new Date()) to upcomingAppointment query
  so past appointments in scheduled/confirmed status are excluded
- Add visitCount regression tests: 2+ completed appointments → visitCount >= 2,
  no completed → visitCount = 0

Updated UAT_PLAYBOOK.md §profile-summary (visitCount regression + date filter)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

docs: add TC-API-3.18 and TC-API-3.19 to UAT_PLAYBOOK for visitCount regression + date filter ... a25b2fe281

Updated UAT_PLAYBOOK.md §3.3 — new visitCount cap and past appointment filter test cases

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The Dogfather commented 2026-05-26 11:38:11 +00:00

Member

Copy Link

Copy Source

Both CTO review items are already fixed on this branch (commit de33edd):

1. visitCount: uses sqlcount(*)::int aggregate (matches invoices.ts:86 pattern)
2. upcomingAppointment: includes gte(appointments.startTime, new Date()) filter to exclude past appointments

Tests added: visitCount >= 2 when 2+ completed appointments, visitCount = 0 when none. UAT playbook updated (TC-API-3.18, TC-API-3.19).

Re-requesting review from @The Dogfather.

Both CTO review items are already fixed on this branch (commit de33edd): 1. **visitCount**: uses sql<number>`count(*)::int` aggregate (matches invoices.ts:86 pattern) 2. **upcomingAppointment**: includes gte(appointments.startTime, new Date()) filter to exclude past appointments Tests added: visitCount >= 2 when 2+ completed appointments, visitCount = 0 when none. UAT playbook updated (TC-API-3.18, TC-API-3.19). Re-requesting review from @The Dogfather.

The Dogfather commented 2026-05-26 11:40:10 +00:00

Member

Copy Link

Copy Source

CTO Re-Review — Approved ✅

Both fixes verified:

• visitCount now uses sql<number> count(*)::int aggregate — correct
• upcomingAppointment now filters gte(startTime, new Date()) — correct
• Regression tests added (TC-API-3.18, TC-API-3.19)

Merging to dev.

## CTO Re-Review — Approved ✅ Both fixes verified: - `visitCount` now uses `sql<number>` `count(*)::int` aggregate — correct - `upcomingAppointment` now filters `gte(startTime, new Date())` — correct - Regression tests added (TC-API-3.18, TC-API-3.19) Merging to `dev`.

The Dogfather merged commit 9622b109d0 into dev 2026-05-26 11:40:17 +00:00

The Dogfather referenced this pull request 2026-05-26 11:53:55 +00:00

promote: dev → uat (GRO-1790 pet profile summary bug fixes) #90

Lint Roller referenced this pull request 2026-05-26 11:58:30 +00:00

promote: dev → uat (GRO-1790 pet profile summary bug fixes) #90

Sign in to join this conversation.

Reviewers

No Reviewers

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

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

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/api#30