Phase 4 checkpoint: chat-screen perf refactor + retryable blast-failure + repo-wide dispose-ref guardrail
Chat-screen performance (customer + mitra): - Parent screens have zero `ref.watch` — only `ref.listen` for side effects - Body extracted into its own `ConsumerStatefulWidget`; AppBar parts split into narrow `.select` consumers (mode, sensitivity, timer) - Per-second timer ticks routed to dedicated providers (`chatRemainingSecondsProvider` + new `mitraChatRemainingSecondsProvider`) so WS `session_tick` frames don't invalidate the rest of the chat state Dispose-in-ref bug fix: - `home_screen.dart`, `payment_screen.dart`, `mitra_chat_screen.dart` — ref-using cleanup moved from `dispose()` to `deactivate()`. Modern Riverpod invalidates `ref` the moment `dispose()` runs; the resulting silent error corrupts the widget-tree finalize and the next screen appears frozen - `halo_lints` package added at repo root with `no_ref_in_dispose` rule to catch this pattern in CI / IDE analysis - `custom_lint` activated in both apps' `analysis_options.yaml` (was installed but never wired in — also brings `riverpod_lint`'s `avoid_ref_inside_state_dispose` online) - CLAUDE.md Pitfalls section added to client_app + mitra_app Phase 4 §3 retryable blast-failure (Option A): - Backend `expirePairingRequest` + all-rejected use `recordIntermediateFailure` instead of `failPaymentSession` so the payment session stays `confirmed` for re-blast - WS `pairing_failed` payload carries `is_terminal: false` on the retryable paths; client parses the flag and exposes `retryBlast()` - "Coba cari lagi" CTA on S7 Timeout now re-blasts on the same payment - Pairing service test updated to reflect the new semantics Customer waiting-payment screen navigation patch: - `_navigateTerminal` uses `Future.microtask` + `addPostFrameCallback` redundancy after a release-mode bug where polling stopped but `context.go` never fired, leaving the screen visually stuck on "menunggu pembayaran" See requirement/resume-2026-05-15.md for next-day pickup checklist (mitra release rebuild + S21 Ultra install + retest is the gating item). Bundles unrelated in-flight Phase 4 §2.x work that was already on disk (ESP screen removal, USP one-time gate scaffolding, bestie-availability public route, OTP service edits, Maestro flow tweaks) — kept together to avoid a partial-rebase mess. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
224
requirement/phase4-esp-removal-usp-gate.md
Normal file
224
requirement/phase4-esp-removal-usp-gate.md
Normal file
@@ -0,0 +1,224 @@
|
||||
# ESP Removal + USP One-Time Gate — Implementation & Test Plan
|
||||
|
||||
> Sub-plan of [phase4-customer-flow-plan.md](phase4-customer-flow-plan.md).
|
||||
> Source-of-truth diagram: [flow_customer.mermaid.md §2](flow_customer.mermaid.md).
|
||||
> Business decision: 2026-05-12 — retire S5 ESP entirely; show S5b USP at most once per user.
|
||||
|
||||
## Scope
|
||||
|
||||
1. Remove the S5 ESP screen + state from `client_app`.
|
||||
2. Rewire `VerifChoiceSheet` to go straight to a `usp_seen?` gate, then USP, then the original next step (S3a for verified, PickMethod for anon).
|
||||
3. Add a `customers.usp_seen` boolean to the backend; expose on `/api/client/me`; add `POST /api/client/usp-seen` to set it.
|
||||
4. Add a Riverpod `uspSeenProvider` backed by `SharedPreferences` with DB sync on login and on dismissal.
|
||||
5. Update Maestro flows + add new flows for the gate behaviour.
|
||||
|
||||
Out of scope: changing USP copy/visuals; reordering USP relative to OTP (business accepted the cross-device first-view edge case).
|
||||
|
||||
---
|
||||
|
||||
## Build order
|
||||
|
||||
The work splits into 5 ordered stages. Backend lands first so the client has a real endpoint to call.
|
||||
|
||||
### Stage 1 — Backend: schema + read + write
|
||||
|
||||
**Files:**
|
||||
- `backend/src/db/migrate.js` — append a Phase-4 ALTER block:
|
||||
```sql
|
||||
ALTER TABLE customers ADD COLUMN IF NOT EXISTS usp_seen BOOLEAN NOT NULL DEFAULT FALSE;
|
||||
```
|
||||
- `backend/src/services/customer.service.js`
|
||||
- Add `usp_seen` to `CUSTOMER_SELECT` so every read includes it.
|
||||
- Add `markCustomerUspSeen(customerId)` — idempotent UPDATE that sets `usp_seen = TRUE` and returns the row via `CUSTOMER_SELECT`.
|
||||
- `backend/src/routes/public/client.auth.routes.js`
|
||||
- Already returns the customer from `getCustomerById` on `/api/client/me`. The new column rides along automatically once it's in `CUSTOMER_SELECT`.
|
||||
- Add `POST /api/client/usp-seen` handler: requires JWT, calls `markCustomerUspSeen`, returns the updated customer. No request body needed.
|
||||
|
||||
**Acceptance:**
|
||||
- New customer row has `usp_seen = false`.
|
||||
- `POST /api/client/usp-seen` flips it; second POST is a no-op (still returns true).
|
||||
- `/api/client/me` response includes `usp_seen` for both first-time and returning users.
|
||||
|
||||
### Stage 2 — client_app: `uspSeenProvider` + DB hydrate
|
||||
|
||||
**New file:** `client_app/lib/features/onboarding/usp_seen_provider.dart`
|
||||
- Async-init `Notifier` (or `AsyncNotifier`) that:
|
||||
- On build, reads SharedPreferences key `usp_seen` (default false).
|
||||
- Exposes `bool get hasSeen` synchronously after init.
|
||||
- `Future<void> markSeen()`:
|
||||
1. Write `true` to SharedPreferences.
|
||||
2. If JWT is present (authProvider state is `AuthAuthenticatedData`), call `POST /api/client/usp-seen` via `ApiClient` — fire-and-forget with logging; don't block UX on the network call.
|
||||
- Add `hydrateFromCustomer(Customer c)` — call from auth bootstrap (e.g. wherever `/api/client/me` is fetched and stored in `AuthAuthenticatedData`). OR-merge: if `c.uspSeen == true`, write `true` to SharedPreferences.
|
||||
|
||||
**Edit:** the auth notifier that already calls `/api/client/me` on app boot — add a call to `uspSeenProvider.hydrateFromCustomer(...)` after the response lands. (Per Explore: `auth_notifier.dart` has the `AuthAuthenticatedData` carrying the profile.)
|
||||
|
||||
**Acceptance:**
|
||||
- Fresh install: provider returns `false`.
|
||||
- After `markSeen()`: provider returns `true`; SharedPreferences key set; backend hit (if auth'd).
|
||||
- Login on a fresh device where DB has `usp_seen=true`: provider returns `true` after auth hydrate completes.
|
||||
|
||||
### Stage 3 — client_app: rewire VerifChoiceSheet → USP gate
|
||||
|
||||
**Edit:** `client_app/lib/features/auth/widgets/verif_choice_sheet.dart`
|
||||
- Replace `routeForVerifChoice()` body. New logic (pseudo):
|
||||
```dart
|
||||
final seen = ref.read(uspSeenProvider).hasSeen;
|
||||
if (choice == VerifChoice.verifWA) {
|
||||
if (seen) context.push('/auth/register'); // straight to S3a
|
||||
else context.push('/onboarding/verif/usp');
|
||||
} else {
|
||||
if (seen) context.push('/payment/method-pick'); // straight to PickMethod
|
||||
else context.push('/onboarding/anon/usp');
|
||||
}
|
||||
```
|
||||
(Exact target routes per existing `router.dart` registrations.)
|
||||
|
||||
**Edit:** `client_app/lib/features/onboarding/screens/usp_screen.dart`
|
||||
- On the primary "Continue" / next CTA tap, `await ref.read(uspSeenProvider.notifier).markSeen()` BEFORE navigating to the next route.
|
||||
- The existing post-USP routing (verified → S3a, anon → PickMethod) stays — the `markSeen()` call just precedes it.
|
||||
|
||||
**Edit:** `client_app/lib/features/auth/screens/otp_screen.dart` (or wherever the OTP-Blocked popup lives) — the fallback to anon path. Currently it pushes ESP; change to the same USP-gate logic above (`uspSeenProvider.hasSeen ? PickMethod : USP`).
|
||||
|
||||
**Acceptance:**
|
||||
- First-time verified flow: VerifChoice "verif WA" → USP (with markSeen on continue) → S3a.
|
||||
- Second-time verified flow: VerifChoice "verif WA" → S3a directly.
|
||||
- First-time anon flow: VerifChoice "tanpa verif" → USP → PickMethod.
|
||||
- Second-time anon flow: VerifChoice "tanpa verif" → PickMethod directly.
|
||||
- OTP-Blocked fallback respects the gate.
|
||||
|
||||
### Stage 4 — client_app: delete ESP
|
||||
|
||||
This is the cleanup step; it intentionally runs *after* the new gate is wired so we never have a moment where the build is broken.
|
||||
|
||||
**Delete:**
|
||||
- `client_app/lib/features/onboarding/screens/esp_screen.dart`
|
||||
- `client_app/lib/features/onboarding/esp_state.dart` (the two `espSelectionProvider` / `espSkippedProvider`)
|
||||
|
||||
**Edit:** `client_app/lib/router.dart`
|
||||
- Remove the two `/onboarding/verif/esp` and `/onboarding/anon/esp` `GoRoute` entries.
|
||||
- Leave the `/onboarding/*` redirect carve-out intact (USP still uses it).
|
||||
|
||||
**Verify:** `flutter analyze` clean — no dangling imports, no orphan references to `EspTopic`, `espSelectionProvider`, `espSkippedProvider`.
|
||||
|
||||
### Stage 5 — Tests
|
||||
|
||||
Detailed below in the **Test plan** section. Updates:
|
||||
- Existing Maestro flows `02_onboarding_verified.yaml` + `03_onboarding_anon.yaml` need to drop ESP steps and add gate-aware assertions.
|
||||
- New Vitest cases for the migration + service + route.
|
||||
- New Maestro flow for the USP-skip-on-second-run case.
|
||||
|
||||
---
|
||||
|
||||
## Data contract
|
||||
|
||||
### `customers.usp_seen` (new column)
|
||||
- Type: `BOOLEAN NOT NULL DEFAULT FALSE`
|
||||
- Set true only on `POST /api/client/usp-seen` or via direct backfill.
|
||||
- Never read by anything except `/api/client/me` and the dedicated POST handler.
|
||||
|
||||
### `POST /api/client/usp-seen`
|
||||
- Auth: JWT required (same middleware as `/me`).
|
||||
- Request: empty body.
|
||||
- Response: 200 with the updated customer object (same shape as `/me`).
|
||||
- Errors: 401 on missing/invalid JWT; 404 if customer row doesn't exist (shouldn't happen in normal flow).
|
||||
- Idempotent: calling twice is fine.
|
||||
|
||||
### `/api/client/me` (extended)
|
||||
- Existing payload + `usp_seen: boolean`.
|
||||
|
||||
### SharedPreferences key (client)
|
||||
- Key: `usp_seen`
|
||||
- Value: `bool` (default `false`).
|
||||
- Owned by `uspSeenProvider`; no other code reads/writes it directly.
|
||||
|
||||
---
|
||||
|
||||
## Test plan
|
||||
|
||||
### Unit tests (Vitest, in `backend/test/`)
|
||||
|
||||
**New file:** `backend/test/customer.usp-seen.test.js`
|
||||
|
||||
| # | Test | Setup | Assert |
|
||||
|---|------|-------|--------|
|
||||
| 1 | Migration default | Insert customer via `createCustomerWithIdentity` with no usp_seen | `getCustomerById(...).usp_seen === false` |
|
||||
| 2 | `markCustomerUspSeen` flips flag | Customer with `usp_seen=false` | After call, row has `usp_seen=true`; return value's `usp_seen` is `true` |
|
||||
| 3 | Idempotent | Customer with `usp_seen=true` | Calling again still returns `usp_seen=true`; no error |
|
||||
| 4 | `POST /api/client/usp-seen` requires auth | No `Authorization` header | 401 |
|
||||
| 5 | `POST /api/client/usp-seen` happy path | Authed customer, `usp_seen=false` | 200, response `usp_seen=true`, DB row `usp_seen=true` |
|
||||
| 6 | `/api/client/me` includes flag | Authed customer | Response has `usp_seen` key (true or false) |
|
||||
|
||||
Target: 6/6 green via `npm test` in `backend/`.
|
||||
|
||||
### Flutter widget/integration tests
|
||||
|
||||
These are not currently a major surface in this repo (Maestro is the main client-side gate). Skip Flutter-level widget tests unless something breaks.
|
||||
|
||||
### Maestro flows (`client_app/.maestro/flows/`)
|
||||
|
||||
**Existing flow updates:**
|
||||
|
||||
- **`02_onboarding_verified.yaml`** — first-time-verified path
|
||||
- Remove: any `assertVisible` / tap targets on ESP chips ("Hubungan", "Lewati").
|
||||
- Update sequence: VerifChoiceSheet → tap "Verif WA Rp2k" → **assert USP visible** → tap continue → S3a WhatsApp input → ...
|
||||
- Add at the start: `runScript: scripts/reset_phone_and_local.js` so the run always starts from a clean state (no `usp_seen` in DB, no SharedPreferences value).
|
||||
|
||||
- **`03_onboarding_anon.yaml`** — first-time-anonymous path
|
||||
- Remove: ESP chip taps / "Lewati" tap.
|
||||
- Update sequence: VerifChoiceSheet → tap "tanpa verif Rp5k" → **assert USP visible** → tap continue → `/payment/method-pick`.
|
||||
- Add reset script at start.
|
||||
|
||||
**New flows / deferrals:**
|
||||
|
||||
The "second-run skip" and "DB hydrate" cases turned out hard to script in Maestro: once a customer logs in (anonymously or with phone), the app is in an authenticated session and the only path back to `VerifChoiceSheet` (where the gate is consulted) is via logout, which clears local SharedPreferences too. Returning-user CTAs go through `BestieChoiceSheet`, not `VerifChoiceSheet`, so the gate is never re-evaluated on the returning path.
|
||||
|
||||
What we *do* have:
|
||||
- Flows `02` and `03` (first run, USP visible, ESP not visible) — covered above.
|
||||
- Vitest 8/8 covers the backend: column default, `markCustomerUspSeen`, route 401/200/403, `/me` payload before + after the flag flips.
|
||||
|
||||
What stays in **manual smoke** (operator-driven, documented in the next section):
|
||||
1. Local flag persists across `stopApp/launchApp` — `adb shell pm clear` should NOT happen between runs; verify USP is skipped on second walk through VerifChoice.
|
||||
2. DB hydrate — pre-seed a customer row with `usp_seen=true` via control center or psql, sign in via phone OTP, verify USP is skipped on first ever appearance.
|
||||
3. OTP-blocked popup — exit via "lanjut tanpa verif" still lands at `/payment/method-pick`. (Pre-USP-gate this was a direct redirect; the gate doesn't fire on this path because USP has already been shown/skipped upstream.)
|
||||
|
||||
**Known blocker (2026-05-12):** flows `02` and `03` had their ESP steps removed and USP gate assertions added, but the runtime can't currently execute them end-to-end. The new `SHome1st` view (Phase 4 Stage 9) wraps the home column in a single Semantics node, so the `"aku mau curhat"` CTA's `text` attribute reads as empty (its label only lives in the parent's merged `accessibilityText`). Maestro's `text:` matcher can't locate the button, blocking the entire flow before USP is even reached. This is a Stage-9 accessibility regression, not an ESP/USP issue — the flow YAML edits are correct and will pass once SHome1st's CTA is wrapped in its own `Semantics(label: '…', button: true)` or given a `Key`.
|
||||
|
||||
### Manual smoke (real device)
|
||||
|
||||
After Maestro is green on emulator, hand-run on the physical Samsung:
|
||||
1. Fresh install → "aku mau curhat" → name → VerifChoice → verif WA → USP (visible) → S3a → OTP → S6 paywall.
|
||||
2. Force-stop → relaunch → "aku mau curhat" → VerifChoice → verif WA → S3a (USP skipped) → ...
|
||||
3. Same but anon path.
|
||||
4. Uninstall + reinstall → login with same phone → "aku mau curhat" → verify USP skipped (DB hydrate proved end-to-end).
|
||||
|
||||
### Visual regression
|
||||
|
||||
`flutter analyze` clean. Spot-check VerifChoiceSheet, USP screen, OTP-Blocked popup in the emulator — no broken navigation, no leftover ESP icon/copy.
|
||||
|
||||
---
|
||||
|
||||
## Rollout & migration
|
||||
|
||||
- Migration is additive (NOT NULL DEFAULT FALSE), safe to run on existing DB. All existing customers come out with `usp_seen=false` — meaning every returning user will see USP one more time on next "aku mau curhat". Business accepted this.
|
||||
- No backfill needed.
|
||||
- Cloud Run rollout: backend first (migration runs on boot), then client_app build.
|
||||
|
||||
## Risks
|
||||
|
||||
1. **Provider init race** — if `uspSeenProvider` is read before SharedPreferences finishes loading, the gate could return false (default) and show USP unnecessarily. Mitigation: use `AsyncNotifier` and gate the VerifChoice navigation on the loaded state, or read SharedPreferences synchronously at app boot before the first VerifChoice render.
|
||||
2. **Network failure on `markSeen()`** — if `POST /usp-seen` fails after local flag is set, DB stays false. Next session uses local (still true) so user UX is fine. On a new device, USP shows once more. Acceptable per the cross-device edge case decision.
|
||||
3. **Two-tab race** — not applicable; mobile app.
|
||||
|
||||
---
|
||||
|
||||
## Done criteria
|
||||
|
||||
- [ ] `customers.usp_seen` column exists in dev DB.
|
||||
- [ ] `POST /api/client/usp-seen` returns 200 for authed call.
|
||||
- [ ] `/api/client/me` payload includes `usp_seen`.
|
||||
- [ ] Vitest 6/6 green on the new test file.
|
||||
- [ ] No `esp_screen.dart` / `esp_state.dart` / `/onboarding/*/esp` routes in client_app.
|
||||
- [ ] `flutter analyze` clean on client_app.
|
||||
- [ ] Maestro: `02`, `03`, `04`, `05`, `06` green on the Client_Phone emulator.
|
||||
- [ ] Manual smoke 1–4 above pass on physical device.
|
||||
- [ ] `TECH_DEBT.md` "S5 ESP screen retired" entry closed / removed once cleanup lands.
|
||||
Reference in New Issue
Block a user