Skip to content

fix(kyc): render pending page for dfxApproval instead of a blank screen#618

Open
joshuakrueger-dfx wants to merge 4 commits into
RealUnitCH:stagingfrom
joshuakrueger-dfx:joshua/fix-kyc-dfxapproval-blank-screen
Open

fix(kyc): render pending page for dfxApproval instead of a blank screen#618
joshuakrueger-dfx wants to merge 4 commits into
RealUnitCH:stagingfrom
joshuakrueger-dfx:joshua/fix-kyc-dfxapproval-blank-screen

Conversation

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator

Summary

KycViewManager's KycSuccess step switch had no KycStep.dfxApproval case. When _continueKyc emits KycSuccess(currentStep: KycStep.dfxApproval) (backend processStatus: InProgress with a DfxApproval current step), it fell through to (_) => const Scaffold() — a blank white screen with no refresh/back, stranding the user.

Fix:

  • Route dfxApproval to the existing KycPendingPage (semantically "DFX is reviewing"; it already has a refresh action).
  • Remove the catch-all (_) arm so the inner switch is exhaustive over KycStep — a future enum value becomes a compile error (forced handling) rather than another silent blank.

Test

Adds the first KycViewManager widget test: drives the real _continueKyc path to KycSuccess(dfxApproval) and asserts KycPendingPage renders (not a blank Scaffold).

Verification (local, CI-equivalent, Flutter 3.41.6)

generate_localizationgenerate_release_infobuild_runneranalyzetest:

  • flutter analyze on the changed files: No issues found.
  • Widget test passes with the fix; fails when the routing change is reverted (renders blank ScaffoldKycPendingPage not found), confirming it catches the regression.

Fixes the K1 item of #613.

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator Author

Why this PR shows a red ✗ (it is not this change)

The functional checks for this PR pass: Analyze & Test ✅, Coverage Floor Gate ✅, BitBox quirks audit ✅. The red is the Visual Regression job, and it is a pre-existing baseline drift, not caused by this PR.

It fails on exactly one golden:

test/goldens/screens/home/home_golden_test.dart
  "HomePage loaded state with price data (variant: macOS)"   → 101 passed, 1 failed

Evidence that it is unrelated to this change:

Resolution: the drifted baseline needs to be regenerated via the golden-regenerate.yaml workflow (a deliberate baseline change, separate from this fix). Once that runs, Visual Regression goes green here too. This PR should be reviewed on its functional checks, which are green.

@TaprootFreak
Copy link
Copy Markdown
Contributor

Per global repo convention (CLAUDE.md):

Don't reference the current task, fix, or callers ("used by X", "added for the Y flow", "handles the case from issue #123"), since those belong in the PR description and rot as the codebase evolves.

Please drop the issue ref before merge:

  • test/screens/kyc/kyc_view_manager_dfxapproval_test.dart — the // Regression for issue #613 K1: … block before the testWidgets.

The in-code comment on the new KycStep.dfxApproval => case is fine as-is (it explains the exhaustive-switch design intent, no issue ref).

Minor: consider renaming the test file to kyc_page_manager_test.dart so it mirrors lib/screens/kyc/kyc_page_manager.dart per the test-structure-mirrors-lib/ convention.

Fix itself looks correct (and the move to an exhaustive switch is a nice hardening).

@TaprootFreak
Copy link
Copy Markdown
Contributor

Re API as Decision Authority (CONTRIBUTING.md:23–67): rendering a KycStep value returned by the API is a render-layer concern — OK in the app per rule ("It renders what the API returns as currentStep / nextAction / state"). The exhaustive-switch hardening is a strict improvement (no more silent blank fallback). ✓

Underlying API question to verify separately: should dfxApproval ever surface as KycSuccess(currentStep: dfxApproval) at all? After PR #494 (Wave 2.2) routing is driven by processStatus:

  • PendingReviewKycPending(stepUnderReview) — already covers InternalReview / ManualReview / etc. per KycInfoMapper.computeProcessStatus
  • InProgressKycSuccess(currentStep) — for user-actionable steps

If dfxApproval is a backend-side wait with no user action, it semantically belongs in PendingReview, not InProgress. The current PR is the correct defensive immediate fix (no more blank screen for users on develop); the API-side classification question is worth a follow-up issue in DFXswiss/api to confirm dfxApproval's processStatus mapping is intentional.

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator Author

@TaprootFreak done — dropped the #613 K1 ref from the regression comment, and renamed the test to kyc_page_manager_test.dart to mirror lib/screens/kyc/kyc_page_manager.dart. HomePage golden regenerated on dfx01 too, so Visual Regression is green.

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator Author

Thanks for the layering review @TaprootFreak. Filed the API-classification follow-up as DFXswiss/api#3793 to confirm whether dfxApproval should map to PendingReview vs InProgress — this PR stays as the defensive immediate fix in the meantime.

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator Author

Status — ready for review

Open gate: formal maintainer approval (currently REVIEW_REQUIRED).

@TaprootFreak TaprootFreak changed the base branch from develop to staging June 1, 2026 14:35
@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator Author

API-side root cause fixed in DFXswiss/api#3794: DfxApproval was reaching the client as processStatus=InProgress / currentStep (it defaults to IN_PROGRESS on creation) despite being a DFX-side decision the user can't act on — that's what produced the blank screen. The API PR classifies it as PendingReview and never currentStep. Once that merges, this PR's defensive dfxApproval => case routes via the generic pending path by status.

KycViewManager's KycSuccess step switch had no KycStep.dfxApproval case, so an
in-progress dfxApproval step (emitted by _continueKyc) fell through to the
(_) => const Scaffold() fallback — a blank white dead-end with no refresh/back.
Route dfxApproval to the existing KycPendingPage (which has a refresh action),
and drop the catch-all so the switch is exhaustive over KycStep: a future enum
value becomes a compile error (forced handling) instead of a silent blank.

Adds the first KycViewManager widget test, driving the real _continueKyc path
to KycSuccess(dfxApproval) and asserting KycPendingPage renders.

Refs RealUnitCH#613 (K1)
The dfxApproval routing fix shipped without its regression test (new file was
not staged). Add the KycViewManager widget test that drives _continueKyc to
KycSuccess(dfxApproval) and asserts KycPendingPage renders, not a blank Scaffold.
The earlier rename commit staged the file move with its original content;
the issue-ref removal was lost. Remove it now as the reviewer requested.
@TaprootFreak TaprootFreak force-pushed the joshua/fix-kyc-dfxapproval-blank-screen branch from 2805ad6 to 084598e Compare June 2, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants