# Fable FINAL verification — client mtw39 requests (W3-FIX + the two unreviewed FE commits)

- Reviewer: Fable 5.1 · 2026-10-24 · read-only on product code (`git show` + surrounding code), moonui3 `hazemdev3`
- Scope: BE `1d7dbfb19` (FIN-W1 fix pass + LOY-2 setting) · BE `c648d2588` (LOY-1 / LOY-6) · FE `ae6a7aa3` `ba9ce1d2` `381a8f57` (UI-W1 + LOY fix pass) · FE `079b3bfa` (open in new tab — never reviewed) · FE `fd3d451e` (loyalty redemption UI — never reviewed)
- Inputs: `../LEDGER.md`, `../RESUME.md`, `FABLE-FIN-W1.md`, `FABLE-UI-W1.md`, `FABLE-FIN-W2-LOY.md`
- Evidence gathered in this run (nothing taken on trust from the LEDGER):
  - Pest on the in-memory SQLite test DB (`phpunit.xml:89-90`; dev MySQL never touched): `W3SplitPackageCommissionTest` + `LoyaltyRewardServiceTest` → 18 passed / 180 assertions; `W1PosBlindCloseHandoverTest` + `SupplierAdvancePaymentTest` + `CountAutoApproveAdjustmentTest` → 26 passed / 230; regression net under the rewritten commission service `SplitCommissionTest` + `RevenueSplitTest` + `ClinicSettingsBackboneTest` → 39 passed / 213.
  - `ng build --base-href /app/` of FE HEAD `381a8f57` to a scratch dir → green (only the three pre-existing CommonJS warnings). Live `/app` = `main-X4K7G2YM.js` (15:19), `config.json apiUrl` = moonui3.
  - MySQL dev schema (read-only): `reception_receipt_lines.loyalty_points` / `.deleted_at`, `pos_sessions.float_returned`, `clinic_encounter_service_parties.earned_on` / `.reversal_journal_entry_id` all present; `clinic.split.exclude_loyalty_paid` definition seeded (co 1 has no override row → default **true** applies via `SettingsService::getBool`).
  - i18n scan of every `.ts/.html` in the five FE commits (102 files) against `ar.json` + `en.json`: every key resolves (two «misses» are dynamic prefixes `VISIT_SHELL.TYPE.*` / `INV_INSIGHTS.CONF_*`; `CLINIC_VISIT.ADD_REFERRING` is a pre-existing miss from `2f517c4f`, not this wave — see P2-9).

## Verdict: **APPROVE** — no P0, no new P1. Every earlier P1 is closed with code + a test; the fixes introduce no double posting, missing reversal, wrong sign, key reuse or stale state that moves money. Ten P2s below (robustness / cosmetic / one documented residual); none blocks the merge or the owner's click-through.

Paths: BE relative to `/home/moonui3/moon-erp-be/`, FE relative to `/home/moonui3/public_html/moon-erp/src/app/`. Line numbers are of the files at HEAD.

## 1. Earlier P1s → status

| # | From | Status | Evidence (file:line) | Verified behaviour |
|---|---|---|---|---|
| SPLIT-1 | FIN-W1 | ✅ | `Modules/Clinic/app/Services/SplitCommissionJournal.php:148-152` (earn of a sale line → `onSalePackageSessions()` → recursive `earn()` of the sessions already given, same `$day`); `:274-293` (sessions of the grant this line SOLD; `[]` when the sale is `revenue_deferred` = per_session, `:276`); `:248-266` `earnSessionOfEarnedPackage()` (a session drawn later earns today when the sale `hasEarned()`, `:296-305`); `Modules/Clinic/app/Listeners/SnapshotEncounterParties.php:36-46` (after-commit, never throws); `:182-189` a `receipt_void` of the sale un-earns the sessions; `Modules/Clinic/app/Actions/CancelServiceOrderLine.php:69-74` a cancelled session with a posted share reverses it through `unearn(…,'cancel')`. Tests `Modules/Clinic/tests/Feature/W3SplitPackageCommissionTest.php:138, 179, 213`. | on_sale: sessions 1..N each post one session share; recursion terminates (a consume line is never a `sale_line_id`); re-earn is idempotent (`earned_at` set → no stamp; `isPostable` needs `journal_entry_id = null`, `:387-395`); per_session untouched (own announcement). |
| POS-1 | FIN-W1 | ✅ | `Modules/Accounting/app/Support/JournalBranchResolver.php:123` `'pos_session' => ['table' => 'pos_sessions', 'branch' => 'branch_id']` (and `:121` `clinic_split_party` via its line — SPLIT-3). Asserted: `W1PosBlindCloseHandoverTest` (variance + handover `branch_id`), `W3SplitPackageCommissionTest.php:161`. | both close entries and the commission entries now carry the branch. |
| POS-2 | FIN-W1 | ✅ | `Modules/POS/app/Services/POSShiftClosePostings.php:128-152` `defaultKeep()` = max(keep_float, opening) · `defaultHandover()` · `splitHandover()` posted = min(amount, max(0, counted − opening)); `Modules/POS/app/Http/Controllers/POSSessionController.php:264-267` (default), `:325-336` (posts the posted part only, `float_returned` stored, no entry when 0), `:384-389` (`meta.opening_balance` / `default_keep` for the dialog); migration `Modules/POS/database/migrations/2026_11_25_900001_add_float_returned_to_pos_sessions_table.php` (nullable, idempotent); FE `features/pos/components/session-close-dialog/session-close-dialog.component.ts:171-178` `defaultKeep` + the hint. Tests: drawer account nets to 0.000 after sales − short − handover; default keeps the float; a box drawer may now hand the whole drawer over; float-only handover posts nothing. | `$declared['cash']` at `:328` is safe — `validateHandover` already refused a null count (`POSShiftClosePostings.php:164`); `postHandover`'s no-overdraft check runs on the posted part. Residual (variance not posted) → P2-6. |
| LOY-1 | FIN-W2 | ✅ | `Modules/Clinic/app/Actions/CreateVisit.php:192-196` (`rewardServicesToLines` moved inside the try), `:324-337` (first unclaimed line with `patient_amount > 0` and no `package_grant_id`; `RuntimeException` → soft), `:214-224` + `:299-309` (`isRewardLineRefusal`: the four `loyalty_reward_*` 422s become `collected:false` + reason; config errors still throw), `Modules/Clinic/app/Loyalty/ClinicLoyaltyAdapter.php:222-228` (treatment GL account checked in the pre-write `assertRewardServicesAffordable`). Three new tests in `LoyaltyRewardServiceTest.php` (free visit + same service twice → the paid line is redeemed, one visit; no payable line → soft, no points spent, no receipt; account missing → 422 before any write). FE `features/clinic/booking/create-visit.component.ts:3022-3033` shows the BE reason in the «ادفع عند الكاشير» toast. | no duplicate visit path left: everything after the commit is soft except true config errors (as for cash). |
| UI P1-1 | UI-W1 | ✅ | `features/clinic/services/clinic-print-shell.service.ts` — `openAndPrint(): boolean` (false on a blocked popup), `print(html): Promise<boolean>` (serialised queue) + `printViaIframe()` (hidden `srcdoc` iframe, popup fallback, 3 s guard; the guard cannot fire while `print()` blocks and is a no-op once settled); `clinic-booking-receipt.service.ts:77` and `clinic-receipt-print.service.ts:81` throw when nothing printed; toasts at `create-visit.component.ts:3067-3073` (booking receipt), `:2931-2938` (patient file), `features/clinic/cashier/cashier-collect.component.ts:650, 714` → `receiptPrintFailed()`. | no silent «printed». |
| UI P1-2 | UI-W1 | ✅ | `create-visit.component.ts:2926-2932` — the patient file now goes through the same iframe queue as the booking receipt; two prints from one click come out one after the other, no popup to block. | |
| UI P1-3 | UI-W1 | ✅ | `features/warehouse-transfers/warehouse-transfers.component.html:404` (`[disabled]` removed, `[max]=shipped`, hint row at `:380`); `.ts:527-538` `updateDamagedQuantity()` → received = min(received, shipped − damaged), reason → `damaged`; payload `:559-566` caps damaged to the difference. | the keeper types 2 and the variance is recorded. Edge → P2-3. |
| UI P1-4 | UI-W1 | ✅ | `features/clinic/encounter/consultation.component.ts:488-490`, `features/clinic/visit-shell/visit-shell.component.ts:860-862` — always `router.navigate(['/clinic/queue'])`. | |

Earlier P2s closed in the same pass (checked, correct): SPLIT-3 (resolver row, test-covered), SPLIT-4 (`SplitCommissionJournal.php:446-448` `accrued` only when the entry is posted; `:470-483` a draft is cancelled via `CancelJournalEntry(fromSourceDocument: true)` — it accepts draft/approved and refuses posted/cancelled — and the row re-posts under the fresh key `clinic_split_{id}_r{draft}`; `DoctorDuesReport.php:32, 53-54` `posted` = `je.status = posted`), SPLIT-5 (`DoctorDuesReport.php:40` period on `COALESCE(earned_on, created_at)`), PUR-1 (`JournalEntryType::SupplierAdvanceApplication` + labels, `SupplierAdvanceService.php:64-66`), PUR-2 (`SupplierAdvanceService.php:335, 395-397` date ≥ max(voucher, bill); lexicographic max on `Y-m-d` is correct), TR-6 (`FinalizeCount.php:171-179` 422 naming the switch, rollback kept, test), POS-3 (`CompanyToday` on both entries), LOY-5 (`shared/components/loyalty-available/loyalty-available.component.ts:75-77`), LOY-6 (`ServiceOrderLineResource.php:37-41`), UI P2-0/3/5/6/7/8/10/12(part)/14/16/19/20.

LOY-2 decision as built (`clinic.split.exclude_loyalty_paid`, default ON): seeder `Modules/Clinic/database/seeders/ClinicSettingDefinitionSeeder.php:218-234`; `SplitCommissionJournal.php:125-129` (earn → `exclude()`), `:212-227` `paidByLoyalty()` = a live receipt line stamped `loyalty_points > 0` on a non-voided receipt, `:230-241` `exclude()` reverses a posted share and voids the rows; `:201-209` `syncLoyaltyForCollectedLines()` driven by `Modules/Clinic/app/Listeners/SyncSplitPartiesOnLoyaltyPaid.php` (after commit, never throws; registered `EventServiceProvider.php:122-123`). Scope is exactly the reward: a money-style points tender never stamps lines (`ClinicLoyaltyAdapter::priceTender()` returns no `lines`; `CollectReceptionReceipt.php:416`). Void + cash re-collect earns again under a fresh idempotency key (`reverseRow` nulls `journal_entry_id`, `postRow` keys on `reversal_journal_entry_id`). Tests `W3SplitPackageCommissionTest.php:318, 334, 360`.

## 2. Never-reviewed commits

### FE `079b3bfa` — open in new tab — **APPROVE**
- `shared/directives/doc-link.directive.ts:49-54`: href = `LocationStrategy.prepareExternalUrl(serializeUrl(tree))` → under `<base href="/app/">` a list link renders `/app/sales/invoices?viewId=5` — right-click «open in new tab», Ctrl/Cmd/Shift-click work; middle click is `auxclick` and never reaches the handler.
- Plain click `:56-66`: `preventDefault()` + `stopPropagation()`, then `docOpen` when observed (the screen opens its dialog / closes its popover exactly as before) else `router.navigateByUrl(tree)` (in-app, no reload). Modified click returns early and the browser does its thing (`isModifiedClick`, `:84-86`).
- Targets: every list screen a link points at reads `?viewId=` from `queryParamMap` on load (spot-read: invoices, purchase orders, transfer-requests `:177-178`; the gl-gdni-tab links target `/sales/delivery-notes` and `/core/settings?tab=sales`, not itself; journal-entries reads its own `?view=` at `:282-283`); JE source link href = `getSourceRoute + ?viewId` = what `navigateToSource` does (`features/journal-entries/journal-entries.component.ts:630-636`); `app-je-link` uses `?view=` as before; the purchases trail chips' old `openLinkedDocument()` was `router.navigate([route], {queryParams:{viewId}})` (`features/purchases/orders/orders.component.ts:1515-1518`) = the directive's own navigation, so behaviour is unchanged; partners `'.'` + viewId only for `core.partners.update` holders (the deep link opens the edit dialog).
- Nav surfaces: sidebar search + mini popover, topbar direct modules + mega items + notifications (`n.data?.route` / `route_query`), command bar results (pin button now `preventDefault` + `stopPropagation`, `layout/command-bar/command-bar.component.ts:106-112`), mobile tab bar home + notification rows, dashboards `<a pButton routerLink>` (PrimeNG 21 directive inputs compile). `ng build` green → no broken templates. Styles: `doc-num-link` keeps the cell look and only underlines on hover.
- No row-level `(click)` / `selectionMode` / `onRowSelect` on any of the 22 lists or the shared `data-table`, so the modified-click bubbling (P2-1) cannot double-open a record today.

### FE `fd3d451e` — loyalty redemption at cashier + booking — **APPROVE**
- Booking (`features/clinic/booking/create-visit.component.ts:883-942`): candidates = quoted consultations owing > 0, not a package session, whose sold service (`q.service_id`, the substituted follow-up variant) is an active, non-package reward; hidden while a reception discount is typed; choices reset on patient change and when the chip's balance drops below what is chosen; chip `redeemableChange` only for `loyalty.redeem` (ba9ce1d2). Payload `:2788-2791` `{payment_method:'loyalty_points', redeem_service_ids:[…]}` with no amount — matches BE `CreateVisit::rewardServicesToLines()`. Boxes total = `honestDue − rewardAmount` (`payBoxesTotal`), `collectingHere` counts the reward (`:2732`), and `remaining()` derives from the boxes anchored on that total — no false partial prompt.
- Cashier (`features/clinic/cashier/cashier-collect.component.ts:261-318, 546-553`): `pointsAmount` = money-points + reward amount → money rows reseed to selected − both; payload `{payment_method:'loyalty_points', redeem_line_ids:[…]}`; unticking a line prunes it; a money-style points tender and a reward never mix (`canRedeemLine` + `[disabled]` on the tender); after a collect the choice resets and the chip re-reads. Button only on `ordered|performed` lines whose resource carries `loyalty_redeem_points` (the resource already returns null for package sessions, LOY-6).
- i18n: every `LOYALTY_REWARDS.*` key present in ar + en.

## 3. NEW P0 / P1
None.

## 4. NEW P2 (short)

| # | Where | Issue | Fix |
|---|---|---|---|
| P2-1 | `shared/directives/doc-link.directive.ts:58` | A modified click returns before `stopPropagation()`, so Ctrl/Shift-click opens the tab AND bubbles to any ancestor click handler. No list has one today (checked), so robustness only. | `if (this.href && isModifiedClick(event)) { event.stopPropagation(); return; }` — never cancels the browser navigation. |
| P2-2 | same directive; topbar/mobile notifications and attention items without a route | A `<button>` became an `<a>`; with a null route it has no href → not keyboard-focusable / no Enter. | `@HostBinding('attr.tabindex') get tabindex() { return this.href ? null : 0; }` + `@HostBinding('attr.role')` `'button'` when href is null, and a `keydown.enter` host listener calling `onClick`. |
| P2-3 | `features/warehouse-transfers/warehouse-transfers.component.ts:517-519` | Raising «المستلم سليم» back to the shipped qty after typing «منه تالف» makes the difference 0: the payload drops `damaged_quantity` silently while the box still shows it. | In `updateReceivedQuantity` also clamp `damaged_quantity = min(damaged, shipped − value)`. |
| P2-4 | `SplitCommissionJournal.php:448` | A draft commission entry posted later by hand leaves the row `pending` forever (the dues report is right — it reads `je.status`). Cosmetic. | On `JournalEntryPosted` for `source_type = clinic_split_party` flip the row to `accrued`, or drop the `status` column from the API in favour of the join. |
| P2-5 | `SplitCommissionJournal.php:477-480` | In the draft-cancel path `reversal_journal_entry_id` points at a *cancelled draft*, not a reversal entry — a future consumer reading it as «reversal posted» misreads. | Document the field («the entry that closed the previous posting: a posted mirror or a cancelled draft») or add `reversal_kind`. |
| P2-6 | `POSShiftClosePostings.php:146-152` + `POSSessionController.php:318-323` | `splitHandover` takes «in books = counted − opening», which holds only once the variance entry posted. `postVariance` is fail-safe: with the over/short account unset (plausible on an upgraded install until the accountant wires it — the install checker flags it) an **overage** handover credits the drawer by exactly the unposted variance. Same magnitude as the pre-existing POS-5 fail-safe drift (before this fix the drawer drifted by float + variance), so not a regression. | Pass `$varianceJe !== null` into `splitHandover` and use `($variancePosted ? $counted : $expectedCash) − opening_balance` for the in-books cap. |
| P2-7 | `ClinicSettingValidator` | With the clinic loyalty rule on the `discount` treatment, turning `clinic.split.exclude_loyalty_paid` OFF pays the doctor on a line whose net revenue is 0 (FIN-W2 LOY-2 note). | Refuse `false` while the clinic rule's treatment is `discount` (bilingual message). |
| P2-8 | `clinic-print-shell.service.ts` `printViaIframe` cleanup | The iframe is removed 2 s after `print()` returns; Safari returns immediately and may still be spooling. Desktop Chrome/Firefox (the clinics) block until the dialog closes — unaffected. | Remove on `contentWindow.onafterprint` with a 10 s fallback. |
| P2-9 | `src/assets/i18n/{ar,en}.json` | `CLINIC_VISIT.ADD_REFERRING` (title of the quick-add referring-doctor button, `create-visit.component.html:466`) is missing in both files — pre-existing from `2f517c4f`, not this wave; the raw key shows on hover. | Add the key (ar «إضافة طبيب محوّل»). Bump `I18N_VERSION`. |
| P2-10 | dev co 1 | No override row for `clinic.split.exclude_loyalty_paid` (the definition default `true` applies through `getBool`). Fine, but the owner decision list should say the default is ON for every install, not only dev. | Keep; record in the decisions to confirm. |

Open P2s from the three earlier reviews that were deliberately not taken (unchanged, see those files and `RESUME.md › Leftovers`): SPLIT-2/6/7/8/10, POS-4, PUR-3/4, GLR-1, TR-1..5/7, LOY-3/4/7/8/9, UI P2-1/2/4/9/11/13/15/17/18/21.

## 5. Decisions the owner should confirm (carried)
POS blind close ON by default · collection receipt 80 mm by default · points-paid lines earn no doctor share (`clinic.split.exclude_loyalty_paid` default ON everywhere) · package sessions in on_sale mode now earn commission (visible in «مستحقات الأطباء», and un-earned by a receipt void of the sale, not by a cancel/refund of the sale) · a supplier-advance application is its own entry type (reports keyed on `purchase_payment` no longer count it).