1. 01 Sep, 2026 10 commits
    • Mahmoud Aglan's avatar
      feat(portal): consent, deletion, requests that take effect, and the seeder bug... · e5ce6f7f
      Mahmoud Aglan authored
      feat(portal): consent, deletion, requests that take effect, and the seeder bug that would have erased it all
      
      The additions the addendum marked P0 and the programme had not built, plus
      two ordering bugs found by testing a from-scratch install rather than only the
      incremental one.
      
      The seeder bug (would have broken the portal on the second deploy)
      ------------------------------------------------------------------
      PermissionSeeder deletes every permission_role row for a role and reinserts
      its own list, and db:seed runs on EVERY container start when
      RUN_SEED_ON_FIRST_DEPLOY is true. So the portal.* grants added by migration
      would have worked exactly until the next deploy and then vanished — the
      portal 403'ing for every member, with nothing in the logs and no migration to
      blame.
      
      And on a brand-new client the migration runs before any academy exists, so it
      created no player role and granted nothing at all.
      
      Both are fixed where they belong: the permissions and the `player` role are in
      PermissionSeeder and RolesAndPermissionsSeeder now, so a fresh install gets
      them and the seeder stops erasing them. The migration stays for existing
      tenants. Verified by migrating an empty database from zero, seeding it, then
      booting it a second time and re-checking every grant.
      
      attendance.scan reaches trainers, head trainers and reception — the people who
      actually stand at a gate. payments.approve_proof reaches accountants.
      
      B2 — consent and deletion (a store-submission blocker)
      ------------------------------------------------------
      Apple 5.1.1(v) and Google both refuse an app that creates accounts and cannot
      delete them, so this is what makes a submission possible rather than a
      refinement to add later.
      
      There was no consent record anywhere in the schema — not a column — while this
      product publishes children's photographs on a public website and sends
      marketing over WhatsApp. Consents are versioned and append-only, enforced by a
      database trigger: a consent is a statement about a particular text at a
      particular moment, so editing one destroys the only thing that makes it
      evidence. Withdrawal is a new row. Bumping the document version invalidates
      previous answers, because a boolean would silently claim a member agreed to
      text they have never seen.
      
      Deletion is redaction, not erasure. This is also an accounting system:
      invoices, payments and ledger rows are the academy's books, and a member must
      not be able to delete them by tapping a button. The person's identifying data
      is destroyed, the login is destroyed, the financial record survives without
      their name. Three gates before that: re-authentication, a cooling-off window,
      and a blocked state with the reason shown when money is owed or an enrolment
      is live — shown up front, because a refusal at the last step is not respectful.
      
      Data export is the other half of the same obligation and is streamed, never
      stored: a file of somebody's whole record sitting on disk waiting to be
      collected is a second copy of the data they asked to control.
      
      B4 + E7 — requests that actually do something
      ---------------------------------------------
      `grep -rln ServiceRequest` found a model, an event, a listener and a provider,
      and no admin screen. Approving a freeze never called ParticipantService::freeze()
      — the column changed and the subscription kept running. A member was told
      their subscription was frozen when it was not.
      
      Approval is now defined by its effect, and the effect runs in the same
      transaction: if it fails the approval fails with it and the request stays
      pending. "Approved, and nothing happened" is worse than "still pending".
      
      E7 decided: an excuse is a service_request, never a direct attendance write.
      Two contradictory implementations existed and neither worked — ParentExcuseForm
      validated, stored its medical attachment to the PUBLIC disk, then discarded the
      record behind a `// TODO` while telling the parent it had succeeded; and the
      deleted API wrote status='excused' with no marker and no check that the session
      belonged to the participant, so a player could excuse himself and corrupt every
      attendance figure the product reports. Approval goes through
      AttendanceMarkingService with the approving staff member as marker, and a
      coach's existing observation is never overwritten. ParentExcuseForm is deleted:
      it never worked, so there was nothing to preserve.
      
      B1, B3, B5, B7, B11, B12, B13
      ------------------------------
      - Document upload and renewal. The admin half has been complete for a long
        time — DocumentApprovalList, a nightly documents:expire, a
        MedicalCertificateAlert — and the member half did not exist, so a certificate
        expired at 06:00 and the member had no way inside the product to fix it.
      - Payable instalments. reminders:installments and push:installment-due fire
        daily and the only payment path ever built charged the whole due_amount: the
        push said pay and the app could not. Settled from the wallet, which is the
        one payment the portal can complete immediately — money the academy already
        holds.
      - Waitlist accept/decline. The offer, the expiry and the push all existed with
        no accept surface anywhere, so the offer expired and the place went to nobody.
      - Renewal surface, for RenewalPolicy::ManualRenew, which explicitly means a
        human decides.
      - can_authorize_payment gates instalment payment as well as proof submission.
      - Every member upload is streamed from the private disk with attachment,
        nosniff and no-store. A member upload is never a URL.
      - /health asserts the schema this code needs and names what is missing.
        Verified against a deliberately half-migrated database: 503, and
        ["payment_proofs","invoices.branch_id"].
      
      /parent retired
      ---------------
      Permanent redirects to the equivalent portal screen, parameters preserved so a
      bookmarked invoice still lands on that invoice. Two member portals must not
      coexist: they diverge, and the one nobody updates is the one a member has
      bookmarked.
      
      E2 recorded in config/compliance.php: 18, hardcoded rather than per-academy,
      because a settings row nobody tunes is a false choice and a twelve year old
      must never open the app and see the household's arrears.
      
      Verification
      ------------
      - 144 migrations from zero on an empty database, then db:seed, then a second
        boot — every grant intact.
      - The same on the restored oc_sport tenant: nothing to migrate, seed clean,
        713 invoices / 356 participants / 649 payments untouched.
      - 11 portal screens render 200 for a real member; 4 staff screens for an owner;
        members refused on both staff screens.
      - Suite: 76 pass on SQLite; on the tenant, PortalSmoke 3/3, AdminScreens 2/2,
        PaymentProof 11/11, CheckInScan 10/10, ServiceRequestEffect 14/14.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      e5ce6f7f
    • Mahmoud Aglan's avatar
      fix(routing): register the scanner above the wildcard that was swallowing it · fb2519c6
      Mahmoud Aglan authored
      /attendance/{session} binds its parameter to a uuid, so /attendance/scan
      registered after it never matched: the wildcard took 'scan' first and died
      casting it to a uuid — a 500, not a 404, so it did not look like a routing
      problem at all.
      
      Found by rendering the staff screens rather than by trusting that they route:
      route:list sorts its output, so it showed the scanner sitting above the
      wildcard when the file has it below. The list is not the matcher.
      
      Adds the staff-screen smoke test that caught it, which also asserts a member
      account is refused the approval queue and the scanner.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      fb2519c6
    • Mahmoud Aglan's avatar
      feat(portal): PWA, push, the check-in gate, the native shell, and the docs that were wrong · 1819b943
      Mahmoud Aglan authored
      S6, S7, the staff half of S8, S9 and S10, plus the admin screens S3 and S5
      were waiting on.
      
      PWA (S6)
      --------
      The worker lives at /app/sw.js and is generated per deploy, because the
      precache list comes from public/build/manifest.json and the cache name is a
      hash of it — a deploy evicts the old cache instead of leaving a worker
      serving asset URLs that no longer exist.
      
      Its scope is /app/, not the root: a root worker would control /dashboard and
      /api too, serving admins a stale shell and leaving cached credentialed
      responses on a shared front-desk tablet.
      
      HTML is never precached. wire:navigate swaps <head> wholesale and prefetches
      on hover, so an HTML cache fills with unvisited pages and then injects @vite
      hashes from a build that no longer exists — a blank page with no error.
      /livewire/* is never cached at all: its snapshot checksum is bound to APP_KEY
      and the session, so a replayed one is a corrupt-snapshot error rather than a
      stale render. The only offline artifact is a static page with no session and
      no CSRF token in it.
      
      nginx gets exact-match locations for /app/sw.js and /app/manifest.webmanifest.
      Both end in an extension the static-asset regex claims, and that regex ends in
      try_files $uri =404 — so without these the worker 404s before reaching PHP.
      
      Push (S7)
      ---------
      FCM, not VAPID. kreait/firebase-php is installed, device_tokens exists, twelve
      listeners already funnel through PushNotificationService, every client has
      their own Firebase project, and FCM HTTP v1 delivers to Web Push endpoints
      with the same CloudMessage and the same token column. VAPID buys independence
      from Google — not a constraint here — for a second sender, table, log path and
      prune policy.
      
      So the change is: platform CHECK widened to include 'web', a user_agent column
      for sensible pruning, and a unique index on (device_token, user_id) — never on
      the token alone, which is what the deleted DeviceController keyed on, letting
      anyone claim anyone's token so the victim's phone received the attacker's
      notifications. Duplicates are cleared before the index, because a failed
      migration blocks every later one on that client forever.
      
      The check-in gate (S8)
      ----------------------
      Staff-scan only. The printed-poster direction stays cut: a printed QR is a
      public, permanent, non-secret string, so rotation is impossible by
      construction — it proves the member once visited, or knows someone who did.
      
      The scanner screen works with a connected barcode reader by default and uses
      BarcodeDetector where the browser has it, because most reception desks have
      the reader and not the camera permission.
      
      A real bug the tests caught: participants.status is cast to an enum, so
      comparing it to the string 'active' was always false — the gate would have
      turned everyone away.
      
      The native shell (S9)
      ---------------------
      flutter_shell/ holds one long-lived Sanctum token in the Keychain or
      EncryptedSharedPreferences with the single ability portal:session, and
      exchanges it at /app/session-exchange for an ordinary web session in the
      WebView's own jar. The token never reaches JavaScript. /app/* is never
      exempted from CSRF — that shortcut is what turns a wrapper from safe into
      trivially exploitable.
      
      Every bridge is an exported native capability, so each is narrow and checked
      natively: the host allowlist is compared against the origin read from the
      controller, never from the page; biometrics gate a native action and return
      nothing the page can use as an authorisation decision; QR is decoded natively
      and only the string crosses.
      
      flutter_inappwebview rather than webview_flutter, because <input type="file">
      is inert in a bare Android WebView without onShowFileChooser — and that single
      gap breaks the transfer-proof upload, which is the portal's whole money path.
      
      Two endpoints only, and they are the only routes on the sanctum guard. The
      deleted API minted tokens with mobile:* — every endpoint it would ever grow.
      
      E3's recommendation stands and the shell is not shipped this cycle. It exists
      so that shipping is a decision rather than a project.
      
      App content (S10)
      -----------------
      One `channel` column on website_news and website_sections instead of the
      parallel CMS the plan called for. website_sections, website_news,
      website_menus, media, a page builder and website:blueprint export|import all
      already exist; a second CMS is a second migration surface, a second editor to
      keep in step, and a second place for content to go missing, forever.
      
      Admin screens
      -------------
      Portal invitations, where the raw link exists for exactly one render and is
      never recoverable afterwards. The duplicate-account merge screen E4 asked for
      — the prerequisite for ever putting a unique index on users.phone, and the
      reason ambiguous phone logins can be refused rather than guessed. Both move
      references rather than deleting rows: a deleted user id in a financial record
      is worse than a duplicate account.
      
      Documentation that was actively wrong
      -------------------------------------
      docs/agent-rules/05-financial-integrity.md described double-entry as two rows
      with a type of 'debit' or 'credit', and 16-enums-and-checks.md registered that
      vocabulary. That schema has never existed — 2024_01_01_000013 created the
      single-row shape with both account columns from the start. Anyone writing code
      from that text got a mass-assignment no-op and a row that silently said
      nothing. CLAUDE.md repeated the same claim, and also said Livewire 3 while
      composer.json says ^4.3 — a difference that decides whether a public property
      is an IDOR.
      
      The test suite is now symmetrical: tests that build their own tables skip off
      SQLite, tests that need a real tenant skip off Postgres, so the whole file
      runs clean under either connection instead of one of them being a lie.
      
      Suite: 76 pass on SQLite (24 skipped), and against the restored tenant
      PortalSmokeTest 3/3, PaymentProofTest 11/11, CheckInScanTest 10/10.
      portal.css is 4.98 kB gzipped against app.css at 31.10 kB.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      1819b943
    • Mahmoud Aglan's avatar
      feat(financial): InstaPay transfers, reviewed before they become money · 2c95f617
      Mahmoud Aglan authored
      S5. The academy publishes a handle, the member transfers and records what
      they sent, and staff turn that claim into a payment only after matching it
      against the academy's own statement.
      
      An unverified screenshot must never create a Payment. `transactions` are
      immutable and recordPayment() forces status = Confirmed and posts to the
      ledger immediately, so a proof is a separate object with its own lifecycle
      and only approval calls recordPayment() — double-entry happens exactly once
      and nothing in the ledger is ever edited.
      
      **A screenshot is not evidence.** It is a convenience. The control is
      `sender_reference`, unique per academy per method behind a partial index,
      which kills replay, cross-invoice reuse and "someone else's transfer against
      my invoice" in one constraint. The reviewer types the amount from the
      statement; `amount_claimed` is what the payer said and is never what gets
      posted.
      
      Concurrency is a conditional UPDATE, not a disabled button. Two reviewers
      open the queue and both see an enabled Approve; the second one's UPDATE
      matches zero rows and raises InvalidStatusTransitionException. A row that has
      left `pending` is frozen by a BEFORE UPDATE trigger — approving a proof is
      the moral equivalent of taking cash, and Auditable::createAuditLog() takes
      its user from auth() at boot and silently writes nothing when it cannot
      resolve an academy, so the approval facts are columns on the row rather than
      an audit-log dependency.
      
      Overpayment is capped at what is due and the excess is deposited to the
      member's wallet in the same transaction. InvoiceStatus::Overpaid exists but
      nothing consumes it and it drives due_amount negative, after which
      getCollectionRate() and ParticipantBillingService start summing negatives.
      
      branch_id is NOT NULL on a proof. Revenue is branch-attributed only through
      payments, so a NULL-branch payment lands in the all-branches total and in no
      branch — the columns stop summing with no error anywhere.
      
      E6 decided as recommended: all five method CHECKs that lacked `instapay` get
      it, the till included. Reception will take an InstaPay transfer within a
      month of launch, and the failure mode of leaving the POS out is a Postgres
      23514 at the till in front of a customer. pos_transactions and
      pos_split_payments also gain `bank_transfer`, which they never had.
      
      The review queue ships before the member-facing upload, on purpose: a proof
      that can be submitted and never reviewed is a promise to a member that nobody
      is keeping.
      
      Proof files go to the private disk and are streamed by a controller that
      authorises the submitter, a co-guardian of the same member, and staff holding
      payments.approve_proof — Content-Disposition: attachment, nosniff, no-store.
      The parent excuse form wrote its medical attachments to the public disk; that
      is the mistake not to repeat.
      
      Verified against a restored copy of backups/oc_sport-20260831-081053.dump: a
      600 EGP transfer against a 500 EGP invoice posts 500 to the invoice
      (Dr 1010 Bank / Cr 4000 Training Revenue, branch attributed) and 100 to the
      wallet; duplicate reference, self-approval, zero amount, second approval and
      editing a settled row are all refused.
      
      Suite: 87 passed, 3 skipped locally; 11 InstaPay tests pass against the
      restored tenant.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      2c95f617
    • Mahmoud Aglan's avatar
      feat(portal): identity, the member portal shell, and the check-in pass · acca60b0
      Mahmoud Aglan authored
      S3, S4 and the core of S8.
      
      Identity (S3)
      -------------
      GuardianResolver replaces ten hand-copied
      `Guardian::where('person_id', …)->first()` lookups, every one wrong in the
      same two ways: `->first()` on a column with no unique constraint, so a
      guardian holding two rows saw one set of children and was 403'd on the rest,
      silently; and no answer at all for an adult member, because all eleven
      app/Livewire/Parent/* components end in ->firstOrFail() and a player has no
      guardian row. That is why a player given the `parent` role saw empty lists —
      the domain had no path from a user to his own participant.
      
      PermissionService::getChildParticipantIds() also carried
      `->where('person_id', …)->orWhere('user_id', …)`, which with the tenant
      global scope appended compiles to `person_id = ? OR (user_id = ? AND
      academy_id = ?)` — the first branch escaping the tenant filter entirely. The
      closure is what keeps both branches inside it.
      
      A real `player` role and the portal.* permissions ship as a guarded
      migration, not a seeder: db:seed only runs when RUN_SEED_ON_FIRST_DEPLOY is
      true, so a client deployed outside the one-click template would never receive
      them. Same pattern as 2026_09_01_000001.
      
      portal_invitations stores only the SHA-256 of its token — a raw token in a
      row is a password in a row — and consumption is one conditional UPDATE whose
      WHERE clause carries every condition, so two taps on the same link on a phone
      cannot both create an account. Activation lives in a plain controller, never
      Livewire: a single-use token in a public property is serialised into the page
      on every round-trip.
      
      users.email stays NOT NULL UNIQUE, deliberately. 2024_01_01_000002 declares
      it inside Schema::create, so Postgres emits a UNIQUE CONSTRAINT that cannot
      be made partial without a DROP CONSTRAINT in up(); CREATE INDEX CONCURRENTLY
      cannot run in a migration transaction; and password_reset_tokens.email is the
      primary key the broker keys on. Portal accounts get p{uuid}@portal.invalid
      (RFC 2606, never routable) plus an email_is_synthetic flag every mail path
      checks. No unique index on users.phone either: 2026_08_30_000004 logged that
      it left duplicates in place, so one would hard-fail on at least one live
      client and then block that client's migrations forever.
      
      Phone login now refuses when one number matches several different people —
      signing someone into a stranger's account — while still resolving a genuine
      duplicate pair for the same person.
      
      config/branch_lock.php gains portal.* and parent.*: RequireBranchSelection
      runs on the whole web group, so without it any user holding branches.view_all
      in all-branches mode is bounced out of the portal by middleware.
      
      The portal (S4)
      ---------------
      Five tabs at /app — الرئيسية, التدريب, المدفوعات, الأكاديمية, حسابي — with
      the pass as a header affordance because it is per active profile: a guardian
      with three children needs three.
      
      PortalContext is the scope rule the IA turns on, decided once instead of
      eleven times: training is member-scoped, money is family-scoped. The old
      components each re-read session('active_child_id') independently while
      ParentFinances ignored it and aggregated everyone — the domain saying out
      loud that a household has one balance. The active id is re-validated against
      GuardianResolver on every read, so a value put into the session, or left
      there after a withdrawal, cannot widen what an account sees.
      
      No participant id is held in a public property anywhere in the namespace.
      This is Livewire v4, where a plain public property is settable from the
      browser, so a check in mount() that is not repeated in render() is
      decoration, not a check.
      
      portal.css is the only entrypoint built with `source(none)`. app.css and
      website.css are each a bare `@import 'tailwindcss'`, so v4 auto-detects from
      the project root and both emit the identical complete utility set — a third
      file written the same way would have been a third identical copy. Measured:
      portal.css 17.11 kB / 4.54 kB gzipped against app.css at 208 kB / 31 kB.
      
      Screens surface what was always one join away and never loaded: the coach
      taking each session and the reason for a substitution, cancelled_reason so an
      empty week does not read the same as Eid, and per-event registration for the
      right child — answerable only since event_registrations gained participant_id
      in S1.
      
      The check-in pass (S8 core)
      ---------------------------
      qr_check_in_enabled has been a toggle in system settings with zero functional
      readers since 2026_07_27: the product advertised a feature that did not exist.
      
      The pass asserts identity and never authorizes. Enrolment, participant
      status, session existence and branch are fresh reads at every scan, which is
      what makes a suspension take effect at the next scan rather than the next
      token rotation. The secret is derived by HKDF from a pepper that is
      deliberately not APP_KEY, revocation is one integer column, and a scanned
      code is consumed by INSERT … ON CONFLICT DO NOTHING inside the same
      transaction as the attendance write — a Cache::has/put pair would be a
      time-of-check race, and two scanners at one gate is exactly when it loses.
      Relay is not solvable; it is made worthless instead.
      
      SelfCheckInService writes through AttendanceMarkingService with the scanning
      staff as the marker rather than adding a second attendance write path. The
      deleted API had one of those: POST /v1/absences/report wrote status='excused'
      with no marker, no transition check, no audit and no check that the session
      belonged to the participant.
      
      QrCode is written rather than pulled in — there is no Composer step here that
      can add to the committed lock file, and the alternative was the existing
      pattern of an <img> pointing at api.qrserver.com, which sends the member's
      token to a third party and fails when the venue's wifi does.
      
      It was verified module-for-module against an independent implementation
      across versions 1-10 and all eight masks, given identical codewords. That
      found two bugs neither visible nor throwing: a Reed-Solomon generator
      polynomial built with its terms reversed, and missing version-information
      blocks for versions 7 and up, whose 36 modules were being filled with payload
      and shifting the whole stream. Both produced a plausible square of black and
      white that no scanner accepts. tests/Fixtures/qr_golden.php freezes that
      verification.
      
      Verified against a restored copy of backups/oc_sport-20260831-081053.dump:
      all seven portal screens render 200 for a real member account, the manifest
      is tenant-branded and no-store, and a member opening another family's invoice
      gets 403.
      
      Suite: 76 passed, 3 skipped (the tenant smoke test skips off Postgres rather
      than pretending SQLite is production).
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      acca60b0
    • Mahmoud Aglan's avatar
      feat(branding): one resolved brand, and the fields that were collected but never read · 2dff900b
      Mahmoud Aglan authored
      S2 of the mobile-portal programme. Branding lived in four uncoordinated
      stores with no sync, and each layout resolved it itself in an `@php` block
      issuing one SettingsService::get() per field — about sixteen SELECTs per
      admin render, repeated on every Livewire round-trip, each with its own
      fallback. That is how primary_color came to be defined three times with
      three different defaults.
      
      BrandingService returns a readonly BrandProfile, cached under the academy's
      new `branding_version` and bumped on save, so it is held until branding
      actually changes rather than for a guessed number of minutes, and a queue
      worker cannot serve last week's colours. Verified on the restored oc_sport
      copy: a second resolve inside one request issues 0 queries.
      
      Defects fixed, each verified against that copy:
      
      - `academies.address` did not exist. AcademySettings has been reading and
        writing it on every save since it was written, and Eloquent silently
        dropped it — no academy has ever had an address stored.
      - `branding.academy_name` was read by the parent layout and by every printed
        sheet and written by nothing, so both showed the literal string "الكابتن"
        on every tenant. It is seeded from the academy's own name and is now
        editable. The login page now reads "او سي سبورت" on the verified tenant.
      - components/print/sheet.blade.php emitted the raw storage path into an
        <img src>, so the logo was broken on every printed sheet. Paths become
        URLs in BrandingService and nowhere else.
      - Guests had no academy bound at all, so the login screen — and the member
        portal's own sign-in, when it exists — rendered under the fallback brand on
        every client. An installation with exactly one academy now resolves it for
        guests too; more than one is ambiguous and binds nothing.
      - AcademySettings had no authorize() call while every sibling settings screen
        does.
      
      Dead fields: the plan's rule is wire it or delete it, and none of them
      survived as collect-but-ignore. login_background now grounds the login
      screen, invoice_header and invoice_footer_text and show_logo_in_invoice
      reach the printed invoice, header_bg colours the topbar, compact_sidebar
      narrows the rail, and success_color/danger_color colour the flash strip.
      
      Colour derivation. sidebar.blade.php hardcoded `color: #fff` on the brand
      accent — this is a tenant-branded product, so a client whose brand is yellow
      got white on yellow at 1.53:1. ColorRamp derives a 50…900 OKLCH ramp plus a
      foreground chosen by WCAG contrast: that same yellow now gets #111827 at
      11.58:1. Nine brand colours are asserted at AA or better.
      
      The ramp is anchored on the tenant's own lightness rather than fixed
      targets, because fixed targets are non-monotonic for an inherently light
      brand: yellow sits at L 0.86, so a table putting 400 at L 0.70 makes 400
      darker than 500. Chroma falls steeply at the pale end — at L 0.97 a chroma
      of 0.10 is outside sRGB and clips to mud.
      
      E1 decided as the addendum recommends: `@custom-variant dark` is declared
      against the `.dark` class. Roughly 900 `dark:` utilities have been compiling
      to prefers-color-scheme and rendering an untested dark ERP for every OS-dark
      user, while the toggle did nothing. The OS-driven rendering stops here and
      the toggle becomes the only thing that switches themes.
      
      App icons are generated with GD directly rather than by adding
      intervention/image: GD is the only image extension in the Dockerfile, and
      the whole job is decode, letterbox, resize, write PNG. Dimensions are read
      from the header before decoding, since a small file can declare enormous
      dimensions. Filenames are content-hashed because nginx serves assets
      `expires 1y; immutable`.
      
      SVG uploads are refused everywhere they were accepted. An SVG on the
      academy's own origin executes script with the site's privileges and
      clean_html() never sees it.
      
      npm run build byte baseline before portal.css exists:
      app.css 208.10 kB / 31.02 kB gzip, website.css 217.08 kB / 33.25 kB gzip.
      
      Suite: 67 passed, 0 failed.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      2dff900b
    • Mahmoud Aglan's avatar
      fix(financial): make the double-entry ledger say what actually happened · 8d251d14
      Mahmoud Aglan authored
      S1 of the mobile-portal programme. Every item here is a live defect, and
      each one blocks the portal's money path rather than merely preceding it.
      
      The ledger. PaymentService::resolveDebitAccount() returned the literal 1
      and resolveCreditAccount() returned 2, both with a `// TODO`. Seeder order
      made that Dr Cash / Cr Bank on every payment the product has ever taken —
      627 of 701 rows on the restored oc_sport copy — so no revenue account was
      ever credited and FinancialOverview::getRevenueBySource(), which groups
      transactions by credit_account_id restricted to revenue accounts, could
      only ever return []. Accounts now resolve by code within the academy and
      hard-fail when absent, and a payment is split across revenue accounts in
      proportion to the invoice's own lines, floored with intdiv() and the
      remainder on the last row. Routing InstaPay into the old ledger would have
      multiplied a broken ledger across a new channel.
      
      The guards. Every rule 05-financial-integrity.md names lived in the UI, in
      two hand-copied Livewire components, so any new caller inherited none of
      them. amount > 0, amount <= due re-read under lockForUpdate inside the
      transaction, invoice not cancelled/paid, academy and currency agreement all
      sit in the service now. Draft is deliberately still payable: the POS issues
      an invoice as a draft and settles it in the same transaction.
      
      updatePaidAmount() was a read-modify-write on money with no lock — two
      settlements landing together each read the old paid_amount and one
      increment was lost.
      
      Paymob confirmed callbacks inline: no lock, no Transaction row at all, and
      an idempotency guard that was dead code because the finder already filtered
      status = Pending, so a retried webhook credited the invoice twice. It goes
      through PaymentService::confirmPending() now, which asserts the captured
      amount matches.
      
      POS cash sales double-counted the drawer: POSService incremented
      total_cash_in and UpdateCashSessionTotals incremented it again, inflating
      the expected drawer 2x and producing phantom variance at close. One writer
      each now. A split tendered above the total (cash handed over, change given)
      capped at the amount due instead of producing an overpaid invoice.
      
      RefundService refunded the full payment only, so an over-approved amount
      could not be corrected; it also hardcoded accounts 2/1 with a comment
      claiming A/R, which is account 3, and debited the refunding user's own
      drawer rather than the one that took the money.
      
      Migrations, all guarded and all verified against a restored copy of
      backups/oc_sport-20260831-081053.dump:
      
      - chart of accounts seeded for every academy, not just Academy::first().
        The verified tenant was missing 4060, and db:seed only runs on first
        deploy — so a hard-failing resolver had to be preceded by this.
      - invoices.branch_id and transactions.branch_id, backfilled. Revenue was
        branch-attributed only through payments.branch_id, and getCollectionRate()
        scopes invoices through whereHas('payments'), so an invoice with no
        payment yet belonged to no branch. Portal invoices awaiting a proof would
        have vanished from every branch's overdue figure. 588/713 invoices and
        644 transactions attributed.
      - academy_id on invoice_items, installments and notification_preferences,
        participant_id on event_registrations — four tenant tables that broke the
        tenancy invariant, all reachable from the portal.
      - notification channel CHECK widened to push and whatsapp.
        PushNotificationService writes 'push' and the CHECK allowed only
        in_app|email|sms, so every push delivery log insert raises 23514 today and
        the catch block writes another failing insert.
      - guardians and guardian_participant relationship_type CHECKs reconciled to
        their union. NewRegistrationWizard validates one field against the pivot's
        vocabulary and writes it to both tables, so picking أخ / أخت / وصي crashes
        registration on the guardians CHECK right now.
      - invoice_number_counters replaces generateNumber()'s count()+1 against a
        UNIQUE(academy_id, number) index — a guaranteed collision the moment
        members can check out without a receptionist serialising them, and it
        reissued numbers soft-deleted invoices still hold.
      - the deleted mobile API's INV-MOB lines repaired: it wrote line_total,
        which is not a column, so total_amount defaulted to 0 and every downstream
        allocation read the sale as worthless.
      
      tests/Feature/ExampleTest.php deleted: the stock Laravel scaffold test has
      failed since `init` (it GETs / with no tenant database), permanently
      red-lighting the suite and masking real failures.
      
      Suite: 61 passed, 0 failed.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      8d251d14
    • Mahmoud Aglan's avatar
      fix(security): revoke the tokens the removed mobile API issued · 80cc4497
      Mahmoud Aglan authored
      AuthOtpController::verify() accepted a constant '0000' in the mode every instance
      shipped with, and minted a Sanctum token with ability 'mobile:*' for whichever
      active user matched the submitted phone number — staff included. The routes were
      deleted in 883391c7, so the tokens reach nothing today, but a credential that was
      issuable without authentication should not sit in the table waiting for the next
      surface that accepts Sanctum.
      
      Every client gets this, so it is a migration rather than an SSH per instance.
      Two such rows exist on the one instance that used the API; the others have none,
      and the table guard covers instances that never ran Sanctum's migration.
      
      Deleting rows in up() is a deliberate exception to "destructive operations live
      in down() only" — that rule protects schema and client data, and this is neither.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      80cc4497
    • Mahmoud Aglan's avatar
      docs: the test is all-clients or one-client, not structure or records · 05a6a63e
      Mahmoud Aglan authored
      Corrects the rule I wrote two commits ago, which was wrong in the other
      direction. I had said rows never belong in a migration. They do, whenever every
      client needs them — a lookup table, reference data, a permission the code checks,
      a default setting. Seeding those from a migration is the correct pattern, not a
      workaround, and add_branches_view_all_permission is the example.
      
      The repository is common ownership: it defines what every client gets. So the
      only question worth asking is whether a change is for all clients or for one
      specific client. All clients means the repo, and anything touching the database
      goes in a migration whether it is schema or data. One client means SSH to that
      instance and it never enters the repo — because a migration applies to every
      tenant at once and cannot be scoped to one.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      05a6a63e
    • Mahmoud Aglan's avatar
      docs: separate structural changes from record changes · d20598a2
      Mahmoud Aglan authored
      I reached for a migration to delete rows — stale API tokens — because the change
      needed to apply to every client. That is the wrong test, and the rule it violated
      was written down nowhere.
      
      The deciding question is structure or records, never reach. Schema belongs in a
      migration because it is structure. Rows belong in the client's own database
      because they are that client's data, even when several clients need the same
      correction. A migration that edits rows edits them on every tenant at once, with
      no review and no way to do it for one client only — and this product is one
      install per client, each running the same software over their own records.
      
      Also states plainly that we do not deploy: code is committed and pushed, and the
      platform ships it. Nothing here triggers a CapRover build.
      
      Records the code cannot run without — a permission it checks, a default setting
      it reads — are genuinely ambiguous, and the repo has precedent both ways. Noted
      as a grey area to ask about rather than pretending the line is clean.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      d20598a2
  2. 31 Aug, 2026 14 commits
    • Claude's avatar
      Give every expense a receipt you can open · cc417ba9
      Claude authored
      An expense recorded with a scan attached arrived in the database with no
      scan at all. ExpenseForm uploaded the file and passed the path to
      ExpenseService::recordExpense(), which builds its Expense::create() array
      by hand and never copied the two attachment keys across — so the file
      landed on disk and the row forgot about it. It landed on the `public`
      disk too, which needs a storage symlink the containers never create, so
      even a persisted path would have 404'd.
      
      Receipts now go to the private disk and are read back through
      ExpenseAttachmentController, which checks the permission, the academy and
      the active branch before streaming a byte.
      
      The list was also a dead end: a row showed a number and a description and
      offered nothing but "cancel". Rows are now clickable and carry a view
      button, with a paperclip marking the ones that have evidence behind them.
      
      The new detail page is where the expense explains itself — amount,
      category, recipient, method, receipt reference, branch, notes, who
      recorded it and when, and, if it was cancelled, by whom and why. Below
      that sit the journal entries it produced, the original debit/credit pair
      and any reversing entry, so the accounting effect is visible rather than
      implied. The receipt itself previews inline: images as images, PDFs in a
      frame, with download beside them.
      
      An expense recorded without a scan is no longer stuck that way — attach
      one from the detail page, replace it (the displaced file is deleted), or
      remove it. Every attachment records who uploaded it and when. A cancelled
      expense refuses all three: its evidence is frozen with its journal.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      cc417ba9
    • Mahmoud Aglan's avatar
      docs(mobile-portal): the approved programme, its adversarial review, and what it supersedes · 2ef6e088
      Mahmoud Aglan authored
      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>
      2ef6e088
    • Mahmoud Aglan's avatar
      fix(security): remove the mobile API surface and stop the 500 page leaking sessions · 883391c7
      Mahmoud Aglan authored
      Two live disclosures and one latent account takeover, plus the infrastructure
      defects that hid them.
      
      AuthOtpController::verify() accepted a constant '0000' whenever auth_otp_mode was
      'demo' — the value every instance was seeded with — and then minted a Sanctum
      token for whichever active user matched the submitted phone number, staff
      included. It was not exploitable as written, because 2026_08_30_000004 had
      normalised users.phone to digits-only local form while normalizePhone() produced
      +20…, so the lookup missed. That is one plausible bug-fix away from being live,
      which is why the whole surface goes rather than the branch.
      
      Deleting /api/v1 also removes: broadcast/send pushing to every device in the
      academy with no permission check; ReceiptController's inverted ownership check,
      which made any non-participant invoice world-readable to any token;
      PaymentController::initiate with no ownership check at all; and
      DeviceController keying updateOrCreate on the FCM token alone, letting one user
      claim another's device. None of it is replaced — the member-facing surface is the
      session-authenticated web portal, so a second token-authenticated surface meant
      building and authorizing everything twice.
      
      bootstrap/app.php built a full diagnostic payload for any 500 and errors/500
      rendered it to the browser, ungated by APP_DEBUG. The session it printed carries
      password_hash_web — the signed-in user's bcrypt hash — alongside the last ten
      queries, the request input and the headers. Now gated on debug, auth keys
      stripped by prefix even there, and the production page is self-contained with no
      CDN. Detail still reaches storage/logs, keyed by the error id shown to the user.
      
      ParentHome::$activeChildId was validated in mount() and selectChild() but used
      raw in render() at eight query sites. Livewire is ^4.3, where a public property
      is settable from the browser, so those checks were decoration: a guardian could
      walk participant ids and read any child's balance, attendance and evaluations.
      Locked, and re-validated in render() since the child list can change between
      requests.
      
      ParentExcuseForm wrote the attachment — typically a child's medical note — to the
      PUBLIC disk, then discarded the record and flashed success. The parent believed
      the absence was excused; nothing was stored, and the record kept feeding the
      consecutive-absence threshold that auto-suspends a participant. It now stores
      nothing and says so, until excuses are modelled properly.
      
      Infrastructure, because each one hid a failure rather than causing one:
      entrypoint continued booting after a failed migration, which serves a stale
      schema and silently blocks every later migration forever; the env whitelist had
      no PAYMOB_, so config:cache baked null credentials and the gateway failed closed
      with no error anywhere; nginx's static-asset regex answered =404 for /sw.js
      before PHP saw it; and Route::fallback returned 200 for every unrouted path, so
      a deleted endpoint served a website page instead of 404.
      
      Verified: 43/44 tests pass. The one failure is ExampleTest, which fails
      identically on unmodified main — confirmed by stashing. Two new tests pin both
      disclosures so they cannot return.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      883391c7
    • Mahmoud Aglan's avatar
      fix(groups): show what a player paid this month, and why it is that number · d2f4bf17
      Mahmoud Aglan authored
      The roster's الدفع column answered a different question from the one it
      appeared to answer, in three compounding ways.
      
      It showed a LIFETIME subscription total beside a monthly bill. A player who paid
      650 in July, 1,200 for a kit bag and 650 in August read as "2,500" for the
      current month. Two months of subscriptions were simply added together.
      
      "Subscription" was defined as "an invoice line with no product link" — a
      negative definition, so every hand-typed line became subscription money. That
      kit bag was typed as free text, so it landed in the subscription figure, was
      missing from product revenue, and left the same screen reporting the player had
      never bought the kit they had paid for.
      
      The red "has not paid" flag came from an unrelated calculation: matching invoice
      text with ilike %اشتراك% plus the programme name. Substring matching on Arabic
      also decides that تجهيزي contains زي. On live data the flag and the amount
      disagreed on 28 of 247 active enrolments — red rows showing a green figure. The
      template's "show unpaid only if flagged AND the amount is zero" guard was not
      defensive coding; it was two sources of truth being reconciled where the
      disagreement stopped being visible.
      
      The figure is now this billing cycle only, derived from the programme's own
      cycle rather than the calendar month, and one computation feeds the amount, the
      row flag and the header counts — so they cannot contradict each other again.
      
      Each figure is colour-coded by WHY it is that number, with a legend above the
      table: paid in full, pro-rated for a mid-month join, admin discount, line price
      override, instalment, partial, unpaid, not yet billed, free. All of it was
      already recorded in invoice and line metadata and never surfaced; the reason,
      who applied it and the original price now appear on the row. Colour never
      carries the meaning alone — each amount also shows a glyph, a label and a
      screen-reader sentence, and every case sits at 4.5:1 against white.
      
      The migration links hand-typed product lines to their product where the full
      trimmed description matches a product name exactly. Substrings are deliberately
      not matched and ambiguous lines are left alone: 44 lines / 209,200 EGP link
      safely, 23 lines / 42,800 EGP are reported for a human instead of guessed at.
      
      Verified by replaying the real production rows behind both reported screenshots
      through the service: every figure the user questioned is now explained.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      d2f4bf17
    • Mahmoud Aglan's avatar
      docs: pin the branch and ref checks into the push rule · 820ea078
      Mahmoud Aglan authored
      Standing authorisation to push fixes without asking was already in place; what
      was missing were the checks that make it safe to exercise.
      
      Both failed today. The session-start git snapshot said `main` while a parallel
      session had since checked out a feature branch in the same working copy, so a
      verified fix was committed to the wrong branch — and `git push origin main`
      then reported "Everything up-to-date" and exited 0 while the fix sat elsewhere.
      A no-op push is indistinguishable from a successful one unless the remote ref
      is checked.
      
      Also makes explicit-path commits mandatory. This checkout is shared with other
      sessions whose in-flight work can be staged in the index; `-a` or `git add -A`
      would sweep it into a fix commit and deploy it to every tenant.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      820ea078
    • Mahmoud Aglan's avatar
      fix(financial): stop binding NULL as a filter Postgres cannot type · 75fbca7a
      Mahmoud Aglan authored
      The financial overview 500'd with SQLSTATE 42P08 on `($4 IS NULL OR
      p.branch_id = $4)`. Postgres fixes each prepared-statement parameter's type
      during parse analysis, and `:branch_id IS NULL` gives it nothing to work from
      — the statement is rejected before it ever reaches the comparison that would
      have typed it. `:academy_id` was the same shape and would have failed next.
      
      The idiom came in with 35200985 and could not be caught here: phpunit runs
      SQLite in memory, which types placeholders at bind time and executes the
      broken form happily.
      
      Fixed by appending the branch and academy filters only when they apply, with
      their bindings, rather than passing NULL as a sentinel — which is what the
      ->when() filters in the same method already do, and keeps the
      (academy_id, branch_id) index usable instead of hiding it behind an OR.
      
      The SQL build is extracted to buildTopProgramsQuery() so it can be asserted on
      without a database. The test pins four things: the placeholders and the
      bindings agree in all four filter combinations, the clauses are omitted rather
      than nulled, the built SQL executes, and no raw SQL under app/ binds a
      placeholder as a NULL sentinel again. That last one is a source scan on
      purpose — the suite's driver is not the production driver, so it cannot
      observe this failure by running.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      75fbca7a
    • Mahmoud Aglan's avatar
      docs: push fixes to main without asking · 6d59b789
      Mahmoud Aglan authored
      Standing authorisation from the user: a verified fix goes out in the same turn
      it is finished, rather than waiting in the working tree for approval.
      
      Written with the order fixed (verify, then commit, then push) and with the
      boundary spelled out, because a push here is a deploy to every tenant at once
      — entrypoint.sh runs migrate --force and db:seed on every container start, and
      there is no staging. Features, schema changes and anything destructive still
      get confirmed.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      6d59b789
    • Mahmoud Aglan's avatar
      fix(branches): stop the branch switcher 500ing on an unbuilt executive view · fc7060b9
      Mahmoud Aglan authored
      config/branch_lock.php names executive.dashboard as the lock's destination,
      but that route was never built. route() throws on an undefined name, so
      switching to "كل الفروع" crashed in production after the session had already
      been written — the user landed in all-branches mode via an error page.
      
      isLocked() already refused to lock without the route, and that was believed
      to make the whole feature dormant. It only made the *gating* dormant: the
      guard sits on the decision, while the crash is at the dereference. Four other
      sites turned the same name into a URL, and BranchSwitcher's was outside the
      gate entirely. Auth/Login reached it only through the config key, so it does
      not even contain the string "executive".
      
      Route every caller through BranchContext::redirectRouteName(), which returns
      the configured route when it exists and degrades to the dashboard when it
      does not. The dashboard is the right fallback while the view is unbuilt: the
      lock is dormant, so it is already unfiltered and showing the every-branch
      numbers the user asked for.
      
      The test pins the resolver in both directions and fails if any Livewire
      component or middleware reads branch_lock.redirect_route directly again.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      fc7060b9
    • Mahmoud Aglan's avatar
      fix(attendance): stop the roster moving under the coach's finger · 7af211fc
      Mahmoud Aglan authored
      Taking attendance re-sorted the list by status on every render, so the
      moment a coach marked someone the row jumped somewhere else and everyone
      below it shifted. Coaches lost their place, could not tell who was already
      handled, and recorded the same player several times.
      
      The roster is now ordered by name with the record id as a tie-break —
      never by anything the coach can change from this screen — so the list
      holds still. Marking a player takes them out of the working list entirely
      and into a collapsed "تم تسجيلهم" section, grouped by status with counts,
      where the decision can be reviewed or changed. A confirmation toast names
      the player and the status that was saved, and a progress card shows how
      many are left.
      
      Also here:
      - markAs/markPresent/saveRecordNote now resolve the record within this
        session instead of by bare id, and reject statuses outside the four the
        screen offers
      - service calls are wrapped in try/catch, so a blocked medical certificate
        shows an Arabic message instead of an error page
      - the polymorphic subject relation is eager-loaded with morphWith (was an
        N+1 on every player row)
      - one responsive card list replaces the duplicated mobile/desktop markup;
        targets are ≥36px, the progress bar carries progressbar semantics and
        the toast is an aria-live region
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      7af211fc
    • Mahmoud Aglan's avatar
      feat(branches): make "all branches" a state the app can actually hold · 48a79a76
      Mahmoud Aglan authored
      Session::has() is `! is_null(get($key))`, so it reports false for a key
      holding null — which is exactly how "all branches" was stored. Three
      call sites tested presence that way, so selecting كل الفروع silently
      reverted to a single branch on the next navigation and isAllBranches()
      was unreachable dead code. All three now use exists().
      
      BranchContext is the one place that reads that state. It lives in
      Context, not Services, because the project rule keeps services free of
      session/auth so they stay queue-safe; this is the adapter that turns
      request state into the explicit ?int $branchId services receive. A null
      left by a user whose permission was revoked is repaired rather than
      honoured, and stamping deliberately does not follow branchId() — API
      routes and queued listeners run outside the request, and a record filed
      against no branch would vanish from every per-branch total for good.
      
      The lock itself is dormant on purpose: isLocked() returns false while
      the executive dashboard route does not exist, since locking would
      otherwise 500 every page including its own redirect target. The
      permission ships as a migration as well as a seeder entry, because
      db:seed only runs when RUN_SEED_ON_FIRST_DEPLOY is set.
      
      Also stops enabling the query log outside debug — it retained every
      statement of every request in production memory for nothing.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      48a79a76
    • Mahmoud Aglan's avatar
      feat(groups): show per-product ownership and real amounts paid · b6fd3fb7
      Mahmoud Aglan authored
      A programme can now bundle products it requires (program_products), so
      the group view can answer "who has not bought their registration card"
      — which nothing in the system could express before. products.is_essential
      is global; this is per-programme.
      
      Each bundled product gets its own column: bought or not, a progress bar,
      and the amount settled against the amount billed. Instalments fall out
      of this for free rather than needing their own column.
      
      Reading a payment off a line is not possible here — a subscription and a
      registration card routinely share one invoice. ParticipantBillingService
      allocates each payment across the lines it covers, pro rata on
      subtotal_amount, rounding down so the remainder stays unallocated rather
      than inventing money. Allocation is capped at the amount billed: a
      payment settles total_amount, which also carries tax and fees, so paying
      in full would otherwise allocate over 100% of a line. Verified against
      production — no invoice over-allocates.
      
      The payment column now shows the amount paid rather than a bare "paid",
      with مجاني for free players and لم يدفع for unpaid, and participants
      carry their عضو / غير عضو tag. The enrolment-date column is gone.
      Total collected is shown to users with invoices.list.
      
      The bundling migration is conditional: it acts only where an academy has
      both an active product named قيد and programmes named فريق. Elsewhere it
      does nothing, which is what makes it safe for every tenant. On oc-sport
      that is exactly one product across 12 programmes.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      b6fd3fb7
    • Mahmoud Aglan's avatar
      fix(financial): attribute revenue to what was actually sold · 35200985
      Mahmoud Aglan authored
      Measured against the live oc-sport database, subscription revenue read
      457,970 EGP against a genuine 345,257 — overstated by 32.6% — while the
      per-programme breakdown summed to 39,873, about 12% of reality.
      
      Three distinct causes:
      
      POSService::buildInvoiceItems() discarded the item_type/item_id it was
      handed, so every POS line landed with a NULL itemable_type. Reporting
      reads NULL as "programme subscription", which moved 102,000 EGP of
      product sales into subscription revenue — 90% of the error — and meant
      no product-ownership check could ever pass. Lines now carry their
      Product or Kit. A migration backfills history by matching invoice lines
      to their POS lines, filling only NULL rows and only where the match is
      unambiguous; a production dry run matched 44 of 45 with 0 ambiguous.
      
      Pro-rata allocation divided by invoices.total_amount, but line totals
      sum to subtotal_amount — total_amount also carries discount, tax and
      service fees. Every bundled invoice was therefore split on the wrong
      denominator (10,713 EGP).
      
      topPrograms joined enrolments to invoices and dropped anything without
      an invoice_id. Only 88 of 350 enrolments have one, so 75% of programmes
      reported zero. Now a UNION: the exact link where it exists, participant
      fallback where it does not, split evenly across a participant's
      programmes. Reconciles at 333,105 EGP.
      
      Also: the mounted revenue widgets and the receptionist dashboard omitted
      direction='inbound', counting refunds as income, and the widgets' raw
      queries bypassed SoftDeletes and cancelled invoices.
      
      EnrollExistingWizard read BasePrice directly, ignoring membership type
      and every pricing rule, so it quoted a different figure than the
      registration wizard for the same player. Both now go through
      PricingService.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      35200985
    • Mahmoud Aglan's avatar
      fix(website): validate section + theme edits and keep unpublished sites private · fa4c5088
      Mahmoud Aglan authored
      Section and theme editors wrote straight to columns that carry CHECK
      constraints, so a bad value surfaced as a 500 rather than a field error.
      Adds rules() mirroring the constraints, Arabic messages(), and an error
      summary in both forms.
      
      home() also served unpublished sites to the public. Staff keep their
      preview route; everyone else is sent to login.
      
      Drops a redundant invalidateAll() from ThemeEditor::save(): the call
      passed an argument the method does not take, and would have flushed
      every tenant's cache. WebsiteSettingService::update() already
      invalidates the one academy that changed.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      fa4c5088
    • Claude's avatar
      Stamp every transaction with the branch it happened in · cd2faefd
      Claude authored
      Records were reaching the database with no branch, so they belonged to
      no branch and were invisible in every branch view. Three causes:
      
      1. Invoices have no branch_id column, yet three call sites read
         $invoice->branch_id and stored the result. It was always null.
         POSService did this for every point-of-sale payment, which is why the
         walk-in ("عميل عابر") sales had no branch. POS now uses the branch the
         sale was rung up in; the mobile payment controller and InvoiceShow
         take it from the participant being billed.
      
      2. PaymentService::record() only set a branch if its caller happened to
         pass one, and most callers did not.
      
      3. Nothing enforced the rule centrally.
      
      New BelongsToBranch trait stamps the active branch at creation, mirroring
      BelongsToAcademy. It is applied to the models that record an action —
      Payment, Expense, CashSession, FacilityRentPayment, POSTransaction,
      PurchaseOrder, Participant, TrainingGroup — and deliberately not to
      catalogue models such as BasePrice, PricingRule, Product and Employee,
      where a null branch legitimately means "shared across all branches".
      
      The trait adds no global scope on purpose: branch is a reporting lens,
      not an isolation boundary, and scoping globally would break console
      commands, cross-branch reports and the switcher's "all branches" mode.
      It also returns null rather than guessing when there is no request
      context, so scheduled jobs do not misfile academy-wide records.
      
      Also adds a migration trimming stray whitespace — including the
      non-breaking space U+00A0 that survives copy-paste — from names shown to
      users. Those characters are invisible in forms but render as a gap in
      page titles and receipts, and break exact-match lookups.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      cd2faefd
  3. 30 Aug, 2026 13 commits
    • Claude's avatar
      Itemise financial expenses and stop double-counting refunds · 99c1d2b6
      Claude authored
      Three separate defects made the financial figures wrong.
      
      1. Refunds were counted as revenue. Eighteen queries summed payments on
         status='confirmed' with no direction filter, so outbound refunds were
         added to income across the dashboard, the revenue/product/subscription
         widgets, the financial report, the print report and ReportService.
         That inflated revenue by 40,048 EGP all-time, 32,510 this month.
      
      2. Refunds were simultaneously counted as an expense. The refunded
         original already drops out of revenue when its status becomes
         'refunded', so adding the outbound payment to expenses deducted the
         same money a second time. Refunds are now contra-revenue: the revenue
         card shows gross collected, refunds, and the net, and the expense side
         no longer includes them.
      
      3. Expenses were presented as vague lumps, the worst being "مدفوعات أخرى"
         — which was in fact customer refunds. The breakdown is now one line
         per real cost (payroll, facility rent, purchases, and each expense
         category separately), sorted by size, each stating where it comes
         from.
      
      Payroll was missing from expenses entirely; approved and paid payslips
      plus trainer compensation are now included, scoped by branch through the
      trainer's employee record.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      99c1d2b6
    • Claude's avatar
      Scope financial overview expenses and P&L to the active branch · 944c5001
      Claude authored
      The financial overview filtered revenue by branch but not expenses, so
      every branch showed the same expense figure. The "مدفوعات أخرى" line was
      academy-wide outbound payments — 32,510 EGP of customer refunds issued
      this month, all belonging to Zayed — displayed identically under all 7
      branches. Purchase orders had the same problem, and in the 6-month P&L
      chart both income and outbound were unfiltered.
      
      Also: PaymentService::refund() created the outbound payment without
      copying branch_id from the payment being refunded, so refunds taken
      through that path landed in no branch at all and were invisible in every
      branch view. RefundService already did this correctly.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      944c5001
    • Claude's avatar
      docs: move loose specs out of .claude into docs/specs · aeab7b7d
      Claude authored
      102 KB of planning documents (website-builder-v2 spec, mobile app plan, mobile
      API implementation, data snapshots) were sitting directly in .claude/ rather
      than in docs/. They are reference material, not agent configuration.
      
      .claude/ is now empty of markdown, so nothing in this repo is auto-loaded into
      agent context except CLAUDE.md.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      aeab7b7d
    • Claude's avatar
      docs: move agent rules out of auto-injection, compact CLAUDE.md · f114bf42
      Claude authored
      The 18 files in .claude/rules were injected into every agent turn — 38 KB of
      POS, inventory and attendance rules loaded even while editing CSS. They are now
      in docs/agent-rules/ and read on demand.
      
      CLAUDE.md keeps every hard invariant inline (money as piasters, tenancy scoping,
      double-entry immutability, migration-first, RTL logical properties, no dead
      links, safe_url/clean_html) and indexes the detail, so nothing that protects
      code quality was dropped.
      
      Also documents the deployment constraint that governs every migration: all
      tenants build from main and the entrypoint runs migrate --force plus db:seed on
      every container start.
      
      Net: ~57 KB less agent context per turn.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      f114bf42
    • Claude's avatar
      chore(reference): add OC-Sport site mirror for the website migration · fa497ab9
      Claude authored
      Captured 2026-08-30 from oc-sport.com as the source spec for rebuilding the
      client's site in the v3 builder. Contains their 7 pages in both locales, the
      static assets, the public images, their published OpenAPI description and the
      extracted bilingual content catalogue (604 strings x en/ar).
      
      analysis/FINDINGS.md holds the gap analysis that motivated the v3 builder.
      
      Excluded from the Docker build context via .dockerignore, so it never ships to
      a tenant image.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      fa497ab9
    • Claude's avatar
      fix(website): guard editor-supplied URLs against unsafe schemes · c3f2ae53
      Claude authored
      Menu items, button links, announcement bars, popups, floating buttons and
      navbar CTAs all wrote editor-supplied values straight into href attributes.
      Any user with settings.manage could store a `javascript:` URL and have it run
      for every visitor of that tenant's public site.
      
      Adds safe_url(), which allow-lists http/https/mailto/tel/whatsapp, site-relative
      paths and fragments, and rejects protocol-relative URLs, data:, vbscript: and
      entity/whitespace/control-character obfuscation before testing the scheme.
      
      Applied at four layers so no writer can bypass it:
        - input     MenuManager rules + BlockField Link validation
        - model     WebsiteMenuItem::href()
        - render    every editor-supplied href in every website view
        - import    WebsiteBlueprintService, since blueprint files skip form rules
      
      safe_url() returns null rather than '#', so blocks skip the link entirely
      instead of emitting a dead anchor — this satisfies the project's no-href="#"
      rule with the same mechanism.
      
      Also fixes Alpine expression injection in the gallery and schedule lightboxes,
      where a quote inside an image URL could break out of the inline handler: Blade
      escapes ' to &#039; but the browser decodes it before Alpine parses.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      c3f2ae53
    • Claude's avatar
      feat(website): add page + block tree builder (v3) · 39f468a9
      Claude authored
      The v2 builder could not express more than one page: website_sections had a
      unique(academy_id, section_key) constraint, there was no pages table, and
      SectionManager exposed only toggle + reorder. A client with a seven-page site
      had no way to represent page two.
      
      Adds an additive page/block model alongside v2:
      
      - website_pages + website_blocks (nested tree, JSONB data/style)
      - BlockRegistry of BlockType classes: 31 types, 133 layout variants, 237
        fields, 442 validation rules derived from the field schema
      - Page/Block/Menu/Blueprint services, BlockRenderer, BlockDataResolver
      - Builder UI: page manager, block tree editor, schema-driven field forms,
        repeaters, content/design/motion panels, image upload
      - Authored navigation (website_menus) with dropdowns, replacing nav links
        that were previously derived from enabled sections
      - Blueprint import/export via `php artisan website:blueprint`
      - Extended motion library: entrance effects, delay, stagger, parallax
      
      A new block type now costs one PHP class — no migration, no enum case, no
      CHECK constraint.
      
      Nothing here is destructive. website_sections is untouched and "/" falls back
      to the legacy renderer when no builder homepage exists, so already-deployed
      tenants are unaffected until they opt in.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      39f468a9
    • Claude's avatar
      Fix branch leaks in the two receptionist wizards · c54f5aea
      Claude authored
      The enrolment wizard looked up a programme's base price without any
      branch filter, so with per-branch pricing across 7 branches the
      receptionist could be quoted another branch's price.
      
      The registration wizard's printed receipt resolved the branch with
      Branch::first(), so every receipt printed the first branch's details
      regardless of where the registration actually happened.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      c54f5aea
    • Claude's avatar
      Filter every management screen by the active branch · f282f7bc
      Claude authored
      The system predates branches, and adoption of the branch switcher was
      partial: 44 of 186 Livewire components used UsesBranchScope, and several
      that imported it never actually called it. The dashboard was the worst
      case — half its widgets were branch-aware and half silently reported
      academy-wide totals next to them, so the numbers on one screen were not
      comparable with each other.
      
      OC-Sport runs 7 active branches, so every unscoped widget was showing
      six other branches' data.
      
      Dashboard: scoped trainers-present, pending payslips, pending documents,
      low stock and expiring medical certificates, which were academy-wide.
      All six dashboard widgets (revenue, product revenue, subscription
      revenue, enrolment trends, overdue renewals, trainer dues) now filter by
      branch, including the raw-SQL CTEs in the revenue breakdowns.
      
      Lists and reports: events, evaluations, base prices, pricing rules,
      promotions, stock counts, kits, document approvals, trainers, trainer
      advances, payroll, essential deliveries and the financial report.
      
      Pickers: participant, group, program, facility, warehouse, product and
      employee selectors now offer only the active branch's records, so a
      transfer or invoice cannot silently reference another branch.
      
      POS and InvoiceShow used auth()->user()->branch_id directly, ignoring
      the switcher entirely — a user who switched branch still transacted
      against their home branch. Both now read the active branch.
      
      Deliberately left unscoped: parent- and guardian-facing screens, which
      are scoped to their own children and have no branch switcher, and
      single-record detail screens, which are already scoped by the record and
      would hide legitimately related history for participants who moved
      between branches.
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      f282f7bc
    • Claude's avatar
      Make login credentials case- and format-insensitive · 60eabb70
      Claude authored
      Postgres '=' is case-sensitive, so a user stored as 'Km...@gmail.com'
      could not log in from a phone keyboard that lowercases the email field.
      The lookup in AuthService returned null before Hash::check ever ran, so
      this presented as "wrong password" and was invisible in login_history —
      that table is only written once a user has been found.
      
      On OC-Sport this affected 8 of 26 accounts, and had already produced one
      duplicate registration: a user who could not get in simply signed up
      again with the same address in lowercase.
      
      - CredentialNormalizer: one canonical shape for emails and phones
      - AuthService: case-insensitive email lookup, deterministically ordered
        so a pre-existing case-duplicate pair resolves to the account actually
        in use rather than an arbitrary row; phone lookup matches local and
        +20 forms
      - User: set-mutators so new rows are stored canonical
      - Migration: normalises existing rows, skipping and logging any that
        would collide, since those are duplicate accounts needing a human
        merge rather than a guess
      Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
      60eabb70
    • Mahmoud Aglan's avatar
      Add branded printable schedule sheets with the space grid · 3d472374
      Mahmoud Aglan authored
      "The schedule" is not one document. Different people print it for different
      reasons and a single layout serves none of them, so this ships three sheets
      sized for their actual use scene:
      
      - Facility day board (A4/A3 landscape, auto-picked by segment count) — time
        down the side, the facility's physical grid segments across the top, every
        booking in its own cell. This is the one that carries the grid, and the one
        that did not exist. Built to be pinned at the court entrance and read from
        a few metres away.
      - Facility week board (A3 landscape) — seven days x time, segments as a badge
        per booking. The notice-board overview.
      - Trainer day cards (A4 portrait, two-up, cut lines) — one pocket card per
        trainer: when, where, which segment, how many players. A trainer does not
        want an A3 off the wall.
      
      Rendered as branded HTML and printed from the browser rather than through a
      PDF library: Arabic shaping survives intact, Cairo and the academy's brand
      colour render exactly, and nothing queues on the server. print-color-adjust
      is set explicitly, without which browsers strip every fill and the whole
      colour-coded board arrives as blank boxes.
      
      Group colours move to a shared GroupColor palette used by both the builder
      and every sheet, so a coach who learns "our group is the teal one" on the
      wall sees the same teal in the app. The hues stay distinguishable in
      greyscale, because plenty of academies print on a mono laser.
      
      Bookings read as a filled, tinted cell rather than a thin coloured stripe —
      a 3px rail is invisible at the distance these are actually read from.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      3d472374
    • Mahmoud Aglan's avatar
      Replace emoji with icons; scope schedule builder by branch and sport · 9454aa03
      Mahmoud Aglan authored
      Icons
      - No emoji anywhere in the UI. Extracted the sidebar's inline SVG map into
        a single <x-ui.icon name="..."> component and added the icons the pricing
        work needed, so there is one source instead of a per-view copy. Discount
        recipes now carry icon NAMES, not glyphs.
      
      Schedule builder
      - Facilities are scoped to the selected branch. The screen listed every
        branch's facilities, which is how someone books the wrong building. A
        ?facility_id= carried over from another branch (bookmark, back button) is
        now dropped instead of silently overriding the branch scope.
      - Groups are scoped to the facility's branch AND to the sports that facility
        hosts, so a football court no longer offers swimming groups. That link did
        not exist, so this adds a facility_activities pivot. A facility that
        declares no activities still hosts anything, so nothing breaks for academies
        that have not filled it in.
      
      Facility grid
      - Removed the arbitrary ceilings (rows/columns capped at 10, lanes at 20).
        Physical space is not limited to a number we picked.
      - New facilities never got a layout, which is why the grid silently failed to
        appear on them. FacilityService::create now seeds one, the migration
        backfills every existing facility that has none, and the default is a 1x1
        grid — "one whole space, not subdivided yet" — rather than inventing a
        subdivision nobody asked for.
      - Grid size is editable straight from facility settings, with a live preview
        of the cells being described. Shrinking onto a segment that holds a
        confirmed future reservation is refused rather than silently dropping
        someone's booking.
      - Sports and starting grid are both settable at creation time too.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      9454aa03
    • Mahmoud Aglan's avatar
      Fix pricing engine and rebuild discounts around recipes and a picker · b3f127c8
      Mahmoud Aglan authored
      Two bugs meant no pricing rule has ever applied correctly:
      
      1. Condition keys never matched. The engine reads min/max/values;
         the wizard wrote min_age/min_children/target_gender and the form
         blade wrote a third set. Ranges saw null bounds and list rules saw
         an empty allow-list, both of which passed, so every rule applied to
         every participant.
      2. Percentages were 100x too small. applyAdjustment divides by 10000
         (basis points) but both screens stored a plain percent, so "20%"
         discounted 0.2%.
      
      They masked each other, which is why the symptom looked like a broken
      engine rather than two bugs — and why everyone moved to the untyped
      super-admin price override instead.
      
      Engine
      - ConditionSchema is now the single owner of the conditions vocabulary;
        builder, engine, simulator and migration all read keys from it.
      - Percent handles all basis-point conversion; nothing else touches the
        raw column.
      - evaluateInList fails closed instead of treating an empty allow-list
        as "match everyone".
      - custom rules no longer auto-apply; they are picker-only.
      - enrollment_timing honours days_before_start (fails closed without a
        program start date instead of silently passing).
      - Global discount cap reads system_settings rather than a hardcoded
        constant with a TODO.
      - New: explain(), audience(), wouldApply(), and role-capped manual
        discounts.
      
      Per-branch
      - pricing_rule_branches pivot so one rule targets many branches,
        instead of one near-identical row per branch that drifts apart.
      
      Stacking
      - is_stackable now defaults to false; best-of-one is the normal case
        and stacking is an explicit opt-in.
      
      Authoring
      - The five-step column editor becomes a recipe gallery plus an Arabic
        sentence, with a live simulator on a real participant and an audience
        count that warns when a rule would hit everyone. Saving a
        conditionless rule is refused.
      
      Checkout
      - ManagesDiscounts trait plus <x-pricing.discount-picker>: branch-scoped,
        searchable, pinned favourites, replace-vs-stack inline, blocked rows
        show why. Wired into CollectPaymentWizard renewals; discount names are
        frozen onto invoice.metadata so receipts survive later rule changes.
      - NewRegistrationWizard now prices through the engine using a
        provisional context built from the form, since the participant row
        does not exist yet. The step-4 guard still checks the base price, so a
        100% discount is not mistaken for an unpriced program.
      
      Migration
      - Rewrites conditions onto the canonical keys and scales percentages to
        basis points. Rules whose conditions cannot be mapped confidently are
        deactivated rather than guessed, with the old JSON kept in
        metadata.legacy_conditions.
      
      Also fixes list and coupon views that rendered the raw column (a 20%
      rule would have displayed as 2000%), and adds the [x-cloak] CSS rule
      that was missing app-wide.
      Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
      b3f127c8
  4. 27 Aug, 2026 2 commits
  5. 24 Aug, 2026 1 commit