fix: appointment conflict detection, soft-delete, and auth guardrail (#18-22) #24

2026-03-17T19:30:47Z

ghost commented

2026-03-17 19:30:47 +00:00

(Migrated from github.com)

Summary

Fixes five bugs flagged in the CEO code review, all of which were merged without fixes in PRs #15 and #16.

#18 — Race condition in conflict detection: The hasConflict check and subsequent INSERT/UPDATE are now wrapped in a db.transaction() call, preventing two concurrent requests from both passing the conflict check and creating double-bookings.
#19 — PATCH skips conflict when staffId omitted: The condition now falls back to the existing appointment's staffId when the field is absent from the request body. Rescheduling an appointment without resending staffId correctly checks conflicts against the already-assigned groomer.
#20 — DELETE hard-removes records: DELETE /appointments/:id now sets status = 'cancelled' instead of removing the row, preserving audit trail and financial records.
#21 — Staff DELETE ignores active appointments: DELETE /staff/:id now checks for existing non-cancelled appointments referencing the staff member and returns 409 with a clear message if any exist.
#22 — AUTH_DISABLED has no production guardrail: On module load, if AUTH_DISABLED=true is set, the API now:
- In production (NODE_ENV=production): logs a fatal error and calls process.exit(1).
- Otherwise: logs a startup warning so developers know auth is bypassed.

Files changed

apps/api/src/routes/appointments.ts — transaction wrapping, PATCH conflict fix, soft-delete
apps/api/src/routes/staff.ts — appointment check before staff delete
apps/api/src/middleware/auth.ts — production guardrail for AUTH_DISABLED

Test plan

POST two appointments with the same groomer/timeslot concurrently — only one should succeed (409 on second)
PATCH appointment to overlap existing booking without sending staffId — should return 409
DELETE appointment — record should remain in DB with status cancelled
DELETE staff member with active appointments — should return 409
DELETE staff member with no active appointments — should succeed
Start API with AUTH_DISABLED=true and NODE_ENV=production — process should exit with error
Start API with AUTH_DISABLED=true and NODE_ENV=development — should log warning and start

Closes #18, #19, #20, #21, #22

🤖 Generated with Claude Code

## Summary Fixes five bugs flagged in the CEO code review, all of which were merged without fixes in PRs #15 and #16. - **#18 — Race condition in conflict detection**: The `hasConflict` check and subsequent INSERT/UPDATE are now wrapped in a `db.transaction()` call, preventing two concurrent requests from both passing the conflict check and creating double-bookings. - **#19 — PATCH skips conflict when staffId omitted**: The condition now falls back to the existing appointment's `staffId` when the field is absent from the request body. Rescheduling an appointment without resending `staffId` correctly checks conflicts against the already-assigned groomer. - **#20 — DELETE hard-removes records**: `DELETE /appointments/:id` now sets `status = 'cancelled'` instead of removing the row, preserving audit trail and financial records. - **#21 — Staff DELETE ignores active appointments**: `DELETE /staff/:id` now checks for existing non-cancelled appointments referencing the staff member and returns `409` with a clear message if any exist. - **#22 — AUTH_DISABLED has no production guardrail**: On module load, if `AUTH_DISABLED=true` is set, the API now: - In production (`NODE_ENV=production`): logs a fatal error and calls `process.exit(1)`. - Otherwise: logs a startup warning so developers know auth is bypassed. ## Files changed - `apps/api/src/routes/appointments.ts` — transaction wrapping, PATCH conflict fix, soft-delete - `apps/api/src/routes/staff.ts` — appointment check before staff delete - `apps/api/src/middleware/auth.ts` — production guardrail for AUTH_DISABLED ## Test plan - [ ] POST two appointments with the same groomer/timeslot concurrently — only one should succeed (409 on second) - [ ] PATCH appointment to overlap existing booking without sending staffId — should return 409 - [ ] DELETE appointment — record should remain in DB with status `cancelled` - [ ] DELETE staff member with active appointments — should return 409 - [ ] DELETE staff member with no active appointments — should succeed - [ ] Start API with `AUTH_DISABLED=true` and `NODE_ENV=production` — process should exit with error - [ ] Start API with `AUTH_DISABLED=true` and `NODE_ENV=development` — should log warning and start Closes #18, #19, #20, #21, #22 🤖 Generated with [Claude Code](https://claude.com/claude-code)

This repo is archived. You cannot comment on pull requests.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/app#24