Commit 05a6e6f8 authored by DevPilot's avatar DevPilot

docs: extreme-detail reference for all 5 system wizards

Member retroactive entry, SA player registration, SA hourly booking,
swimming pool reservations, and the accounting revenue-mapping wizard.
Every field, validation rule, permission key, button, DB write, and
error message, sourced from reading the actual controller/service code
rather than the UI — plus a list of real defects found along the way
(transaction leaks, dead endpoints, schema mismatches, silent failures)
flagged for a deliberate fix pass rather than folded in unannounced.
parent 9236b6a1
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
# Wizard: حجوزات السباحة (Swimming Pool Reservations Wizard)
`/sa/swimming/pool-reservations/wizard` · `SportsActivity\Controllers\Swimming\PoolReservationController` · permission `sa.pool_reservation.create`
## What this actually is (correcting the obvious assumption)
Despite the name, this is **not** a pool-zone/time-slot grid booking screen. It never touches `sa_pool_zone_templates` or the Facility Grid tables at all. It's a **sales wizard**: it sells a lane-rental or session-card *package* (a fixed number of sessions at a fixed price) to a booker — typically a freelance coach or an outside entity — and creates a synthetic "group" to hold the sessions. **Actual scheduling happens afterward, manually, in the separate Facility Grid drag-and-drop screen** (`/facility-grids/{gridId}`) — the detail page for an unscheduled reservation says so in plain text: *"لم يتم جدولة حصص بعد — اسحب المجموعة في المراية لتحديد المواعيد."*
## Architecture — single-page, no server-side step tracking
One `GET` route renders one page with all 3 steps + inline JS; state lives entirely in a client-side object and is never persisted between steps. **A page refresh at any point restarts at step 1.** The only server round-trips before final submit are a one-time pricing fetch (cached client-side) and, optionally, a capacity-check endpoint that **the wizard's own JS never actually calls** (see Known Issues).
## Routes
| Method | Path | Handler | Permission |
|---|---|---|---|
| GET | `/sa/swimming/pool-reservations` | `list` | `sa.pool_reservation.view` |
| GET | `/sa/swimming/pool-reservations/wizard` | `wizard` | `sa.pool_reservation.create` |
| GET | `/sa/swimming/pool-reservations/{id}` | `show` | `sa.pool_reservation.view` |
| POST | `/sa/swimming/pool-reservations/{id}/use-session` | `recordSession` | `sa.pool_reservation.manage` |
| POST | `/sa/swimming/pool-reservations/{id}/cancel` | `cancel` | `sa.pool_reservation.manage` |
| GET | `/api/sa/swimming/pool-reservations/pricing` | `pricing` | `sa.pool_reservation.create` |
| POST | `/api/sa/swimming/pool-reservations/capacity-check` | `capacityCheck` | `sa.pool_reservation.create`**dead, never called** |
| POST | `/api/sa/swimming/pool-reservations/store` | `store` | `sa.pool_reservation.create` |
## Step 1 — Booker info
Fields: booker name (**required**, only client-enforced), phone (optional), booker type (select: freelance_coach / entity / individual — **hardcoded in the view, not DB-driven**), participant count (number, 1–100, clamped client-side). No server call on this step at all.
## Step 2 — Package selection
On entry, fetches `GET /api/sa/swimming/pool-reservations/pricing` once, returning two lists from `sa_academy_pricing` (`category IN ('lane_rental','session_card')`, active, within effective date range):
- **Lane rentals**, grouped by `lane_type` (50m/25m/mix), each with `sessions_per_month` and price. Seeded prices: 50m → 12/8/4/1 sessions at 4100/2750/1400/410 EGP; 25m → 3000/2100/1200/350; mix → 24 sessions at 6350.
- **Session cards**: `total_sessions` at a flat price — 24/12/8/6/4/1 sessions at 1450/800/580/520/440/140 EGP.
Selecting a lane package triggers an overflow check: the wizard has a **hardcoded, client-side-only** lane-capacity table (`{50m:12, 25m:8, mix:24}`) duplicated from a server-side constant — if `participant_count` exceeds it, the user must additionally pick "extra lane" or "overflow session cards," priced by re-using the session-card list. Session-card packages have no capacity concept, so this check doesn't apply to them.
**No field on this step is server-validated for correctness before final submit** — the price, session count, and overflow amount the client computes are trusted as-is at `store()` time (see Known Issues).
## Step 3 — Confirm
One field: notes (optional). Confirm button (**"تأكيد وإرسال للخزينة"**) POSTs to `/api/sa/swimming/pool-reservations/store` with the full accumulated state.
## Server-side validation on submit (`PoolReservationService::create()`)
| Condition | Message |
|---|---|
| Booker name blank | اسم الحاجز مطلوب |
| Reservation type not one of lane_50m/lane_25m/lane_mix/cards | نوع الحجز غير صالح |
| Sessions total < 1 | عدد الحصص غير صالح |
| Unit price ≤ 0 | السعر غير صالح |
| Matching `sa_programs` row missing | برنامج الإيجار غير موجود — يرجى تشغيل المايقريشن |
| Any DB exception | فشل إنشاء الحجز: {raw exception text — leaked verbatim to the UI} |
**Not validated at all:** participant count (only clamped to a minimum of 1), phone format, and — critically — the price/session-count/overflow figures themselves are taken as-is from the client, not recomputed from `sa_academy_pricing`. The `pricing` and `capacity-check` endpoints are advisory only; nothing stops a crafted request from posting an arbitrary price.
## What gets written on success
Inside one transaction:
1. **`sa_groups`** — a synthetic group is created to host the future sessions (`source_type='pool_reservation'`, `coach_id=null`, `max_capacity=current_count=participant_count`, `is_full=0` — note it's created already at full capacity by definition but flagged as not full).
2. **`sa_pool_reservations`** — the reservation itself: `reservation_number` (`PR-YYYYMMDD-NNNN`, generated by a `MAX()+1` query with **no locking** — two concurrent submissions on the same day can race into a duplicate, which then throws on the table's unique constraint and surfaces as the generic DB-exception message above), `total_amount = unit_price + overflow_amount`, `payment_status='pending'`, `status='active'`, `expiry_date = today + 2 months` (hardcoded).
3. `sa_groups.pool_reservation_id` backfilled.
After the transaction commits, if `total_amount > 0`, a `payment_requests` row is created (`payment_type='pool_reservation'`, `member_id=0` — explicitly whitelisted for this type since a freelance coach isn't a member). **The resulting request ID is never written back onto `sa_pool_reservations.payment_request_id`** — a column that exists specifically for this — which breaks the reverse lookup the void-handling listener needs; voiding this payment in the treasury will silently fail to revert the reservation's payment status.
Cashier collection later fires `payment_request.completed``handlePoolReservationPaid()`, which sets `payment_status='paid'` and a `payment_id`.
## Known issues found in this review
- **`capacityCheck` endpoint is fully built server-side but never called** — the wizard reimplements the same math in raw client JS with a duplicated, hand-maintained capacity table that will silently drift if the server-side constant ever changes.
- **No server-side re-validation of price or overflow amount** — advisory-only client math is trusted at submit time.
- **`payment_request_id` never persisted** on the reservation, breaking void-reversion.
- **Cancelling a reservation does not cancel its pending payment request** — it's left dangling in the treasury queue (a consequence of the same missing link above).
- **No branch scoping at all**`sa_pool_reservations.branch_id` exists in the schema but this flow never populates it; reservations are effectively branch-less club-wide, unlike most other screens in the system.
- Possible schema/code mismatches worth checking directly against the live DB before relying on them: `sa_groups.coach_id` may be `NOT NULL` in the original migration while this flow inserts `null`; `handlePoolReservationPaid()` writes to a `payment_id` column that the recorded migration for `sa_pool_reservations` never defines. If either is true on the live database, the affected write silently fails (caught and logged, not surfaced) and the reservation's payment status can get stuck.
- Raw exception text is shown verbatim to the end user on any DB failure during creation.
## Related screens
List (`/sa/swimming/pool-reservations`, launch point for "حجز جديد") · detail/show screen ("تسجيل حصة" decrements remaining sessions and auto-completes + archives the group when they hit zero; "إلغاء الحجز" cancels both the reservation and its group) · **Facility Grid** (`/facility-grids/{gridId}`) — where the created group actually gets scheduled onto real lanes/times; legacy `/pool/{id}/grid` URLs redirect here · Groups module (every reservation spawns one, but the detail page's "view group" link always points at the generic group list, not the specific group) · Cashier/Treasury queue · pricing source (`sa_academy_pricing`, currently only maintainable via migration/seed — no admin CRUD screen for these prices was found in this module).
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment