# [FIN] Review — W2-LOY: redeem loyalty points for a clinic service

- Reviewer: Fable 5.1 ([FIN]), 2026-10-24
- Scope: BE `c988b878d` (`/home/moonui3/moon-erp-be`), FE `fd3d451e` (`/home/moonui3/public_html/moon-erp`), dev DB `moonui3_dev_be` co 1 (read-only checks)
- Brief: `../tasks/W2-LOY-loyalty.md`, `_COMMON.md`, LEDGER Decision 8

## Verdict: APPROVE WITH FIXES — no P0, one P1 (booking-form failure after the visit is committed), rest P2

The posting is right for the engine as it exists. `Modules/Loyalty` has only two treatments (`Enums/AccountingTreatment.php`: `expense`, `discount`) and books nothing at earn — expense-at-redemption, no points liability account (`plans/lis-loyalty-program/RESUME.md:9`). So the brief's "clear the liability at carrying value, difference to expense" collapses to one line: the whole line price P goes to the treatment account. That is the correct treatment under this engine, and the implementer documented it as a deviation. Verified end to end:

| Event | JE (collection mode, dev co 1) | Points | Where |
|---|---|---|---|
| Collect (reward line P, cash C) | Dr redemption expense P · Dr cash C / Cr clinic AR P+C ; Dr AR P+C / Cr revenue P+C | −reward pts; `money_value` = −pts × point value; `meta.redeem_price` = P | `CollectReceptionReceipt.php:270-274, 416, 429-432`; `PostReceiptJournalEntry.php:70-92, 124`; `LoyaltyLedger.php:268-305` |
| Collect (accrual mode) | recognizer books Dr AR / Cr revenue first (`:424`); receipt JE only Dr expense P · Dr cash C / Cr AR | same | `CollectReceptionReceipt.php:424`, `PostReceiptJournalEntry.php:109-111` |
| Refund of a redeemed line | Dr revenue P / Cr AR P ; Dr AR P / Cr expense P (no box line) | + exactly the stamped points, capped at what is still unreturned; reversal `meta.redeem_price` = P | `ClinicLoyaltyAdapter.php:432-435, 496-515`; `RefundServiceOrderLine.php:230-265` |
| Refund of a money line on the same receipt | unchanged (all cash) | none; earn clawed back pro-rata to cash | `ClinicLoyaltyAdapter.php:500-503` (null) |
| Void | full flip of the receipt JE (Cr expense P) | + every unreturned point; `redeem_price` = P − Σ already returned | `VoidReceptionReceipt.php:58-65`; `LoyaltyLedger.php:1086-1105` |
| Earn | — | on `Σ tenders − points tender` only (kind `loyalty` excluded) | `ClinicLoyaltyAdapter.php:348-359` |
| Reports | redemption totals + GL check read `meta.redeem_price` (redeem and reversal) | | `LoyaltyReportService.php:118-126, 207-226, 272-280` |

Reward redemption skips the money caps / minimum points / minimum invoice but keeps balance, account status, program and rule gates (`LoyaltyLedger.php:1055-1077`). Concurrency: the preview is unlocked, but `redeem()` re-checks under the account + lot locks (`:255-266`); the loser gets `insufficient_balance` and the whole collect transaction (receipt, payments, stamps, recognizer) rolls back. One points tender per receipt and "reward XOR points amount" are enforced server-side (`ClinicLoyaltyAdapter.php:126-141`). Tenant scoping holds: reward lines come from the company-scoped locked `$selectedLines`, booking services from a company-scoped query, refund stamps from a company-scoped query. Pay-before-consult gate: the points tender is in `paid_total` → `billingTotals()['remaining']` = 0 (test proves it). Per-session packages: sale lines (`is_package`) and session lines (`package_grant_id`) can never be rewards (`ClinicService::rewardPoints()`, `rewardPointsForLine():191`). Migration columns exist on MySQL; co 1 is `unified`, program on, point value 0.1, clinic rule earn 0.1/EGP + redeem on `expense` → account 121 (5304); rewards set on services 14 and 42 (500 pts each); no redemption rows yet on dev.

## Findings

| # | Sev | Where | Problem | Failure scenario | Fix |
|---|---|---|---|---|---|
| LOY-1 | **P1** | `Modules/Clinic/app/Actions/CreateVisit.php:75` (visit txn) vs `:189-216` (collect after commit), `:292-313` (`rewardServicesToLines`), `ClinicLoyaltyAdapter.php:154-182` | The booking-form redemption can fail AFTER the appointment + order are committed, and three of those failures surface as HTTP 422 (not the soft `collected:false` the money path uses): (a) `rewardServicesToLines` throws `loyalty_reward_service_missing` post-commit (its comment "refuses before writing anything" is wrong — the visit is written); (b) `priceRewardTender` throws `loyalty_reward_line_invalid` / `not_collected` (HttpException) inside the collect → rethrown by the `catch (HttpException)`; (c) the pre-write check `assertRewardServicesAffordable():209-232` never checks the treatment GL account (`$settings->accountFor`), so `account_missing` is only raised by `priceTender():245-248` after the visit exists (the implementer listed this). | Free visit + a second consultation of the same service: FE offers the paid one, BE maps the service id to the lowest-id line = the free one (`patient_amount` 0) → 422 after the visit exists. Same for a fully covered line, or a race where the balance drops. The operator sees a red toast on the booking form, re-saves → duplicate visit in the queue, the first one unpaid. | (1) In `rewardServicesToLines` pick the first unclaimed line with `patient_amount > 0` and `package_grant_id null`, and throw a `\RuntimeException` (soft failure, visit stands with `payment.reason`) instead of `HttpException`. (2) In the collect `try`, treat the three `clinic::errors.loyalty_reward_*` HttpExceptions as soft failures too (`payment = ['collected'=>false,'reason'=>…]`). (3) Add the GL-account check to `assertRewardServicesAffordable` (two lines copied from `priceTender:243-248`) so a config gap is a 422 before any write. (4) Add a test: free visit + same reward service twice → one visit, `collected:false`. |
| LOY-2 | P2 (decision) | `PostReceiptJournalEntry.php:124, 263-269` → `EarnSplitPartiesOnRevenueRecognized` → `SplitCommissionJournal.php:84-116` | A points-paid line recognises revenue at its full `patient_amount`, so the doctor's split share is EARNED on P although the clinic collected no cash and booked P as loyalty expense. Consistent with the recognition contract and with the brief ("revenue recognised as usual"); refund/void un-earn it correctly. But the dues report (payout basis; `clinic.split.post_to_gl` is off on dev) will show commission on a service the clinic gave away — a real cash cost. Under the `discount` treatment the base would be plainly wrong (net revenue 0). | Doctor 20 % on a 3,000 تبييض redeemed for 500 pts: dues report shows 600 owed; the clinic paid 3,000 expense + 600 commission for 50 EGP of points. | Owner decision. Cheapest implementation: a clinic setting `clinic.split.exclude_loyalty_paid` (default per owner) — in `SplitCommissionJournal::earn` skip (or base on the cash share of) lines whose receipt line is stamped `loyalty_points`. If the rule's treatment is ever `discount`, the exclusion must be on. |
| LOY-3 | P2 (FIN visibility) | `LoyaltyReportService::liability()` (Σ open lots × point value) vs `rewardPoints()` | A reward decouples points from money: 500 pts carry 50 EGP in the liability report but buy a 600 / 3,000 EGP service. The liability report no longer approximates the program's real exposure. (`redemptionTotals.amount` is uniformly the posted figure — points × point value for plain rows, `redeem_price` for reward rows — but for reward rows `points × point_value ≠ amount`, which the UI does not explain.) | Finance reads "liability 50" while 3,000 can walk out tomorrow. | Add a "reward exposure" line to the liability report: Σ over accounts of the cheapest active reward each balance can buy (or simply list active rewards with price ÷ points so finance sees the effective point value per reward). Flag the dev config (1 pt / 10 EGP, 500 pts = 3,000 EGP service → 60 % give-back) as test-only in the KB. |
| LOY-4 | P2 | `ClinicLoyaltyAdapter::rewardPointsForLine():189-201`; cashier `cashier-collect.component.ts:273-276` vs booking `create-visit.component.ts:906` | Points are fixed per service while the money posted is the line's remaining patient share: an insured line (copay 60 of 300) or a cashier-discounted line costs the full reward points for a fraction of the price. The booking form refuses a reward when a reception discount is typed; the cashier does not — inconsistent policy. | Patient spends 500 pts to cover a 60 EGP copay. | Decide one rule in `rewardPointsForLine` (server) and mirror it in both screens: either refuse a reward on a line with `discount_amount > 0` or `coverage_amount > 0`, or allow it everywhere and show the effective value. |
| LOY-5 | P2 | `CreateVisit.php:55-62` (pre-check) + FE `loyalty-available.component.ts:57` (`canSee = loyalty.view OR loyalty.redeem`) | The chip emits a redeemable balance to users holding only `loyalty.view`, so both screens offer «استبدال بالنقاط»; the server then refuses with 403 `loyalty_redeem_forbidden` (before any write — correct). | View-only receptionist picks a reward, gets a 403 after filling the form. | Emit `redeemableChange` only when `perms.can('loyalty.redeem')` (keep showing the chip for `loyalty.view`). |
| LOY-6 | P2 | `ServiceOrderLineResource.php:38`; `cashier-collect.component.ts:273-276` | The resource's `loyalty_redeem_points` ignores `package_grant_id`; the cashier offers the button on any line with `patient_amount > 0`. A package session line with a patient share (rare) shows the button and the server rejects at collect. | Rare 422 at the cashier. | Return `null` in the resource when `$this->package_grant_id !== null` (same rule as `rewardPointsForLine`). |
| LOY-7 | P2 | `AddServiceOrderLine.php:149-155` (follow-up substitutes variant R), `create-visit.component.ts:892-906` (looks up R) | Consistent — the FE sends the quote's substituted `service_id` and the line stores the same id. But the reward must be configured on the follow-up variant R too; a reward set only on the base service S is not offered on a follow-up visit. | Owner sets 500 pts on «كشف», follow-ups never redeemable. | In `ClinicService::rewardPoints()` fall back to `parent_service_id`'s reward when the variant has none (and mirror in the booking FE lookup), or document. |
| LOY-8 | P2 (lifecycle, pre-existing) | `TransitionAppointmentStatus.php:103-105`; `CancelServiceOrderLine.php:25-27`; `RefundServiceOrderLine.php:83-87` | Cancelling a visit after redemption does nothing to the receipt: points stay spent until an explicit void or line refund (same as cash). A partial receipt (`allow_partial` with a reward-only tender) is `partially_paid` → the redeemed line can only be undone by void, never by credit note. Both are pre-existing rules; not defects of this WP. | Reception cancels a redeemed booking and nobody voids → the patient lost 500 pts. | Reception UX: when cancelling an appointment whose order has a paid receipt, prompt «الإيصال لسه قايم — عايز تلغيه؟» (link to void). KB note for the partial case. |
| LOY-9 | P2 (tests) | `Modules/Clinic/tests/Feature/LoyaltyRewardServiceTest.php` | Good coverage of the four posting paths, balanced asserts, GL tie. Missing: partial receipt with a reward tender (status, JE, debt); two reward lines on one receipt refunded in sequence (cap at remaining); quantity > 1; accrual-mode collect; concurrent redemption (two orders, one balance) — at least a unit test that `redeem()` under lock refuses when the balance moved after preview; booking: free visit + same service twice (LOY-1). | | Add the five cases; the first and last are the ones that protect money. |

## Checked and found correct (no action)

- Refund idempotency: a second refund of the same line is a 422 at the status guard (`RefundServiceOrderLine.php:57, 155`); the reversal is keyed on the credit note (`insertOrExisting`). Void idempotency: status guard + `insertOrExisting` on (`reception_receipt`, id). Void after a line refund is blocked (`VoidReceptionReceipt.php:39-43`), so `rewardReversalMeta`'s "final" branch always starts from 0 returned.
- Reward + money-style tender on one receipt: FE disables each when the other is set; BE rejects two points tenders (`loyalty_one_tender`) and a `points` value on a reward tender (`loyalty_reward_mixed`).
- The money-style points path is byte-identical: `redeemError` only changes when `$reward` is true, `rewardReversalMeta` returns the caller's meta when the redeem has no `redeem_price`, `refundPointsShare` branches only on that key, reports fall back to `money_value`.
- Partial receipts: tender ≤ due still enforced; reward amount counts toward `paid_total`; the remainder is AR debt as before. Repayment receipts have no lines → a reward tender is refused (`loyalty_reward_line_not_collected`).
- FE payloads: cashier sends `{payment_method:'loyalty_points', redeem_line_ids:[…]}` + `line_ids` of the selection, money rows reseeded to `selected − (points + reward)`, reward ids pruned when a line is unticked; booking sends `{payment_method:'loyalty_points', redeem_service_ids:[…]}` with no amount (request rule `required_unless` honours it), boxes total = `honestDue − reward`, partial dialog counts the reward as collected here, choices reset on patient change and when the balance drops. Booking services list = `/clinic/services` → `ClinicServiceResource`, which emits the two new fields.
- `redeem_line_ids` are de-duplicated; a line in two receipts is impossible (line lock + status re-check). Reward points use `quantity`, amount uses `patient_amount` (already × qty) — consistent.
- Lock order unchanged: order → lines → loyalty (account, lots) → receipt JE/box. In accrual mode the recognizer's AR/revenue JE runs before the loyalty locks, but it touches no cash box, so the box-before-loyalty concern does not arise.

## Dev co 1 state (read-only, 2026-10-24)

`loyalty_company_states.mode = unified` (cutover 14:37), `loyalty_program.enabled = true`, `point_value = 0.1`, expense account 121 (5304 Loyalty program expense), discount account 15, `refund_policy = allow_negative`; clinic rule: earn 0.1 / EGP on paid amount, redeem on, `max_redeem_type = unlimited`, treatment `expense`; `clinic.revenue_recognition = collection`, `package_revenue_mode = per_session`, `allow_partial_payment = true`, `require_payment_before_start = true`, `split.post_to_gl = false`. Rewards: service 14 (600 EGP) and 42 (3,000 EGP) at 500 pts. No redeem rows yet.