# NVG-BEA-011 — Group Bookings — Gap Analysis + Implementation Plan

**Status: COMPLETE — started 2026-07-22, finished 2026-07-23 (W0–W5 all done).** Live tracker at the bottom.
Spec: NVG-BEA-011 BRD (user-provided). Audited 2026-07-22 (agent, file:line evidence).

## Verdict: shop-portal substrate DONE; the customer-facing heart is MISSING

### DONE (shop portal "walk-in party" tool)
- Schema: `booking.group_booking_id` (indexed) + `guest_label` + `is_group_organiser`
  (m260623_211500). No parent table — groups aggregate by id (by design).
- `GroupBookingService`: atomic N-child creation w/ per-guest placement re-validation +
  per-child charge derivation; whole-group cancel (per-child refunds via ledger) /
  collect / complete / start / reschedule; summaries.
- Portal UI: booking-calendar "New group booking" modal, day-grid "group of {n}" badge,
  detail drawer shows group id, AgentsBookings Parties drawer/modal, backend admin view.

### GAPS
1. **No CUSTOMER_APP surface at all** (api/ has zero group awareness) — create/view/cancel
   group as lead booker doesn't exist. **[api-tier — mobile contract]**
2. **No single-transaction Paymob group payment** — `payment_timing` just sets numeric
   fields; api `actionPay` is strictly single-booking. **[api-tier]**
3. **No max group size** anywhere (BRD: default 10, platform + per-shop configurable);
   only min≥1 (service) / ≥2 (UI).
4. **Cancellation is whole-group only** — no per-participant cancel API; a solo cancel of
   one child bypasses the group path unguarded.
5. **TOCTOU race**: placement re-check inside the transaction but no row lock — two
   concurrent creates for the same specialist/time can both pass.
6. Group id format is `GRP-` + random hex, not the spec's `GRP-YYMMXXXXX` (date-encoded).
7. Guest-label fallback is "Guest {n}", BRD wants the lead booker's name.
8. Bookings LIST surfaces (frontend list/backend list) lack a group indicator (calendar
   has one).

### Product flags (recorded)
- **API contract**: W3 adds NEW api endpoints only (additive — existing mobile endpoints
  untouched, nothing breaks). The mobile team must build the Group Booking UI against
  them. Flagged per project rule.
- **Per-participant classification**: BRD says each participant's classification applies
  independently, but participants share the LEAD's account/phone (no identities). True
  per-participant attribution needs optional per-guest phone capture (api + app change) —
  **deferred/flagged**; today classification keys to the lead booker (documented).

## Waves
- **W1 Config (additive):** `commercial_config.max_group_size` INT DEFAULT 10 +
  `shop.max_group_size` INT NULL (migration); admin fields (Commercial Config form +
  Shop form); resolution helper (shop ?? global ?? 10).
- **W2 Engine hardening (common + portal):** enforce the cap in `createGroupBooking` (+
  create-UI hint); date-encoded id `GRP-YYMM#####` (new bookings only); per-participant
  cancel `cancelParticipant()` (standard policy on that child, siblings untouched, group
  summary stays consistent) + portal UI action in the Parties drawer; row-lock the
  placement window (FOR UPDATE inside the create transaction — locally, without editing
  shared BookingScheduleService); guest-label fallback → lead booker's name.
- **W3 CUSTOMER_APP endpoints (api-tier, ADDITIVE ONLY — new actions, zero edits to
  existing ones):** v1 group-booking create (participants: services/specialist?/slot,
  names optional; per-participant conflict errors; cap enforced), group view, group
  cancel + participant cancel (lead-scoped), and **group-pay**: ONE Paymob transaction
  covering the group total (mirrors actionPay's idempotency FOR UPDATE guard; payment
  failure ⇒ whole group fails, no partial confirmation).
- **W4 Portal polish:** group indicator + linked-group view in the frontend bookings list
  + backend bookings list (calendar already has it); month-view dot where sensible.
- **W5 Translations + unit tests (acceptance criteria: 3-linked-creation, conflict
  identifies participant, per-participant cancel refund, name fallback) + smoke + docs.**

### Sequencing
W1 → W2 → (W3 ∥ W4) → W5. Coordinated with the still-running BEA-003 W3 + BEA-005 W3
agents (disjoint files).

### Out of scope (per BRD)
~~Split payment~~ · group pricing/discounts · multi-day groups.

> **Scope expansion — 2026-08-04 (owner decision, mobile-audit follow-up).** Two items
> were pulled INTO scope in response to the mobile team's `NVG_FINAL_REVIEW` audit and
> an explicit owner go-ahead:
> - **W6 — per-participant time slots (B1):** `group-booking/create` now accepts an
>   optional `time_slot` (`HH:MM`) per participant. **Additive + backward-compatible:**
>   when omitted, every guest keeps the single shared start (BR-G06) exactly as before,
>   so the walk-in flow (AgentsBookings/BookingCalendar) is byte-for-byte unchanged. The
>   distinct-specialist guard (BR-G06) is generalised to a **per-specialist interval
>   overlap** check — identical rejection when guests share a start, but two guests may
>   now share a specialist at non-overlapping times.
> - **W6 — per-participant package redemption / mixed payment (B2):** each participant
>   may carry a `package_redemption_id` (a `PackageEntitlement`). That guest is paid by a
>   package session (payment_mode `PACKAGE`, `package_entitlement_id` set, cash value 0,
>   session decremented), and is **excluded from the group Paymob total** — i.e. a party
>   may now mix package-paid and cash-paid guests. This supersedes the original
>   "split payment out of scope" line above. Redemption is validated + decremented
>   **controller-side** (mirroring the solo `/subscription-package/redeem` guards) so the
>   shared `GroupBookingService` core stays intact.

## Live tracker
- [x] W0 audit + this plan
- [x] W1 config — DONE 2026-07-22. Migration `m260722_180000_add_group_booking_config_schema`
  (`commercial_config.max_group_size` INT NOT NULL DEFAULT 10, `shop.max_group_size` INT NULL)
  applied + verified via `SHOW COLUMNS` and `migrate/history`. Models: `CommercialConfig`
  (+`@property`, `integer` rule min 1/max 50) and `Shop` (+`@property`, nullable `integer`
  rule min 1/max 50) updated to mirror the BEA-002/BEA-005 override idiom.
  `GroupBookingService::maxGroupSize(?Shop $shop): int` added (resolution only — shop
  override ?? commercial_config ?? `DEFAULT_MAX_GROUP_SIZE = 10`; no other
  GroupBookingService code touched — enforcement is W2). Admin fields: "Group Bookings"
  card on `commercial-config/index.php` + "Max group size (override)" on `shop/_form.php`
  Section 2 (blank = inherit), both flow through the existing `load()`/`saveAll()` mass
  assignment (no controller changes needed — `max_group_size` is a safe attribute on
  both models). New `Yii::t('backend', ...)` keys (untranslated pending a separate i18n
  pass): 'Group Bookings', 'Max group size', 'participants', 'Platform default cap on
  participants per group booking. Shops can be given a lower or higher override.',
  'Max group size (override)', 'Blank = inherit the platform default group-size cap.'.
  Lint clean; backend smoke 200 on both `/commercial-config/index` and `/shop/update?id=1`
  with fields present, no error markers.
- [x] W2 engine hardening — DONE 2026-07-22. Files: `common/components/
  GroupBookingService.php`, `frontend/controllers/{AgentsBookingsController,
  BookingCalendarController}.php`, `frontend/views/booking-calendar/_group_booking.php`,
  `frontend/views/agents-bookings/{_group_drawer,_group_modal,index}.php`.
  (1) **Cap (gap 3):** `createGroupBooking()` now rejects `count(guests) > maxGroupSize
  ($shop)` with `'A party can have at most {max} guests.'` before any heavier
  validation; the cap is passed from both controllers (`AgentsBookingsController::
  actionIndex`, `BookingCalendarController::actionGroupBooking`) into both create UIs
  (`_group_booking.php`'s aurora iframe modal + `agents-bookings/index.php`'s embedded
  `_group_modal.php`) as a hint under "Add guest" + a client-side disable on the
  button once the party count reaches the cap (defense-in-depth guard on the click
  handler too — server is the real gate). (2) **Date-encoded id (gap 6):**
  `newGroupBookingId()` now returns `GRP-YYMM-XXXXX` (5-char uppercase alnum, e.g.
  `GRP-2607-G2BG2` — confirmed via the regression command), collision-checked against
  existing rows with up to 3 regenerate attempts; old `GRP-<9 hex>` ids stay valid
  forever (lookup is always by exact string, never by parsing the format). (3)
  **Per-participant cancel (gap 4):** new `GroupBookingService::cancelParticipant
  (Booking $child, ?Shop $shop): array` mirrors `cancelGroup`'s per-child refund/
  status logic (same `FinanceLedgerService` calls) for ONE child, atomic in its own
  transaction; refuses when `$child` is already terminal, isn't a group child, or
  would be the LAST active sibling (directs the caller to "Cancel all" instead —
  keeps BR-G20 the sole path that can zero out a party). Wired via new shop-scoped
  POST `AgentsBookingsController::actionCancelParticipant` (verb-filtered) + a
  per-guest cancel button in the Parties drawer (`_group_drawer.php`, shown only for
  non-terminal guests) using `ngConfirm` (never native `confirm`). (4) **Row lock /
  TOCTOU (gap 5):** `createGroupBooking()` now opens its transaction BEFORE
  `checkPlacement`, takes `SELECT id FROM booking WHERE agent_id IN (...) AND
  DATE(booking_date) = :d FOR UPDATE` on the candidate specialists' rows for the
  target day first, THEN runs the per-guest `checkPlacement` loop and persists — all
  inside the one transaction (mirrors the `SELECT ... FOR UPDATE` idiom in
  `api\controllers\BookingController::actionReschedule/actionPay`; `checkPlacement`
  itself/`BookingScheduleService` untouched, still shared/read-only). (5) **Name
  fallback (gap 7):** `resolveLabel()`'s no-label/non-organiser fallback is now the
  organiser's (lead booker's) name per the BRD, falling back further to `'Guest {n}'`
  only if the organiser has no name on file; explicit labels + the organiser tag are
  unchanged. New `Yii::t('frontend', ...)` keys added untranslated (deferred to W5
  per this wave's scope — NO `common/messages/*` edits made): the cap-exceeded
  message, `cancelParticipant`'s two error strings, `'Cancel this guest'`,
  `confirmCancelParticipant`, `maxGuestsHint`, `partyFull`, `'Guest not found.'`,
  `'The guest was cancelled.'`. Verify: `php -l` clean on all 7 touched files;
  `php console/yii group-booking-check/run` → PASS (new id format, cap default 10
  unaffected at party-size 2) and `group-booking-check/user-shop-service` → PASS;
  targeted unit suites green — `FinanceLedgerServiceTest` 33/33, `PaymentProcessingFeeTest`
  13/13, `BookingCompletionServiceTest` 2/2 (48/48, 0 regressions); curl smoke —
  `shop.navagoo.localhost/booking-calendar/index` → 200, `/agents-bookings/index` → 302
  (auth redirect, no 500), `/booking-calendar/group-booking?real=1` → 302,
  `POST /agents-bookings/cancel-participant` (no auth) → 302 — no 500s anywhere.
- [x] W3 customer-app endpoints (additive, flagged) — DONE 2026-07-23. NEW
  `api/controllers/GroupBookingController.php` (extends MyActiveController — bearer
  auth + ResponseHelper envelope) + ADD-only route block in `api/config/_urlManager.php`
  (`git diff --stat api/` = _urlManager +16 lines, controller untracked-new; zero other
  api files touched). Endpoints: POST `/group-booking/create` (lead booker = auth
  customer; shop-active check; cap via `GroupBookingService::maxGroupSize`; participants
  {name?, specialist_id? → auto-assign via user_shop_service capability +
  checkPlacement, service_ids[], is_organiser? default idx 0}; per-participant 422s
  carry `participant_index`; service placement conflicts surfaced VERBATIM as 409
  "«label» · «specialist»: «reason»"); GET `/group-booking/view/<id>`;
  POST `/group-booking/cancel-group` + `/cancel-participant` (lead-scoped, last-child
  guard mirrored); POST `/group-booking/pay`. **Create app-stamping:** service is
  called with on_visit timing inside an OUTER controller transaction (service tx nests
  as savepoint), then children re-stamped `booking_method='mobile'` (classification ⇒
  app/navagoo_sourced path) + real payment_mode; online/deposit parties are left in
  STATUS_SELECTED_NOT_PAID (actionBook's pending-payment status, hidden from the list)
  until pay — avoids the walk-in 'online'-timing premature amount_collected/
  processing-fee stamp (idempotent fee guard would have blocked the real fee).
  **Group pay:** ONE Paymob transaction for the whole group (sum of totals, or sum of
  per-child deposits w/ ShopPaymentSettings pct); server-side inquiry verification +
  amount_cents ≥ group-charge guard; SAME `SELECT payment WHERE tran_ref FOR UPDATE`
  idempotency idiom as actionPay (+unique tran_ref index); ONE Payment(+Transaction)
  row attached to the ORGANISER child (payment has no meta column — reconciliation via
  organiser child's group_booking_id; every child stamped invoice_id=tran_ref);
  existing /webhook/paymob settles by tran_ref w/ no booking-type assumptions — group
  row compatible unchanged; children flip to SCHEDULED atomically, whole group fails
  together (gateway not-Paid / underpay / DB error ⇒ all children stay
  SELECTED_NOT_PAID). **Customer-cancel semantics:** one ADDITIVE service method
  `GroupBookingService::cancelAsCustomer()` (existing methods untouched) — cancelGroup/
  cancelParticipant are the SHOP paths (unconditional full refund, CANCELED_BY_SHOP,
  cancelled_by='shop'), wrong for a lead-booker cancel; the new method applies the
  standard customer policy (zone refunds via FinanceLedgerService, STATUS_CANCELED,
  cancelled_by='customer', ledger re-derive as customer cancel). Docs: Group Bookings
  section (§3.2) added to `ai_specs/03_API/API_CONTRACTS.md` with request/response
  examples. New Yii::t('backend', …) strings untranslated (deferred to W5; no
  common/messages edits). Verify: php -l clean ×3; unauth curl via Host
  api.navagoo.localhost on all 5 routes → 401 unauthorized envelope (routing proven,
  no 404/500); unit suites green — FinanceLedgerServiceTest 43/43,
  PaymentProcessingFeeTest 13/13, BookingCompletionServiceTest 2/2. **Mobile-team
  flag:** these are NEW contracts for the app's Group Booking UI; nothing existing
  changed.
- [x] W4 portal polish — DONE 2026-07-22. Re-audited GAP 8 first: the frontend
  booking-calendar List view (`_list.php` + `BookingListService.php`) and the backend
  `booking/index.php` list ALREADY carried a group pill (`Group`/`users`-icon +
  visible `group_booking_id`), a "Party of {n}" chip, and (backend) an inline
  `guest_label` — landed in earlier commits (`4d37eda`, pre-dating this plan's audit),
  so GAP 8's premise was partly stale. What was actually missing, now added: (1)
  frontend group-child rows didn't surface `guest_label` — `BookingListService::
  groupRow()` now emits `guestLabel` per kid and `_list.php` appends it in muted text
  after the child's name ("Sara — Guest 2"), skipped when equal/blank; (2) the backend
  pill wasn't a link — wrapped in `Html::a(['booking/view','id'=>$model->id])` +
  `title` attr carrying the full `group_booking_id`; (3) month view had zero group
  awareness — `BookingCalendarViewModel::month()` (route-local, NOT the shared
  `BookingCalendarPresenter`) now flags `isGroup` per event off the AR rows it already
  loads (no extra query), and `_month.php` renders a small users-icon dot + "Group
  booking" title on grouped event pills. New i18n key (untranslated, deferred to W5
  per this task's scope — NO common/messages edits made): `Yii::t('frontend', 'Group
  booking')`. Lint clean (`php -l`) on all 5 touched files. Smoke: backend
  `/booking/index` → 200, no error markers; frontend `/booking-calendar/index?view=list`
  and `?view=month` → 200, no error markers, real page titles rendered (guest session,
  so rows likely empty — no crash either way).
- [x] W5 translations + tests + verify — DONE 2026-07-23.
  **Translations:** every `Yii::t` key across the 15 BEA-011-touched files audited
  against `common/messages/en/*`; api-tier strings confirmed to use the `backend`
  category (matches every existing api controller; i18n resolves from
  `@common/messages` via the shared fileMap — api has no own messages dir). Missing
  keys added to BOTH en+ar with real Arabic, placeholders verbatim: **frontend 11**
  (cap-exceeded, per-guest cancel strings, 'Group booking', 'Guest not found.',
  party-full/max-hint, last-guest guard, terminal-guard, non-group guard) +
  **backend 28** (the api GroupBookingController error strings incl. the 6
  `Participant {n}: …` messages, the W1 admin config labels 'Group Bookings'/'Max
  group size'/'Max group size (override)'/'participants'/2 hints, plus 4 pre-existing
  untranslated commercial-config messaging labels living in the BEA-011-touched
  view — Sell/Cost/Refund-on-fail/Margin). Parity verified in-container: frontend
  en=ar=1911, backend en=ar=3348, common 232/232, shop 142/142 — 0 one-sided keys;
  `php -l` clean on all 4 message files.
  **Unit tests:** NEW `common/tests/unit/components/GroupBookingServiceTest.php` —
  15 tests / 72 assertions, ALL GREEN: (a) maxGroupSize resolution (shop override →
  commercial_config insert-or-update in-tx → DEFAULT 10; over-cap createGroupBooking
  rejected with the exact translated message) (b) `GRP-\d{4}-[A-Z0-9]{5}` id format
  + current-YYMM prefix (c) resolveLabel via reflection (explicit label → organiser
  tag → organiser-name fallback → 'Guest {n}' only when organiser nameless)
  (d) cancelParticipant guards (terminal child, non-group child, last-active-sibling
  refusal — pure + DB-backed) (e) real 3-child party creation (shared GRP id, one
  organiser row, distinct specialists, all labelled) + overlap conflict error naming
  the guest AND the specialist (f) cancelAsCustomer customer semantics
  (STATUS_CANCELED + cancelled_by='customer' + zone-derived refund; terminal child
  skipped) vs cancelParticipant's shop semantics (CANCELED_BY_SHOP +
  cancelled_by='shop' + refund stamped, sibling untouched). DB tests use an OUTER
  always-rolled-back transaction (service tx nests as savepoint — the exact nesting
  the W3 api controller relies on) with GroupBookingCheckController-style fixtures;
  nothing leaks into the dev DB. **Full suite: 350 tests / 828 assertions, 6 errors +
  15 failures — IDENTICAL to the documented pre-existing baseline (0 regressions;
  finance/entitlement/classification suites green).**
  **Smoke:** backend `/booking/index`, `/commercial-config/index`, `/shop/update?id=1`
  → all 200, 0 error markers; frontend (dev shop-owner session)
  `/booking-calendar/index` + `?view=list` + `?view=month` → 200, 0 markers, group UI
  present ('New group' button + group-booking modal link); api unauth ×5
  (`POST /group-booking/{create,cancel-group,cancel-participant,pay}`,
  `GET /group-booking/view/<id>`) → all 401 Unauthorized envelopes;
  `php console/yii group-booking-check/run` → PASS (id `GRP-2607-…`, 2 children,
  fixtures cleaned).