From 71ca986dcd9c5a62b8ba2987ec3e39fe0fe8ffe9 Mon Sep 17 00:00:00 2001 From: hamid Date: Sun, 2 Aug 2026 23:42:39 +0330 Subject: [PATCH] remove blocker phases after done --- mvp/blocker-phases/01-admin-rbac.md | 78 ------------------- mvp/blocker-phases/02-reviews-moderation.md | 23 ------ mvp/blocker-phases/03-address-edit-bug.md | 19 ----- mvp/blocker-phases/04-payment-timezone.md | 47 ----------- mvp/blocker-phases/05-bnpl-setup.md | 31 -------- mvp/blocker-phases/06-catalog-admin-page.md | 26 ------- .../07-card-payment-redirect.md | 36 --------- mvp/blocker-phases/08-refunds-demock.md | 32 -------- .../09-nurse-verification-badge.md | 44 ----------- .../10-search-dedup-and-trust.md | 33 -------- mvp/blocker-phases/11-nurse-payouts.md | 72 ----------------- mvp/blocker-phases/12-patient-records.md | 48 ------------ mvp/blocker-phases/13-booking-lifecycle.md | 45 ----------- mvp/blocker-phases/14-partner-center.md | 45 ----------- .../15-debug-mode-production.md | 54 ------------- 15 files changed, 633 deletions(-) delete mode 100644 mvp/blocker-phases/01-admin-rbac.md delete mode 100644 mvp/blocker-phases/02-reviews-moderation.md delete mode 100644 mvp/blocker-phases/03-address-edit-bug.md delete mode 100644 mvp/blocker-phases/04-payment-timezone.md delete mode 100644 mvp/blocker-phases/05-bnpl-setup.md delete mode 100644 mvp/blocker-phases/06-catalog-admin-page.md delete mode 100644 mvp/blocker-phases/07-card-payment-redirect.md delete mode 100644 mvp/blocker-phases/08-refunds-demock.md delete mode 100644 mvp/blocker-phases/09-nurse-verification-badge.md delete mode 100644 mvp/blocker-phases/10-search-dedup-and-trust.md delete mode 100644 mvp/blocker-phases/11-nurse-payouts.md delete mode 100644 mvp/blocker-phases/12-patient-records.md delete mode 100644 mvp/blocker-phases/13-booking-lifecycle.md delete mode 100644 mvp/blocker-phases/14-partner-center.md delete mode 100644 mvp/blocker-phases/15-debug-mode-production.md diff --git a/mvp/blocker-phases/01-admin-rbac.md b/mvp/blocker-phases/01-admin-rbac.md deleted file mode 100644 index e4eb4e6..0000000 --- a/mvp/blocker-phases/01-admin-rbac.md +++ /dev/null @@ -1,78 +0,0 @@ -# Phase 01 — Admin can't do anything - -**Blocker:** blockers.md § "Admin can't do anything" (the #1 leverage item — a large share of the other -phases in this folder are gated behind it). -**Depends on:** nothing. -**Unlocks:** [02-reviews-moderation.md](02-reviews-moderation.md) outright; makes every other admin-facing -phase (09, 10, 11, 14) independently testable via the seeded `super_admin`/`finance` demo accounts. - ---- - -**Root cause.** `server/src/Infrastructure/Baya.Infrastructure.Identity/Identity/PermissionManager/DynamicPermissionService.cs:9` only bypasses for the literal role `"admin"`: -```csharp -if (user.IsInRole("admin")) { return true; } -``` -`RoleNames` (`server/src/Core/Baya.Domain/Entities/User/RoleNames.cs`) defines `Admin`, `Support`, `Finance`, -`Moderation`, `SuperAdmin` as **five sibling roles**, not a hierarchy — `super_admin` doesn't imply `admin`. -The seeded demo accounts (`DemoWorldDefinitions.cs:142-146`) hold `super_admin` and `finance` — **never** -the literal `admin` — so neither passes this check. The only path that ever grants the literal `admin` role -is the config-gated bootstrap-admin (`SeedDataBase.cs:50-71`, needs `Seed:AdminUsername`/`Seed:AdminPassword`, -neither set anywhere), and it's a username/password account that can't sign in through the phone-OTP web UI -anyway. - -The fallback branch (a `DynamicPermission` claim matching `"{area}:{controller}:"`) is **dead code** — nothing -anywhere writes that claim to any role. The commands that could (`Features/Role/*`: `AddRoleCommand`, -`UpdateRoleClaimsCommand`, `GetAllRolesQuery`, `GetAuthorizableRoutesQuery`) are fully implemented but never -wired to a controller — confirmed zero references under `Controllers/V1/`. - -**Why manual QA doesn't catch it:** `client/src/services/admin/constants.ts:9` — `USE_ADMIN_MOCK = true` — -19 of 21 admin consoles are served from an in-browser mock and never call the API. Only `/fa/admin/tickets` -and `/fa/admin/reviews` are wired to the real seam, and both visibly 403 today. - -## The fix, in two parts - -1. **Broaden the bypass** in `DynamicPermissionService.CanAccess` to also accept `RoleNames.SuperAdmin` - (`super_admin` is definitionally the top role — this isn't a guess, it mirrors what the client's own - permission matrix already assumes, see below): - ```csharp - if (user.IsInRole(RoleNames.Admin) || user.IsInRole(RoleNames.SuperAdmin)) { return true; } - ``` - This alone closes BL-002 (the seeded `super_admin` demo account, `09120000020`, becomes fully functional) - and fixes every admin action that only needs `super_admin`. - -2. **Grant the `finance` role (and, for completeness, `support`/`moderation`, even though no demo account - holds them yet) real `DynamicPermission` claims**, scoped to the consoles they own. Don't guess this - mapping — it's already designed and reviewed, just not backed by anything: `client/src/hooks/capabilities.ts` - encodes the exact intended matrix (`useAdminCapabilities`), and reading the *code* (not just its stale - doc-comment) gives: - - `finance` → `canRefund`, `canPayout`, `canConfig` → controllers **`AdminRefunds`**, **`AdminPayouts`**, - **`PlatformConfig`**. - - `support` → `canVerify`, `canManageAlerts`, `canManageTickets` → **`AdminVerifications`**, - **`AdminVerificationStepTypes`**, **`SupportAlerts`**, **`AdminTickets`**. - - `moderation` → `canModerate` → **`AdminReviews`**, **`Reviews`** (the latter only for its - `PATCH {id}/status` action, which is separately method-gated). - (Everything else — `AdminInvoices`, `AdminClawbacks`, `AdminCancellationPolicies`, `AdminBnpl`, `AdminGeo`, - `Holidays`, `AdminCatalog`, `AdminBookingRequests`, `AdminEvv`, `AdminSearch`, `AdminPartnerCenters`, - `Audit`, `InternalCenters` — stays `super_admin`/`admin`-only, matching "admin — everything except role - management" from the same matrix.) - - Concretely: add a step to `SeedDataBase.Seed()` (same file that already idempotently ensures all 7 roles - exist) that, for each of those three roles, calls the already-built - `IRoleManagerService.ChangeRolePermissionsAsync(new EditRolePermissionsDto { RoleId = role.Id, Permissions = [...] })` - with the claim values `$":{controller}:"` (area is always empty today — no controller uses `[Area]`) for - its owned controllers. This reuses existing, tested plumbing rather than hand-rolling `RoleClaim` inserts. - `SeedDataBase` will need `IRoleManagerService` injected (it's already registered in DI for the identity - assembly). - -## Deliberately out of scope - -Wiring the `Features/Role/*` commands to a real `AdminRolesController` so `super_admin` can grant/revoke -roles through the UI. The client's own `/fa/admin/roles` screen is explicitly marked "DEFERRED-IF-MISSING" -with its own banner (`admin/roles/page.tsx:2-9`) and served by the mock on purpose — leave it deferred; the -seeding approach above makes the two blockers real without needing that screen built. - -## Bonus, required by forgotten-features.md - -"The nurse-verification approve/reject buttons point at server actions that don't exist yet" is real and -separate from the RBAC bug — see [09-nurse-verification-badge.md](09-nurse-verification-badge.md) for the -two missing endpoints. diff --git a/mvp/blocker-phases/02-reviews-moderation.md b/mvp/blocker-phases/02-reviews-moderation.md deleted file mode 100644 index 731d448..0000000 --- a/mvp/blocker-phases/02-reviews-moderation.md +++ /dev/null @@ -1,23 +0,0 @@ -# Phase 02 — Reviews can never go live - -**Blocker:** blockers.md § "Reviews." -**Depends on:** [01-admin-rbac.md](01-admin-rbac.md) — same root cause, no separate code fix. - ---- - -**Root cause.** Same broken policy as phase 01, nothing else. `AdminReviewsController.cs:20` (the moderation -queue) and `ReviewsController.cs:33` (the `PATCH {reviewId}/status` publish action) are both gated by -`[Authorize(ConstantPolicies.DynamicPermission)]` → the same `CanAccess` check. - -**The fix:** none needed beyond phase 01 — once `moderation`/`super_admin` pass `CanAccess`, both endpoints -become reachable immediately. - -## Two secondary things worth doing in the same pass (not required to "unstick" reviews) - -- `ReviewModerationStatus.Rejected` is currently unreachable end-to-end even after the RBAC fix — the - submit-time banned-word path maps to `Hidden`, not `Rejected` (`SubmitReviewCommand.Handler.cs:77`), and the - only other producer of `Rejected` is the admin PATCH. Once RBAC is fixed this stops being a dead state (an - admin really can reject a review), so no code change is actually required — just noting the client's - `rejected` chip (`review/page.tsx:48`) was previously unreachable and will start being exercised. -- **Flag, don't guess:** should `Seams:ReviewModeration:AutoApproveClean` be turned on so reviews aren't stuck - pending an admin action that may not happen promptly? That's a product call, not inferable from code. diff --git a/mvp/blocker-phases/03-address-edit-bug.md b/mvp/blocker-phases/03-address-edit-bug.md deleted file mode 100644 index 21ebfe7..0000000 --- a/mvp/blocker-phases/03-address-edit-bug.md +++ /dev/null @@ -1,19 +0,0 @@ -# Phase 03 — Address edit wipes recipient name/phone - -**Blocker:** blockers.md § "Data safety." -**Depends on:** nothing. Small and contained. - ---- - -**Root cause.** `AddressForm.tsx` (shared by add and edit) has **no** `recipientName`/`recipientPhone` fields -at all (`:36-42`, `:91-102`). `clientApi.ts:21-34` (`toBody`) sends `null` for both on every create/update -because the form never collects them. The server does a full overwrite, not a merge: -`UpdateAddressCommand.Handler.cs:53-59` unconditionally sets both fields from the request, and the command's -DTO declares them non-nullable `string` — no "field omitted" signal is even possible today. These fields are -consumed downstream (`bookings/request/page.tsx:269-270` snapshots them into a booking request), so a wipe -silently degrades the nurse's day-of-visit contact info for future bookings from that address. - -**Fix:** add `recipientName`/`recipientPhone` fields to `AddressForm.tsx` (both add and edit gain them for -free, since it's one shared form) and thread through `AddressFormValues`/`submit()`/`CreateAddressInput`. This -is the root fix. A server-side partial-update stopgap is possible but only prevents further data loss — it -can't let a user actually set these fields, since the form still wouldn't collect them. diff --git a/mvp/blocker-phases/04-payment-timezone.md b/mvp/blocker-phases/04-payment-timezone.md deleted file mode 100644 index e43acf7..0000000 --- a/mvp/blocker-phases/04-payment-timezone.md +++ /dev/null @@ -1,47 +0,0 @@ -# Phase 04 — The 30-minute payment countdown can lie - -**Blocker:** blockers.md § "Payments" (the timezone item). -**Depends on:** nothing. Small and contained. -**Related:** [13-booking-lifecycle.md](13-booking-lifecycle.md)'s "today's visits" fix needs a consistent -timezone decision — resolve that the same way once you land this. - ---- - -**Root cause, precise.** `BookingRequest.PaymentDeadlineAt`/`NurseResponseDeadlineAt` are `DateTime` (not -`DateTimeOffset`, deliberately, per a comment about SQLite test-provider compatibility — -`BookingRequest.cs:53-60`). The value is written correctly as UTC -(`AcceptBookingRequestCommand.Handler.cs:41-50`), but **SQL Server's `datetime2` carries no timezone**, and -EF's SQL Server provider returns `Kind = Unspecified` on read — nothing resets it to `Utc` -(`BookingRequestConfig.cs:19-26` has no `.HasConversion(...)` for these two properties, unlike -`BaseEntity.CreatedAt`/`ModifiedAt`, which correctly use `DateTimeOffset`). System.Text.Json therefore -serializes it **without** a trailing `Z`. `Date.parse()` on the client -(`client/src/components/CountdownTimer/CountdownTimer.tsx:84`) reads a `Z`-less ISO string as **local time**, -not UTC — for a Tehran browser (UTC+3:30) that's ~3.5 hours added to the true deadline, so the countdown -shows hours more time than actually remains and expires while still showing time left. - -**The fix.** Add an EF Core value converter (new file, e.g. -`server/src/Infrastructure/Baya.Infrastructure.Persistence/ValueConversion/UtcDateTimeConverter.cs`, same -pattern as the existing `EncryptedStringConverter.cs`): `convertFromProvider: v => -DateTime.SpecifyKind(v, DateTimeKind.Utc)` (no-op on write). Apply it to both `PaymentDeadlineAt` and -`NurseResponseDeadlineAt` in `BookingRequestConfig.cs:19-26`. No migration needed — same column type, only -the in-memory `Kind` tag changes on read. - -**Flag:** the same defect class (a `DateTime` read back from `datetime2` losing `Kind`) likely recurs -anywhere else a server-frozen instant feeds a client countdown (e.g. `dispute_window_ends_at`, BNPL -`settled_at`) — worth a grep for other `DateTime` (not `DateTimeOffset`) properties before considering this -class of bug fully closed, not just the one already reported. - -**Follow-up from that grep (done, not yet fixed).** Swept every other `DateTime`/`DateTime?` entity property -(`BaseEntity.CreatedAt`/`ModifiedAt` already correctly use `DateTimeOffset` and are excluded). Most are -internal/audit-only (payout batches, webhook events, ASP.NET Identity tables) and never reach a client. Four -are real, lower-severity instances of the same bug — never wired to a live countdown, so the failure mode is -"can show the wrong day near a Tehran midnight boundary," not "actively expires while showing time left": -- `Booking.DisputeWindowEndsAt` (`Entities/Booking/Booking.cs:95`) — rendered via `formatShamsiDate` in - `BookingDetailView.tsx:444`. -- `Booking.ConfirmedAt` / `CancelledAt` / `CompletedAt` (`Entities/Booking/Booking.cs:73,74,91`) — booking - timeline timestamps shown to the client. - -All four are configured in `BookingConfig.cs` (no `.HasConversion(...)` today) and would take the exact same -fix as this phase: `.HasConversion(new UtcDateTimeConverter())`. Deliberately left unfixed here — scoped out -of this phase on request, tracked so it isn't dropped. Pick up as a follow-up phase (or fold into -[13-booking-lifecycle.md](13-booking-lifecycle.md), which already touches `Booking` timezone handling). diff --git a/mvp/blocker-phases/05-bnpl-setup.md b/mvp/blocker-phases/05-bnpl-setup.md deleted file mode 100644 index fb7764a..0000000 --- a/mvp/blocker-phases/05-bnpl-setup.md +++ /dev/null @@ -1,31 +0,0 @@ -# Phase 05 — BNPL doesn't work at all - -**Blocker:** blockers.md § "Payments" (installments). -**Depends on:** nothing. Two distinct bugs, both small. - ---- - -## Bug A — server-side: no active BNPL gateway is ever seeded - -`SeedPaymentGatewaysAsync` (`ServiceCollectionExtensions.cs:135-154`) seeds exactly -one `PaymentGatewayType.Standard` (card) gateway row — **never** a `PaymentGatewayType.Bnpl` row. -`CheckBnplEligibilityQuery.Handler.cs:45-47` and `InitiateBnplOrderCommand.Handler.cs:42-44` both short-circuit -with "No active BNPL gateway is configured" **before** ever calling `IBnplProvider` (mock or real — both are -correctly registered, just unreached). - -**Fix:** extend `SeedPaymentGatewaysAsync` (or add a sibling seeder called next to it in `Program.cs:137,156`) -to idempotently insert a `PaymentGatewayType.Bnpl` row (`ProviderCode="balinyaar"`, the in-house net-of-fee -provider per `archive/docs/rules/server/money.md` §6 — needs no external credentials), same -`AnyAsync(...)` idempotency guard as the existing seeder. - -## Bug B — client-side: still fully mocked, and the mock itself is broken - -`client/src/services/bnpl/constants.ts:18` — `USE_BNPL_MOCK = true`, never flipped. -Even as a demo the mock is broken: `client/.../bnpl/apis/mockApi.ts:5-8` imports directly from the retired -`bookingRequests` mock store, bypassing that domain's own (real) seam — so any real booking-request id 404s -inside the BNPL mock. That store only ever seeds ids `1`/`2`, which age out to `expired_no_response` almost -immediately, so the D1–D5 wizard is practically unreachable today regardless. - -**Fix:** once Bug A lands, flip `USE_BNPL_MOCK` to `false` — the real `CheckoutBnplController` endpoints are -otherwise complete. The mock's cross-import isn't worth fixing in isolation; it becomes moot once the flag -flips. diff --git a/mvp/blocker-phases/06-catalog-admin-page.md b/mvp/blocker-phases/06-catalog-admin-page.md deleted file mode 100644 index 0e282ca..0000000 --- a/mvp/blocker-phases/06-catalog-admin-page.md +++ /dev/null @@ -1,26 +0,0 @@ -# Phase 06 — Catalog/pricing has no admin page - -**Blocker:** blockers.md § "Catalog / pricing." -**Depends on:** nothing. Pure client work — the server side is already done. - ---- - -**Root cause — confirmed as two separate, correctly-separated things, not one bug.** The 5 MVP service -categories are seeded everywhere via migration `HasData` (`CatalogConfig/CatalogSeed.cs:1-19`) — categories -are fine in every environment. Pricing **options** (the EAV dimensions) are deliberately **not** seeded via -migration — the seed file's own comment says option groups/values are "admin-authored data... never a -migration." The only place they get created today is `DemoWorldSeeder.cs:74-101`, which only runs in -Development (`Program.cs:135-159` gates both seeders to `IsDevelopment()`). Production genuinely has zero -option groups for any category. - -**The admin API to manage this already exists and is fully permission-gated:** -`AdminCatalogController.cs:25-61` — `CreateCategory`, `UpdateCategory`, `SetCategoryActive`, -`CreateOptionGroup`, `UpdateOptionGroup`, `CreateOptionValue`, `UpdateOptionValue`. The gap is **entirely -client-side** — there is no `admin/catalog` route anywhere in the client tree, and zero references to any of -those endpoints in `client/src`. - -**Fix:** build the admin catalog page (categories + option-group/value CRUD) under -`client/src/app/[locale]/(private-routes)/admin/catalog/`, calling the already-built server endpoints. This -is genuinely pure client work now — running `DemoWorldSeeder` in production is explicitly the *wrong* fix -(it bundles fake demo nurses/customers/reviews alongside the option data, which is demo content, not real -launch content). diff --git a/mvp/blocker-phases/07-card-payment-redirect.md b/mvp/blocker-phases/07-card-payment-redirect.md deleted file mode 100644 index 552205f..0000000 --- a/mvp/blocker-phases/07-card-payment-redirect.md +++ /dev/null @@ -1,36 +0,0 @@ -# Phase 07 — Card payment can never complete - -**Blocker:** blockers.md § "Payments" (the card-payment dead end). -**Depends on:** nothing. - ---- - -**Root cause.** `MockPaymentProvider.cs:16-21` returns `RedirectUrl: https://mock-psp.local/pay/{reference}` -— a host that doesn't exist. `client/.../checkout/page.tsx:137-140` does a **hard browser navigation** -(`window.location.assign`) to that URL with no catch, so the user is stranded outside the SPA. - -The page that used to catch this redirect and bounce back into the app — -`client/.../bookings/checkout/gateway/page.tsx` — was **deleted** in commit `64f6aa4` ("refinement phase 4") -and never replaced. The exact same pattern still lives today for BNPL: -`client/.../bookings/checkout/bnpl/gateway/page.tsx` (env-gated to `NODE_ENV=development` via `notFound()`, -pay-success/pay-fail buttons, redirects into `ROUTES.CHECKOUT_BNPL_RETURN`). The target it should redirect -to already exists and already works: `client/.../bookings/checkout/return/page.tsx` -(`useConfirmGatewayReturn` + bounded backoff poll → success routes to `/checkout/confirmation`). - -Nothing about the money logic is broken — `ZarinPalPaymentProvider.cs` (the real provider) is fully -implemented and simply unreachable pre-e-namad (blockers.md §B.5), which is a legal/business gate, not a code -gap. - -## The fix (two parts, both needed) - -1. Restore `client/.../bookings/checkout/gateway/page.tsx`, mirroring the still-live BNPL harness pattern - (env-gate with `notFound()` outside development), targeting `ROUTES.CHECKOUT_RETURN`. -2. Point `MockPaymentProvider.InitPaymentAsync`'s `RedirectUrl` at that harness (a relative or - client-origin-absolute URL) instead of the dead `mock-psp.local` host. - -## Flag, don't guess (latent, not blocking the mock fix) - -Once a real gateway 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`). This needs a translating -endpoint before the real path can work, separate from the mock-harness fix above. diff --git a/mvp/blocker-phases/08-refunds-demock.md b/mvp/blocker-phases/08-refunds-demock.md deleted file mode 100644 index 64462c4..0000000 --- a/mvp/blocker-phases/08-refunds-demock.md +++ /dev/null @@ -1,32 +0,0 @@ -# Phase 08 — Refunds are demo-only, and off by 100× - -**Blocker:** blockers.md § "Payments" (refunds). -**Depends on:** nothing. - ---- - -**Root cause — the mock.** `client/src/services/refunds/constants.ts:18` — `USE_REFUNDS_MOCK = true`. The -real server side (`RefundsController.cs`, `CreateRefundCommand`, `GetCancellationPolicyPreviewQuery`) is live -and correct; the client just never calls it for anything but the bare status read. - -**Root cause — the ×100 bug, exact.** The server's `refundPercentageApplied`/`feePercentage` are **0–100** -values (`GetCancellationPolicyPreviewQuery.Handler.cs:74`: `100m - policy.RefundPercentage`). The client's -own type comment claims **0–1** (`refunds/types.ts:110-112`) — true of the *mock* (`MOCK_POLICY_TIERS`, -`refunds/constants.ts:45-52`, uses `1 | 0.5 | 0`) but not the real server. The render function -(`CancellationPolicyDisclosure.tsx:19-21,36-37`) does `Math.round(fraction * 100)` — correct by luck against -the mock's 0–1 scale, but against the real server's `100.00` it produces **"10000%"**. - -Important nuance: the actual IRR amounts shown are server-supplied strings passed straight through — **not** -multiplied by 100 themselves. It's specifically the percent-chip labels that are 100× too large. - -## The fix - -1. Flip `USE_REFUNDS_MOCK` to `false` — the real endpoints are live and correct. -2. Fix the scale mismatch at the contract level, not just the one render site: type - `CancellationPolicyPreview.refundPercentageApplied`/`feePercentage` as 0–100 (matching the real server), - fix `CancellationPolicyDisclosure.tsx`'s `toPercent` to stop re-multiplying an already-0–100 value, and fix - the mock (`mockApi.ts:127-128`, `MOCK_POLICY_TIERS`) to also emit 0–100 so mock and real can never - silently disagree on scale again. Check `refunds/types.ts:167,201` for other render sites reusing the same - field before considering this fully closed. -3. Same cross-mock-import problem as BNPL (phase 05): `refunds/apis/mockApi.ts:3` imports directly from the - (real) `bookings` mock store — moot once step 1 lands. diff --git a/mvp/blocker-phases/09-nurse-verification-badge.md b/mvp/blocker-phases/09-nurse-verification-badge.md deleted file mode 100644 index 08eb875..0000000 --- a/mvp/blocker-phases/09-nurse-verification-badge.md +++ /dev/null @@ -1,44 +0,0 @@ -# Phase 09 — Nurse verification badge doesn't reflect reality - -**Blocker:** blockers.md § "Trust — nurse verification." -**Depends on:** nothing to start; do together with [10-search-dedup-and-trust.md](10-search-dedup-and-trust.md) — same root cause on two of the four call sites. - ---- - -**Root cause.** `client/src/services/verification/constants.ts:9` — `USE_VERIFICATION_MOCK = true`. The real -backend is fully built and shape-matched: `GET nurse_verification` → `NurseVerificationController.cs:32-35`; -`GET nurses/{id}/trust_badge` (anonymous) → `NursesController.cs:26-29`; all admin queue/decide endpoints -(`AdminVerificationsController.cs`) match the client's `DecideStepInput` fields exactly. - -## Two real gaps that must ship in the same change as flipping the flag, or the admin half 404s - -- `verificationClientApi.approveVerification`/`.rejectVerification` (`clientApi.ts:269-278`) POST to - `admin_verifications/{id}/approve`/`/reject` — **routes that don't exist.** They're wired into the live UI - (`admin/verification/[nurseId]/page.tsx:81-82`), so this is a real break, not theoretical. This is also - exactly the gap forgotten-features.md calls out for nurse approval. The real - workflow already achieves the same outcome via `Decide` on the last pending step - (`AdminReviewStepCommand.Handler.cs:72` → `VerificationAggregator.Finalize`) — so either add the two - explicit endpoints (thin wrappers that resolve "the last pending step" and call the same finalize path), or - rewire the two admin buttons to call `decideStep` directly. Not a business-rule question — pure engineering - gap. -- `getDocumentSignedUrl` (`clientApi.ts:255-256`) targets `admin_verifications/documents/{id}/url` — also - missing; the case detail already carries a signed URL per document, so a re-open should just refetch the - case instead. - -## Small DTO gap, currently harmless - -Server's `VerificationStepDto.IsRequired` has no client-side -equivalent (`verification/types.ts:62-71`), and `blockingSteps` is computed from *all* non-passed steps, not -just required ones (`VerificationRepository.cs:138-141`) — fine while every seeded step is required; would -misrepresent "what's blocking" the day an optional step type is added. Worth a note, not urgent. - -**Fix:** flip `USE_VERIFICATION_MOCK` to `false`, ship the two missing/rewired admin actions in the same -change. - -## Same-symptom bugs, actually filed under Search — fix together since they're one root cause - -Hardcoded literals, not the mock: `search/nurse/[nurseId]/page.tsx:173` and -`NurseResultCard.tsx:87` both hardcode `state="verified"` instead of deriving it from the real -`profile.isVerified` / index-guarantee data. `verification/types.ts:328-330` already has the exact helper -built for this (`publicBadgeState`) — neither call site uses it. See -[10-search-dedup-and-trust.md](10-search-dedup-and-trust.md) for the rest of that phase. diff --git a/mvp/blocker-phases/10-search-dedup-and-trust.md b/mvp/blocker-phases/10-search-dedup-and-trust.md deleted file mode 100644 index b6a2217..0000000 --- a/mvp/blocker-phases/10-search-dedup-and-trust.md +++ /dev/null @@ -1,33 +0,0 @@ -# Phase 10 — Search results aren't de-duplicated, and trust info is hardcoded - -**Blocker:** blockers.md § "Search." -**Depends on:** nothing to start; do together with [09-nurse-verification-badge.md](09-nurse-verification-badge.md) — same root cause on the badge half. - ---- - -## 10a. Not de-duplicated — architectural, not accidental - -`NurseSearchIndex.cs:7-11` documents itself as -"one flat row per (bookable variant × covered service area)." `SqlNurseSearch.SearchAsync:20-59` filters/ -sorts/paginates the raw rows with no `GroupBy(NurseId)` — `total` is a row count, so pagination itself is -row-counted. This is a stated, accepted fact of the contract client-side too -(`search/types.ts:14-15`), not previously flagged as a bug. - -**Fix:** move the dedup into `SqlNurseSearch.SearchAsync` itself (must be SQL-layer, since page size needs to -be nurse-counted, not row-counted, for correct pagination) — group by `NurseId` inside the existing filter -chain, pick one representative row per nurse (e.g. lowest matching price, keep the existing rating sort), -count/paginate over the grouped set. `NurseSearchResultDto` needs a shape change to represent "one card, N -matching services." - -## 10b. Trust info on the result card is hardcoded, and an unverified nurse's profile can show as verified - -Both `NurseResultCard.tsx:87` and `search/nurse/[nurseId]/page.tsx:173` hardcode `state="verified"` instead of -deriving it from real data — the exact same bug as phase 09's "badge doesn't reflect reality," just a -different call site. `verification/types.ts:328-330`'s `publicBadgeState()` helper already exists for this -and isn't used by either. Fix both call sites in the same change as phase 09's verification-badge fix. - -**Flag, don't guess:** whether an anonymous customer should be able to view an unverified nurse's profile at -all (vs. a clean 404) isn't stated anywhere in `mvp/`/`archive/product/`. Note this is a **display** bug, not -a money-safety bypass — `CreateBookingRequestCommand.Handler.cs:56-57` independently gates actual booking -creation on `IsVerified && IsAcceptingBookings`, so an unverified nurse reached by direct link can be shown -misleadingly but not actually booked. diff --git a/mvp/blocker-phases/11-nurse-payouts.md b/mvp/blocker-phases/11-nurse-payouts.md deleted file mode 100644 index b9fb692..0000000 --- a/mvp/blocker-phases/11-nurse-payouts.md +++ /dev/null @@ -1,72 +0,0 @@ -# Phase 11 — Nurse pay / payouts - -**Blocker:** blockers.md § "Nurse pay," and closes §B.6 (real bank-transfer rail) as a side effect — see the -note at the end. -**Depends on:** nothing to start; benefits from [01-admin-rbac.md](01-admin-rbac.md) to actually exercise the -admin-side actions with the seeded `finance` account. - ---- - -Three distinct sub-issues, all independently real. - -## 11a. The mock hides four already-real endpoints - -`client/src/services/payouts/constants.ts:13` — -`USE_PAYOUTS_MOCK = true`, with a stale comment claiming 3 of 4 nurse reads are server gaps. All four are -real (`NursePayoutsController.cs:27-48`, fully implemented handlers, and `clientApi.ts:162-271` already -correctly calls them). The mock/comment simply predate the server catching up (server phase merged after the -client wrote the mock). - -**Fix:** flip `USE_PAYOUTS_MOCK` to `false`. Two secondary defects surface immediately and should land in the -same change: `clientApi.ts:60` hardcodes `failureReason: null` though the wire carries it; and -`previewPayoutBatch` (`clientApi.ts:205-227`) computes money client-side (sums, fabricates a processing -date) — violates the client's own hard rule 18 ("the client never computes money"). The real fix needs a -server-side preview endpoint (already tracked as REQ-036); until that lands, this is a documented gap, not -something to silently patch client-side. - -## 11b. No client entry point for the one irreversible action - -The real, correct implementation already -exists: `AdminPayoutsController.cs:49-52`, `POST admin_payouts/batches/{id}/process` → -`ExecutePayoutBatchCommand.Handler.cs` (submits to `IBankTransferProvider`, posts the ledger, nets clawbacks, -idempotent). **`PayoutsApi` has no method for it at all** — grepped the whole client, nothing calls `/process`. -The admin UI's scary-looking "Run" button with a typed confirmation -(`admin/payouts/page.tsx:220-235`, `useRunPayoutBatch.ts`) actually calls **Generate** -(`POST admin_payouts/batches`, draft-only, moves no money) — the UX implies it sends money; it doesn't. A -generated batch sits in `draft` forever with no way to advance it. - -**Fix:** add `processPayoutBatch(batchId)` to `PayoutsApi` (both `clientApi.ts`/`mockApi.ts`), a -`useProcessPayoutBatch` hook, and a distinct, clearly-separate "Process" action on -`admin/payouts/[batchId]/page.tsx` (gated on `caps.canPayout`). Relabel the existing "Run"/"payout_run" copy -so it's honestly "Generate a draft batch," not "send money." Bundle in the same change: `POST -admin_payouts/{id}/mark_failed` also exists server-side with no client op — add the console action to record -a manually-reconciled bank rejection (this is also the forgotten-features.md item -"no way to confirm a payout succeeded or failed"). - -*(This is also where blockers.md §B.6's "mock the bank-transfer payment pages for now" mostly resolves -itself: `IBankTransferProvider`/`MockBankTransferProvider` already exist server-side — the "mock" is already -built and DI-registered. What's actually missing is exactly 11a/11b/11c — the client UI in front of that -existing mock. No new bank-transfer mock needs to be invented; de-mocking + adding the process/mark-failed -actions above is the whole of it.)* - -## 11c. "Paid" is set at link-time, not bank-confirmation-time - -`PayoutRepository.cs:279`, inside `DeriveEarningsState`: -```csharp -if (payout is not null) - return NurseEarningsState.Paid; -``` -`payout` is non-null as soon as `GeneratePayoutBatchCommand` links the booking to a batch -(`GeneratePayoutBatchCommand.Handler.cs:107-112`) — while `NursePayout.Status` still defaults to `Pending`. -The struct carries `Status` already; it's fetched and never read here. - -**Fix:** gate on `payout.Status == PayoutStatus.Paid` instead — the definition the codebase already treats as -authoritative elsewhere (`NursePayoutLinkStatusService.cs:22-33`). For the mocked bank rail this'll be set at -`process` time; for a real async rail it should only flip via the reconciliation webhook -(`WebhooksPayoutsController.cs:33-43`). - -**Flag:** once fixed, a linked-but-unpaid booking falls into **no** bucket at all in the 4-state summary model -(`pending`/`eligible`/`paid`/`clawback_applied` — none currently represents "batched, awaiting settlement"). -Needs a product decision: add a state, or fold it into `eligible`. Separately flagged (needs its own look, -not necessarily the same root cause): the earnings-balance buckets and the raw ledger net don't reconcile on -a live probe — which one is authoritative isn't documented; don't pick one silently. diff --git a/mvp/blocker-phases/12-patient-records.md b/mvp/blocker-phases/12-patient-records.md deleted file mode 100644 index 6039e1c..0000000 --- a/mvp/blocker-phases/12-patient-records.md +++ /dev/null @@ -1,48 +0,0 @@ -# Phase 12 — Patient records & visit notes are fake demo data - -**Blocker:** blockers.md § "Patient records & visit notes." -**Depends on:** nothing to start, but items #3/#4 below need a product decision made before you flip the mock -flag — don't skip straight to "turn off the mock," it will break on day one exactly as blockers.md warns. - ---- - -**Root cause.** `client/src/services/patientRecords/constants.ts:13` — `USE_PATIENT_RECORDS_MOCK = true`, with -a stale comment claiming "no backend at all." The backend is fully built and every route the client calls -already exists (`PatientCareRecordsController.cs`) — confirmed 1:1 against -`archive/docs/integration/domains/patient-records.md`. - -## Six distinct shape/behavior mismatches, ranked by severity - -1. **Silent data-wipe (fix first — this is the dangerous one).** - `UpsertCarePlanCommand.Handler.cs:27-45` always **fully replaces** all three lists - (medications/routine/tasks) from the request — no merge with what's stored. The client's - `EditableTabs.save()` (`patients/[id]/record/page.tsx:173-198`) **saves one tab at a time**, sending only - that tab's field and omitting the other two from the JSON entirely. Today this "works" only because the - **mock** does a partial merge (`mockApi.ts:183-191`) — against the real endpoint, saving one medication - would silently erase every routine item and task for that patient on the next save. Structurally identical - to the address-edit bug (phase 03). - *Fix (pick one, it's a design choice more than a bug fix):* (a) client always sends the full current plan - on every save — smaller, safer, matches "PUT = full replace" semantics the integration doc already asserts - — or (b) server only replaces a list when its field is non-null, preserving the rest. Lean toward (a). -2. **Id types.** Client ids are `string` (`'m1'`, `newTempId()` = `` `new-${Date.now()}` ``); wire ids are - `long`. Stringify/parse at the boundary; **omit** the id for new items rather than inventing one. -3. **Medication fields don't match.** Client has structured `doseAmount`/`doseUnit` + - `frequencyCode`/`frequencyText` + `timeOfDay: TimeOfDayCode[]` (added post-hoc in a later UI pass); server - `MedicationDto` only has one free-text `Dosage`, one required `Frequency` string, and **no** `timeOfDay` - field at all. **Flag — genuine product decision, not inferable from code:** either build the richer schema - server-side to match the UI, or simplify the UI back to the server's simpler shape. The richer client UI - was built after the backend shipped, targeting a table that was never added to match. -4. **RoutineItem `timeOfDay` is multi-select client-side, single `string?` server-side.** Same - decision as #3 — array support needs a backend field, or the UI needs to drop to single-select. -5. **`taskResults` is a stale mapper bug, not a contract gap.** The real `CareRecordDto` already serves - `TaskResults: [{label, done}]` — an exact structural match to the client's own type — but the client's - `toVisitNote` (`clientApi.ts`) hardcodes `taskResults: []` on read, and on write it folds the checklist - into a free-text summary line instead of sending the wire's native `taskResults` array - (`WritePatientCareRecordCommand.cs:12-16` already has a first-class `TaskResultsJson` column for this). - *Fix:* stop discarding on read, send the array directly on write. Pure bug, not a design choice. -6. **`RecordAccess.deniedReason` enum mismatch.** Client union is `'no_access' | 'not_found'`; server actually - sends `'not_authorized'` for the denied-not-found-owner case (`PatientAccess.cs:19-20`). Fix the client - union to match the real value (the archived integration doc is also stale here — trust the code). - -**Fix order:** #1 first (safety), then #2/#6 (mechanical), then resolve #3/#4 as a product decision before -flipping the mock flag, then #5 (independent bug fix, can land anytime). diff --git a/mvp/blocker-phases/13-booking-lifecycle.md b/mvp/blocker-phases/13-booking-lifecycle.md deleted file mode 100644 index 4a27cb8..0000000 --- a/mvp/blocker-phases/13-booking-lifecycle.md +++ /dev/null @@ -1,45 +0,0 @@ -# Phase 13 — Booking lifecycle - -**Blocker:** blockers.md § "Booking lifecycle." -**Depends on:** 13a's edge case needs a product decision (see below); 13b should use whatever timezone -approach you land on in [04-payment-timezone.md](04-payment-timezone.md). - ---- - -## 13a. Stuck partial-missed bookings - -The domain's own comment -(`BookingStatus.cs:19`) states `Completed` means "every session is completed/cancelled/missed" — implying a -mixed completed+missed booking should reach `Completed`. The only place that actually checks this is -`CheckOutVisitCommand.Handler.cs:64-70` (`allSettled` check, runs after a real check-out). The no-show sweep -(`DetectNoShowSessionsCommand.Handler.cs:25-76`, run by `NoShowSweepJob`) marks individual sessions `Missed` -but **never re-checks `allSettled`** afterward — so a booking with one completed session and the rest -auto-missed later gets permanently stuck at `InProgress`, and `PayoutRepository`'s eligibility query only -considers `Status == Completed`, so the nurse's completed-session payout never becomes eligible. An admin can -manually rescue this specific case (`TransitionBookingStatusCommand.Handler.cs:43-55` allows `InProgress → -Completed`), but nothing does it automatically. - -**Fix:** extract the `allSettled` check from `CheckOutVisitCommand.Handler.cs:64-70` into a shared helper, call -it from `DetectNoShowSessionsCommand.Handler.cs` after marking sessions `Missed`, per affected booking in that -sweep batch. - -**Flag, don't guess:** what should happen to a booking where *zero* sessions were ever completed (all missed -straight from `Confirmed`)? `BookingTransitions.cs` has no `Confirmed → Completed` edge today, and `Cancel` -is explicitly refused as a substitute (`TransitionBookingStatusCommand.Handler.cs:28-29`). Nothing in -`mvp/`/`archive/product/` answers this — it needs a product decision (does it still reach `Completed` with a -zero payout, or some other terminal state?) before writing the edge-case code. - -## 13b. "Today's visits" is unfiltered by default - -`ListSessionsForNurseQuery` documents itself as -"today's visits by default," but `BookingRepository.cs:185-192` only applies the date filter when `date` is -explicitly passed — both real client call sites (`nurse/visits/page.tsx:26`, -`NurseDashboardScreen.tsx:85`) omit it, so the entire history returns. The **mock** already implements the -intended default correctly (`mockApi.ts:438`: `date ?? isoDate(0)`). - -**Fix:** default `request.Date` to "today" via `IDateTimeProvider` inside the handler/repository when null, -mirroring the mock. - -**Flag:** "today" needs a timezone decision (Iran local date vs. UTC day boundary) — there's no -`Asia/Tehran`-aware date logic anywhere in the codebase yet (same class of gap as -[04-payment-timezone.md](04-payment-timezone.md)). Resolve consistently with that fix, not ad hoc here. diff --git a/mvp/blocker-phases/14-partner-center.md b/mvp/blocker-phases/14-partner-center.md deleted file mode 100644 index f336611..0000000 --- a/mvp/blocker-phases/14-partner-center.md +++ /dev/null @@ -1,45 +0,0 @@ -# Phase 14 — Partner / business-center accounts - -**Blocker:** blockers.md § "Partner / business-center accounts." Largest single phase in this folder — mostly -new server surface. -**Depends on:** nothing structurally, but exercising it end-to-end benefits from -[01-admin-rbac.md](01-admin-rbac.md) for the staff-authorized paths. - ---- - -Two distinct sub-issues. - -## 14a. No real "which center am I" resolution - -`client/src/services/partnerCenter/constants.ts:10,13` — -`USE_PARTNER_MOCK = true`, `MOCK_MY_CENTER_ID = 1` — every signed-in caller gets center #1. The "real" path -(`clientApi.ts:105`, `GET centers/me`) targets a route that **doesn't exist anywhere server-side.** The only -partner portal route is `GET centers/{id}/dashboard` (`CentersController.cs:16-26`), which requires already -knowing the id — it authorizes, but never resolves one. `IPartnerCenterRepository` has no reverse -`userId → centerId` lookup at all. - -**Fix:** add a reverse lookup to `IPartnerCenterRepository` (the join shape already exists in -`GetDashboardAsync`, `PartnerCenterRepository.cs:114-144`), a `GetMyPartnerCenterQuery` keyed off -`ICurrentUser.UserId`, and `GET api/v1/centers/me` on `CentersController`. - -**Flag:** should this same signal also close the separately-tracked REQ-038 (surfacing "you administer a -center" on `/me` so nav/routing can auto-detect the role)? Product call, not required for the fix itself. - -## 14b. Booking list and settlement report have zero server endpoint - -Confirmed by exhaustive search — no -query/command exists anywhere for "list a center's sponsored bookings" or "list a center's settlement -invoices." The only thing that exists is bare counts on `CenterDashboardDto`. There's no invoice *list* -capability anywhere in the codebase (only a single-by-booking-id read), and even that single read -(`GetInvoiceQuery.Handler.cs:25-27`) authorizes only `Admin` or the booking's own customer — a center owner -reading its own issued invoice currently 404s. The client is fully built against these phantom routes -(`partner/bookings/page.tsx`, `partner/settlement/page.tsx` — real, complete UIs with nothing to call). - -**Fix:** new `ListSponsoredBookingsQuery` (join `Booking` → `NurseProfile.PartnerCenterId`) and -`ListCenterInvoicesQuery` (`Invoice.PartnerCenterId == centerId`), each authorized like -`GetCenterDashboardQuery` (owning admin or staff), routed under `centers/me/bookings` and -`centers/me/settlement` to match what the client already expects. - -**Flag, don't guess:** should a non-merchant-of-record center (settlement runs through Balinyaar) get read -access to platform-issued invoices for its sponsored bookings? The client already UI-special-cases this -(`settlement_not_mor` state) but the data question isn't answered anywhere in `mvp/`/`archive/product/`. diff --git a/mvp/blocker-phases/15-debug-mode-production.md b/mvp/blocker-phases/15-debug-mode-production.md deleted file mode 100644 index af0bf6d..0000000 --- a/mvp/blocker-phases/15-debug-mode-production.md +++ /dev/null @@ -1,54 +0,0 @@ -# Phase 15 — Turn off developer/debug mode on the live site - -**Blocker:** blockers.md §B.2. Do this deliberately, on its own, right before any real stranger is let near -the site — it touches deploy config and secrets handling, not app logic, and has real prerequisites. -**Depends on:** nothing code-wise, but bundle with credential rotation (blockers.md §B.1) when you do it — -currently deferred per your own call, revisit before real users arrive. - ---- - -**Root cause.** The exposure (`GET api/v1/dev/last_otp/{phone}`, -unauthenticated by design, guarded only by `environment.IsDevelopment()`) is correctly implemented in code -— the entire gap is that the deployed `docker-compose.yml` sets `ASPNETCORE_ENVIRONMENT: Development`, -**deliberately and already documented** (a comment in the compose file and a full section in `DEPLOY.md` -spell out why and what depends on it). - -This is **not** a one-line flip — five real prerequisites, already documented in `DEPLOY.md`, need to land -together: - -1. Create `server/src/API/Baya.Web.Api/appsettings.Production.json` with real (non-`not-for-production` - sentinel) `IdentitySettings:SecretKey`/`Encryptkey` — `StartupSecretsGuard` rejects the sentinel outside - Development, so a bare env-var flip with the current file would refuse to boot. - **`Seams:FieldEncryption:Key`/`:HashKey` must be copied byte-identical** from the current Development - value — never regenerated (root `CLAUDE.md` rule 6 — this key decrypts all existing PII). -2. Set `ASPNETCORE_ENVIRONMENT: Production` in `docker-compose.yml`. -3. Switch migrations from boot-time auto-apply (Development-only branch) to the documented one-shot - (`... migrate`) deploy step — and confirm the demo/lifecycle seed data the shared DB currently relies on - survives the transition, since those seeders stop running automatically post-flip. -4. Swap `Seams:Sms:Provider` from `telegram` (a broadcast-to-a-fixed-chat-list relay, fine for trusted - internal testers, not for strangers) to `kavenegar` with a real API key — flipping only the environment - variable closes the `/dev/last_otp` leak but leaves OTPs going out over the Telegram relay instead of to - the real phone. -5. Bundle with credential rotation (blockers.md §B.1) when you do it — the same config file carries the DB - password alongside the encryption keys. - -**Flag:** whether to keep the Telegram relay live in parallel with `kavenegar` for internal/test accounts -post-launch, or retire it entirely, is a product/ops call, not answered by the code. - ---- - -## Not phases — noted here so they aren't lost, but no code to write - -- **§B.1 (rotate committed credentials)** — per your call, skipped for now (private repo, single developer). - Revisit before any real user touches the deployed site; bundle with step 5 above when you do. -- **§B.3 (legal review of Terms/Privacy)** — not a code task; needs an actual legal review. -- **§B.4 (مودیان e-invoicing)** — per your call, not built now. When you pick it up: the invoice data and tax - math are already correct; the only missing piece is the actual government-facing registration call, and it - needs a chosen provider/gateway before any adapter code makes sense. Add a note to forgotten-features.md - saying exactly that, rather than building a speculative seam with nothing real to point it at. -- **§B.5 (e-namad certification)** — a business/paperwork step, not a code change; also gates the real - ZarinPal card-payment path ([07-card-payment-redirect.md](07-card-payment-redirect.md)) from ever going - fully live, independent of any code fix. -- **§B.6 (real bank-transfer payout rail)** — per your call, stays mocked. Resolved by - [11-nurse-payouts.md](11-nurse-payouts.md) — the seam already exists server-side; only the client UI in - front of it was missing.