- 02 Sep, 2026 4 commits
-
-
Mahmoud Aglan authored
The previous check asked whether a dashboard query mentions branch_id. That is the weaker half of the question. A widget that hardcoded the main branch, or read a stale id off the URL, or took auth()->user()->branch_id instead of the session, would mention branch_id on every query and still show the wrong branch's numbers — and it would look perfectly correct in testing, because the main branch is the one usually selected. So the assertion is now on the bound value, not the SQL text. Laravel's bindings are positional, so the value belonging to a `branch_id = ?` predicate is found by counting the placeholders before it. Every dashboard is rendered under each branch that carries data, and every branch id bound into a branch_id comparison must equal the branch the session selected. The seven widget components are also mounted directly, rather than only through the pages that embed them. Two of them — EnrollmentTrends and RevenueWidget — are written but on no view today, so page-level coverage alone would have said nothing about either. Measured on the live tenant: with branch 1 selected, 536 branch bindings, all of them 1. With branch 2, 1,182 bindings, all of them 2. No dashboard binds a branch other than the selected one. Co-Authored-By:Claude Opus 5 (1M context) <noreply@anthropic.com>
-
Mahmoud Aglan authored
The two existing suites cannot catch a leaking dashboard. BranchIsolationTest proves the scope narrows a model; BranchScopedScreensTest looks for another branch's records in the rendered HTML. But a dashboard renders totals, not records — a revenue widget quietly summing every branch shows a number that is simply wrong, with no uuid anywhere to give it away, and both suites pass. Dashboards are also where the raw query builder lives, because that is what aggregates are written in, and a raw DB::table() goes straight past every global scope. So this asserts at the only layer that sees both: the wire. Two checks: - Every SQL statement each of sixteen dashboards runs is captured with DB::listen, and any query reading a branch-owned table without mentioning `branch_id` anywhere — its own WHERE, a join, a subquery the scope added — is a failure. Deliberately crude, because a strict SQL parse would be worse than useless here: a query that never says the word never asked. - The per-branch figures for participants, enrolments, invoices, payment totals, attendance and groups must add up to the academy-wide figure. A widget ignoring the branch returns the whole academy for every branch, so the sum comes out a multiple of the truth. Current state: all sixteen render, 1,354 queries captured, 902 of them touching branch-owned tables, and every one names a branch. Co-Authored-By: -
Mahmoud Aglan authored
The first pass put programmes, products, prices, promotions, warehouses and receipt templates in the *shared* bucket, where `branch_id IS NULL` means "every branch uses this row". The reasoning was that a missing base price is a hard failure that stops a sale, so hard-filtering the catalogue risked leaving a branch unable to sell anything. That bought safety with the wrong currency. A branch is meant to read as its own installation — its own programmes at its own prices, its own products, its own stores — and a programme offered at one branch turning up in another branch's dropdown is the same bug as a group doing it. The shared bucket just hid it behind a plausible-sounding rule. The data said the caution was unnecessary. Across the live tenant no group points at a programme in another branch, no base price prices a programme in another branch, and exactly three catalogue rows had no branch at all. The catalogue was already per-branch in practice and merely unlabelled. - Programmes, base prices, pricing rules, promotions, products, product categories, kits, warehouses, receipt templates and wallets move to the strict bucket. `kits` and `product_categories` gain the column; the rest only needed their nulls resolved, from actual usage where a link existed and from the main branch otherwise. - Events go the other way and lose the trait entirely, with their registrations. An event is an academy-wide occasion and is genuinely not per branch. The column stays — dropping it would be destructive — but nothing reads it. - Only people and the academy calendar stay shared: employees, trainers, guardians, holidays. A coach who works two pitches needs one record visible from both, not two that drift. Even there the nulls are narrowed — anyone who demonstrably belongs to one branch is pinned to it, which in this tenant leaves none shared at all. - Pricing rules are the one model whose branch is genuinely many-valued: the wizard targets a list through `pricing_rule_branches` and deliberately leaves the legacy column null. A column scope would have hidden every rule it has ever created, so PricingRule supplies its own scope reading the pivot — and BelongsToBranch now lets a model do that. - Forms that let a user save a catalogue row with no branch now require one, and the "كل الفروع" option is gone from those pickers: on a strictly scoped table that choice does not mean every branch, it means none. The setup wizard's seeded prices are filed against their programme's branch rather than null, so a new academy does not finish setup unable to sell. - Comments throughout said "SHARED — branch_id NULL means every branch uses this row". They now say what is true. tests/Feature/PricingSurvivesBranchScopeTest.php is the guard on the original worry: it prices every live enrolment inside its own branch on each run. It passes, and reports the one programme that has no active base price at all — a pre-existing gap, unrelated to scoping. Branch suites: 14 tests / 23,060 assertions against a restored tenant. Standard suite: 224 tests, no failures. Co-Authored-By: -
Mahmoud Aglan authored
Branch was built as a reporting lens. Nine tables carried `branch_id`, the trait that stamped it deliberately added no scope, and each of 212 Livewire components was individually responsible for remembering to filter. Most remembered for their main list query. Almost none remembered for the <select> rendered beside it — so picking a branch narrowed the table and left every dropdown, typeahead, count badge and print template still offering the other branches' groups, players, facilities and trainers. On OC-Sport, where only ZSC is live and six other branches sit half-configured, that surfaced as stray rows turning up in pickers all over the app. An audit of the whole surface found 434 such leaks across 165 files. Per-screen patching would not have held: the next component added would forget again. So branch now works the way academy already does — a global scope on the model, which no screen can route around. - BranchScopeState owns the on/off switch, BranchScope the filter itself. ResolveBranchContext activates enforcement; it stays off for console and queues, guests, member-portal accounts and the routes in config/branch_scope.php. A cron that silently billed one branch, or a parent hidden from their own child, would each be worse than the leak being fixed. - Models fall into four buckets. STRICT (operational records — players, groups, facilities, invoices, payments, attendance) filter `branch_id = :active`. SHARED (catalogue and configuration — products, base prices, pricing rules, programmes, trainers) filter `branch_id = :active OR branch_id IS NULL`, because null there means "every branch uses this row" and hard-filtering it would leave other branches with no active base price — a hard failure that stops a sale, not a tightening. CHILD line items scope through their parent relation rather than growing a denormalised branch_id that drifts. The rest are academy-level and get no trait. - The migration adds branch_id to 22 more tables and backfills every existing row parent-first (a session from its group, an attendance record from the session), falling back to the academy's main branch. Every step is guarded and additive, and it is safe to re-run: verified by rollback and re-migrate against a populated tenant copy. - The residue a scope cannot catch is fixed by hand: `exists:` validation rules run raw SQL and accepted any id in the academy, browser-settable `#[Url] public $branch_id` properties were an authorisation bypass rather than a filter, raw DB::table aggregates bypassed Eloquent entirely, and several figures attributed a row through the wrong table (an invoice's branch read from its payments'). Screens that are cross-branch by design — transfers, executive roll-ups, branch administration — now opt out explicitly and narrowly. Verified against a restored OC-Sport copy: BranchIsolationTest and BranchScopedScreensTest, 13 tests / 22,897 assertions, comparing what Eloquent returns under each branch against what raw SQL says is in it, rendering every staff screen, and proving a branch id in the URL cannot override the selected one and a detail route cannot open another branch's record. Standard suite: 223 tests, no failures. docs/agent-rules/19-branch-isolation.md records the rule and how to add a new branch-owned table. Co-Authored-By:
-
- 01 Sep, 2026 21 commits
-
-
Mahmoud Aglan authored
On 1 September OC-Sport had 230 active players due for renewal and the nightly command raised zero invoices. It resolved the invoice actor with User::find($enrollment->created_by); `enrollments` has no created_by column — it is enrolled_by — and Eloquent answers null for a missing attribute, so User::find(null) returned null and every enrolment took the "no valid user" branch. Introduced by da344cda on 9 August, which is why August still billed (2 and 5 August) and September did not. The command printed a summary and exited SUCCESS the whole time, so nothing in the system disagreed for four weeks. The column name was the trigger; the silence was the bug. Four things change so this class of failure cannot repeat: * The actor comes from enrolled_by, and falls back (programme creator, then an academy admin) rather than skipping. A receptionist leaving the academy must never be the reason a paying member goes unbilled. * A run that finds players and bills none of them exits FAILURE. The scheduler now reports a broken run instead of a tidy one. * Cycles are caught up. The old code advanced next_billing_date by addMonth() and raised one invoice, so a run lost to a container restart at 07:00 skipped that month permanently. It now bills every cycle between next_billing_date and today. * Invoices are dated the cycle they buy, never the day the job ran, and carry metadata.month — the signal SubscriptionLine trusts above Arabic month names and above issue_date. A September renewal raised on the 20th is still September money. The "renew on the 1st" rule was hand-written in five places and three disagreed: the command drifted the anchor a day every time a run was late, ReconciliationWizard advanced from now() instead of from the cycle it was closing (skipping one), and EnrollmentService ignored billing cycles longer than a month. They now share App\Domain\Training\Support\ BillingCycle, which is the only definition of the rule. Also fixed along the way: * Discounts were put on the invoice header AND the line, and recalculateTotals() computes total = sum(line totals) - header discount, so every discounted renewal charged the discount twice. OC-Sport has an active sibling-discount rule, so this was live money. Lines now carry the undiscounted price, which is the convention ParticipantBillingService already documents. * CollectPaymentWizard deduplicated renewals with notes LIKE %{programme name}% over unpaid statuses only. OC-Sport has three programmes called "فريق 2018", so one player suppressed another's; and once a renewal was paid the guard stopped seeing it and the next visit billed the month again. It now matches on the cycle and the enrolment, and invoice creation shares a transaction with the next_billing_date advance. * An active paying enrolment with a null next_billing_date was invisible to every renewal query in the system, permanently. Such rows are now adopted onto the current cycle — never retroactively. Verified against a copy of the OC-Sport tenant: 230 invoices dated 2026-09-01, 65 members at 650 EGP and 162 non-members at 900 EGP, the sibling discount applied once, re-running adds nothing. Six players on عبدالعال 2012 fail loudly because that programme has no base price — a hard fail by design, and now visible instead of silent. Co-Authored-By:
-
Mahmoud Aglan authored
Every branded page on the live tenant answered 500 with "BrandingService::for(): Return value must be of type BrandProfile, __PHP_Incomplete_Class returned". config/cache.php shipped Laravel's default serializable_classes => false, which unserializes cache values with allowed_classes: false. That is safe for an app that caches only scalars and arrays, and fatal for one that does not. We cache whole value objects deliberately: BrandProfile is the entire tenant brand, resolved once and held until branding changes, and read by every admin, portal and print layout. With classes refused it came back as __PHP_Incomplete_Class, the return type threw, and the admin went dark. The setting exists to stop a gadget chain in a cache an attacker can already write to. Ours is the tenant's own Postgres, reachable only by the app; anyone who can write there can do worse directly. CACHE_SERIALIZABLE_CLASSES lets a deployment pass its own allowlist without a code change. BrandingService now also checks what the cache handed back before trusting it, and rebuilds when it is not a profile. A cache that cannot return this class should cost a rebuild per request, never a 500 — the failure has to degrade, not detonate. Verified: every admin screen and the group roster render against a restored copy of the live tenant with CACHE_STORE=database. Co-Authored-By: -
Mahmoud Aglan authored
The terminal read products.selling_price and never called the pricing engine, so a product priced per membership type — a member rate and a walk-in rate, which is how base_prices has always modelled it — sold at whichever single number the catalogue carried. Members were charged the non-member price and nothing on the receipt showed it had happened. Every product line now comes from PricingService, which resolves the base price for THIS buyer and then applies the academy's rules to it. selling_price stays the fallback for a product nobody has priced through the engine: it is the price the catalogue advertises, and refusing the sale outright would close the shop over a configuration gap. Cashiers scan first and identify the customer afterwards at least as often as the other way round, so selecting or clearing a participant re-prices what is already in the cart, and the product grid shows that buyer's price with the list price struck through beside it — a rate the cashier cannot quote out loud is a rate that gets argued about at the counter. Checkout prices everything again before it believes any of it. The cart is a public Livewire property, so the unit prices arriving at checkout are whatever the browser last sent; if the engine's answer differs from what the cashier is looking at, the sale stops rather than charging a total nobody saw. Verified against a restored copy of the live tenant: with member/non-member base prices on a real product, member 900, non-member 1,200, walk-in 1,200, and a product with no base price falls back to its catalogue 8,000. Co-Authored-By: -
Mahmoud Aglan authored
The roster column called itself الدفع and answered two questions wrongly. WHAT COUNTS. "Subscription" was every invoice line with no itemable — which is every line a receptionist typed by hand. On the live tenant that swept in 42,800 EGP of federation registration fees ("قسط القيد", "قيد اشتراك", "أقساط متبقية من قيد اتحاد الكرة") and kit ("الزي", "شنطة لبس") and reported it as training money players had paid. SubscriptionLine now reads the text: the academy's own product names first, then its programme names, then the words for registration and kit — matched on whole normalised WORDS, never substrings, because "تجهيزي" contains the letters of "زي" and a substring match turns a subscription into merchandise. Anything still unrecognised keeps counting as subscription, so no line disappears unannounced. WHICH MONTH. The month was the invoice's issue_date, which is only the covered month when the invoice was raised inside it. August's subscription typed up in September belonged to no cycle at all, and one invoice covering July and August counted twice over. Months now come from the invoice's own metadata, then any month named in the line or the notes ("اشتراك يوليو 2026", "اشتراك شهر 8", ranges), then the issue date — and a line covering several months is split evenly across them, remainder on the last, so the parts can never exceed what was billed. A line that names no month is still judged by its date, which is the only evidence there is. A year is only read as a date when it sits near the invoice, so the age group in "فريق 2011/2012" cannot date a 2026 subscription to 2011. Verified against a restored copy of the live tenant: subscription billed for July 123,173 -> 101,073 EGP and August 200,538 -> 187,038 EGP, and every excluded line was checked one by one — all 14 are registration or kit, none is a programme. No line currently moves month; that half is future-proofing plus the combined-invoice case the old data is full of. Column renamed الدفع -> الاشتراك and the header now carries the year as well as the month, because "سبتمبر" alone does not say which September. Co-Authored-By: -
Mahmoud Aglan authored
A pointer-level test and a page-by-page diff against the reference site, rather than reading the markup. Dropdowns could be opened but not used. The panel sat 4px below its trigger with nothing in the gap, and the menu closed the instant the pointer left the trigger's box — so travelling to an item crossed dead space and the menu shut before the click landed. The panel now starts flush against the trigger (`top-full`, padding inside the panel rather than a margin outside it), and the close is delayed and cancellable. Same contract applied to the language switcher, which had the identical gap. Verified by dispatching real mouse moves along the path a hand takes: the menu survives the trip and the click lands on /en/football. `columns` silently dropped its children. It reads them from slots col1..colN, so a child in the generic `default` slot — what a blueprint import or a hand-built tree naturally produces — matched no column and vanished with no error and no empty box. That is how the about-us video disappeared. Loose children are now dealt out across the columns. Nine of eleven pages had no h1 at all. Blocks hardcoded their heading level, so whichever section carried a page's title rendered as h2 and the document had no top-level heading. `heading_level` is now part of the section-header contract (and of text_image, video, profile_card, contact_form, app_download, cta, rich_text), so the section carrying the page title is an authoring decision rather than an accident of which block was used. Every page now has exactly one. Also from the sweep: - El3ab's entire privacy policy — nine sections, both languages — was absent from our page. Restored from the reference render. - The line the reference shows above its contact form was missing; contact_form gains `form_intro`. - The chairman's name was a <p> on top of the portrait; it is a heading. - Footer column titles were <h4> with no <h3> above them, a skipped level in the outline screen readers navigate by. Promoted to <h3>. - `heading_rule` had been added to app_download *inside* its features repeater, so every feature row offered a meaningless "underline" control and the section heading offered none. Moved to where the heading actually is. Verified: 22 page/locale combinations answer 200 with no logged block failures, 137 block/variant combinations render, every page has exactly one h1, the carousel still does not scroll the page, and the suite passes. Co-Authored-By: -
Mahmoud Aglan authored
fix(website): stop the partner carousel scrolling the page, and turn the hero showcase into a real flipping carousel Three faults, all found by watching the built site rather than reading it. The partner carousel dragged the whole page down to itself. Its timer called `el.scrollIntoView()` every few seconds, and scrollIntoView walks up and scrolls *every* scrollable ancestor including the document — so a reader anywhere on the page was hauled to the partners section on a loop. It now sets `scrollLeft` on its own track, which cannot move anything but itself. Verified: twelve seconds on the homepage, `window.scrollY` never leaves 0, while the track's own scrollLeft still advances. The hero showcase was manual when it should play itself. The reference plays each design for a beat, turns it on its Y axis to show the back, holds again, then hands over to the next — an Embla autoplay at `delay: 2500` with a `duration-700` `rotateY(180deg)` flip and `backfaceVisibility`, read out of their own bundle rather than guessed. Ours sat still until someone clicked a swatch. It now runs that cycle: a genuine 3D flip of one object, not a crossfade between two pictures, with both faces backface-hidden so they never show through one another mid-turn. Hovering pauses it; reduced-motion skips it entirely. The colour swatches were large filled circles under the garment, which read as loud page furniture rather than a control. They are gone by default. A site that wants them gets `showcase_controls`, and they render as the same discreet progress dashes the other carousels use. New fields: `showcase_autoplay` (default on), `showcase_interval` (default 2500ms, matching the reference), `showcase_controls` (default off). Verified on a local replica carrying the full 11-page site: sampling the hero across a cycle shows active=1 front → active=1 flipped → active=2 front, so the flip and the hand-over both really happen; 22 page/locale combinations answer 200 with no logged block failures; 137 block/variant combinations render; suite passes. Co-Authored-By: -
Mahmoud Aglan authored
Checking the rendered result against the client's own branch data showed the block was reproducing the wrong rule. Their site lists fifteen branches and dims nine of them, each captioned with the dates it opens — the dimming tracks whether the branch is switched on, and the season window is the explanation shown to the reader, not the test. Ours filtered `is_active = false` out of the query entirely, so those nine simply did not exist on the page. A branch under construction is exactly the thing a marketing site wants to show. - `getBranches()` takes `$includeInactive`, cached under its own key so the two result sets cannot overwrite each other. - `data_branches` offers `show_inactive`, and treats a branch as dormant when it is switched off OR outside its declared season — a branch with no window stays open all year, as before. - Open branches sort first, so an announced-but-closed location never pushes a working one below the fold. Verified on the local replica with two dormant branches alongside seven live ones: the dormant pair render dimmed with their location and opening dates while the rest keep the accent border. 22 page/locale combinations answer 200 with no logged block failures, 137 block/variant combinations render, suite passes. Co-Authored-By: -
Mahmoud Aglan authored
Building an existing client site inside the builder surfaced the gaps between "the blocks exist" and "the blocks can express a finished design". Each of these was found by rendering the target and comparing it, not by reading the code. Blueprint export/import (v2) now carries the whole design, not a third of it. It exported pages only — so importing a design gave you the content with a default theme and no navigation, which looks nothing like its source. It now carries theme settings and menus too. Menu items record their page by SLUG, because a page id means nothing in another tenant's database and would import a navigation pointing at whatever happened to hold that id. Tracking identifiers are deliberately excluded from the whitelist: importing a design must never start reporting one client's traffic into another's account. v1 files still import. Isolation that did not isolate. BlockRenderer catches a failing block so the rest of the site survives, but Laravel's View::render() calls flushState() when any view throws, which clears the section stack of the page *around* it. Rendered inside the layout's @section, one bad block therefore killed the whole page at @endsection with an unrelated "Cannot end a section" error — the exact opposite of the intent. Blocks are now rendered before the layout runs, where there is no open section to corrupt. Bilingual content reached templates raw. Translatable repeater sub-fields (a button label, a card title, a partner name) are stored as ['ar'=>…,'en'=>…] and read straight out of the data array, so they arrived at {{ }} as arrays and took the block down with "htmlspecialchars(): array given". website_text() resolves them, and 49 such reads across 15 block views now use it. The English site rendered right-to-left. website.css hardcoded `direction: rtl` on .website-body, silently overriding the dir attribute the layout computes from the locale. Direction now follows the document. There was no English at all. No lang/ directory existed, so every __() returned its Arabic key and English visitors read Arabic form labels, buttons and helper text. lang/en.json covers all 125 public-site strings. Smaller gaps, each of which made a real design impossible to express: - 'glass' was a valid navbar template and a forbidden column value; the CHECK constraint predated it, so choosing it failed at write time. - navbar_cta_text had no English twin, so a bilingual site showed one language's button to both audiences. - An empty navbar CTA fell back to the default label, so the button could not be turned off. - An anchor menu item resolved to a bare "#id", which points at nothing from a sub-page; it now addresses the homepage in the reader's language. - A footer column title was a plain string, so it could not be bilingual; the 'about' column dropped the social row whenever columns were configured. - product_showcase stacked its showcase under a centred headline instead of laying out as the split it is. - A map field stores coords as ['lat'=>…,'lng'=>…] and the view passed the array to urlencode(), killing the block. - New: 'stacked' info cards, a footer spacer, heading rules and eyebrows on the contact block, and a nowrap on highlighted heading fragments so "Welcome to {OC-Sport}" cannot break mid-phrase. Verified against a local Postgres replica carrying a real 11-page bilingual site: 22 page/locale combinations answer 200 with zero logged block failures, all 137 block/variant combinations render, reserved and unknown paths still 404, the four migrations apply and roll back on both an existing tenant and a from-scratch install + seed, and the suite passes (140 tests, 483 assertions). Co-Authored-By: -
Mahmoud Aglan authored
The page + block builder shipped in 39f468a9 could not render a single page. Two independent faults, both fatal: 1. `website/page.blade.php` passes no `$sections`, but `layout.blade.php` includes `navbar.blade.php` and `footer.blade.php`, which both dereference it. Every builder page died with "Undefined variable $sections" before the first block was rendered. `$sections` belongs to the legacy section site; both dispatchers now default it and prefer the authored menu tree, so one navbar and one footer serve both worlds and existing tenants keep the navigation they have. 2. The `ec-*` class layer every block partial styles itself through (ec-heading, ec-muted, ec-surface, ec-btn, ec-eyebrow, ec-prose, ec-marquee, ec-block) was used in 25+ views and defined in no stylesheet. Even past the crash, a rendered page had no colours, no cards, no buttons. The layer is now written against the --site-* variables the layout already emits, so a theme change repaints the whole site. Alongside the fix, the capabilities a data-driven marketing site needs: - Per-language URLs. `/en/...` and `/ar/...` address the same page, SetLocale reads the prefix ahead of the session, and the layout emits lang, dir, canonical and hreflang alternates from the active locale instead of a hardcoded rtl. A shared link now opens in the language it names. `en` and `ar` are reserved slugs so a page cannot hide behind a locale prefix. - One section-header contract: eyebrow, heading, accent rule, subtitle, shared by 21 block types through `_header`, with `{braced}` fragments of a heading rendered in the accent colour. - New variants: glass navbar, `overlay_split` profile card, `focus_carousel` logo strip, `season_cards` branches. Ambient motion (float/glow/pulse/ shimmer) exposed in the motion panel; showcase motion on the hero image. - A footer "powered by" accent band, and footer columns driven by a menu so they cannot drift from the navbar. - Branches gain photo_path and a season window; the block dims a branch that is out of season. A branch with no window is open all year, so nothing changes for existing data. Also fixed while in here: `getBranches()` bypasses Eloquent and never checked `deleted_at`, so a branch deleted in the ERP kept appearing on the public site. Verified on a local Postgres replica: all 136 block/variant combinations render with no logged failures, the locale routes answer 200 and reserved paths still 404, migrations apply and roll back cleanly on both an existing tenant and a from-scratch install + seed, and the suite passes (140 tests, 483 assertions). Co-Authored-By:
-
Mahmoud Aglan authored
The last of the programme, plus the exit gate CLAUDE.md requires before any UI work counts as finished. Self-registration (W10) — shipped, and closed --------------------------------------------- This is the only path in the product that writes into `people` and `participants` with no member of staff in the loop, so it ships **off**: `portal.self_registration_enabled` defaults to false on every tenant including new ones, and the middleware answers 404 — never 403, because a 403 advertises that there is a signup form here and invites someone to look for the setting. A public form that starts accepting strangers because a deploy happened is not a decision anybody made. Phone verification is the precondition, not a feature. The flow it replaces was a study in how not to do this: `verify()` accepted the constant '0000' whenever a seeded setting said 'demo', then resolved *any* active user by phone — academy owners included — and minted a token with `mobile:*`; and in the other mode it generated a code, cached it, and never sent it anywhere, so turning the bypass off locked everyone out rather than securing anything. So: no bypass exists, in any mode, behind any flag. Only the SHA-256 is stored, with a bounded attempt count, in a table rather than the cache — a code you cannot audit is a code you cannot investigate. A send that fails deletes the record, because a stored code nobody received is precisely the old failure. There is a test asserting '0000' and '1234' are refused. Registration goes **through** ParticipantService rather than around it. Writing the row directly skipped the participant number, the already-a-member check, the audit columns and ParticipantRegistered — a second creation path that looked identical and was not. It does not enrol and it does not take money: EnrollmentService::enroll() needs an actor authorised to enrol and a self-registering guardian is not one. The member asks; staff enrol. `people.created_by` is NOT NULL and there is no staff member here, so the account is created first with no person attached and becomes the author of its own records — which is also the truth about who typed them. Loosening the column would have weakened it for every other path. DuplicateDetectionService runs on every signup and its findings are stored on the row and shown in the approval queue. It has existed for a long time with nothing surfacing what it found, so a second Person for an existing member appeared silently and the two drifted apart forever. The accessibility gate ---------------------- Eleven checks against the HTML the portal actually renders for a real member, not against the templates: language matching direction, image alternatives, accessible names on every icon-only control, a label for every form control, named landmarks, focus never globally removed, reduced motion honoured, a stated focus ring, announced errors, and dir=ltr on numeric inputs. Two things it found. There was no explicit focus-visible style, so the ring was the browser default — a thin blue line that disappears against a tenant whose brand is blue; it is now `currentColor`, which inherits an already contrast-checked colour and is legible on every surface in both themes. And validation messages were rendered as plain text: a screen-reader user submitted a form and heard nothing. Every one is a live region now, asserted at the source, because an error block only renders when there is an error and a clean page proves nothing either way. Also: `:focus` gets scroll-margin so a focused control is never left under the sticky header or the bottom tab bar (2.4.11). Verification ------------ - 145 migrations from zero on an empty database, seeded, booted a second time: every portal grant intact, self-registration absent and therefore closed (SettingsService returns the default, which is false — it fails closed). - The restored oc_sport tenant: nothing to migrate, seeders clean, health 200, 713 invoices / 356 participants / 649 payments untouched. - Suite: 76 pass on SQLite; on the tenant PortalSmoke 3/3, AdminScreens 2/2, PaymentProof 11/11, CheckInScan 10/10, ServiceRequestEffect 14/14, SelfRegistration 13/13, PortalAccessibility 11/11. Co-Authored-By: -
Mahmoud Aglan authored
Three P1 items, and a latent bug each of them depended on. SetLocale was registered nowhere. The middleware has existed since early on and no middleware group ever included it, so `app()->getLocale()` returned the config default on every request and the bilingual half of an Arabic-first product was dead code. Worse, that default was 'en' — so every page announced `lang="en"` while being marked `dir="rtl"`, telling a screen reader two contradictory things about the same text. The default is now 'ar', the middleware runs, and the portal's direction follows the locale instead of being hardcoded. notification_preferences had per-event, per-channel columns and no interface anywhere: it was written to by the deleted API and by nothing else, so every member received everything on every channel with no way to say otherwise. The preferences screen defaults an unset choice to ON — the member has not asked for less, and silently defaulting to off means a missed instalment nobody was told about. The device list is on the same screen, because push is the one channel whose recipients a member cannot otherwise see: an old phone, a browser at work, a device someone else now owns, all receiving silently until the token rotates. Transfer reconciliation is the control that makes proof approval honest. A screenshot is not evidence; matching the day's total against the academy's own statement is. The report is per branch per day with deliberately blank statement and signature columns, because a report that cannot be signed is not a control. It also surfaces ageing proofs — a member told "we will check" who heard nothing — and transfers recorded with no proof behind them, which are legitimate but which a reconciler needs to expect. One column asserts a database constraint rather than a number: an approved proof with no payment is made unrepresentable by payment_proofs_approved_payment_check, so a non-zero count there means the constraint is gone, and the screen says exactly that. A constraint nobody ever looks at is one you find out about the hard way. notification_preferences.academy_id added to the model's fillable — S1 added the column and the model was still writing rows that belonged to no tenant. Verified on the restored tenant: 12 portal screens and 6 staff screens render 200; suite 76 pass on SQLite. Co-Authored-By: -
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: -
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: -
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: -
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: -
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: -
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: -
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: -
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:
-
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: -
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:
-
- 31 Aug, 2026 14 commits
-
-
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: -
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: -
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: -
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: -
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: -
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:
-
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: -
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: -
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: -
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: -
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: -
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: -
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: -
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:
-
- 30 Aug, 2026 1 commit
-
-
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:
-
