# Task 1 (BE) — م0 money-guard: 4 fixes — REPORT

Repo: `/home/moonui/moon-erp-be` · Branch: `hazemdev` (base `5b66277ad`)
Method: superpowers TDD — failing test FIRST (run → RED) → implement → GREEN. One commit per fix.
NO migration. Nothing pushed/deployed/migrated. `VoidReceptionReceipt.php` untouched.

## Commits (4 fixes + 1 collateral test-setup = 5, in order)
1. C4 — `39989dd00` fix(clinic): C4 collect fails before write when AR/revenue GL unset; JE listener throws
2. C3 — `47b2e21b5` fix(clinic): C3 refund refuses a pending / no-JE paying receipt
3. C2 — `b6bf6a752` fix(clinic): C2 lab-complete only flips an ordered line to performed
4. M4 — `778f45c87` fix(clinic): M4 refuse deleting an order with financial history
5. C4-collateral — `22d0b4270` test(clinic): configure clinic AR/revenue in CreateVisit TEST 6 (C4 collateral)

Deviation from "4 commits": a 5th, test-only commit was required. The full-suite run
surfaced that `CreateVisit` (visit creation with pay.mode=now) calls
`CollectReceptionReceipt` and, by its existing design, re-throws HttpException config
errors. The C4 guard now throws such a 422 when AR/revenue is unset, which masked
CreateVisitTest TEST 6's no-cashier-session SOFT-failure path (its company never
configured AR/revenue). Fix is test-setup only (configure AR/revenue so the sole
failure is the missing session); NO production code changed. Kept as a separate,
clearly-labeled commit rather than rewriting the 4 immutable fix commits' history on a
live repo.

---

## Fix C4 — collect must refuse BEFORE any write when clinic AR/revenue GL unset
**Files changed**
- `Modules/Clinic/app/Actions/CollectReceptionReceipt.php` — injected `Modules\Core\Services\SettingsService` via the constructor (3rd param, alongside `CashierRoutingService` + `SequenceService`). Added a fail-before-write guard at the TOP of `execute()`, BEFORE `DB::transaction(...)` opens: resolves `clinic.ar_account_id` + `clinic.revenue_account_id` for `$companyId`; if either falsy → `throw new HttpException(422, 'Clinic AR/revenue GL accounts are not configured. Cannot collect receipt.')`. Mirrors the existing tender-GL 422 style.
- `Modules/Clinic/app/Listeners/PostReceiptJournalEntry.php` — replaced the silent `return;` (lines 27-29) with `throw new \RuntimeException("Clinic AR/revenue accounts are not configured; cannot post receipt journal entry for {$receipt->receipt_number}.")`. Runs synchronously inside the collect transaction → a throw rolls the collection back atomically (defense-in-depth; unreachable after edit #1 but closes the exact audited bug).

**Test file (new):** `Modules/Clinic/tests/Feature/CollectGlAccountGuardTest.php`
- `C4a: collect with clinic AR/revenue GL accounts unset returns 422 and writes nothing` — full valid collect setup (cash account+method, fiscal period, open session, ordered line, matching tender) EXCEPT clinic AR/revenue settings left unset. Asserts HTTP 422 + the message + ZERO writes (reception_receipts, reception_payments, reception_receipt_lines, patient_ledger_entries, journal_entries source_type=reception_receipt) + line stays 'ordered'.
- `C4b: PostReceiptJournalEntry throws (does not silently return) when AR/revenue unset` — invokes the listener directly with a pending/no-JE receipt and unset settings; asserts it throws RuntimeException AND the receipt stays 'pending' with journal_entry_id null.

**RED→GREEN evidence**
- RED: C4a `Expected response status code [422] but received 201` (collect wrote a pending receipt — the bug). C4b `Exception "RuntimeException" not thrown` (silent return).
- GREEN: both pass (2 passed, 11 assertions).

---

## Fix C3 — refund must refuse a pending / no-JE paying receipt
**File changed:** `Modules/Clinic/app/Actions/RefundServiceOrderLine.php` — right after the existing voided-receipt check (fail-before-write section), added:
```php
if ($receipt->status !== 'paid' || ! $receipt->journal_entry_id) {
    throw new \RuntimeException(
        "The receipt for line {$line->id} has no posted journal entry (status: {$receipt->status}); cannot refund against it."
    );
}
```

**Test (added to `Modules/Clinic/tests/Feature/RefundLineTest.php`):** `M0-C3: refund is refused when the paying receipt is pending with no journal entry (no CN, no JE)` — collect a line (receipt posts → paid+JE), then force the receipt to `status='pending', journal_entry_id=null`, attempt refund. Asserts RuntimeException thrown + zero new `ClinicCreditNote` + zero new `clinic_credit_note` JE + line stays 'billed'.

**RED→GREEN evidence**
- RED: `Exception "RuntimeException" not thrown` (refund proceeded and would post a reversing JE against an unrecognized receipt).
- GREEN: full RefundLineTest passes (16 passed, 90 assertions) — new M0-C3 + all L2-8-* untouched.

---

## Fix C2 — lab-complete must not un-bill a paid line (one-liner)
**File changed:** `Modules/Clinic/app/Listeners/MarkEncounterLabComplete.php` — the update filter changed from
`->where('status', '!=', OrderLineStatus::Cancelled->value)` → `->where('status', OrderLineStatus::Ordered->value)`
(+ comment: only an ordered line auto-flips to performed; a billed line stays billed; lifecycle deferred). Mirrors `MarkEncounterRadComplete`.

**Test file (new):** `Modules/Clinic/tests/Feature/MarkLabCompleteBillingGuardTest.php`
- `C2: LabRequestCompleted flips only an ordered fulfilling line to performed; billed stays billed, cancelled stays cancelled` — a real Encounter (Department + LabDoctor + LabPatient + Encounter, so the `lab_requests.encounter_id` FK — activated by clinic migration `2026_06_22_120003` — resolves) + a LabRequest with that encounter_id + three fulfilling ServiceOrderLines (billed/ordered/cancelled). Fires `LabRequestCompleted`; asserts billed→billed, ordered→performed, cancelled→cancelled.

**RED→GREEN evidence**
- RED: `-'billed' +'performed'` — the billed line was un-billed to performed (the bug).
- GREEN: passes; `OrderLabFromEncounterTest` (which exercises this listener, AC3b) still green (10 passed).

Note: initial RED runs hit a `lab_requests` FK failure — root cause was the clinic migration `2026_06_22_120003_activate_encounter_id_fk_on_lab_requests.php` (activates FK encounter_id→encounters), so the test now uses a real Encounter. Not a code issue.

---

## Fix M4 — cannot delete a service order that has financial history
**File changed:** `Modules/Clinic/app/Http/Controllers/ServiceOrderController.php` — imported `ReceptionReceipt`, `ClinicCreditNote`, `PatientLedgerEntry`. In `destroy()`, BEFORE `$orderModel->delete()` and AFTER the existing status 409, added a company-scoped financial-history guard: `abort_if($hasFinancialHistory, 409, '...')` where history = a `ReceptionReceipt` (any status incl. voided) OR `ClinicCreditNote` with `service_order_id=order->id`, OR a `PatientLedgerEntry` with `source_type='service_order_line'` and `source_id IN (order line ids)`. Existing status 409 kept.

**Tests (added to `Modules/Clinic/tests/Feature/ServiceOrderTest.php`):**
- `M4: DELETE a cancelled order that has a reception receipt returns 409 and does not delete` (voided receipt = history).
- `M4: DELETE a cancelled order with a patient ledger entry on its line returns 409`.
- `M4: DELETE a clean open order with no financial history still returns 200` (don't over-block; soft-deleted).

**RED→GREEN evidence**
- RED: both should-409 tests `Expected response status code [409] but received 200` (cancelled order with history was deletable — the gap); clean-order test already 200.
- GREEN: full ServiceOrderTest passes (16 passed, 56 assertions).

---

## Regression / full-suite
- **4 L2-regression named tests confirmed PASS**: `PartialCollectTest` + `MoneyPathRegressionTest` + `ReceptionCashierTest` (26 passed together, run after C4) and `RefundLineTest` (16 passed, run after C3). Re-verified in the full-suite runs.
- Full `Modules/Clinic/tests/Feature` suite — initial run: 2 failed, 343 passed (1174 assertions):
  - `CreateVisitTest` TEST 6 — caused by the C4 guard (see collateral commit `22d0b4270`). Fixed → CreateVisitTest now 9/9 pass in isolation.
  - `ClinicRoomTest > it returns HRM departments` (assertJsonCount(3) got 0) — **PRE-EXISTING, unrelated**. Proven: reverting my 5 source files to base and running the 2 files still fails ClinicRoomTest (1 failed, 12 passed); ClinicRoomTest references NONE of my changed files (no clinic room/department code was touched).
- Full-suite RERUN (after collateral fix): **1 failed, 344 passed (1178 assertions)**, 305s.
  The ONLY remaining failure is the pre-existing
  `ClinicRoomTest > it returns HRM departments` (confirmed still failing when my 5 source
  files are reverted to base). All CreateVisitTest tests pass. Net effect of my work on the
  suite: zero regressions; the pre-existing ClinicRoomTest failure is untouched (out of scope).

## Bottom line
DONE. 4 money-guards implemented TDD (RED→GREEN each), + 1 collateral test-setup fix.
4 L2-regression tests pass. No migration, nothing pushed/deployed. VoidReceptionReceipt
untouched. Only pre-existing, unrelated ClinicRoomTest remains red.