hamid
122 lines
11 KiB
Markdown
122 lines
11 KiB
Markdown
# Fix plan — how to close every item in blockers.md
|
||
|
||
An index into [`blocker-phases/`](blocker-phases/), an engineering-facing companion to
|
||
[blockers.md](blockers.md): one numbered file per fix, in a suggested working order. Each file has the exact
|
||
root cause (file:line), the proposed fix, and anywhere the fix depends on a business-rule decision that
|
||
shouldn't be guessed. Written from a full read of the current code (commit `fb58ca5`, 2026-08-02). Phases 01
|
||
and 02 are applied (see Status column); the rest are unapplied — work through them at your own pace, in
|
||
whatever order you prefer.
|
||
|
||
> **Two blockers already got a scope decision, recorded so it isn't re-litigated:** §B.4 (مودیان
|
||
> e-invoicing) gets a note in [forgotten-features.md](forgotten-features.md) instead of a build — no chosen
|
||
> provider to integrate against. §B.6 (bank-transfer payout rail) stays mocked — the seam already exists, see
|
||
> phase 11.
|
||
|
||
## Phases
|
||
|
||
| # | File | Blocker | Depends on | Status |
|
||
| --- | --- | --- | --- | --- |
|
||
| 01 | [admin-rbac](blocker-phases/01-admin-rbac.md) | Admin can't do anything (the #1 leverage item) | — | ✅ Done |
|
||
| 02 | [reviews-moderation](blocker-phases/02-reviews-moderation.md) | Reviews can never go live | 01 | ✅ Done (no separate fix needed — unblocked by 01) |
|
||
| 03 | [address-edit-bug](blocker-phases/03-address-edit-bug.md) | Editing an address wipes recipient name/phone | — | ✅ Done |
|
||
| 04 | [payment-timezone](blocker-phases/04-payment-timezone.md) | The 30-minute payment countdown can lie | — | ✅ Done (follow-up filed below) |
|
||
| 05 | [bnpl-setup](blocker-phases/05-bnpl-setup.md) | Installments (BNPL) don't work at all | — | — |
|
||
| 06 | [catalog-admin-page](blocker-phases/06-catalog-admin-page.md) | No admin page for service categories/pricing | — | — |
|
||
| 07 | [card-payment-redirect](blocker-phases/07-card-payment-redirect.md) | Card payment can never complete | — | ✅ Done (follow-up filed below) |
|
||
| 08 | [refunds-demock](blocker-phases/08-refunds-demock.md) | Refunds are demo-only, off by 100× | — | ✅ Done (customer surface only, follow-up filed below) |
|
||
| 09 | [nurse-verification-badge](blocker-phases/09-nurse-verification-badge.md) | Verification badge doesn't reflect reality | pairs with 10 | ✅ Done |
|
||
| 10 | [search-dedup-and-trust](blocker-phases/10-search-dedup-and-trust.md) | Search isn't de-duplicated; trust info hardcoded | pairs with 09 | ✅ Done |
|
||
| 11 | [nurse-payouts](blocker-phases/11-nurse-payouts.md) | Nurse pay/payouts are fake, no "process" action | benefits from 01 | — |
|
||
| 12 | [patient-records](blocker-phases/12-patient-records.md) | Patient records & visit notes are fake demo data | needs a product decision first | 🟡 Partial (see follow-up below) |
|
||
| 13 | [booking-lifecycle](blocker-phases/13-booking-lifecycle.md) | Stuck bookings; "today's visits" unfiltered | pairs with 04 | 🟡 Partial (see follow-up below) |
|
||
| 14 | [partner-center](blocker-phases/14-partner-center.md) | Partner/business-center accounts are fake | benefits from 01 | — |
|
||
| 15 | [debug-mode-production](blocker-phases/15-debug-mode-production.md) | Turn off dev mode on the live site (§B.2) | do last, deliberately | — |
|
||
|
||
## Follow-ups filed (not yet phases of their own)
|
||
|
||
- **Phase 08 closed the customer-facing refund surface only — the admin refund console stays mocked.**
|
||
`AdminRefundsController` only implements create-and-execute (`POST admin_refunds`, matching
|
||
`initiateRefund`); there is no real read-only preview, no retry/approve, and no reject route. The phase
|
||
doc's own read ("real server side is live and correct") only checked the customer half
|
||
(`RefundsController`, `CreateRefundCommand`, `GetCancellationPolicyPreviewQuery`) — the admin gaps weren't
|
||
called out and would have 404'd the console had the single mock flag been flipped wholesale. Split into
|
||
two independently-selected flags instead (`client/src/services/refunds/constants.ts`):
|
||
`USE_CUSTOMER_REFUNDS_MOCK = false` (real — closes the actual ×100 money-safety bug) and
|
||
`USE_ADMIN_REFUNDS_MOCK = true` (stays mocked; the four admin methods are kept together rather than mixed,
|
||
since a real `initiateRefund` executing against a mocked preview's numbers would be actively dangerous).
|
||
Building the missing preview/retry/reject endpoints needs real design (retry semantics re-executing a
|
||
channel call, what "reject" reverses) that isn't specified anywhere — filed as its own future phase, not
|
||
guessed here.
|
||
- **Same timezone bug as 04, lower severity — now fixed, folded into phase 13.** Phase 04 fixed
|
||
`BookingRequest.PaymentDeadlineAt`/`NurseResponseDeadlineAt` — a `DateTime` (not `DateTimeOffset`) read back
|
||
from SQL Server's `datetime2` loses its `Kind` tag (comes back `Unspecified`), so JSON serialization drops
|
||
the trailing `Z` and a client `Date.parse()` misreads it as local time. The same grep found four more real
|
||
instances, all on `Booking`: `DisputeWindowEndsAt`, `ConfirmedAt`, `CancelledAt`, `CompletedAt`
|
||
(`server/src/Core/Baya.Domain/Entities/Booking/Booking.cs:73,74,91,95`). Applied the existing
|
||
`UtcDateTimeConverter` to all four in `BookingConfig.cs`. No migration needed (same column type, only the
|
||
in-memory `Kind` tag changes on read).
|
||
|
||
- **Phase 13 closed both items except one deliberately-deferred edge case.**
|
||
- **13a (stuck partial-missed bookings) fixed.** Extracted the `allSettled` check `CheckOutVisitCommand`
|
||
already ran after a real check-out into `Booking.IsAllSessionsSettled()`
|
||
(`server/src/Core/Baya.Domain/Entities/Booking/Booking.cs`) and called it from
|
||
`DetectNoShowSessionsCommand.Handler.cs` after the no-show sweep marks sessions `Missed`, re-checking every
|
||
booking touched in that batch. A booking with one completed session and the rest auto-missed now correctly
|
||
reaches `Completed` and opens its dispute window (so the nurse's completed-session payout becomes eligible)
|
||
instead of staying stuck at `InProgress` until an admin manually rescues it.
|
||
**Still open, on purpose — the zero-completed-sessions case (every session auto-missed straight from
|
||
`Confirmed`/`InProgress`).** Asked whether that should still reach `Completed` with the nurse paid in full
|
||
(today's payout query is booking-level only — `PayoutRepository.EligibleBookingsQuery` pays the whole
|
||
`NursePayoutAmount` off `Status`+`DisputeWindowEndsAt`, with no per-session proration, so this would pay a
|
||
nurse who did zero visits), reach `Completed` with the payout fields zeroed (a bigger deviation from the
|
||
"money snapshot, never mutate" convention), or use a new terminal status with no payout path at all. The
|
||
answer was to leave it deferred rather than pick one now — it stays stuck exactly as before, rescuable only
|
||
via the admin `TransitionBookingStatusCommand` (`InProgress → Completed`). Revisit once the payout query
|
||
itself is made session-aware, or once there's a concrete need to close these out.
|
||
- **13b ("today's visits" unfiltered) fixed.** `ListSessionsForNurseQueryHandler` now defaults
|
||
`request.Date` to "today" when null, mirroring the mock (`mockApi.ts:438`). "Today" needed a timezone
|
||
decision the codebase had never made (no `Asia/Tehran`-aware date logic existed anywhere): added
|
||
`TehranClock` (`server/src/Core/Baya.Application/Contracts/Common/TehranClock.cs`), a fixed UTC+03:30
|
||
offset (Iran abolished DST in 2022, so no tzdata/`TimeZoneInfo` lookup is needed) — a technical default,
|
||
not a business-rule guess, so it wasn't flagged for a decision the way 13a was.
|
||
|
||
- **Phase 12 closed everything except the medication/routine schema decision, which stays deliberately
|
||
unresolved.** Fixed: the silent data-wipe (`patients/[id]/record/page.tsx`'s `EditableTabs.save` now always
|
||
resends the full `medications`/`routine`/`tasks` triple, not just the tab being edited — the real
|
||
`UpsertCarePlanCommand` fully replaces all three from the request, no server-side merge); the
|
||
`RecordAccess.deniedReason` enum (client now matches the server's real `'not_authorized' | 'not_found'`,
|
||
was `'no_access' | 'not_found'`); and the visit-note `taskResults` mapper bug (`clientApi.ts` was hardcoding
|
||
`taskResults: []` on read and folding the checklist into free text on write, even though `CareRecordDto`
|
||
already serves a structured `TaskResults` array and `WriteCareRecordBody` already accepts one — both sides
|
||
now pass the array through directly). Also corrected the stale "no backend at all" comments in
|
||
`constants.ts`/`types.ts`/`clientApi.ts`/`mockApi.ts` — `PatientCareRecordsController` implements every
|
||
route this domain needs.
|
||
**Still open, on purpose:** the client's medication/routine shape (structured dose amount/unit, frequency
|
||
preset codes, a multi-select `timeOfDay`) doesn't match the server's (one free-text dose, one required
|
||
frequency string, no `timeOfDay` on either medications or routine items) — see phase 12's items #3/#4. Asked
|
||
which side should change; the answer was to leave both as they are for now and just keep this note so it
|
||
isn't lost. `USE_PATIENT_RECORDS_MOCK` therefore stays `true`, and the id-type mismatch (client `string` ids
|
||
vs. the wire's `long`, phase 12 item #2) is left unfixed too — it can't be finished without first knowing
|
||
the target field shape, since `getFamilyRecord`/`updateFamilyRecord` would need a real mapping function
|
||
between the two, not just an id conversion.
|
||
|
||
- **ZarinPal-callback translator, latent until a real gateway is switched on.** Phase 07 restored the
|
||
mock-gateway harness and pointed `MockPaymentProvider.InitPaymentAsync`'s `redirectUrl` at it (relative
|
||
URL, no more dead `mock-psp.local` host) — the card-payment dead end (blockers.md § "Payments") is closed
|
||
for the mock path, which is the only path reachable pre-e-namad (§B.5). Once a real acquirer is ever
|
||
switched on, ZarinPal's callback redirects the browser with its **own** query params (`Authority`,
|
||
`Status`) — nothing under `Controllers/V1` today translates that into what `checkout/return/page.tsx`
|
||
expects (`request_id`/`transaction_id`/`outcome`). Needs a translating endpoint/route before the real path
|
||
can work; deliberately out of scope of the mock fix, and moot until §B.5 clears.
|
||
|
||
## Suggested order
|
||
|
||
1. **01 (admin RBAC)** — unlocks 02, and makes 09/10/11/14 independently testable via the seeded
|
||
`super_admin`/`finance` accounts.
|
||
2. **Small, contained, no dependencies:** 03, 04, 05, 06.
|
||
3. **De-mock passes, each roughly self-contained:** 08, 09+10 together, 11.
|
||
4. **Needs a product decision before coding:** 12 (medication/routine schema), 13a's all-missed edge case
|
||
(deferred on purpose — see follow-up above).
|
||
5. **14 (partner center)** — the largest single phase, mostly new server surface.
|
||
6. **15 (debug mode)** — on its own, right before any real user is let near the site.
|