Commit Graph

35 Commits

Author SHA1 Message Date
groombook-qa[bot] aa36d4ba87 Merge branch 'main' into fix/gro-485-oobe-staff-middleware 2026-04-05 20:16:12 +00:00
Flea Flicker 8348f1c152 fix(api): resolve CI typecheck failures in GRO-485 fix
Fix type errors that caused CI Lint & Typecheck job to fail:
- setup.ts: replace unavailable isNull import with sql template tag
  (isNull not exported from @groombook/db; sql IS exported)
- setup.ts: add non-null assertion on newStaff after insert.returning()
- setup.test.ts: add sql mock template tag to @groombook/db mock
- setup.test.ts: fix evaluateCond to handle sql template tag type
- setup.test.ts: add type assertions for body.staff in OOBE regression tests
- setup.test.ts: fix dbStaffRows type casts in mock insert function

All 18 tests pass, full typecheck clean.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-05 20:05:53 +00:00
Flea Flicker fa18c41677 fix(api): exempt OOBE setup from staff middleware and auto-create staff (GRO-485)
Exempt POST /api/setup from resolveStaffMiddleware so OOBE users (with no
pre-existing staff record) can complete the out-of-box experience without
getting blocked by the "no staff record found" 403 error.

Changes:
- rbac.ts: add /api/setup to path exemption alongside /api/auth/
- setup.ts POST /: add find-or-create logic that:
  - Looks up existing staff by userId from JWT
  - Auto-links legacy staff records by email if userId is null
  - Creates a new staff record if none exists (OOBE case)
  - Returns 400 if JWT has no email and no staff record found
- setup.test.ts: add regression tests for all scenarios

Fixes GRO-485 (OOBE regression introduced by GRO-480).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-05 19:37:23 +00:00
Paperclip 78a6758349 fix(db): generate unique random salt per encryptSecret call (GRO-453)
Use a 16-byte random salt per encryption instead of the fixed
"groombook-auth-provider-config" salt. This prevents identical
plaintexts from producing identical ciphertexts, closing the
timing/anagram security gap identified in GRO-452.

New format: salt:iv:ciphertext:authTag (all base64).
Legacy format (iv:ciphertext:authTag) is still accepted for
backward-compatible decryption of existing stored values.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-04 21:37:30 +00:00
Paperclip 1c7628459f fix(db): use random per-encryption salt in crypto.ts (GRO-453)
Generate a unique 16-byte random salt for each encryptSecret() call
and store it as a prefix in the ciphertext. Format changed from
  iv:ciphertext:authTag → salt:iv:ciphertext:authTag

decryptSecret() detects legacy 3-part format and uses the fixed
package salt for backward compatibility with existing encrypted rows.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-04 13:14:18 +00:00
groombook-engineer[bot] 2c1752f178 test(authProvider): mock reinitAuth to prevent BETTER_AUTH_SECRET dependency
vi.mock the auth module so reinitAuth() is a no-op in tests.
This decouples the tests from the BETTER_AUTH_SECRET env var.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 13:02:13 +00:00
groombook-engineer[bot] 3f23781493 test(api): add OOBE bootstrap integration tests for setup endpoints (GRO-393)
- GET /api/setup/status: verify showAuthProviderStep logic for all cases
  (fresh install, env vars present, setup complete, DB config exists)
- POST /api/setup/auth-provider: 403 after complete, 409 if already configured,
  creates config with encrypted secret, Zod validation
- POST /api/setup/auth-provider/test: 403 after complete, unreachable issuer,
  valid issuer, invalid issuer (non-200)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 13:02:13 +00:00
groombook-engineer[bot] 2a50850217 feat(oobe): conditional auth provider bootstrap step + fix(rbac): requireRoleOrSuperUser for /admin/* (GRO-392, GRO-412)
Merges GRO-392 (OOBE auth provider bootstrap step) and GRO-412 (fix admin route RBAC to use requireRoleOrSuperUser). QA  CTO . Approved by CEO.
2026-04-03 01:55:13 +00:00
groombook-engineer[bot] 1044cdfec3 fix(api): correct transaction mock closing bracket in authProvider test
Syntax error: `))` was closing the arrow function body prematurely.
Change `)),` to `}),` to properly close the values-returning object.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 00:55:42 +00:00
groombook-engineer[bot] 63c829bfd3 feat(api): auth provider CRUD endpoints + test-connection (GRO-388)
Implements admin API endpoints for managing auth provider configuration.
All gated by requireSuperUser().

Endpoints:
- GET /api/admin/auth-provider - returns config with clientSecret=redacted
- PUT /api/admin/auth-provider - encrypts clientSecret before DB write
- POST /api/admin/auth-provider/test - validates OIDC discovery endpoint
- DELETE /api/admin/auth-provider - removes DB config

Fixes CTO review findings:
- PUT uses db.transaction() for atomic upsert (was non-atomic delete+insert)
- Rebased on latest main (drops stale GRO-404/406 commits)
- Added EOF newlines to authProvider.ts and authProvider.test.ts

Unit tests with 9 passing test cases covering all endpoints and RBAC.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-02 21:50:40 +00:00
groombook-engineer[bot] 883af15fbe feat(api): DB-first auth config loading with env-var fallback (GRO-389)
Refactor auth initialization to support three config states:
1. DB config (auth_provider_config table) — primary source
2. OIDC_* env vars — fallback when DB config absent
3. Unconfigured — graceful handling when neither source available

Changes:
- auth.ts: Add initAuth() async factory, getAuth() getter, getAuthPromise()
- index.ts: Call initAuth() at startup before serve()
- middleware/auth.ts: Use getAuth() instead of direct auth import
- Add auth.test.ts covering all three config states

Preserves AUTH_DISABLED=true behavior and original hairpin NAT pattern.

Co-authored-by: groombook-engineer[bot] <3141748+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-04-02 19:58:17 +00:00
groombook-engineer[bot] f4acf5be23 feat(db): auth_provider_config table + AES-256-GCM encryption helpers
Renumbered migration 0021 → 0023 to resolve conflict with pet_image and
logo_key migrations that landed on main after this branch was created.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-02 17:16:35 +00:00
groombook-engineer[bot] 004e23f8bc fix(api): enforce requireSuperUser on settings PATCH and fix dev-mode auth bypass (#206)
* fix(api): enforce requireSuperUser on settings PATCH and fix dev-mode auth bypass

- Add requireSuperUser() middleware to PATCH /api/admin/settings route
  to ensure only super users can modify business settings

- Fix dev-mode (AUTH_DISABLED=true) force-set of isSuperUser:true
  for all staff records in resolveStaffMiddleware. Now preserves
  actual database value with isSuperUser ?? false fallback.
  This prevents non-super-users (e.g., receptionists) from
  bypassing RBAC checks in dev mode.

- Fix test data: RECEPTIONIST and GROOMER now correctly have
  isSuperUser: false (was incorrectly inheriting true from MANAGER)

- Add 7 new tests for requireSuperUser middleware covering:
  - Super user access allowed
  - Non-super-user receptionist blocked with 403
  - Non-super-user groomer blocked with 403
  - Unresolved staff record returns 403
  - Receptionist cannot grant super user via PATCH
  - JSON error response format

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(api): remove dead code in rbac test

Remove unused `app` variable from 'returns 403 when staff record is
not resolved' test - the test uses `testApp` instead.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: groombook-engineer[bot] <3141748+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-04-02 12:57:56 +00:00
groombook-engineer[bot] 3a31ad71c2 feat(schema): add is_super_user to staff table (GRO-201)
Add boolean is_super_user column (default false) to staff table.
Update Staff interface in shared types.
Mark first manager as super user in both seed modes.
Update test fixtures to include isSuperUser field.

Co-authored-by: groombook-ci[bot] <ci@groombook.bot>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-28 20:39:46 +00:00
groombook-engineer[bot] ad1f32eb8f feat(auth): replace OIDC/jose with Better-Auth (#136)
* feat(db): add Better-Auth schema tables (GRO-118)

Add user, session, account, and verification tables required by
Better-Auth's Drizzle adapter. Add nullable userId FK on staff to
link business identity to auth identity. Fix test fixtures and
factory to include the new column.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): mount Better-Auth handler at /api/auth/** (GRO-118)

- Import toNodeHandler from better-auth/node and auth from ./lib/auth.js
- Mount Better-Auth HTTP handler before auth middleware block
- Handles OAuth callbacks, sign-in/sign-out, session management
- Supports GET/POST/PUT/PATCH/DELETE/OPTIONS methods

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): replace JWT auth with Better-Auth session validation (GRO-118)

- Replace jose/jwtVerify with auth.api.getSession()
- Session token validated via cookie/header, DB-backed
- jwtPayload.sub now = Better-Auth user ID (not OIDC sub)
- Dev mode bypass preserved; production guard against AUTH_DISABLED preserved
- rbac.ts and tests updated in subsequent tasks

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): update resolveStaffMiddleware for Better-Auth userId (GRO-118)

- Remove JwtPayload import; use inline type in AppEnv
- Production and dev mode lookups now use staff.userId (not oidcSub)
- Backward compat: jwtPayload.sub now = Better-Auth user ID

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* chore(api): remove jose and openid-client deps (GRO-118)

- Remove unused jose and openid-client packages
- Regenerate pnpm lockfile
- Pre-existing Zod type errors resolved (1 remaining: JwtPayload in test)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(api): remove stale JwtPayload import from impersonation test (GRO-118)

auth.ts no longer exports JwtPayload — replace with inline type.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* test(api): update RBAC tests for Better-Auth userId (GRO-128)

- Add userId field to mock staff records (MANAGER, RECEPTIONIST, GROOMER)
- Update jwtPayload.sub to use userId instead of oidcSub in test helpers
- Update dev mode X-Dev-User-Id header to use userId

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* chore(api): upgrade zod to v4 with v3 compat layer (GRO-131)

- Bump zod from ^3.24.1 to ^4.3.6
- Bump @hono/zod-validator from ^0.4.3 to ^0.7.6
- Update all 12 route files to import from "zod/v3" compat layer

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): add Better-Auth configuration (GRO-118)

Exports the better-auth() instance configured with:
- Drizzle PG adapter
- genericOAuth plugin for Authentik OIDC
- 7-day session with 5-min cookie cache

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(web): install Better-Auth client and create config (GRO-118)

- Add better-auth to apps/web/package.json dependencies
- Create apps/web/src/lib/auth-client.ts with createAuthClient config
- Export signIn, signOut, useSession from the client
- Add vite-env.d.ts for Vite client types

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(web): use Better-Auth session state in App.tsx (GRO-126)

Add useSession hook to check Better-Auth session for production auth.
Redirect to Authentik sign-in when no session in production mode.
Dev mode flow (DevLoginSelector) preserved.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): scope devFetch interceptor to dev mode only (GRO-127)

* fix(api): validate BETTER_AUTH_SECRET and fix lockfile specifier (GRO-118)

- Add startup validation for BETTER_AUTH_SECRET when auth is enabled
- Fix pnpm-lock.yaml typescript specifier mismatch (^5.9.3 → ^5.7.3)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): mock authDisabled=true in App.test.tsx to fix CI failures

App.test.tsx "App navigation" tests were failing because the beforeEach
set authDisabled=false (production mode), which triggers the Better Auth
useSession() path. Since useSession() was not mocked in tests, the
component rendered null instead of the admin nav.

Now uses authDisabled=true + dev user in localStorage for those tests,
bypassing the Better Auth dependency while still testing the nav render.

Also removes duplicate App.test.js (compiled artifact).

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(e2e): set authDisabled=true in fixtures to bypass Better Auth

The App.tsx production auth path calls signIn.social() when
authDisabled=false, causing E2E tests to render blank. The fixtures
must mock authDisabled=true so the dev login selector is used instead.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(e2e): add dev/config, dev/users, and branding mocks to navigation.spec.ts

Playwright matches routes in last-registered-first-served order, so the
catch-all /api/** handler was overwriting the authDisabled: true fixture.
Added specific handlers before the catch-all to ensure auth config,
user list, and branding responses are properly shaped.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): gate DevLoginSelector on API authDisabled, not import.meta.env.DEV

Move the DevLoginSelector rendering check from import.meta.env.DEV to the
API-driven authDisabled state, after the loading guard. Simplify the redirect
condition to remove the now-redundant pathname exception.

Fixes E2E login tests that were failing because DevLoginSelector was never
rendered in Docker production builds where import.meta.env.DEV is false.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(db): add missing migration journal entries 0012-0017

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): import App.tsx (not App.js) in App.test.tsx (#137)

* fix(web): mock /api/auth/get-session in Dev login selector test

The "redirects to /login when auth is disabled and no user selected" test
fails because useSession() from better-auth/react calls /api/auth/get-session
which wasn't mocked, causing sessionLoading to stay true indefinitely.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): import App.tsx (not App.js) in test to get authDisabled bypass

The Dev login selector test was importing the compiled App.js instead of
the source App.tsx. App.js has different logic (uses import.meta.env.DEV
instead of API-based authDisabled) and doesn't implement the
sessionLoading bypass needed for tests to pass.

Also applied the rawSession/rawSessionLoading refactor in App.tsx that
bypasses useSession result when authDisabled=true.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): use extensionless import for App in test

The `.tsx` extension in the import path is not allowed without
`allowImportingTsExtensions` (TS5097). Use extensionless `../App`
which resolves correctly via moduleResolution: "bundler".

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>

* fix(auth): dev login resolve staff by id, not userId

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(rbac): fallback lookup for staff records predating Better-Auth userId (#140)

GRO-153: /api/staff returned 403 for all staff because resolveStaffMiddleware
looked up by staff.userId (Better-Auth ID) but dev login sent staff.id (PK),
and existing staff records had userId=NULL.

Changes:
- resolveStaffMiddleware: try userId first, fall back to staff.id (dev mode)
- resolveStaffMiddleware: try userId first, fall back to oidcSub (production)
- GET /api/dev/users: include userId field for DevLoginSelector
- DevLoginSelector: send userId (not staff.id) as X-Dev-User-Id
- Migration 0018: backfill userId for known demo staff

Co-authored-by: groombook-engineer[bot] <groombook-engineer@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Barkley Trimsworth <barkley@groombook.farh.net>

* fix(rbac): allow all staff roles to READ /api/staff

GRO-156 follow-up: RBAC middleware was blocking groomer/receptionist
from GET /api/staff. The QA review found 403 with "role groomer is not
permitted" after PR #140 deployment.

Fix: split the /staff/* guard — GET requests allow all roles
(groomer, receptionist, manager); write operations remain manager-only.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-engineer[bot] <269742240+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Flea Flicker <flea-flicker@paperclip.ing>
Co-authored-by: groombook-engineer[bot] <groombook-engineer@users.noreply.github.com>
Co-authored-by: Barkley Trimsworth <barkley@groombook.farh.net>
2026-03-28 03:50:45 +00:00
The Dogfather b78e45b5c5 fix(auth): dev login 403 — resolve staff by id, not oidcSub (GRO-150)
The DevLoginSelector stores the staff database id in localStorage and
sends it as X-Dev-User-Id. The resolveStaffMiddleware incorrectly
looked up staff by oidcSub instead of id, causing all API endpoints
to return 403 for every user in dev mode.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-28 01:23:10 +00:00
groombook-engineer[bot] 9eb0c3d151 fix(gro66): E2E selector fix + groomer isolation + portal confirm/cancel
* Implement confirm/cancel in customer portal (GRO-50)

Backend:
- Add POST /api/portal/appointments/:id/confirm endpoint
  - Validates impersonation session auth and ownership
  - Rejects past/in-progress, non-pending, or already-cancelled/completed
  - Sets confirmationStatus="confirmed", confirmedAt, updatedAt
- Add POST /api/portal/appointments/:id/cancel endpoint
  - Same auth/ownership pattern
  - Rejects past/in-progress or already-cancelled/completed
  - Sets status="cancelled", confirmationStatus="cancelled", cancelledAt, updatedAt

Frontend (Appointments.tsx):
- Add confirmationStatus field to Appointment type and mock data
- Add ConfirmationSection component: shows status badge + confirm button
- Add CancelAppointmentButton: wires to cancel API with loading/error state
- Wire existing Cancel button to CancelAppointmentButton
- Show confirmation status badge in expanded view for upcoming appointments

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(gro-48): row-level data scoping for groomer role (RBAC Phase 2)

Filter query results at the route handler level when staff role is groomer:

- GET /api/appointments: WHERE staffId = groomer OR batherStaffId = groomer
- GET /api/appointments/🆔 403 if not assigned to groomer (as staff or bather)
- GET /api/clients: Clients with ≥1 appointment for this groomer (via exists subquery)
- GET /api/clients/🆔 403 if no appointment linkage
- GET /api/pets: Pets owned by groomer-linked clients (via exists subquery)
- GET /api/pets/:petId: 403 if no appointment linkage

Managers and receptionists: no change.

Added exists to @groombook/db exports (was missing from re-export).
Added groomerIsolation unit tests for role guard and filter logic.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-50): add portal confirm/cancel tests and fix ConfirmationSection state

- Add test coverage for POST /portal/appointments/:id/confirm endpoint
- Add test coverage for POST /portal/appointments/:id/cancel endpoint
- Fix ConfirmationSection not updating local status after successful confirm
- Remove unused onCancel prop from ConfirmationSection call site
- Fix Appointments.test.tsx missing confirmationStatus field

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

* test(gro-50): add ConfirmationSection UI component tests

Add tests for the ConfirmationSection component:
- Renders correct badge for each confirmationStatus state
- Shows/hides Confirm button based on status
- Calls confirm API with correct headers
- Handles sessionId null case
- Shows error messages for 401/403/422 responses
- Shows loading state while confirming
- Shows success message briefly after confirm
- Does not call API if user cancels confirm dialog

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-48): address QA review feedback — staffRow?.role and portal TS guards

- appointments.ts: use staffRow?.role (consistent with clients.ts/pets.ts)
  to handle undefined staff context safely
- portal.ts: add null guards on .returning() results for confirm and cancel
  endpoints (TS18048: 'updated' is possibly undefined)
- All 188 tests passing; TypeScript typecheck clean

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro66): use specific selector for banner visibility assertion

Replace ambiguous `getByText("STAFF VIEW")` that matched both the
ImpersonationBanner and the CustomerPortal watermark with a precise
`getByTestId("impersonation-banner")` selector to eliminate strict
mode violations.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-66): add missing afterEach to vitest import

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-48): add icalToken to MANAGER mock after rebase

After rebasing onto origin/main (which added icalToken to the staff
schema via GRO-107), the MANAGER mock in groomerIsolation.test.ts was
missing the new required field. Added icalToken: null to the MANAGER
constant. factories.ts is clean (no duplicate icalToken after rebase).

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-47): add non-null assertions on Drizzle RETURNING results

Drizzle's update().returning() types the array element as T | undefined.
After the if (!appt) guard, updated is still typed as possibly undefined
because RETURNING can succeed with no rows. Add ! assertions since
we already guard with the existence check.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Flea Flicker <fleaflicker@groombook.ai>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Flea Flicker <flea-flicker@paperclip.ing>
2026-03-27 14:23:19 +00:00
groombook-engineer[bot] 6539eb4554 feat: iCal calendar feed (GRO-107)
feat: iCal calendar feed (GRO-107)

Closes GRO-107
2026-03-27 02:37:06 +00:00
groombook-engineer[bot] e3220af9ce fix(gro-38): prod/demo auth and API-based seed (#117)
Closes GRO-38. Adds POST /api/admin/seed (manager-only, gated by SEED_KNOWN_USERS_ONLY) and separates dev vs prod seeding paths. Reviewed and approved by CTO and QA.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-26 20:51:08 +00:00
groombook-engineer[bot] d0b4baf5aa feat: customer-facing appointment notes (GRO-106) (#109)
* feat: add customer-facing appointment notes (GRO-106)

- Migration 0014: add customer_notes column to appointments
- Schema update: add customerNotes field to appointments table
- Factory update: include customerNotes in buildAppointment
- Portal route: PATCH /api/portal/appointments/:id/notes
  - Ownership validation via impersonation session
  - Future-only validation (no edits after start)
  - 500 character limit
- Register portal router in index.ts

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* Fix confirmationToken leak and add unit tests for portal notes endpoint

- Return only id, customerNotes, updatedAt instead of full appointment row
- Add comprehensive unit tests covering auth, ownership, time-gating, and validation
- Fix: confirmationToken no longer returned to portal session

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat: add customer notes UI to portal and staff views (GRO-178)

- Add customerNotes field to Appointment type
- Add read-only customer notes display in staff appointment detail modal
- Add customer notes textarea with save, char counter (500 max), and disabled state
- Wire up PATCH /api/portal/appointments/:id/notes in portal UI
- Update mockData with customerNotes field

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix: address QA review feedback - null check and portal route auth

- Add null check after db.update().returning() in portal notes endpoint
- Move portal router registration before auth middleware so clients can access it
- Remove unused ENDED_SESSION variable from test file

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(portal): address QA review - isUpcoming time parsing and session header

- Fixed parseTimeTo24Hour to handle 12-hour AM/PM format correctly
- Added X-Impersonation-Session-Id header to CustomerNotesSection fetch
- Added comprehensive tests for CustomerNotesSection and time parsing
- Fixed TypeScript strict null checks for parseTimeTo24Hour

Fixes QA review issues:
- isUpcoming() now correctly parses 12-hour time format
- CustomerNotesSection sends session ID header for auth
- Added unit tests for new UI component

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix: thread sessionId as prop instead of sessionStorage

CustomerNotesSection was reading sessionStorage for the impersonation
session ID, but CustomerPortal stores it in React state. Pass sessionId
as a prop through AppointmentsSection and AppointmentCard instead.

Also update tests to pass sessionId prop and add test for null sessionId
case.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Scrubs McBarkley <scrubs@groombook.app>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-cto[bot] <269737991+groombook-cto[bot]@users.noreply.github.com>
2026-03-26 08:24:21 +00:00
Scrubs McBarkley d1ab91adfa feat: appointment confirmation and cancellation (GH #98, GRO-153)
Add customer confirmation/cancellation flow for appointments:

- DB migration (0013): add confirmation_status, confirmed_at, cancelled_at,
  confirmation_token to appointments table with index on token column
- schema.ts + factories.ts + types: expose new columns and ConfirmationStatus type
- GET /api/book/confirm/:token — tokenized confirm via email link (redirects)
- GET /api/book/cancel/:token — tokenized cancel via email link, single-use token
- POST /api/appointments/:id/confirm — portal/staff confirm endpoint
- POST /api/appointments/:id/cancel — portal/staff cancel endpoint
- Reminder emails now include Confirm/Cancel CTA buttons with tokenized links
- Reminder service generates confirmation token if missing before sending
- Staff calendar shows confirmation status indicator on appointment cards
  and in the detail modal (confirmed ✓ / customer cancelled ✗)
- /booking/confirmed, /booking/cancelled, /booking/error redirect pages
- 23 new unit tests covering all new endpoints and edge cases

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-24 16:02:58 +00:00
groombook-ceo[bot] 020e758916 Merge branch 'main' into feat/pet-photo-upload-gh93 2026-03-22 19:45:35 +00:00
Scrubs McBarkley 90abb28a0d fix: address PR #102 review feedback (GRO-145)
- factories.ts: add photoKey/photoUploadedAt null defaults to buildPet (TS regression fix)
- s3.ts: lazy singleton S3Client to avoid re-instantiation per call
- routes/pets.ts: server-side 5MB file size limit, explicit content-type allowlist (drops image/svg+xml etc), validate confirm key ownership against pets/${petId}/ prefix, delete old S3 object on re-upload, fix RBAC comment on DELETE photo
- PetPhotoUpload.tsx: bypass canvas resize for GIFs (preserves animation), pass fileSizeBytes in upload-url request
- Add PetPhotoDisplay.test.tsx: 7 tests covering fetch states, placeholder, refetch on petId change, custom size
- Add PetPhotoUpload.test.tsx: 8 tests covering idle state, type validation, upload flow, progress, GIF bypass
- Update petPhotos.test.ts: add SVG rejection, 5MB limit, key ownership, and old-photo deletion tests (18 total)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-22 15:41:44 +00:00
Scrubs McBarkley 0c182da366 fix: address CTO review feedback on quick-find search (GH #97, GRO-134)
- Remove unused makeSelectChain function from search.test.ts (lint blocker)
- Fix handleClientClick/handlePetClick to navigate to /admin/clients?highlight={id}
  so the target client is identified in the URL rather than silently ignored
- Add console.warn for fetch errors in GlobalSearch instead of swallowing silently

Auth middleware verified: searchRouter is registered on the api Hono instance
which applies authMiddleware + resolveStaffMiddleware globally — no coverage gap.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-22 04:10:54 +00:00
Scrubs McBarkley c826f65bd6 feat: quick-find search for clients and pets (GH #97, GRO-140)
Backend:
- GET /api/search?q={query} — returns up to 10 matching active clients and 10
  matching pets in a single request; clients matched on name/email/phone,
  pets matched on name/breed with owner name included
- Special chars (%, _, \) escaped before ILIKE to prevent injection/accidents
- Disabled clients excluded; pets from disabled client owners excluded via JOIN filter
- Route registered under protected API (auth + RBAC middleware applies automatically)
- Export `ilike` from @groombook/db alongside existing drizzle-orm helpers

Frontend:
- GlobalSearch component in sticky admin header: debounced input (300ms),
  grouped dropdown (Clients / Pets sections), loading/empty states
- Client results show name + phone; pet results show name, breed, owner name
- Touch-friendly: 44px input height, 48px min row height, full-width dropdown
- Outside-click closes dropdown; selecting a result navigates to /admin/clients

Tests (apps/api/src/__tests__/search.test.ts):
- 400 on missing/empty/whitespace q
- Returns matching clients and pets
- Empty arrays on no match
- Response shape always has clients/pets keys
- Special character inputs handled without errors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-22 00:16:28 +00:00
Scrubs McBarkley 1380848aea feat: pet photo upload via presigned S3 URLs (GH #93, GRO-123)
- DB migration 0012: add photo_key and photo_uploaded_at columns to pets table
- S3 client utility (apps/api/src/lib/s3.ts): presigned PUT/GET, delete via Rook-Ceph RGW
- API photo routes on petsRouter:
  - POST /:petId/photo/upload-url — returns presigned PUT URL + object key
  - POST /:petId/photo/confirm    — records key in DB after successful upload
  - DELETE /:petId/photo          — deletes from storage and clears DB
  - GET /:petId/photo             — returns presigned GET URL
- RBAC: all staff roles (manager, receptionist, groomer) may upload/delete photos;
  restructured index.ts guards so groomer-accessible photo paths don't overlap
  with the manager/receptionist-only general pets write guard
- Frontend PetPhotoDisplay: responsive image with shimmer skeleton and paw placeholder
- Frontend PetPhotoUpload: client-side resize to max 1200px, XHR with progress,
  presigned PUT flow — binary data never passes through the API server
- Wired both components into Clients.tsx staff portal pet cards
- Unit tests: 14 test cases covering all four routes (happy path + error cases)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-22 00:07:48 +00:00
Scrubs McBarkley 1f50fdff54 test(db): add unit tests for test factories (GitHub #94)
Tests cover resetFactoryCounters(), counter determinism, override
merging, and compile-time enforcement of required fields on
buildAppointment. All 16 new tests pass (92 total).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 23:43:47 +00:00
Scrubs McBarkley ad6024f3d9 feat: deterministic seed, impersonation migration, test factories (GRO-110)
Phase 1 — Seed Hardening:
- Replace all Math.random() calls in seed.ts with a Mulberry32 seeded PRNG
  (seed 42) so the same data set is reproduced on every run
- Replace crypto.randomUUID() with a PRNG-based UUID v4 generator
- Add manager (Jordan Lee) and receptionist (Sam Rivera) staff members
  to seed — previously all staff were groomers
- New packages/db/src/reset.ts drops all tables/enums and re-runs
  migrate + seed; exposed as `pnpm db:reset` at root
- Generate migration 0010_impersonation_sessions.sql for the
  impersonation_sessions and impersonation_audit_logs tables that were
  already in schema.ts but had no corresponding migration

Phase 2 — Test Factories:
- New packages/db/src/factories.ts with buildStaff, buildClient, buildPet,
  buildService, buildAppointment and resetFactoryCounters helpers
- Exported via @groombook/db/factories subpath (package.json + vitest alias)
- impersonation.test.ts updated to use buildStaff instead of hand-rolled
  fixture objects

Closes #90 (Phases 1 + 2)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 19:34:52 +00:00
Scrubs McBarkley 543c13f182 fix: correct TypeScript types in rbac.test.ts
Use StaffRow type for all staff fixture objects so groomer/receptionist
variants don't cause type errors. Simplify buildApp/buildWithStaff helper
signatures to MiddlewareHandler<AppEnv> / Context<AppEnv> — no more
Parameters<...> inference gymnastics.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 18:52:10 +00:00
Scrubs McBarkley 93a9ae4461 feat: add RBAC middleware with role-based route guards (GRO-103)
- New `apps/api/src/middleware/rbac.ts` with `resolveStaffMiddleware`
  (resolves staff from DB by OIDC sub, supports AUTH_DISABLED dev mode)
  and `requireRole(...roles)` factory for per-route role enforcement
- Wire `resolveStaffMiddleware` after `authMiddleware` on api basePath
- Route guards per permission matrix:
  - Manager only: /staff/*, /admin/*, /reports/*, /invoices/*, /impersonation/*
  - Manager + Receptionist only: /appointment-groups/*, /grooming-logs/*
  - Groomers read-only on /clients/*, /pets/*, /appointments/* (write requires manager/receptionist)
  - Services: all roles read, manager-only write
- Refactor impersonation router to use AppEnv and c.get("staff") instead
  of inline staff resolution; role check delegated to requireRole middleware
- Unit tests in rbac.test.ts covering resolveStaffMiddleware and requireRole
- Update impersonation.test.ts to inject staff directly via context

Closes #88 (Phase 1)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 15:50:45 +00:00
Scrubs McBarkley b7145271fb fix: assert on deletedId in DELETE test to resolve unused-vars lint error
Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 01:52:18 +00:00
Scrubs McBarkley d4629baaea fix: update ImpersonationBanner tests to match current component API
- Import ImpersonationSession from @groombook/types (component was updated in #78)
- Remove stale tests: "shows customer name" and "returns null when inactive"
  (component no longer renders customer name or checks session.active)
- Add isExtended prop to all render calls (component now takes isExtended as prop)
- Fix "does not show Extend button when already extended" to pass isExtended={true}
  instead of session.extended (prop was extracted from session in #78)
- Fix clients.test.ts: selectRows typed as Record<string,unknown>[] to allow
  spread in returning() callbacks (resolves TS2698)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 01:50:51 +00:00
Scrubs McBarkley d85e09cb11 test: add unit tests for email service, clients route, and ImpersonationBanner
- Email service: 16 tests covering buildConfirmationEmail and buildReminderEmail
  (recipient, subject, body content, groomer presence/absence, reminder timing)
- Clients route: 17 tests covering CRUD endpoints including validation,
  404 handling, soft-disable (disabledAt), and confirm-required delete
- ImpersonationBanner: 8 tests covering render, session expiry auto-end,
  Extend button visibility, and End/Audit button callbacks

Part of GRO-76 (Phase 1 unit/integration tests).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-21 01:50:02 +00:00
groombook-paperclip[bot] 70958542f8 feat: Staff Impersonation backend + frontend wiring (#75)
* feat: implement Staff Impersonation backend and wire frontend

Add server-side impersonation session management with full audit
logging, replacing the frontend-only mock. Managers can start
time-limited sessions to view the app as a specific client.

Backend:
- Add impersonation_sessions and impersonation_audit_logs tables
  (Drizzle schema) with proper FK constraints and status enum
- Add Hono API routes: start/get/extend/end session + audit logging
- Server-side session expiration, one-active-per-staff enforcement
- Staff role validation (manager-only)

Frontend:
- Add CustomerPortal wrapper with URL-param session init
- Add ImpersonationBanner with live countdown timer
- Add AuditLogViewer modal for session audit trail
- Add "View as Customer" button on Clients page
- Auto-log page visits during impersonation

Closes #74

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* chore: remove unused useNavigate import from Clients.tsx

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

* fix: add authorization + expiry checks to impersonation endpoints, add tests

Security: Add ownership verification (resolveStaff + staffId check) to
GET /sessions/:id, POST /sessions/:id/log, and GET /sessions/:id/audit-log
endpoints that were previously unprotected.

Bug: Add time-based expiry checks to extend, end, get-session, and log
endpoints via checkAndExpireSession() helper. Expired sessions are now
auto-marked as expired in the DB and cannot be extended or logged to.

Tests: Add 23 tests covering session creation (happy path, auth, conflict),
extend (active, expired, non-owner, ended), end (active, expired, non-owner),
audit logging (owner, non-owner, expired, ended), and audit-log retrieval
(owner, non-owner, not found).

Addresses QA review on PR #75 (GRO-66).

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

* fix: resolve @groombook/db source in vitest config

Add resolve alias so vitest can resolve @groombook/db from source
TypeScript files without requiring a prior build step. Fixes CI
test failures when dist/ has not been compiled.

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

---------

Co-authored-by: Groom Book CEO <ceo@groombook.dev>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Groom Book CTO <cto@groombook.dev>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Scrubs McBarkley <scrubs@groombook.app>
2026-03-20 08:16:09 +00:00
groombook-paperclip[bot] cba502e35f Set up unit testing infrastructure
Extract slot generation from book.ts into pure utility for unit testing.
Add 8 API unit tests and 4 web component tests with coverage thresholds.

Closes #39

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-18 01:55:02 +00:00