Skip to content

E
El-Captain Sports Management
Commit 2ef6e088 authored by Mahmoud Aglan's avatar Mahmoud Aglan
Browse files
Options

docs(mobile-portal): the approved programme, its adversarial review, and what it supersedes 

Another session builds the portal from here, so the entry point has to survive
being read cold.

docs/specs/mobile-portal/ holds three files. 01-program-plan.md is the approved
programme — decisions, workstreams, art direction, the full feature inventory and
verification. 02-critique-addendum.md is a four-lens review of that plan
(completeness, security/abuse, financial integrity, delivery risk) with every
claim checked against the code; where the two disagree the addendum wins, and it
replaces the plan's build order with S0–S10. 00-README.md is the map.

The README leads with four premises the plan was written on that turned out to be
false, because each changes what gets built: Livewire is ^4.3 not 3 (so a public
property is client-settable and validating in mount() is not enough); `dark:`
compiles to prefers-color-scheme with no @custom-variant declared, so ~900
utilities are live and untested rather than inert; `transactions` is one row with
debit and credit account columns, not a pair, contradicting CLAUDE.md and two
agent-rules files; and most of the domain the portal needs already exists.

That last one is the real hazard on this programme. The block-builder engine, the
parent portal, the push stack and the pricing entry points are all built, so the
README lists them explicitly under "do not rebuild" — the plan originally proposed
a second CMS before the review found the first one is generic enough to reuse.

Banners on mobile-app-plan.md, mobile-api-implementation.md and openapi.yaml:
all three describe the /api/v1 surface deleted in the previous commit, and a
native-Flutter-per-client approach that was replaced. Left in place as history,
marked so nobody builds from them.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent 883391c7
Hide whitespace changes
Inline Side-by-side
Showing with 1047 additions and 0 deletions
docs/api/openapi.yaml
View file @ 2ef6e088
# =====================================================================
# SUPERSEDED — DO NOT BUILD FROM THIS SPEC.
#
# This documents the /api/v1 surface, which has been DELETED from the
# codebase. It carried an authentication bypass, an unauthenticated
# academy-wide push broadcast, and several endpoints with missing or
# inverted ownership checks.
#
# Current programme: docs/specs/mobile-portal/00-README.md
# Kept for history only.
# =====================================================================
openapi: 3.1.0 openapi: 3.1.0
info: info:
title: El Captain Sports Management — Mobile API title: El Captain Sports Management — Mobile API
......
docs/specs/mobile-api-implementation.md
View file @ 2ef6e088
> **⚠️ SUPERSEDED — DO NOT BUILD FROM THIS DOCUMENT.**
>
> It plans a native Flutter app per client against a REST API at `/api/v1`.
> **That API was deleted** (it carried an authentication bypass), and the approach
> changed to a member-facing web portal with a thin WebView shell.
>
> Current programme: [`docs/specs/mobile-portal/`](mobile-portal/00-README.md).
> Kept for history only.
---
# Mobile API Implementation Plan — System Side # Mobile API Implementation Plan — System Side
This is the step-by-step work to prepare the Laravel backend for the Flutter mobile app. This is the step-by-step work to prepare the Laravel backend for the Flutter mobile app.
......
docs/specs/mobile-app-plan.md
View file @ 2ef6e088
> **⚠️ SUPERSEDED — DO NOT BUILD FROM THIS DOCUMENT.**
>
> It plans a native Flutter app per client against a REST API at `/api/v1`.
> **That API was deleted** (it carried an authentication bypass), and the approach
> changed to a member-facing web portal with a thin WebView shell.
>
> Current programme: [`docs/specs/mobile-portal/`](mobile-portal/00-README.md).
> Kept for history only.
---
# Mobile App Plan — Client App (Guardians & Participants) # Mobile App Plan — Client App (Guardians & Participants)
## Vision ## Vision
......
docs/specs/mobile-portal/00-README.md 0 → 100644
View file @ 2ef6e088
# Mobile Portal — programme documents
Everything needed to build the member-facing mobile web portal (players + guardians),
served from each client's own installation, with a thin per-client Flutter WebView shell
on top. **Read this file first.**
Paths in these documents are relative to the **repository root**, not to this folder.
---
## Read in this order
| # | File | What it is |
|---|---|---|
| 1 | `01-program-plan.md` | The approved programme: context, decisions, workstreams W0–W10, art direction, full feature inventory, build order, verification. |
| 2 | `02-critique-addendum.md` | A four-lens adversarial review of that plan (completeness, security/abuse, financial integrity, delivery risk), every claim verified against the code. **Where the two disagree, the addendum wins.** |
The addendum is not commentary. It corrects the plan, adds P0 work the plan missed, cuts
work the plan included, and replaces its build order with the S0–S10 sequence. Do not start
from the plan alone.
---
## Status
| Stage | State |
|---|---|
| **S0 — production safety** | **Done and pushed.** Vulnerable `/api/v1` surface deleted, 500-page disclosure closed, `ParentHome` IDOR locked, excuse-form PII write removed, entrypoint fails on migration error, env whitelist fixed, nginx exact-match locations added, page-builder fallback no longer answers reserved prefixes. |
| S1–S10 | **Not started.** This is the whole project. |
One item from S0 is deliberately **not** shipped: a migration revoking the old `mobile`
Sanctum tokens. It changes live client data, which the CLAUDE.md push rule says to ask
about first. It is not required for safety — the routes it protected no longer exist.
---
## Four premises that are false — do not inherit them
These were wrong in the original plan and are corrected in the addendum. They are repeated
here because each one changes what you build:
1. **Livewire is `^4.3`, not 3.** `CLAUDE.md` says Livewire 3 and is wrong. In Livewire 4 a
plain `public` property is settable from the browser, so validating an id in `mount()`
and then filtering queries on it in `render()` is an IDOR. Use `#[Locked]` **and**
re-validate.
2. **`dark:` is not inert — it is live and untested.** With no `@custom-variant` declared,
Tailwind v4 compiles `dark:` to `prefers-color-scheme`, so ~900 utilities already render
for every OS-dark user. The `.dark` class toggle is the part that does nothing.
3. **`transactions` is a single row** with `debit_account_id` + `credit_account_id` — not a
debit/credit pair. `CLAUDE.md` and two files in `docs/agent-rules/` describe a schema
that does not exist. The migration is the truth.
4. **The domain is largely already built.** See below.
---
## Already built — do not rebuild
The single biggest risk on this programme is writing something that exists. Verified
present:
- **A generic block-builder engine** — `app/Domain/Website/Blocks/` (`BlockType`,
`BlockField`, `BlockRegistry`, 31 block types) plus the generic field renderer
`resources/views/website/builder/field.blade.php`. A new content type needs **only a PHP
class** — no migration, no enum, no CHECK constraint, no form code. The addendum's
recommendation is to reuse this and **not** build a second CMS.
- **A parent portal** — 11 Livewire components in `app/Livewire/Parent/`, mobile-shaped
layout `resources/views/layouts/parent.blade.php`, routed under `/parent`. This is the
migration source for the new portal, and it gets retired at parity.
- **FCM push** — `PushNotificationService`, 12 event listeners, `device_tokens`,
`push_announcements`, `push_analytics`, scheduled reminder commands.
- **Domain services the portal reads from** — pricing (`PricingService::calculate()`, which
already accepts a `contextOverride` for pricing before a participant row exists),
enrolment, attendance, invoicing, wallets, installments, `service_requests`,
`contact_messages`, events with dynamic form fields, documents, evaluations.
- **WhatsApp sending** — `WhatsAppService` via an external messaging hub.
- **Paymob** — `PaymobService`, feature-flagged off.
---
## Superseded documents — actively misleading now
| Document | Why it is wrong |
|---|---|
| `docs/specs/mobile-app-plan.md` | Plans a **native Flutter app per client** against a REST API. The approach changed to a web portal + WebView shell, and **the API it describes has been deleted**. |
| `docs/specs/mobile-api-implementation.md` | Implementation plan for that same deleted API. |
| `docs/api/openapi.yaml` | Contract for endpoints that no longer exist. |
They are kept as history. Do not build from them.
---
## Build order (from the addendum — this replaces the plan's own)
```
S0 production safety ...................... DONE
S1 data integrity + the ledger ............ blocks all money and push UI
S2 branding + BrandingService ............. parallel with S1
S3 identity, roles, invitations, OTP ...... hard dependency for S4–S10
S4 portal shell + read-only screens ....... needs S2 + S3
S5 InstaPay payments ...................... needs S1 + S4
S6 PWA ................................... needs S2 + S4
S7 push to the portal ..................... needs S1 + S3 + S6
S8 QR check-in (staff-scan only) .......... needs S1 + S3 + S4
S9 Flutter wrapper ........................ needs S6 + S7 working in a browser
S10 app content ............................ no dependents, last
```
**S1 is not cleanup.** `PaymentService::resolveDebitAccount()` and `resolveCreditAccount()`
return hardcoded `1` and `2` with a `// TODO`, so every payment ever recorded posts to the
same two accounts. Routing a new payment channel into that ledger just multiplies the
problem.
---
## Decisions already made with the client
| Decision | Choice |
|---|---|
| Portal login | Admin-issued invitation + password; phone or email + password after |
| Accounts | One account, many linked profiles, with a profile switcher |
| QR check-in | Staff-scan only (the static-poster direction was cut — see addendum) |
| Notifications | PWA + push, reusing the existing FCM stack (VAPID was cut) |
| Existing `/parent` | Replace it, redirect, then retire |
| Non-members | Full self-registration — which makes phone verification mandatory |
| REST API v1 | Deleted |
## Still open — need a human answer
E1 dark mode (own it or delete it) · E2 age of majority for self-service · E3 whether the
Flutter shell ships this cycle at all · E4 duplicate-account merge · E5 overpaid proofs ·
E6 which payment-method CHECK constraints get `instapay` · E7 where an excuse lives ·
E8 branch attribution on portal payments.
Each is written up with a recommendation in section E of the addendum.
docs/specs/mobile-portal/01-program-plan.md 0 → 100644
View file @ 2ef6e088
<!--
STATUS: approved program plan, not yet implemented (except S0).
Paths in this document are relative to the REPOSITORY ROOT, not to this file.
Read 02-critique-addendum.md alongside it — the addendum SUPERSEDES parts of this
document and corrects four premises it was written on.
-->
# El Captain — Mobile Portal Program
## Context
El Captain is a per-tenant SaaS: every client gets their own CapRover app + dedicated
Postgres DB, all built from `main`, and every client already gets a public website out of
the box. The goal now is that every client also gets a **mobile app** for their players and
guardians — but built once as a **mobile web portal** served from the client's own
installation, with a thin per-client Flutter WebView wrapper on top, rather than a native
app per client.
Four deliverables were asked for:
1. Audit and fix the branding/appearance settings so tenant branding actually works.
2. A "Mobile App Content" CMS section, mirroring the existing website builder.
3. The portal itself at a public path, with a design language deliberately unlike the ERP.
4. A Flutter wrapper around that path, per client.
**Exploration changed the premise.** This is not greenfield. The repo already contains a
substantial member-facing backend — and one critical live vulnerability that must ship a
fix before anything else in this plan is built.
---
## What already exists (verified by reading the code)
| Already built | Where |
|---|---|
| A parent portal, 11 Livewire components, mobile-shaped layout | [app/Livewire/Parent/](app/Livewire/Parent/), [layouts/parent.blade.php](resources/views/layouts/parent.blade.php), routes at [web.php:602-614](routes/web.php#L602-L614) |
| A full mobile REST API v1 — 24 controllers, ~2,784 lines, Sanctum tokens, phone OTP | [routes/api.php](routes/api.php), [app/Http/Controllers/Api/V1/](app/Http/Controllers/Api/V1/) |
| FCM push: service, 12 event listeners, `device_tokens`, `push_announcements`, `push_analytics`, scheduled reminder commands | [PushNotificationService.php](app/Domain/Notification/Services/PushNotificationService.php), [routes/console.php:30-34](routes/console.php#L30-L34) |
| Paymob payment gateway (feature-flagged off) | [PaymobService.php](app/Domain/Financial/Services/PaymobService.php) |
| WhatsApp sending via an external messaging hub | [WhatsAppService.php](app/Domain/WhatsApp/Services/WhatsAppService.php) |
| `service_requests` (freeze/transfer/cancellation), `contact_messages` | migrations `2026_07_27_000005`, `2026_07_27_000003` |
| Events + registrations with dynamic form fields | [app/Domain/Event/](app/Domain/Event/) |
| A **generic, reusable block-builder engine** | [app/Domain/Website/Blocks/](app/Domain/Website/Blocks/) |
**The block engine is the single most valuable existing asset for this project.**
`BlockType` + `BlockField` + `BlockRegistry` + the generic field renderer
[website/builder/field.blade.php](resources/views/website/builder/field.blade.php) mean a new
content type needs *no migration, no enum edit, no CHECK constraint, and no form code* —
only a PHP class. `website_blocks.type` deliberately has no CHECK constraint. The Mobile App
CMS reuses this machinery wholesale instead of inventing a second one.
---
## Decisions taken
| Decision | Choice |
|---|---|
| Portal login | Admin-issued invitation + password (phone or email + password thereafter) |
| Accounts | One account, many linked profiles, with a profile switcher |
| QR check-in | **Both** directions supported, selectable per branch |
| Notifications | Full PWA + Web Push |
| Security hole | Fix **first, alone**, before any other work starts |
| Existing `/parent` portal | **Replace** it — build `/app`, redirect, then retire the old components |
| Non-members | **Full self-registration** — a stranger can sign up, register a child, and pay |
| REST API v1 | **Delete it entirely** |
Two of these interact in ways worth stating plainly, because they change the work:
**Deleting the API *is* the security fix.** Removing `routes/api.php` and the 24 controllers
eliminates the `0000` bypass, the unauthenticated `broadcast/send`, and every other
unaudited endpoint in one move — strictly safer than patching `AuthOtpController` and
leaving 23 unreviewed controllers exposed. So W0 becomes *delete the surface*, not *patch
it*. I flagged when asking that the Flutter wrapper would need a few endpoints back; the
resolution is that it does not need an API at all — the wrapper wraps the web portal, so it
carries the portal's session cookie and can POST its FCM token to an ordinary portal route.
No token auth, no second identity system.
*Pre-flight check before deleting:* confirm no client currently has a Flutter build in the
wild pointing at `/api/v1`. Everything in the repo says none exists (that is the premise of
this project), but this is a one-command check against the access logs and it must happen
first.
**Self-registration makes phone verification mandatory, not optional.** An open signup form
with no verification is a spam and duplicate-record generator, and it writes into `people`
and `participants` — the tables the whole ERP hangs off. So OTP-over-WhatsApp moves from
"nice to have later" into the critical path, using the existing `WhatsAppService`. It also
pulls in: `DuplicateDetectionService` (already exists) on every signup, an admin approval
queue for self-registered participants, and rate limiting on the signup endpoint.
---
## W0 — Remove the vulnerable API surface (ship on its own, before anything else)
[AuthOtpController.php](app/Http/Controllers/Api/V1/AuthOtpController.php) contains a
universal authentication bypass that is **live on every deployed client right now**:
```php
// AuthOtpController::verify(), ~line 75
$mode = SystemSetting::get('auth_otp_mode', 'demo');
$isBypass = $mode === 'demo' && $request->otp === '0000';
if (!$isBypass) { /* ...the only place the cached OTP is checked... */ }
```
- `auth_otp_mode` is **seeded as `'demo'`** ([migration `2026_07_27_000004`:11](database/migrations/2026_07_27_000004_seed_mobile_app_system_settings.php#L11)) and the code default is `'demo'`.
- The bypass skips the cache check entirely — no prior `otp/request` needed.
- It then resolves `User::withoutGlobalScope('academy')->where('phone', $phone)->where('status','active')` — **any** user, including academy owners and admins — and mints a Sanctum token with `['mobile:*']`.
- `routes/api.php` is registered in [bootstrap/app.php:15](bootstrap/app.php#L15), so this is reachable in production.
**Anyone who knows an active user's phone number can take over that account on every client instance.**
Two compounding problems:
- In `'sms'` mode, `requestOtp()` generates and caches an OTP but **never sends it anywhere** — so flipping the setting is not a mitigation, it locks everyone out.
- [BroadcastController::send()](app/Http/Controllers/Api/V1/BroadcastController.php) — push to the entire academy — has **no permission check at all**; any mobile token can call it.
### The fix (one commit, deployed on its own)
1. Verify no live Flutter client is calling `/api/v1` (access-log check).
2. Remove `api:` from [bootstrap/app.php:15](bootstrap/app.php#L15) and delete `routes/api.php`, `app/Http/Controllers/Api/V1/`, `app/Http/Resources/Api/V1/`, and the API rate limiters in [AppServiceProvider](app/Providers/AppServiceProvider.php).
3. Keep `/up` (framework health) and the `HealthController` web route.
4. **Keep** everything the API merely *used*: `PushNotificationService`, `device_tokens`, `push_announcements`, the 12 push listeners, the scheduled push commands, `service_requests`, `contact_messages`, `PaymobService`. None of these are API-specific and all are reused by the portal.
5. Revoke every existing `mobile` Sanctum token (`personal_access_tokens where name = 'mobile'`) so any token already minted through the bypass dies.
6. Replace the API tests in [tests/Feature/Api/](tests/Feature/Api/) with one regression test asserting `/api/v1/auth/otp/verify` is gone (404), so the surface cannot silently return.
Deleting rather than patching also removes the second bug for free: in `sms` mode
`requestOtp()` generated and cached an OTP but **never sent it**, so the setting was not a
usable mitigation either way. The OTP flow gets rebuilt correctly in W2, over WhatsApp, on
the session guard.
---
## W1 — Branding: one source of truth
Today branding lives in **four uncoordinated stores** with no sync: `academies` columns,
`system_settings` (group `branding`, keys `branding.*`), `website_settings`, and the `media`
table. `primary_color` exists three times with three different defaults (`#2563eb`,
`#1a1a2e`, `#1e40af`).
**Confirmed defects**
- ~13 fields are collected by [BrandingSettings.php](app/Livewire/Settings/BrandingSettings.php) and read by **nothing**: `login_background`, `invoice_header`, `header_bg`, `compact_sidebar`, `show_logo_in_invoice`, `invoice_footer_text`, `success/warning/danger_color`, `secondary_color`, `accent_color`.
- [AcademySettings.php](app/Livewire/Settings/AcademySettings.php) reads and writes `$academy->address` — **there is no `address` column** on `academies` and it is not in `$fillable`. Silently dropped on every save.
- `branding.academy_name` is read by [layouts/parent.blade.php:7](resources/views/layouts/parent.blade.php#L7) but **written by nothing**.
- [components/print/sheet.blade.php:165](resources/views/components/print/sheet.blade.php#L165) emits the raw storage path as an `<img src>` instead of a URL — broken logo on all sheet prints.
- ~900 `dark:` utility classes exist, but Tailwind v4 has no `@custom-variant dark` declared in [app.css](resources/css/app.css), so the class toggle in [DarkModeToggle.php](app/Livewire/Components/DarkModeToggle.php) is **inert**.
- [SettingsService::get()](app/Domain/Shared/Services/SettingsService.php) issues one SELECT per key with no memoisation → ~16 queries per admin page render, repeated on every Livewire round-trip.
- The `mobile_app` settings group (colors, feature flags, OTP mode, Firebase JSON) has **no admin UI whatsoever** — [Admin/SystemSettings.php](app/Livewire/Admin/SystemSettings.php) only knows six other groups.
### Design
- New `app/Domain/Shared/Services/BrandingService.php` returning a readonly `BrandProfile` DTO — the single read API for every surface (admin, portal, website, print, API). Cached per academy, invalidated on save. Replaces the per-layout `@php` blocks that each re-resolve `SettingsService`.
- Derive a full **OKLCH shade ramp** (`50…900`) plus a **WCAG-contrast-safe foreground** for each brand colour, instead of the hardcoded `#fff` at [sidebar.blade.php:184](resources/views/components/layouts/sidebar.blade.php#L184). Follow the existing derivation precedent in [ThemeEditor::generatePaletteFromAccent()](app/Livewire/Website/ThemeEditor.php).
- Add an `@theme` bridge in `app.css` (mirroring what [website.css:8-16](resources/css/website.css#L8-L16) already does) so Tailwind utilities bind to brand tokens rather than only inline `style=""`.
- Add `@custom-variant dark` so the 900 existing `dark:` classes come alive.
- New branding fields for mobile: `app_icon`, `splash_image`, `theme_color`, `theme_mode` (light/dark/auto), plus **write** `academy_name`.
- Guarded migration adding the missing `academies.address` column.
- Idempotent branding seeder so a fresh tenant has sane defaults before anyone visits the settings page.
- Every dead field gets a decision: wire it or delete it from the UI. Collect-but-ignore is not allowed to survive.
- Admin UI for the `mobile_app` settings group (folds into W6).
---
## W2 — Identity and portal authentication
The existing identity model already supports "one account, many profiles" with **no new
identity tables**:
```
users ──(users.person_id)──► people ──(people.user_id back-ref)
│
┌────────────────┴────────────────┐
participants.person_id guardians.person_id
guardian_participant (pivot:
is_primary, receives_notifications,
can_authorize_payment, can_pickup)
```
`people.user_id` becomes the canonical link. (`guardians.user_id` is a redundant second
link — keep it in sync, prefer `people.user_id` on read.)
**Decisions taken with you**
- Login: **admin-issued invitation + password**. Phone or email + password thereafter.
- Accounts: **one account, profile switcher**, so a father who also plays sees everything in one place.
**Correction to my earlier recommendation.** I recommended invite+password partly because
OTP "needs an SMS provider you don't have". That premise was wrong: `WhatsAppService` and
`AuthOtpController` already exist and the deploy already passes `WHATSAPP_*` /
`MESSAGING_HUB_*` env vars. So OTP is far cheaper than I implied. The recommendation stands
for the **primary** flow (session auth for Livewire, works with zero external dependency,
and OTP-only accounts cannot recover if the hub is down), but W0 has to make OTP work
correctly anyway — so we get it as a **second factor and as password recovery** almost free.
### Build
- `portal_invitations` table: `academy_id`, `person_id`, `token` (hashed), `expires_at`, `used_at`, `created_by`. Admin UI to issue / resend / revoke, with a copyable link and a "send via WhatsApp" action using the existing service.
- Activation sets a password on the `users` row, creating it if absent. **Blocker:** `users.email` is `UNIQUE NOT NULL` and most guardians have no email — resolve by dropping `NOT NULL` and adding a partial unique index on non-null values (safe, additive; see W-Migrations).
- New `player` role (level 5) — today there is **none**, and a player given the `parent` role gets empty lists because [PermissionService::getChildParticipantIds()](app/Domain/Identity/Services/PermissionService.php) resolves only via `Guardian`. Extend `applyOwnChildrenScope` to include the user's own participant row.
- `PortalProfileService` resolving which participants a user may view (self + linked children), backing a session-held active profile. Every portal query scopes through it — never through a route-bound id.
---
## W3 — The portal at `/app`
> You said "slash mobile portal". I've used `/app` as the canonical path because it is
> shorter to type and to say; `/mobile-portal` can be registered as a permanent redirect.
> One-line change if you prefer the longer path.
- Its own layout and its own `resources/css/portal.css` + `resources/js/portal.js` bundle, added to [vite.config.js](vite.config.js) exactly as `website.css`/`website.js` already are. This is the established precedent for a second design language in this repo.
- Design language deliberately unlike the ERP. Note the [Beanding Guide.txt](Beanding Guide.txt) brand book explicitly prescribes *"snappy and functional, no bouncy or playful animations"* — that guide governs **El Captain the ERP product**, not the tenant-branded member app. The portal is tenant-branded and intentionally expressive; the two are allowed to diverge, and the plan treats them as separate design systems.
- Existing `/parent/*` becomes the migration source: reuse its query logic (extracting to services where it currently sits in components), then redirect `/parent/*` → `/app/*` and retire the old components once at parity. **Two member portals must not coexist.**
### Art direction
The portal must feel like an app, not a website — and it must look nothing like the ERP.
Direction, from the design-intelligence pass (`ui-ux-pro-max`, variance 7 / motion 8 /
density 5):
- **Palette is derived, never fixed.** The design search proposes a concrete sports palette
(team red + championship gold). That is exactly wrong here: this is a *tenant-branded*
product, so every colour must come from `BrandingService` (W1) and its derived OKLCH ramp.
What we adopt from the recommendation is the *structure* — high-contrast, block-based,
saturated accent over a near-neutral ground — not the hexes.
- **Type:** a display face for numbers and headings paired with a body face, both Arabic-capable. Cairo stays the body default; the display face is the tenant-overridable slot. Large type (32px+) for the things members actually come to check — balance due, next session, attendance streak.
- **Motion tier: complex, but purposeful.** 500–800ms shared-element route transitions (`expo.inOut`), staggered list reveals, spring-based sheet and tab transitions. Every animation must respect `prefers-reduced-motion` and fall back to the final state immediately. Motion conveys spatial continuity — where a card went — never decoration for its own sake.
- **Layout:** bold block sections, generous 48px+ section gaps, scroll-snap on horizontal rails (news, programs, children switcher), bottom sheets rather than modals.
- **Non-negotiables** carried from the repo's own rules and the accessibility gate: logical properties only (`ms/me/ps/pe`, `start-*/end-*`) — no `ml/mr/left/right` anywhere; `dir="ltr"` on numeric inputs; every string through `__()`; SVG icons only, never emoji; 44×44px minimum touch targets with 8px spacing; visible focus rings; 4.5:1 contrast verified against the *tenant's* chosen colours, not against a designer's swatch; `env(safe-area-inset-*)` respected (the existing parent layout already does this).
- **Navigation:** bottom bar capped at 5 items with overflow elsewhere; real URLs per screen so deep links and the Flutter wrapper's back button both work; `history.pushState` semantics preserved — never `location.replace`.
### Feature inventory
You asked me to work out the features myself. Every row below is backed by a table and a
service that **already exist** — this is the argument for why the portal is mostly UI work,
not new domain modelling. "New" marks the genuinely new capability.
**Bottom bar (5 max):** الرئيسية · الجدول · بطاقتي (QR) · الحساب · المزيد
| Screen | Reads / writes | Backing |
|---|---|---|
| **الرئيسية** — next session countdown, balance due, attendance streak, unread announcements, quick actions | sessions, invoices, attendance, news | `training_sessions`, `invoices`, `attendance_records`, `website_news` |
| **الجدول** — week/month view, session detail (time, facility, coach, group), cancellations highlighted, add-to-calendar | `training_sessions` + `training_schedules` + `facilities` + `trainers` | all exist |
| **الحضور** — history, rate ring, late count, streak, monthly summary | `attendance_records`, `AttendanceMarkingService::calculateRate()` | exists |
| **بطاقتي (QR)** — rotating pass, check-in/out state, guest passes | **New** — `SelfCheckInService` | `attendance_records.check_in_at/check_out_at` exist |
| **الحساب / الفواتير** — outstanding, paid, invoice detail with frozen line items | `invoices`, `invoice_items` | exist |
| **الأقساط** — plan progress, next due, overdue | `payment_plans`, `installments` | exist |
| **المحفظة** — balance + ledger | `wallets`, `wallet_transactions`, `WalletService` | exist |
| **الدفع** — InstaPay handle, amount, instructions, screenshot upload, status | **New** — `payment_proofs` | `PaymentService` exists |
| **الإيصالات** — payment receipts, printable | `ReceiptService::buildPaymentReceiptData()` | exists |
| **المتجر** — essential products, member/non-member pricing, installment plans, "already purchased this year" | `products` (`is_essential`, `member_price`, `billing_cycle`), `product_installment_plans` | exist |
| **مستلزمات البرنامج** — required products the player is missing | `program_products.is_required` | exists |
| **البرامج** — browse, prices via `PricingService`, enrol / join waitlist | `training_programs`, `waitlists`, `EnrollmentService` | exist |
| **التقييمات** — coach evaluations shared with the guardian | `evaluations`, `EvaluationStatus::Shared` | exists |
| **المستندات** — medical certificate status + expiry warning, upload, approval state | `documents` (morph), `DocumentStatus` | exists |
| **الفعاليات** — list, detail, register via the dynamic form, my registrations | `events`, `event_registrations.form_data` | exists |
| **الأخبار** — academy feed, article detail | `website_news` | exists |
| **الإشعارات** — history, read state, per-type preferences | `notification_logs`, `notification_preferences` | exist |
| **الطلبات** — freeze / transfer / cancellation request + status | `service_requests` (`freeze|unfreeze|transfer|cancellation|other`) | exists |
| **الاستئذان** — report a planned absence | `attendance_records` → `excused` | **currently discarded — see W9** |
| **راسلنا** — message the academy, threaded replies | `contact_messages` | exists |
| **الملف الشخصي** — profile switcher, child detail, guardian info, photo | `people`, `participants`, `guardians`, `guardian_participant` | exist |
| **الفروع** — locator with map, hours, phone | `branches` (`latitude`, `longitude`, `operating_hours`) | exists |
| **التسجيل الذاتي** — public signup → child → enrol → pay | **New** — W10 | `DuplicateDetectionService`, `PricingService` `contextOverride` exist |
**Deliberately out of scope for v1:** live chat with coaches, in-app video, social/feed
features between members, and anything that needs a payment gateway (Paymob stays
feature-flagged off; InstaPay is the payment path you chose).
Exact IA and per-screen guardian-vs-player differences are being pressure-tested by the
critique pass (see Addendum).
---
## W4 — QR check-in / check-out and guest passes
Nothing QR exists today. `qr_check_in_enabled` is a seeded system setting with **zero
readers**; the only QR in the repo is an `<img>` pointing at the external `api.qrserver.com`
on an admin event page. No QR package is installed.
You chose **both directions, selectable per branch**:
- **A — player displays, staff scans.** Rotating HMAC token (short window + tolerance), rendered as SVG server-side (no CDN, works offline). New scanner screen in the admin app using `BarcodeDetector` with a JS fallback.
- **B — static branch poster, player scans.** Per-branch secret; portal scans and posts.
**Constraint that shapes the design:** [AttendanceMarkingService](app/Domain/Attendance/Services/AttendanceMarkingService.php) requires a staff `User $marker` on every method, and `attendance_records` already has `check_in_at` / `check_out_at` and a unique key on `(training_session_id, subject_type, subject_id)`. Self-check-in therefore needs a new `SelfCheckInService` that resolves the participant's *expected* record for the current session window and marks it, recording provenance — not a loosening of the existing service.
Guest passes ("QR invitations") are a purchasable item → priced through `PricingService`,
paid through W5, issued as a signed pass, and scanned at the gate.
Token construction, replay/screenshot-sharing defences, revocation for suspended
participants, and geofencing for mode B are being hardened by the security critique (see
Addendum) before implementation.
---
## W5 — InstaPay payments (manual proof)
`PaymentMethod` is exactly `cash|card|bank_transfer|wallet|online|cheque|other` — **no
`instapay`**, and `wallet` means the internal academy wallet, not a mobile money wallet.
The financial invariants forbid the obvious shortcut: `transactions` are immutable, every
movement writes a debit+credit pair, and [PaymentService::recordPayment()](app/Domain/Financial/Services/PaymentService.php) forces `status = Confirmed` and writes double-entry immediately. **So an unverified screenshot must never create a `Payment`.**
### Design
- New `payment_proofs` table (pending → approved/rejected), holding the claimed amount in piasters, the uploaded screenshot, the sender reference, and the reviewing user. Approval — and only approval — calls `PaymentService::recordPayment()`, so double-entry happens exactly once and the immutable ledger is never edited.
- Add `instapay` to the enum **and** to the `payments_method_check` CHECK constraint, character-for-character, via a guarded migration.
- Per-branch InstaPay handle / QR image / Arabic instructions in `branch_settings` (that generic per-branch key-value store already exists).
- Admin review queue as a Livewire component.
- Portal: invoice → amount + handle + copy button + instructions → upload screenshot → pending state with clear status.
Lifecycle edge cases (overpayment, proof against an already-paid or cancelled invoice,
reversing an approval made in error, double-approval races, refunding an InstaPay payment,
cash-session interaction, daily reconciliation) are being specified by the financial
critique (see Addendum).
---
## W6 — Mobile App Content CMS
Mirror the website builder by **reusing its engine**, not by copying it:
- New `MobileBlockRegistry` + mobile `BlockType` subclasses, registered in a provider exactly as [WebsiteServiceProvider::BLOCKS](app/Providers/WebsiteServiceProvider.php) does for the 31 website blocks.
- New `app_screens` + `app_blocks` tables, modelled on [`website_pages`/`website_blocks`](database/migrations/2026_08_31_000001_create_website_pages_and_blocks.php) — `data`/`style` jsonb, self-FK tree, no CHECK on `type`, partial unique index for the single home screen.
- The generic field renderer [website/builder/field.blade.php](resources/views/website/builder/field.blade.php) and the `BlockField` DSL work unchanged — a new mobile block type needs only a PHP class.
- Block types: hero banner / carousel, news feed, events strip, quick-action tiles, programs, announcement bar, onboarding slides, contact card, gallery, custom HTML.
- Plus the missing admin UI for the `mobile_app` settings group (feature flags, OTP mode, Firebase JSON, maintenance mode, min version) — currently seeded but uneditable.
- Push composer UI on top of the existing `push_announcements` + targeting.
Two known traps to avoid inheriting: the website builder's `reorderBlocks()` server method
exists but **no drag-and-drop is wired**, and nothing in the v3 path invalidates any cache.
Do both properly here.
---
## W7 — PWA and Web Push
Nothing PWA exists: no manifest, no service worker, no app icons, no splash, no
`theme-color`. `public/` holds only `favicon.ico`, `index.php`, `robots.txt`.
- Per-tenant dynamic manifest at `/app/manifest.webmanifest`, fed by `BrandingService`.
- Service worker + offline app shell.
- Icons/splash: **no image library is installed** (no `intervention/image`, no `spatie/image`). Options are add one, require pre-sized uploads, or generate SVG — the delivery critique is deciding (see Addendum).
- Web Push: VAPID vs reusing the existing FCM stack is a real decision, since `kreait/firebase-php` and `device_tokens` already exist and each client has their own Firebase project. Deferred to the Addendum.
---
## W8 — Flutter wrapper
Thin WebView shell per client, consistent with the existing per-client model already
documented in [docs/specs/mobile-app-plan.md](docs/specs/mobile-app-plan.md) (one
`instance.dart`, one Firebase project, per-client icons).
Loads `{baseUrl}/app`; bridges FCM token registration, camera for QR, file picker for
payment screenshots, biometric re-entry, deep links, and forced-update via the existing
`GET /api/v1/app/config`. App Store rejection risk for a pure WebView wrapper is a real
concern and is covered in the Addendum.
---
## W9 — Latent bugs to fix along the way
| Bug | Location |
|---|---|
| `OrderController` writes invoice-line keys that are not columns (`academy_id`, `description_ar`, `line_total`) → `invoice_items.total_amount` stays `0`, corrupting `ParticipantBillingService`'s allocation maths | [OrderController.php:82-91](app/Http/Controllers/Api/V1/OrderController.php#L82-L91) |
| Same file bypasses `PricingService` entirely and reads `participants.classification`, which actually lives on `people` → always null | [OrderController.php:57-59](app/Http/Controllers/Api/V1/OrderController.php#L57-L59) |
| **Parent excuse form lies to the user.** It validates, stores the attachment, then flashes `تم تقديم العذر بنجاح. سيتم مراجعته من قبل الإدارة` while discarding the excuse entirely (`// TODO: Create excuse record when model is available`). There is no `Excuse` model or table anywhere. A parent believes the absence was excused; the academy never sees it. Treat as P1, not a TODO — either build the model or remove the screen | [ParentExcuseForm.php:103-105](app/Livewire/Parent/ParentExcuseForm.php#L103-L105) |
| `qr_check_in_enabled` is exposed as a toggle in system settings and seeded, but has **zero functional readers** — it advertises a feature that does not exist. W4 gives it a real implementation | [SystemSettings.php:62](app/Livewire/Admin/SystemSettings.php#L62), [SystemSettingsSeeder.php:56](database/seeders/SystemSettingsSeeder.php#L56) |
| `NotificationChannel` enum has `whatsapp` and `push`, but the DB CHECK on `notification_templates`/`notification_logs` allows only `in_app|email|sms` → inserting a push log violates the constraint | `2024_01_01_000042` |
| `guardians.relationship_type` and `guardian_participant.relationship_type` have **different** CHECK value lists | `2024_01_01_000007`, `2024_01_01_000018` |
| `AcademyController` returns `caption` and `type` on media rows; neither column exists → always null | [AcademyController.php:132-133](app/Http/Controllers/Api/V1/AcademyController.php#L132-L133) |
| `SetLocale` middleware exists but is registered nowhere → bilingual support is dead | [SetLocale.php](app/Http/Middleware/SetLocale.php), [bootstrap/app.php:26-39](bootstrap/app.php#L26-L39) |
| Website navbar/footer partials require `$sections`, which v3 builder pages and news pages never pass | [navbar.blade.php:10](resources/views/website/navbar.blade.php#L10) |
---
## W10 — Self-registration (public signup → paid enrolment)
You chose the widest option: a stranger can open the portal, browse, create an account,
register a child, and pay — with no academy staff involved. This is the highest-value and
highest-risk workstream, because it is the only one that writes into `people` and
`participants` without a staff member in the loop.
**What makes it viable:** [PricingService::calculate()](app/Domain/Pricing/Services/PricingService.php)
already accepts a `$contextOverride` parameter explicitly designed to *"price BEFORE a
participant row exists"*, and [DuplicateDetectionService](app/Domain/Participant/Services/DuplicateDetectionService.php)
already exists. The domain was built with this in mind.
**What it needs:**
- Phone verification (OTP over `WhatsAppService`) before any row is written — non-negotiable, see above.
- Public program browsing with prices resolved through `PricingService` using `contextOverride`. The **no-base-price hard fail** rule still applies: a program with no active price must not be offered for self-enrolment at all, never priced at 0.
- Signup → `Person` (+ `User`) → child `Participant` → `Enrollment`. `EnrollmentService::enroll()` requires a `User $actor`; a self-registering guardian *is* a user, but is not authorised to enrol. Needs a `PortalRegistrationService` that performs the enrolment with a system/self actor and an explicit provenance stamp, rather than loosening the existing service.
- `registration_source = 'online'` — the enum value already exists.
- Admin approval queue: self-registered participants land in a reviewable state before they become fully active.
- `DuplicateDetectionService` run on every signup, surfacing "this looks like an existing member" to staff rather than silently creating a second `Person`.
- Rate limiting + abuse controls on signup, and a decision on what an unpaid, unapproved self-registration is allowed to see.
This workstream is sequenced **last among the functional ones** — it depends on identity
(W2), the portal shell (W3), and payment (W5) all being real first.
---
## Migration safety (applies to every workstream)
Every client DB is live and populated, and `migrate --force` runs on **every container
boot** ([docker/entrypoint.sh](docker/entrypoint.sh)). Therefore:
- every `up()` guarded with `hasTable`/`hasColumn`; destructive operations only in `down()`;
- altering the `payments_method_check` CHECK on a populated table, and dropping `NOT NULL`
from `users.email`, both need the specific safe Postgres idiom — being verified in the
Addendum before any of it is written;
- seeders stay idempotent and free of client-specific content.
---
## Build order
Vertical slices, per [docs/agent-rules/14-build-order.md](docs/agent-rules/14-build-order.md) — migration → model → service → Livewire → view → verify in a browser.
1. **W0** delete the API surface — alone, deployed immediately.
2. **W1** branding + `BrandingService` + caching — everything visual depends on it.
3. **W2** identity, roles, invitations, WhatsApp OTP — everything member-facing depends on it.
4. **W3** portal shell: layout, `portal.css`, nav, home, profile switcher.
5. **W3** screens in dependency order, reusing `/parent` logic; redirect and retire `/parent` at parity.
6. **W5** InstaPay (unblocks the whole money path in the portal).
7. **W4** QR check-in, then guest passes.
8. **W6** Mobile CMS (portal must exist before content can target it).
9. **W7** PWA + push.
10. **W10** self-registration (needs W2 + W3 + W5 all real).
11. **W8** Flutter wrapper.
12. **W9** bugs — folded into whichever slice touches that file first, never batched.
W6 and W7 can run in parallel with late W3. W8 cannot start before W7. W10 cannot start
before W5.
---
## Verification
`vendor/` is **not installed** in this working copy — `composer install` first.
- `composer test` (PHPUnit; 9 test files today, all API-focused, **zero** for the website builder).
- New tests required: the W0 regression tests; `payment_proofs` lifecycle including double-approval and the double-entry assertion; QR token forge/replay; portal authorization (a guardian must not reach another guardian's child).
- Browser-first, per [laravel-discipline](docs/agent-rules/15-integration-checks.md) rule 2 — nothing is done until the flow completes in a real browser: invite → activate → login → switch profile → view invoice → upload InstaPay proof → admin approves → balance updates → QR displays → staff scans → attendance marked.
- `php artisan migrate` against a **restored copy** of [backups/oc_sport-20260831-081053.dump](backups/oc_sport-20260831-081053.dump) — a real populated tenant DB, already local. Never against the live instance. This is the only honest proof that the CHECK-constraint change on `payments` and the `users.email` `NOT NULL` drop are safe.
- Lighthouse PWA + installability check; `accessibility` and `web-design-guidelines` audits as the design exit gate, with logical-properties-only RTL enforced.
---
## Open decisions
1. `/app` vs `/mobile-portal` as the canonical path.
2. Web Push via VAPID vs reusing the existing FCM stack (`kreait/firebase-php` and `device_tokens` are already installed, and each client already has their own Firebase project).
3. Icon/splash generation: add `intervention/image` vs require pre-sized uploads vs generate SVG. No image library is installed today.
4. What an unpaid, unapproved self-registration is allowed to see and do before staff approve it (W10).
---
## Addendum — READ THIS BEFORE IMPLEMENTING ANYTHING ABOVE
The four-lens adversarial critique came back and it **supersedes parts of this plan**. Full
text: [velvet-cooking-snowglobe-addendum.md](velvet-cooking-snowglobe-addendum.md) (365
lines, every claim verified against the code).
### It corrected four of my premises
1. **Livewire is `^4.3`, not 3.** `CLAUDE.md` says Livewire 3 and it is wrong. This matters: in Livewire 4 a `public` property is client-settable, which turns `ParentHome::$activeChildId` into a live IDOR across every child in the academy.
2. **`dark:` is not inert — it is live and untested.** I had this backwards. With no `@custom-variant` declared, Tailwind v4 compiles `dark:` to `prefers-color-scheme`, so ~900 utilities are already rendering for every OS-dark user. The `.dark` class toggle is the part that does nothing.
3. **`transactions` is one row** with `debit_account_id` + `credit_account_id` — not a debit/credit pair. `CLAUDE.md` and two files in `docs/agent-rules/` describe a schema that does not exist. The schema is the truth.
4. **An attendance-write endpoint already exists** (`POST /api/v1/absences/report`) — and it writes `status='excused'` with no marker, no transition check, no audit, and no check that the session even belongs to the participant. A player can excuse himself.
### The severity I reported to you was wrong in both directions
**The OTP takeover is not currently exploitable.** `2026_08_30_000004` normalised
`users.phone` to digits-only local form (`01014087672`) while `AuthOtpController::normalizePhone()`
produces `+201014087672`, so the lookup misses and returns 404. I told you it was live; it is
**latent — one plausible bug-fix away**. That is why the bypass deletion and the phone-matching
fix must ship in the *same* commit: fixing login alone re-arms the takeover.
**But something worse is genuinely live right now.** [bootstrap/app.php:89](bootstrap/app.php#L89)'s
HTML 500 branch is **not gated on `APP_DEBUG`**, and [errors/500.blade.php](resources/views/errors/500.blade.php)
renders `$sessionData`, `$inputData`, `$headers`, `$queries` and the stack trace to the browser.
`$sessionData` unsets only `_token`/`_previous`/`_flash` — so Laravel's `password_hash_web`,
**the logged-in user's bcrypt hash**, plus the last 10 SQL queries and the full request input,
are shown to whoever triggers any 500, on every client, today. That page also pulls
`cdn.tailwindcss.com`.
### Other live defects it found that I missed
| Defect | Impact |
|---|---|
| [PaymentService::resolveDebitAccount()](app/Domain/Financial/Services/PaymentService.php) `return 1;` / `resolveCreditAccount()` `return 2;` with a `// TODO` | **Every payment ever recorded** posts Dr Cash / Cr Bank — no revenue, no receivable. `getRevenueBySource()` always returns `[]`. The double-entry "invariant" has never held. |
| `SetCurrentAcademy` runs *before* `auth:sanctum` in the api group | `current_academy` is never bound on any API request → `SystemSetting::get()` returns hardcoded defaults, so `auth_otp_mode`, `maintenance_mode`, `min_version` and all four feature flags are **inert**, and `BelongsToAcademy`'s global scope is a no-op across the whole API |
| `PushNotificationService` writes channel `'push'`; the CHECK on `notification_templates`/`notification_logs` allows only `in_app\|email\|sms` | **Every push log insert throws 23514 today.** Push delivery logging is 100% broken |
| `POSService` increments `cash_session.total_cash_in` *and* `UpdateCashSessionTotals` does too | Every POS cash sale inflates the expected drawer 2× |
| `docker/entrypoint.sh` — `migrate --force \|\| { echo WARN; echo Continuing; }` | A failed migration boots anyway and silently blocks every later migration forever |
| Entrypoint env whitelist has no `PAYMOB_` / `FIREBASE_` | `config:cache` bakes `null` → **Paymob is dead on every client right now**, silently |
| nginx `location ~* \.(js\|css\|…)$ { try_files $uri =404; }` | `/sw.js` 404s before reaching PHP — W7 cannot work without an image-layer change and a redeploy of every client |
| `DeviceController::updateOrCreate(['device_token' => …])` | Any user claims any FCM token — the victim's phone then receives the attacker's notifications |
| `ReceiptController` authorization is **inverted** (runs only if `billable_type === Participant`) | Any non-participant invoice/payment is world-readable to any token |
| `/parent` route group has no permission middleware | Any trainer or cashier walks into the parent portal |
### Major design changes it forces
- **Cut W6's parallel CMS.** `website_news`/`website_sections`/`media`/the page builder already exist — add one `channel` column (`website|app|both`) instead of a second CMS with a second migration surface forever.
- **Cut static-poster QR (W4b).** A printed QR is a permanent public string; rotation is impossible by construction. Keep staff-scan only.
- **Cut purchasable guest passes.** `guardian_participant.can_pickup` already models delegated entry.
- **Cut VAPID** — reuse the installed FCM stack; widen `device_tokens.platform` to include `'web'`.
- **Do not touch `users.email`.** The unique constraint cannot be made partial without a `DROP CONSTRAINT` in `up()`, and `password_reset_tokens.email` is the primary key. Use `email_is_synthetic` + `@portal.invalid` addresses instead.
- **Do not add a unique index on `users.phone`** — the normalisation migration deliberately left duplicates, so it will hard-fail on at least one live client and then block that client's migrations forever.
- **Build order is rewritten** as S0–S10, with a mandatory **S0 production-safety PR** before anything else. My W9-last sequencing was wrong: two of its items gate W5 and W7.
### Open decisions it raises for you (E1–E8 in the addendum)
Dark mode (own it or delete it) · age of majority for self-service · whether the Flutter
wrapper ships this cycle at all · duplicate-account merge · overpaid proofs · which of the
six payment-method CHECKs get `instapay` · where an excuse lives · branch on portal payments.
docs/specs/mobile-portal/02-critique-addendum.md 0 → 100644
View file @ 2ef6e088
<!--
STATUS: binding. This addendum is the output of a four-lens adversarial review
(feature completeness, security/abuse, financial integrity, delivery risk) run
against 01-program-plan.md. Every claim in it was verified against the code.
Where it disagrees with 01-program-plan.md, THIS DOCUMENT WINS.
Paths are relative to the REPOSITORY ROOT.
-->
# ADDENDUM TO THE MOBILE-PORTAL PROGRAM (W0–W9)
Four premises in the brief are false and change the plan: (1) `livewire/livewire` is **v4.3.3**, not 3; (2) `dark:` is not inert — with no `@custom-variant` declared, v4 compiles it to `prefers-color-scheme`, so ~900 utilities are **live and untested** for every OS-dark user while the `.dark` class toggle is what does nothing; (3) an attendance-**write** endpoint exists (`POST /api/v1/absences/report`); (4) `transactions` is **one row with `debit_account_id` + `credit_account_id`**, not a debit/credit pair — verified in `2024_01_01_000013`.
---
## A. CORRECTIONS
### A1 — W0 is half a fix, and half of it is inert
**Contradiction adjudicated.** Security lens: the `otp === '0000'` bypass is live and exploitable. Delivery lens: `2026_08_30_000004_normalize_user_credentials` rewrote `users.phone` to digits-only local form (`01014087672`) while `AuthOtpController::normalizePhone()` (verified, line 135) produces `+201014087672`, so `where('phone',$phone)` misses and OTP login is currently broken. **Both are true: the takeover is one bug-fix away.** Decision: the bypass deletion and the phone-resolution fix ship in **one commit**, using `CredentialNormalizer::phoneVariants()` + `whereIn`, or repairing login re-enables account takeover.
**Second inert layer.** `SetCurrentAcademy` is appended to the **api group** (`bootstrap/app.php:31-34`), which runs before route middleware `auth:sanctum`; `$request->user()` resolves the `web` guard, which has no session on a Bearer request. So `current_academy` is **never bound on any API request**, and therefore:
- `SystemSetting::get()` returns its **hardcoded default on every API call**. `auth_otp_mode` reads `'demo'` regardless of DB. Flipping the admin setting changes nothing.
- `maintenance_mode`, `min_version`, all four `app_features_*` are inert → **W8's forced-update and maintenance kill switches have never worked.** W8 cannot start until this is fixed.
- `BelongsToAcademy`'s global scope is a **no-op on the whole API** — reads unscoped, and `academy_id` is only set because 8 controllers set it by hand.
One-request proof before touching code: `curl https://<tenant>/api/v1/app/config` prints `"otp_mode":"demo"` on a tenant whose DB says `sms`.
**Corrected W0 scope (all of it, one PR, S0):**
| # | File | Fix |
|---|---|---|
| 1 | `AuthOtpController:74-89` | Delete the `$isBypass` branch and the constant `'1234'` cache value. Not gated — deleted. |
| 2 | `AuthOtpController:135`, `:44`, `:92` | `phoneVariants()` + `whereIn`; if >1 row matches, reject with "تواصل مع الأكاديمية" — never `->first()` on a non-unique column. |
| 3 | `bootstrap/app.php` | Move `SetCurrentAcademy` after `auth:sanctum`, or make it `$request->user('sanctum') ?? $request->user('web')`. Add a test asserting `app()->has('current_academy')` inside an authenticated API request. |
| 4 | `AuthOtpController` `sms` mode | Fail closed when no SMS driver is configured. Today it caches an OTP and sends nothing = a lockout switch. |
| 5 | `AppConfigController:46` | Stop returning `otp_mode` on an unauthenticated endpoint. |
| 6 | `routes/api.php:140` | `permission:notifications.broadcast` on `broadcast/send` **and** `broadcast/index`, a scope check inside `AnnouncementBroadcastService`, and `throttle:api-broadcast` 5/hour. Any parent token currently pushes to every device in the academy. |
| 7 | `bootstrap/app.php:89-169` | Wrap the HTML 500 branch in `if (config('app.debug'))`. `$sessionData` unsets only `_token/_previous/_flash`, so **`password_hash_web` — the user's bcrypt hash — renders to the browser** on any 500, with the last 10 queries and full request input. Verify `APP_DEBUG=false` on every tenant today. Drop `cdn.tailwindcss.com` from `errors/500.blade.php:8`. |
| 8 | `PaymentController:33-42` | No ownership check at all on `initiate` — any token creates a `Payment` against any invoice by uuid and reads back its balance, and builds Paymob from `$user->academy_id` while writing `$invoice->academy_id`. Add `verifyAccess` + hard-fail on academy mismatch. |
| 9 | `ReceiptController:17-25`, `:68-76` | Authorization is inverted: it runs *only if* `billable_type === Participant`. `payments.invoice_id` is nullable and `billable` is `nullableMorphs`, so any non-participant invoice/payment is world-readable to any token. Deny-by-default. |
| 10 | `DeviceController:23-34` | `updateOrCreate(['device_token' => …])` keyed on the token alone → any user claims any FCM token and **the victim's phone receives the attacker's notifications**. Key on `(device_token, user_id)`; reject a token owned by another user. |
| 11 | `ParentHome:22` (+ `ParentExcuseForm:19`) | `public ?int $activeChildId` is validated in `mount()`/`selectChild()` but used raw in `render()` at 7 sites. Livewire 4 lets the client set it directly → iterate participant ids and dump every child's balance, attendance rate and last evaluation. `#[Locked]` **and** re-validate in `render()`. |
| 12 | `ParentExcuseForm:99-109` | Stores a medical attachment to the **public** disk and then discards the record (`// TODO`). Orphaned, unauthenticated, undeletable child medical PII plus a free file host on the academy's TLS cert. Stop storing before the record exists; sweep `storage/app/public/excuses/` on every live tenant. |
| 13 | `routes/web.php:602-614` | The `/parent` group carries **no permission middleware** — every sibling group has one. Any trainer/cashier walks in. |
| 14 | `docker/entrypoint.sh:52-55` | `migrate --force \|\| { echo WARN; echo Continuing; }` — a failed migration boots anyway, serves a stale schema, and **blocks every later migration forever**, silently. Fail the boot. |
| 15 | `docker/entrypoint.sh:24` | The env whitelist has no `PAYMOB_`, so `config:cache` bakes `null` → **Paymob is dead on every client right now, silently.** Add `PAYMOB_|FIREBASE_|SMS_|ATTENDANCE_|MAX_|CONSECUTIVE_`. |
| 16 | `docker/nginx/default.conf:26` | `location ~* \.(js\|css\|png…)$ { try_files $uri =404; }` — a regex location outranks prefix, so `/sw.js`, `/manifest.webmanifest` and any dynamic icon **404 before reaching PHP**. Ship `location = /sw.js`, `location = /manifest.webmanifest`, `location = /.well-known/apple-app-site-association` **in S0**: it is an image-layer change requiring a redeploy of every client, and batching it now saves a whole deploy cycle. |
| 17 | env | `SESSION_SECURE_COOKIE=true`, `SESSION_DOMAIN` pinned per tenant, `SameSite=Lax` (correct for a WebView top-level document). |
### A2 — The financial invariant in CLAUDE.md does not describe the schema
`transactions` (verified): single row, `debit_account_id` + `credit_account_id`, `type IN (payment_received, payment_made, refund, transfer, adjustment, fee, discount, write_off, opening_balance)`. `docs/agent-rules/16-enums-and-checks.md` registers `TransactionType` as `'debit','credit'` and `05-financial-integrity.md` describes a two-row pair. **The schema is the truth; fix the two docs.** Also: `transactions` carries `timestamps()` — immutability is a model convention (`const UPDATED_AT = null`), not a constraint.
`PaymentService::resolveDebitAccount()` / `resolveCreditAccount()` (verified, lines 116-125) `return 1;` / `return 2;` with a `// TODO`. Seeder order makes that **Dr Cash / Cr Bank on every payment in the system** — no revenue, no receivable. `FinancialOverview::getRevenueBySource()` therefore always returns `[]`. `RefundService:130-131` hardcodes `2`/`1` with a comment claiming A/R (which is id 3).
**Corrected:** resolve accounts by **code, scoped to `academy_id`, hard-failing when absent** — copy `PayrollService:465-505` verbatim. Split a payment across revenue accounts pro-rata by invoice-line share with `intdiv()`, remainder on the last row. Seed `4070 / إيرادات تذاكر الزوار` if guest passes ever ship. `FinancialAccountsSeeder` seeds only `Academy::first()` — make it per-academy and idempotent. **This is a hard prerequisite for W5**, not cleanup: routing InstaPay into a ledger that posts Cash→Bank just multiplies a broken ledger across a new channel.
### A3 — `PaymentService::recordPayment()` has no guards, and the online path skips it entirely
- Every guard `05-financial-integrity.md` names lives in the **UI** (`InvoiceShow:61-63`, `CollectPaymentWizard:434-438`). Any new caller inherits none. Move them into the service: `amount > 0`, `amount <= due_amount` **re-read under `lockForUpdate()` inside the transaction**, invoice not `cancelled`/`draft`/`paid`, academy + currency match, `branch_id` required.
- `InvoiceService::updatePaidAmount():95-115` is a read-modify-write with no lock → two concurrent settlements lose one increment.
- `PaymobService::processCallback():141-173` mutates `invoice->paid_amount` inline, **writes no `Transaction` row at all**, takes no lock, discards the callback's `amount_cents` after a `> 0` check, and its idempotency guard is dead code (the finder already filters `status = Pending`). Paymob retries → double credit. **Corrected:** route through `recordPayment()` inside one `DB::transaction` with `lockForUpdate()`, and assert `$amountCents === $payment->amount`.
- `POSService:213-222` increments `cash_session.total_cash_in` **and** `UpdateCashSessionTotals` (registered, queue workers running) increments it again → every POS cash sale inflates the expected drawer 2×. Delete the manual increment; audit historical `cash_sessions.variance` first. Any new channel dispatching `PaymentReceived` with `Cash` inherits this.
### A4 — W5 `payment_proofs`: corrected schema and lifecycle
Two lenses proposed different unique indexes. **They are orthogonal — ship both.**
```php
if (Schema::hasTable('payment_proofs')) return;
Schema::create('payment_proofs', function (Blueprint $t) {
$t->id(); $t->uuid('uuid')->unique();
$t->foreignId('academy_id')->constrained()->cascadeOnDelete();
$t->foreignId('branch_id')->constrained('branches'); // NOT NULL — see A5
$t->foreignId('invoice_id')->constrained();
$t->foreignId('submitted_by')->constrained('users');
$t->bigInteger('amount_claimed'); // payer input, never posted
$t->bigInteger('amount_approved')->nullable(); // reviewer types this from the statement
$t->string('method', 20)->default('instapay');
$t->string('sender_reference', 64)->nullable(); // normalized: trim+upper
$t->string('sender_phone', 20)->nullable();
$t->timestamp('transferred_at')->nullable();
$t->string('proof_path'); // private disk, proofs/<sha256>.<ext>
$t->string('status', 20)->default('pending');
$t->foreignId('reviewed_by')->nullable()->constrained('users');
$t->timestamp('reviewed_at')->nullable();
$t->foreignId('second_reviewed_by')->nullable()->constrained('users');
$t->timestamp('second_reviewed_at')->nullable();
$t->string('rejection_reason', 40)->nullable();
$t->text('review_notes')->nullable(); $t->string('review_ip', 45)->nullable();
$t->foreignId('payment_id')->nullable()->constrained();
$t->jsonb('metadata')->default('{}');
$t->timestamps(); // deliberate: a proof is NOT a ledger record
$t->index(['academy_id','status','created_at']);
});
DB::statement("ALTER TABLE payment_proofs ADD CONSTRAINT payment_proofs_status_check CHECK (status IN ('pending','under_review','approved','rejected','superseded'))");
DB::statement("ALTER TABLE payment_proofs ADD CONSTRAINT payment_proofs_method_check CHECK (method IN ('instapay','bank_transfer'))");
DB::statement("ALTER TABLE payment_proofs ADD CONSTRAINT payment_proofs_amount_check CHECK (amount_claimed > 0 AND (amount_approved IS NULL OR amount_approved > 0))");
DB::statement("ALTER TABLE payment_proofs ADD CONSTRAINT payment_proofs_approved_payment_check CHECK (status <> 'approved' OR payment_id IS NOT NULL)");
DB::statement("CREATE UNIQUE INDEX payment_proofs_one_payment ON payment_proofs (payment_id) WHERE payment_id IS NOT NULL");
DB::statement("CREATE UNIQUE INDEX payment_proofs_one_pending ON payment_proofs (academy_id, invoice_id) WHERE status = 'pending'");
DB::statement("CREATE UNIQUE INDEX payment_proofs_sender_ref ON payment_proofs (academy_id, method, sender_reference) WHERE sender_reference IS NOT NULL AND status <> 'rejected'");
```
**A screenshot is not evidence.** The control is reconciliation against the academy's own InstaPay/bank record; the image is a convenience. `sender_reference` is required by validation and the per-academy unique index is the backbone — it kills replay, cross-invoice reuse, and "someone else's transfer against my invoice" in one constraint.
**Concurrency control is the conditional UPDATE, not a disabled button:**
```php
$rows = PaymentProof::where('id',$id)->where('status','pending')->update([...'approved'...]);
if ($rows === 0) throw new InvalidStatusTransitionException(); // another reviewer won
$payment = $this->payments->recordPayment([... 'amount' => $verifiedAmount ...], $reviewer);
```
Everything in one `DB::transaction` with `Invoice::lockForUpdate()` and a fresh `due_amount` read inside it.
Other binding rules: only `amount_approved` is posted, never `amount_claimed`; overpayment → cap at `due_amount` + `WalletService::deposit()` for the excess in the same transaction (never `InvoiceStatus::Overpaid` — nothing consumes it and `due_amount` goes negative through `getCollectionRate()`); proof on a `paid` or `cancelled` invoice → hard fail (nothing today blocks pay-after-cancel, and such payments count in `getRevenue()` while `ParticipantBillingService` excludes them); reversal **only** via `RefundService` (a "negative proof" leaves the original `confirmed` and the participant reads as having paid twice); attachment append-only after `pending`; Postgres `BEFORE UPDATE` trigger rejecting edits once `status <> 'pending'`.
**RefundService must gain partial refunds before W5 ships** — it refunds `$payment->amount` in full only, so an approval of 500 that should have been 300 cannot be corrected. And its cash path debits **the actor's** open session, not the one that took the money.
**Six CHECK constraints carry a payment-method vocabulary and already disagree** (`payments`, `pos_transactions`, `pos_split_payments`, `expenses`, `facility_rent_payments`, `payslips` — the last already contains `'instapay'`). Decide all six in one migration (see E6). Widening a CHECK is safe against populated tables; `DROP CONSTRAINT IF EXISTS` + `ADD CONSTRAINT` with the full new list, both in one transaction. Skip `NOT VALID`/`VALIDATE` — it buys nothing at this volume and adds a half-applied failure mode.
### A5 — Branch attribution: portal payments will silently break every branch column
Revenue is branch-attributed **only** through `payments.branch_id`; `invoices` has no `branch_id` (the `'branch_id'` passed into `InvoiceService::create()` by `POSService:162` is not in `$fillable` and is dropped). `FinancialOverview` uses `when($branch_id, …)`, so a NULL-branch payment lands in the all-branches total and in **no** branch — the columns stop summing with no error. A proof approved while the reviewer is in "all branches" mode produces exactly that, re-introducing what `3520098` fixed. Also `getCollectionRate():416-419` scopes invoices via `whereHas('payments', branch)`, so **an invoice with zero payments belongs to no branch** — portal invoices awaiting proof approval vanish from every branch's overdue figures, inflating the collection rate exactly when collections are worst.
**Corrected:** `branch_id` NOT NULL **at the service boundary** for every portal-originated payment and proof (validate; do not add a NOT NULL constraint to populated `payments`), plus an additive guarded `invoices.branch_id` backfilled from first payment then `participant.branch_id`, and a nullable `transactions.branch_id` backfilled from `payments.branch_id` (A2 must land first, or `getRevenueBySource()` is empty anyway).
### A6 — W2 identity: do **not** touch `users.email`, and do **not** add a unique index on `users.phone`
**Contradiction adjudicated (verified in the repo).** Security lens proposed `DROP NOT NULL` + a partial unique index on `(academy_id, lower(email))`. `2024_01_01_000002:17` is `$table->string('email')->unique()` inside `Schema::create` → Postgres emits a **UNIQUE CONSTRAINT**, which cannot be partial; converting requires `DROP CONSTRAINT` in `up()`, which CLAUDE.md forbids. `CREATE INDEX CONCURRENTLY` cannot run in Laravel's migration transaction, and an `INVALID` index left behind is swallowed by the entrypoint (A1/14). `password_reset_tokens.email` is the **primary key** (`:39`) and the broker keys on it. `DatabaseSeeder` does `firstOrCreate(['email' => …])`. **Delivery lens wins.**
**Corrected:** keep `email` NOT NULL UNIQUE. Add `users.email_is_synthetic` boolean default false. Portal accounts get `p{person_uuid}@portal.invalid` (RFC 2606 — non-routable, so it never bounces off poste.io), and every mail path skips synthetic addresses. Login resolves by phone through `CredentialNormalizer::phoneVariants()`.
**Same adjudication for `UNIQUE(academy_id, phone)`:** `2026_08_30_000004` explicitly logged *"left {column} untouched on user ids {ids} — another account already holds the normalised value… need a manual merge"* (verified). A unique index **will hard-fail on at least one live client**, and that client then silently stops receiving migrations forever. **Cut for now.** Instead: an admin duplicate-merge screen (E4), then the index behind a `GROUP BY … HAVING count(*) > 1` pre-check that logs-and-skips rather than failing. Until then, phone login rejects ambiguous matches rather than `->first()`.
**Identical treatment for `guardians`:** no `UNIQUE(academy_id, person_id)` exists, and ten call sites do `Guardian::where('person_id',…)->first()` — a guardian with two rows sees one set of children and is 403'd on the rest. `PermissionService:224` compounds it: `->where('person_id',…)->orWhere('user_id',…)` with a global scope compiles to `person_id = ? OR (user_id = ? AND academy_id = ?)` — the `person_id` branch **escapes the tenant filter**. Corrected: one `GuardianResolver` service returning a **collection** of guardian rows and the union of participant ids, replacing all ten copies; wrap the `orWhere` in a closure; add the unique index only after a de-dup pass.
**The resolver must also handle adults with no guardian row.** Every one of the 11 `app/Livewire/Parent/*` components ends in `Guardian::…->firstOrFail()`, so a player account hard-fails on every screen. The mobile API already solved this (`$participant->person_id === $personId` first, e.g. `DocumentController:52`). Port that; do not invent a third resolution.
### A7 — `/parent` route group is invisible to `RequireBranchSelection`
`config/branch_lock.php` `unlocked` contains `executive.*`, `reports.*`, `branches.*`, `users.*`, `settings.*`, `dashboard`, `profile.*`, `livewire.*` — verified, **no `parent.*`/`portal.*`**. Any user holding `branches.view_all` in all-branches mode is bounced out of the portal by middleware running on the whole web group. Add the portal prefix in S3.
### A8 — W3's separate bundle isolates nothing as specified
Verified: `app.css` and `website.css` are each a bare `@import 'tailwindcss'` plus one `@theme` block — **no `@source`, no `source(none)`, no `@custom-variant`**. v4 auto-detects from the project root, so both emit the identical complete utility set; `website.css` is app.css plus components. A third file the same way is a third identical copy. `layouts/parent.blade.php` already `@vite`s the full ERP `app.css`.
```css
@import 'tailwindcss' source(none); /* the only thing that isolates anything */
@source '../views/portal/**/*.blade.php';
@source '../views/livewire/portal/**/*.blade.php';
@source '../../app/Livewire/Portal/**/*.php';
@source inline('bg-brand-{50,100,500,600,700} text-brand-{600,700} border-brand-200');
@theme inline { /* inline: utilities read the runtime var, not a copy */
--color-brand-500: var(--brand-500);
--color-brand-fg: var(--brand-fg);
--font-sans: 'Cairo', ui-sans-serif, system-ui, sans-serif;
}
@custom-variant dark (&:where([data-theme="dark"], [data-theme="dark"] *));
```
Order matters: **portal.css first** (new file, no regression surface), website.css second, `app.css` **last** — retrofitting `source(none)` onto app.css silently deletes the 60+ interpolated safelist strings in its header comment (every dynamic status badge in the ERP goes unstyled, with no build error); they must become `@source inline(...)` in the same commit. Never load two Tailwind entrypoints on one document (double preflight, non-deterministic `@layer` order) → **W4's scanner screen is an admin screen and builds with `app.css`**. Take a `npm run build` byte baseline first; there is no `public/build` in the repo, so nobody has measured this. Target: portal.css < 25KB gzipped.
W1's OKLCH ramp: derive in PHP once into `:root` custom properties keyed by `academies.branding_version`; never `color-mix()` at render time against a non-inline `@theme` var.
### A9 — W7 PWA corrections
- SW at **`/app/sw.js`**, `Service-Worker-Allowed` unset, registered `{scope:'/app/'}` — a root worker controls `/dashboard` and `/api/*` and will serve a stale shell to admins and leave cached credentialed responses on a shared front-desk tablet.
- First lines of `fetch`: bail on `method !== 'GET'`, on `/livewire/`, on `/api/`. A cached Livewire POST replays a snapshot whose checksum is bound to `APP_KEY` + session → corrupt-snapshot error, not a stale render.
- **Never precache HTML.** `wire:navigate` (299 usages) swaps `<head>` and will inject `@vite` hashes from a build that no longer exists → blank page, no error; it also prefetches on hover, poisoning a CacheFirst cache with unvisited pages. Navigations are NetworkFirst; the only offline artifact is a static, session-free, CSRF-token-free `/offline.html`.
- Global `Livewire.hook('request', …)` reloading on 419 (a cached shell after session rotation is otherwise a dead page).
- Precache list generated **at runtime** from `public/build/manifest.json`; cache name = hash of that manifest so a deploy purges the old one.
- Manifest served `Cache-Control: private, no-store` (it is a per-tenant response; a shared proxy would serve one tenant's branding to another) with **relative** icon paths — `config/filesystems.php` builds public URLs from `env('APP_URL')` verbatim with no https coercion, and one http URL makes the manifest invalid and the install prompt silently never appears.
- `Route::fallback([WebsitePageController::class,'fallback'])` returns **200 for every unknown URL**, defeating SW navigation fallbacks and deep-link validation. Exclude `app/*`.
- Icons at content-hashed filenames (`icon-{size}-{sha}.png`) — the nginx `expires 1y; immutable` rule otherwise pins a rebranded icon for a year on every installed device.
### A10 — W4: keep the staff-scan direction only, and make the token an identifier
The QR asserts identity; it never authorizes. Enrollment status, participant status, session existence and balance gating are **fresh DB reads at every scan** — that is what makes revocation instant.
- Secret derived, never stored: `HKDF-SHA256(ikm: CHECKIN_PEPPER, salt: academy_uuid, info: participant_uuid || version)`. `CHECKIN_PEPPER` is 32 bytes in env, **distinct from `APP_KEY`** (and must be added to the entrypoint whitelist — A1/15).
- Revocation = `participants.checkin_key_version` (int, default 1); `version++`. One column, atomic, auditable.
- Wire: `v1.<participant_uuid>.<version>.<base32(HMAC-SHA256(academy_uuid||participant_uuid||version||counter)[0:16])>`. Uuid in the clear so the server does one indexed lookup — never iterate participants trying HMACs (DoS + timing oracle). 128-bit tag, `hash_equals`; it is a barcode, so there is no reason to shorten it.
- `counter = floor(unix_time/30)`, computed **server-side**, accept `{C-1,C,C+1}`. Log observed skew; never widen tolerance.
- Consumption: `checkin_consumptions` with `UNIQUE(academy_id, participant_id, counter)` and `INSERT … ON CONFLICT DO NOTHING` **inside the same transaction as the attendance write** — zero rows = replay. A `Cache::has`/`Cache::put` pair is a TOCTOU race. Plus `UNIQUE(participant_id, training_session_id)` on the record, which neutralises a relayed screenshot (relay is unsolvable and any design claiming otherwise is lying; the fix is making it worthless).
- Scanner endpoint: **web session guard**, `permission:attendance.scan`, staff `branch_id` matching the session facility, rate limits 60/min per staff and 10/min per participant. Write through `AttendanceMarkingService` with the scanning staff `User` as `$marker` — **do not add a second attendance write path** (A11 is what happens when you do).
- Token fetched over XHR, rendered client-side, `Cache-Control: no-store`, never in a URL.
### A11 — Excuses are one object, and today they are two contradictory ones
`AbsenceController::report:40-92` writes `status='excused'` straight into `attendance_records` with no `$marker`, no `VALID_TRANSITIONS` check, no event, no audit — and **no check that the session belongs to the participant's groups** (`ParentExcuseForm:86-95` does check). The `already_marked` guard only blocks `present|late|absent`, so `no_show`, `left_early`, `partial` and `expected` are parent-flippable. A player can excuse himself. This corrupts `EnforceAttendanceThresholds`, `SuspendOnThreshold` and every attendance-rate figure.
**Corrected:** an excuse is a **request** (`pending/approved/rejected`, `admin_notes`), never a direct attendance write. Approval calls `AttendanceMarkingService` with the approving staff `User`. `ParentExcuseForm` and `AbsenceController` collapse into one path. See E7 for where the row lives.
### A12 — Member purchase: build `PortalCheckoutService`, do not relax `POSService`
`POSService`'s contract *is* the drawer. Making `cash_session` nullable breaks the reconciliation invariant and spreads A3's double-count into the portal. But the pricing half must be shared or a portal cart and a POS cart of the same items differ in piasters — `POSService:99-101` adds `PlatformFeeService` (whose `getPercentage()` reads `env()`, returning its `3` default under `config:cache`, which the entrypoint runs on every boot), so a portal path that skips it undercharges 3% on every sale.
```php
// app/Domain/Pricing/Services/CartPricingService.php — NEW, shared by POS and portal
public function price(array $lines, ?Participant $buyer, int $branchId, ?string $couponCode = null, ?string $date = null): PricedCart;
// app/Domain/Checkout/Services/PortalCheckoutService.php — NEW
public function quote(CheckoutRequest $r): PricedCart;
public function placeOrder(CheckoutRequest $r, User $actor): Invoice; // creates invoice+items+enrollment; takes no money
public function settle(Invoice $i, PortalPaymentIntent $intent, User $actor): Payment|PaymentIntentResult;
```
Rules: every line through `PricingService::calculate()`, missing base price → `DomainException('لا يوجد سعر محدد')` rolling back the **whole** cart; `branchId` resolved upstream and passed in (never `BranchContext`/`session()` inside a service); `intdiv()` with remainder on the last element for every split; freeze totals — never call the public `Invoice::recalculateTotals()` again; keep `EnrollmentService::enroll(…, User $actor)`'s signature and pass the portal account, gated before the call, with `metadata.source='portal'`; stock deducted via `InventoryService::createMovement()` **only on payment** (an abandoned cart must not drain stock); unique index on `invoices (academy_id, (metadata->>'idempotency_key'))`.
**`InvoiceService::generateNumber()` uses `count()+1` against `unique(academy_id, number)`** — a guaranteed collision under concurrent portal checkout. Replace with a per-academy Postgres sequence. And **do not mint a second series**: `OrderController`'s `INV-MOB-…` with `rand(1000,9999)` forks numbering and every report keyed on it.
### A13 — W9 `OrderController`, with the blast radius the proposal understates
Confirmed: `invoice_items` has exactly `invoice_id, itemable_type, itemable_id, description, quantity, unit_price, discount_amount, tax_amount, total_amount, metadata`. `academy_id`, `description_ar` and `line_total` are **silently dropped by mass assignment** (no `preventSilentlyDiscardingAttributes()` anywhere), so `total_amount` defaults to **0**. Downstream: `ParticipantBillingService::allocate():146` `continue`s on `$part <= 0` → `productPaid()` returns **no key**, so the per-product ownership screen from `b6fd3fb` shows 0 paid for a product demonstrably bought; `productBilled()` gives "owned, billed nothing"; `FinancialOverview::getRevenueBreakdown()` puts the payment in `revenue.total` and in **neither** bucket, so subscription+product stops reconciling to total and the gap grows with mobile adoption; `ReceiptController:48-49` reads the non-existent `line_total` so every mobile invoice line displays `0.00 ج.م`. Latent: any later `recalculateTotals()` on such an invoice sets `total_amount = 0`, `due_amount = -paid_amount`, status `overpaid`, and `getCollectionRate()` starts summing negatives.
Also `OrderController:56-58`: `$participant->classification` — `classification` is a column on **`people`**, and the tier column is `participants.membership_type` (`member|non_member`); `Product::priceForTier()` branches only on those two, so it **always falls through to `selling_price`**. Members never get `member_price`. And `installment_plan_id` is validated at `:27` and **never used** — the member picks "4 أقساط" and gets a lump-sum invoice.
Fix = delete the write, route through `PortalCheckoutService`, plus one guarded repair migration:
```sql
UPDATE invoice_items ii SET total_amount = (ii.quantity*ii.unit_price) - ii.discount_amount + ii.tax_amount
FROM invoices i WHERE i.id = ii.invoice_id AND ii.total_amount = 0 AND ii.unit_price > 0 AND i.number LIKE 'INV-MOB-%';
```
### A14 — IA correction: five tabs, and the switcher is a scope rule, not a screen
Today each component independently re-reads `session('active_child_id')` while `ParentFinances` ignores it and aggregates all children. That is the domain talking: **training is child-scoped, money is family-scoped.** Model the switcher exactly like `BranchContext` (present-but-null = "all"), apply it only to child-scoped surfaces, and decide it **before** the shell is built or all 11 screens re-implement it.
| # | Tab | Contains |
|---|---|---|
| 1 | الرئيسية | today, next session, one action list: مستحقات / مستند منتهي / عرض مقعد انتظار / تجديد / قسط مستحق |
| 2 | التدريب | one session-centric timeline: topic, objectives, coach + substitution reason, facility, `cancelled_reason`, holidays, attendance state inline, "قدّم عذر" **from the session**, evaluations, waitlist offers |
| 3 | المدفوعات | عليك (invoices + installments + renewals) · السجل (payments, POS receipts, refunds) · المحفظة |
| 4 | الأكاديمية | programs, events + registrations, required kit, news, gallery, branch locator, messages |
| 5 | حسابي | profile switcher, documents + upload, emergency contacts + consents, طلباتي, notification prefs, language, devices, terms, delete account |
QR pass is a header/FAB affordance rendered **per active profile** (a guardian with three children needs three), not a tab. Notifications stay in the header bell. Route prefix `/app`, and drop "ولي الأمر" from every `Title` and nav label once a player role exists.
### A15 — Portal auth and W8's credential model
Keep W2's session-on-web-guard decision (CSRF and one logout path come free). Then: the Flutter shell holds **no** cookie-plus-token pair. Mint a long-lived Sanctum token natively, store it in Keychain/EncryptedSharedPreferences, and exchange it for a fresh web session at `/app/session-exchange` on launch — that also buys biometrics and remote force-logout. Never `localStorage` in the WebView, never a token in a JS variable. Never exempt `/app/*` from `VerifyCsrfToken` "because the native app can't get a token" — that single shortcut converts this from safe to trivially exploitable. Every JS bridge is an exported native capability: host allowlist in `NavigationDelegate`, origin read natively from `controller.currentUrl()` (never from the page), file picker returns a handle, **biometrics gate a native action and never return a boolean the page trusts**. Deep links via verified App Links / Universal Links (`assetlinks.json` / AASA served as `application/json` with no redirect — which needs the explicit route from A9, since `Route::fallback` would serve HTML). Use **`flutter_inappwebview`**, not `webview_flutter`: `<input type="file">` is inert in a bare Android WebView without `onShowFileChooser`, which directly gates W5's screenshot upload. Do QR scanning natively (`mobile_scanner`) and pass the decoded string in over a channel.
---
## B. ADDITIONS
### P0 — the portal is broken, illegal, or lying without these
| # | Addition | Why |
|---|---|---|
| B1 | **Member document upload + medical-certificate renewal** | The admin half is fully built (`DocumentApprovalList`, `documents:expire` nightly, `MedicalCertificateAlert`); the member half does not exist and `Api/V1/DocumentController` is `index`-only. Today a certificate expires at 06:00 and the member has no in-product way to fix it. |
| B2 | **In-app account deletion + data export; versioned `consents` table** | Apple 5.1.1(v) and Google both reject without deletion. No consent record exists at all — and this product publishes children's photos on a public website. Blocks W8 outright. |
| B3 | **Payable installments** | `reminders:installments` and `push:installment-due` fire daily; `PaymentController::initiate:34-36` only ever charges the whole `due_amount`. The push says pay and the app cannot. `products.allows_partial_payment` makes it worse. |
| B4 | **Service-request admin queue with domain effects** | `grep -rln ServiceRequest` finds only the API controller, model, event, listener and provider. There is **no admin screen**, and approving a freeze never calls `ParticipantService::freeze()` or `TransferService::transferToBranch()`. W3 ships a screen writing into a void. |
| B5 | **Waitlist accept/decline** | `waitlists.notified_at/expires_at/response`, `WaitlistSpotAvailable`, `SendWaitlistSpotPush` all exist with **no accept surface anywhere**. Push fires, offer expires, revenue lost. One screen. |
| B6 | **Keep the evaluations screen** | `ParentEvaluationDetail` + route `parent.evaluations.show` ship today; W3's screen list drops them. Rebuilding without it is a regression against the thing parents care most about. |
| B7 | **Subscription / renewal surface** | `enrollments:generate-renewals` bills daily; `RenewalPolicy::ManualRenew` explicitly implies a human decision with nowhere to make it. This is the retention surface of the product. |
| B8 | **Notification channel CHECK widened to `push` + `whatsapp`** | `notification_templates`/`notification_logs` allow only `in_app\|email\|sms` while `PushNotificationService` writes `'push'` → **every push log insert throws 23514 today**, and the catch block writes another failing insert. Push delivery logging is 100% broken. Hard prerequisite for W7, not cleanup. |
| B9 | **Tenancy-invariant repairs** — `invoice_items.academy_id`, `installments.academy_id`, `notification_preferences.academy_id`, `event_registrations.participant_id` | All four are tenant tables missing `academy_id`; `event_registrations` links `person_id` only, so "which of my children is registered" is unanswerable as modelled — and W4's guest pass and W3's events screen both assume otherwise. |
| B10 | **`payments.approve_proof` permission, maker-checker above a threshold, hard block on `submitted_by === reviewed_by`** | Approving a proof is the moral equivalent of taking cash. `Auditable::createAuditLog()` takes `user_id` from `auth()->id()` at boot and **silently writes nothing** when it can't resolve an academy, so approval facts must be columns on the proof row, not audit-log dependencies. |
| B11 | **Enforce `guardian_participant.can_authorize_payment`** | The column exists since `2024_01_01_000018:22` and has **zero readers** — every guardian currently has full financial access via `WalletController`, `InstallmentController`, `ReceiptController`. It is the natural gate for proof submission and payment initiation. |
| B12 | **Private-disk + streaming controller for every member upload and read** | `Api/V1/DocumentController:29` returns `asset('storage/'.$file_path)` while the web `DocumentController:38-49` streams from the **`local`** disk behind `authorize('documents.view')` — so the mobile URL either 404s or serves national IDs and medical records unauthenticated. Same for `ProfileController:42,136,179`, `ParticipantController:61`, `ShopController:59`. Validate by magic bytes, generated filename, `Content-Disposition: attachment`, `nosniff`, strip EXIF, rate-limit. |
| B13 | **Schema self-check on `/up`** | With A1/14 fixed the entrypoint fails loudly, but a `hasTable`/`hasColumn` assertion set on the health endpoint is the difference between finding a bad migration in minutes and hearing it from a client in a month. |
| B14 | **Permissions and the `player` role ship as guarded migrations** | `db:seed` runs only when `RUN_SEED_ON_FIRST_DEPLOY=true`, and `SystemSettingsSeeder` is not even wired into `DatabaseSeeder`. Copy `2026_09_01_000001_add_branches_view_all_permission.php` verbatim, including its comment. Same for re-seeding `auth_otp_mode` — `2026_07_27_000004` only touched academies existing at migration time, so a new tenant would otherwise ship the old default again. |
| B15 | **`invoices.branch_id`** (additive, guarded, backfilled) | See A5 — without it, unpaid portal invoices are invisible to every branch's collection figures. |
### P1 — expected, and each is one query from data that already exists
Trainer identity + `substitute_reason` on every session (`training_sessions.trainer_id/assistant_trainer_id`, `trainers.bio_ar` — never loaded by `ParentSchedule`/`ParentHome`); POS purchase history + the existing `/receipt/{uuid}` and `/pos-receipt/{uuid}` routes surfaced; billed-vs-paid per product via `ParticipantBillingService` and `invoice_items.is_delivered` → "زيك جاهز للاستلام"; holidays + `cancelled_reason` + `rescheduled_to_id` on the schedule (an empty week must not read the same as Eid); emergency contacts / blood type / medical notes view + correction request; notification-preferences UI per `event_type` (API-only today); branch/facility locator over `branches.latitude/longitude/operating_hours` (`GET /v1/branches` is already commented "for branch locator"); event fee column + member-side cancel + ticket; **product sizes/variants before any kit shop ships** (`participants.jersey_size/shoe_size` exist and `OrderController::create` never asks); second-guardian invitation over the existing many-to-many (the divorced-parent case currently forces one shared login); wallet top-up + pay-invoice-from-wallet + family wallet (`WalletController` hardcodes `owner_type = Participant`); language toggle (`layouts/parent.blade.php:10` hard-codes `dir="rtl" lang="ar"` in a bilingual product); landing screens for `reports:parent-weekly`, `notifications:birthdays`, `summary:daily` so those pushes deep-link somewhere (W8's link map needs them anyway); **InstaPay daily reconciliation report** (system total vs bank statement per branch per day, with a sign-off row) plus ageing/unmatched-proof reports and a `refund_payouts` table so a promised-but-unsent manual refund is visible; `transactions.branch_id`; device/session security screen over `login_history` once long-lived tokens exist; invoice/receipt PDF and `.ics` export; threaded messaging replacing one-shot `contact_messages`; refund visibility (refunded payments currently just vanish from totals).
### P2 — after the above ships
Attendance streaks and badges from `attendance_records`; evaluation progress charts over `EvaluationScore` history; coupon entry via `PricingService::validateCoupon()`; referrals over `participants.referred_by_id`; photo gallery from `media`; post-session rating writing `trainers.rating` (the column exists and nothing ever writes it); **show the member why they got a discount** — `PricingRuleType::SiblingOrder`/`FamilySize`/`Loyalty` already resolve and `PricingService::explain()` already returns the reasoning.
---
## C. CUTS
| Cut | Why |
|---|---|
| **W4(b): static per-branch poster QR** | A printed QR is a public, permanent, non-secret string; rotation is impossible by construction. It proves the player once visited, or knows someone who did — not presence. It also needs `getUserMedia` inside a WebView, the single most fragile item in the program. Only a powered gate display running a 30s TOTP produces real evidence; paper cannot. |
| **Purchasable guest-pass QR invitations** | New product type + pricing path + redemption state machine + gate flow, for revenue that is a cash sale at the desk today. And `guardian_participant.can_pickup` is the delegated-entry feature actually being asked for — the guest pass reinvents it. Defer until a client asks twice. *(If it ever ships: `billable` = the buying Participant, or NULL + `contact_name` for a non-member — never a `Person` morph, which `ParticipantBillingService` and `getRevenueBreakdown()` silently exclude while payments-based revenue still counts it. Consumption is one conditional `UPDATE … WHERE status='active' AND uses_consumed < max_uses RETURNING *`. Void in the same transaction as a refund, never on a nightly job.)* |
| **W6: a parallel "Mobile App Content" CMS** | `website_sections`, `website_news`, `website_menus`, `media`, a page builder and `website:blueprint export\|import` already exist. Replace the whole workstream with **one additive column** — `channel` (`website\|app\|both`) on `website_news` and `website_sections` — and let the portal read the same tables. A second CMS is a second migration surface forever. |
| **W6's push composer** | `push_announcements` + `BroadcastController` exist. Fix the permission (A1/6) and put a Livewire screen on it. |
| **VAPID Web Push** | `kreait/firebase-php` is installed, `device_tokens` exists, 12 listeners funnel through `PushNotificationService`, each client already has their own Firebase project, and FCM HTTP v1 delivers to Web Push endpoints with the **same** `CloudMessage` and token column. VAPID buys only independence from Google — not a constraint here — at the cost of a second sender, table, log path and prune policy. Instead: widen `device_tokens.platform` CHECK to include `'web'`, add nullable `user_agent`, `importScripts()` the Firebase messaging SW **inside** your own `sw.js` (two root-scope workers otherwise compete), and pass your registration to `getToken({serviceWorkerRegistration})`. |
| **Offline beyond a static `/offline.html`** | A Livewire component *is* server state. There is no offline mode for it. Anything more is weeks of work for a wrong answer. |
| **iOS `apple-touch-startup-image` sets** | 20+ sizes, the classic multi-day sink. Android draws its splash from the manifest; the Flutter shell has a native splash. Accept the iOS white flash. |
| **"Heavy animation" as a stated goal** | On the phones this audience uses, heavy animation is what makes an app feel slow. One shared page transition, then stop. |
| **`PortalProfileService` as a class** | **Contradiction adjudicated.** The completeness lens is right that the switcher is the root of the IA; the delivery lens is right that a service class is premature. Decision: **cut the class, keep the rule** — child-scoped vs family-scoped surfaces (A14) is a scoping decision baked into the layout and a `#[Locked]` computed property, not a new abstraction. Build the service when a second profile *type* actually exists. |
| **`users.email` nullable migration** | A6 — destructive, breaks the password-reset PK, and cannot use `CONCURRENTLY` inside Laravel's transaction. |
| **Unique index on `users.phone`** | A6 — will hard-fail on at least one live client with logged duplicates. |
| **Growing the Sanctum API surface** | The portal is session-authenticated. Freeze `Api/V1` at what the native shell genuinely needs — config, FCM registration, session exchange — rather than building each new feature twice. Every new controller currently inherits a hand-copied `findAuthorized()` and an inert tenancy scope. |
---
## D. BUILD ORDER
**S0 — Production safety. One PR, one deploy, nothing ships before it.**
All 17 rows of A1. Note items 14–16 (entrypoint failure handling, env whitelist, nginx exact-match locations) are **image-layer changes requiring a redeploy of every client** — shipping the nginx rules now, months before the service worker exists, saves a whole deploy cycle later. This is the one sequencing error in the proposal that costs real calendar time.
*No parallelism. Nothing else ships first.*
**S1 — Data integrity (W9 + the ledger). Blocks all money and all push UI.**
`OrderController` rewrite behind `PortalCheckoutService`'s pricing path + the `total_amount` repair migration (A13) · notification channel CHECK widened to `push`/`whatsapp` (B8) · reconcile the two `relationship_type` CHECKs · `PaymentService` account resolution by code (A2) · `recordPayment` guards + `updatePaidAmount` lock (A3) · Paymob through `recordPayment` (A3) · POS cash double-count (A3) · `RefundService` partial refunds + correct accounts · B9 tenancy repairs.
*Depends on: S0. Parallel with S2.*
**S2 — Branding (W1).**
Guarded additive migration (`academies.address` — genuinely absent today and `AcademySettings` writes to it; `theme_color`, `app_icon_path`, `branding_version`) → `BrandProfile` value object → `BrandingService` with one cached read keyed on `branding_version` (kills ~16 queries per admin render) → **delete every dead field not consumed by `BrandProfile`** in the same PR. `intervention/image` on the **GD** driver (GD is in the Dockerfile; imagick, webp and avif are not) generating `icon-{size}-{sha}.png` from the existing `academies.logo_path`. Reject SVG logo uploads or serve them `Content-Disposition: attachment` — an SVG on your own origin is an XSS vector `clean_html()` never sees. Check image dimensions before decode.
*Depends on: S0. Parallel with S1. Excludes the dark-mode decision — that is E1.*
**S3 — Identity + portal auth (W2). Hard dependency for S4–S9.**
`player` role + `portal.*` permissions as guarded migrations (B14) · `portal_invitations` (`token_hash` = `hash('sha256', random_bytes(32))`, UNIQUE, 72h enforced **in the query**, single-use via `UPDATE … WHERE consumed_at IS NULL AND expires_at > now() RETURNING *`, identical response and timing for missing/expired/used, consumed in a **plain controller** — never a Livewire public property, which ships to the browser every round-trip and into A1/7's error page) · `email_is_synthetic` + `.invalid` addresses (A6) · phone login via `phoneVariants()` · **`GuardianResolver`** replacing all ten `->first()` copies, resolving adults by `person_id` (A6) · `PermissionService:224` `orWhere` closure · portal prefix added to `config/branch_lock.php` `unlocked` (A7) · real `authorize()` in every portal `mount()` · duplicate-account merge screen (E4).
*Depends on: S0.*
**S4 — Portal shell (W3), one screen at a time, all reads.**
`portal.css` with `source(none)` (A8) → layout → home → schedule/التدريب → attendance → payments (invoices, installments, POS receipts) → invoice detail → evaluations. Ship there and let a real client use it.
*Depends on: S2 + S3.*
**S5 — InstaPay (W5).**
Six method CHECKs decided in one migration (E6) + `payment_proofs` (A4) → **admin review queue first**, then the portal upload, so a proof can never sit unreviewable → reconciliation report (P1).
*Depends on: S1 + S4. Parallel with S6.*
**S6 — PWA (W7).**
A9. The nginx half already shipped in S0.
*Depends on: S2 + S4.*
**S7 — Push to the portal.** Reuse FCM (C). *Depends on: S1 (B8) + S3 + S6 (one service worker).*
**S8 — QR check-in, staff-scan only (W4a).** A10. *Depends on: S1 + S3 + S4. Parallel with S9.*
**S9 — Flutter wrapper (W8).** *Depends on: S6 + S7 **working and stable in a browser**. Starting earlier means debugging Livewire in a WebView with no devtools. Budget one App Store rejection round: a pure wrapper hits Guideline 4.2, so native FCM, biometrics, native camera/QR, native file picker and share ship on day one, not later. Confirm the reviewer can see that fees and kit are real-world goods (3.1.3(e)/3.1.5) or Apple demands IAP at 30%.*
**S10 — App content.** The `channel` column (C) + a Livewire screen over `push_announcements`. *No dependents; last.*
**Corrections to the proposal's own sequence:** W9 is listed last but two of its items gate W5 and W7 → moved to S1. W1's `@theme` bridge is worthless before `source(none)` exists → the bridge belongs with the portal bundle, not the branding service. W4 is listed before W5/W7 but depends on identity and the attendance write path → moved after S5. W7-after-W6 is inverted: the CMS has no dependents; the PWA gates W8. W8 is listed as independent; it depends on W7, on the nginx change, and on A1/3.
---
## E. OPEN DECISIONS
**E1 — Dark mode: own it, or delete it.**
~900 `dark:` utilities are compiling to `prefers-color-scheme` today, so every OS-dark user already sees an untested dark ERP, while the `.dark` toggle in `dark-mode-toggle.blade.php` does nothing.
*Either:* declare `@custom-variant dark (&:where(.dark, .dark *))` in `app.css` — the OS-driven rendering stops immediately, the toggle starts working, and the 900 utilities become a design surface you now own and must audit. *Or:* strip all 900 and the toggle.
**Recommendation: declare the variant.** One line stops an untested rendering reaching real users today; the audit can then be scheduled instead of being forced.
**E2 — Age of majority for self-service.**
No rule exists anywhere. `people.date_of_birth` and `Participant::getAgeAttribute()` do.
*Either:* self-service unlocks at 18 (money always guardian-gated whenever any guardian holds `can_authorize_payment`). *Or:* per-academy configurable threshold.
**Recommendation: hardcode 18 now, with money always guardian-gated.** A configurable threshold is a settings row nobody will tune, and a 12-year-old must never see the family's arrears.
**E3 — Ship the Flutter wrapper at all this cycle?**
*Either:* PWA-only for the first clients, wrapper later. *Or:* wrapper in the same program.
**Recommendation: PWA-only first.** W8 is the highest-rejection-risk item, depends on S6+S7, and its safety story (forced update, maintenance) rests on an endpoint that has never worked (A1/3).
**E4 — Duplicate phone/email accounts.**
`2026_08_30_000004` left them in place by design.
*Either:* build an admin merge screen in S3 and add uniqueness afterwards. *Or:* live without uniqueness and reject ambiguous logins forever.
**Recommendation: build the merge screen.** Without uniqueness, `->first()` on `phone` stays an account-selection primitive for as long as the portal exists.
**E5 — Overpaid proofs.**
*Either:* cap at `due_amount` and deposit the excess via `WalletService::deposit()` in the same transaction. *Or:* allow `InvoiceStatus::Overpaid`.
**Recommendation: cap + wallet.** Nothing consumes `Overpaid`, `due_amount` goes negative, and `getCollectionRate()` and `ParticipantBillingService` start summing negatives.
**E6 — Which of the six method CHECKs get `'instapay'`.**
Current lists diverge already; `payslips` already contains `'instapay'`, and `pos_transactions`/`pos_split_payments` lack `bank_transfer` entirely.
*Either:* `payments` + `expenses` + `facility_rent_payments` only (portal/office channel). *Or:* also `pos_transactions` + `pos_split_payments` (reception accepts an InstaPay transfer at the till).
**Recommendation: all five.** Reception will take an InstaPay transfer within a month of launch, and the failure mode is a Postgres `23514` at the till in front of a customer.
**E7 — Where an excuse lives.**
*Either:* a new `excuses` table. *Or:* extend `service_requests.type` CHECK with `'excuse'` plus a nullable `attendance_record_id`.
**Recommendation: extend `service_requests`.** Excuses and requests are the same interaction (member-initiated, `pending/approved/rejected`, `admin_notes`), the type CHECK already has an `other` slot, and one queue means B4's admin screen serves both — two tables means two half-built workflows, which is exactly the current state.
**E8 — Branch on portal payments.**
*Either:* `branch_id` required at the service boundary, resolved participant → `users.preferred_branch_id` → group/program → 422. *Or:* allow NULL and accept unattributed revenue.
**Recommendation: required.** A NULL-branch payment appears in the all-branches total and in no branch, with no error — the exact failure `3520098` was written to fix.
\ No newline at end of file
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