feat(kyc): add the PersonalData step page - #883
Conversation
Registration satisfies PersonalData without the user ever seeing it, so the app never had a page for it. The step re-opens when identification rejects the submitted data as not matching the document - the API fails the completed step and opens a fresh one specifically so the account can correct it. The app could not render that step, so instead of a correction form the user got the unsupported-step failure screen and onboarding dead-ended with no way forward. The page submits the same KycPersonalData shape the endpoint expects, via the generic setData PUT already used by the nationality and settings address/name flows. houseNumber is omitted rather than sent empty because the API joins it onto street. Two existing tests encoded the old behaviour and were updated rather than worked around: one used personalData as its example of an unmapped step name, and the enum guard pinned ten variants.
Review follow-ups. The cubit hardcoded accountType: 'Personal'. The API writes that value unconditionally and, whenever it is Personal, explicitly nulls all six organization columns - so submitting this form from an organization or sole-proprietorship account would have destroyed its organization data and dropped five org-only steps from its required set. The account type is now read from the registration payload the parent cubit already fetches, and the form is refused outright for anything that is not personal. The same payload seeds the form. It is a correction form whose copy asks the user to check their details, and every submit rewrites all eight fields, so shipping it empty forced a from-memory re-entry in which a typo could overwrite data that was already correct. houseNumber is always sent: the form requires it, so the omission branch was unreachable, and an omitted key leaves the stored value unchanged rather than clearing it - the opposite of what a correction form should do. Also: build the body with the existing KycPersonalData/KycAddress models instead of a hand-rolled map, reuse the existing personalData string rather than adding an identical key, drop an unreachable submit guard whose comment misdescribed how the form fields validate, and add the widget + golden tests and README row this page owed.
Second review pass. Send the account type the page actually gated on instead of a hardcoded Personal, so the submitted payload can never disagree with the value that was checked. The gate reads the registration snapshot, which the API never re-syncs from user_data, so a drifted account type would otherwise still reach a destructive submit. Do not seed a phone number PhoneNumberField cannot decompose. It matches a seeded value against its own prefix list and leaves prefix null otherwise, and in that state it never writes the user's edits back - a correction form would have silently re-submitted the rejected number. The prefix list is now a shared const so callers can check it. A missing registration payload is transient, so it gets a refresh surface rather than the terminal screen an unsupported account type gets, mirroring KycLinkWalletPage. Also: catch country-lookup failures rather than letting them escape as uncaught async errors, do not let a late country lookup clobber a pick the user already made, and correct the RealUnitRegistrationInfoDto doc - it claimed userData is null for alreadyRegistered, which is the only state that reaches this step and would have made the feature dead on arrival. Tests: pin both hops of the payload plumbing (each survived mutation before), the account-type gate, the retry surface and the phone-seeding guard.
Mirrors the link-wallet defensive refresh page, which carries its own baseline for the same shape.
Third review pass. The previous pass exported the prefix list and made callers guard against seeding a value the field cannot decompose. That documented the bug as a contract instead of fixing it, and left the other seeding caller - the registration prefill, which is the path that provably carries arbitrary dial codes - in violation of the contract this PR itself introduced. Fix it where it belongs. An undecomposable seed no longer leaves prefix null: the dropdown carries no validator, so Form.validate() passed while updatePhoneNumber() refused to write, and the stale value was submitted instead of what the user typed. Falling back to the first prefix keeps the field editable and lets the number validator block submit until it is re-entered. The caller-side guard and the exported const are gone. Pin the two pass-2 fixes that were silently reversible - the country-lookup catch and the racing-pick guard - and correct two comments that the pass-2 split left describing the old behaviour.
|
Three review passes were needed to reach zero findings. Pass 1 — the page had no widget or golden test, duplicated an existing serializer and i18n key, and carried an unreachable submit guard whose comment misdescribed how the form fields validate. Pass 2 — the serious one. The cubit hardcoded Pass 3 — the previous pass exported the Every guard is mutation-checked. Golden baselines came from |
…andoff The app renders 6 of the 24 step names the API can return; every other one fell to the generic failure page, which carries no actions at all, with the step's raw wire identifier printed into the message. The user was left with nothing to do and nothing useful to tell support. The new page offers a retry and a route to support, and names no step. The retry matters because the state is not always terminal - an internal review completing or a step expiring moves the account on by itself, and re-reading the API is the only way the user finds out. The identifier is gone deliberately: it is an internal enum value, not something a user can act on. The organization refusal on the personal-data step now renders the same page, so there is one answer for 'this step cannot be shown here' instead of two. Covers Recommendation, ResidencePermit and every future step name at once.
…sive gate Review follow-ups on the unsupported-step slice. The title used textTheme.titleMedium, which RealUnitTextStyle.theme does not define - it silently fell back to the Material default instead of h2, and the body copy lacked the neutral500 the sibling status pages use. The regenerated baseline had locked that drift in. Register both new sticky-CTA surfaces in the responsive catalog and cover them across the full device x text-scale matrix. CONTRIBUTING makes an unregistered sticky-CTA surface a blocking finding, and the catalog self-test only validates listed rows, so nothing catches this automatically. The personal-data missing-payload surface was missing too. Reuse the existing contactSupport string rather than shipping a second key with identical text in both locales. Pin the support handoff under a hosted GoRouter: repointing it at a different route previously left the whole suite green. The matrix asserts the CTA's layout but cannot tap it, since pumpPage hosts pages without a router.
Both reviewers landed on the same finding independently. The support CTA was asserted with find.byType, which resolves a widget regardless of visibility, clipping or hit-testability - so the dead-CTA regression the matrix exists to catch passed 178/178. The justifying comment was wrong too: ScrollableActionsLayout scrolls an over-tall action block rather than overflowing, so a CTA pushed off-screen raises nothing for the overflow assertion to see. The excuse did not hold either - this file already had pumpCompletedPage hosting a page under a GoRouter for exactly this reason, 130 lines above the group I added. Mirrored it, and the CTA is now hit-tested for real: moving it out of the sticky block turns 16 of the 35 cells red. Also bump the catalog entry count the two new surfaces changed.
|
Scope grew after the first close-out: the generic unsupported-step handoff was folded in rather than stacked, since it edits the same two switch statements. That slice went through two more review passes. Slice pass 1 — the new page was absent from the responsive-surface catalog, which CONTRIBUTING makes a blocking finding and which nothing catches automatically (the catalog self-test only validates listed rows). It also used Slice pass 2 — both reviewers independently found that the new matrix group's second-CTA assertion was decorative: Every guard is mutation-checked. Golden baselines came from |
1 participant
Removes the KYC onboarding dead ends: adds the missing
PersonalDatastep page, and replaces the generic "cannot be completed in this app" failure screen with an actionable handoff. Covers items 4a and 5 of DFXswiss/api#4556, and the dead-end class first flagged as K1/K5 in #613.Problem
Registration satisfies
PersonalDatawithout the user ever seeing it, so the app has never had a page for it. But the step re-opens when identification rejects the submitted data as not matching the document: the API fails the completed step and opens a fresh one specifically so the account can correct it.The app could not render that step. Instead of a correction form the user got the unsupported-step failure screen — "The current KYC step (PersonalData) cannot be completed in this app" — and onboarding dead-ended with no way forward and no way back. It cannot self-heal: the open step blocks the API from opening any later one.
This is not a corner case. It is the standard identification data-mismatch flow, and it recurs every time a submitted name or address does not match the document.
Change
KycStep.personalData+ the_mapStepNamearm, so the step routes to a page instead of the failure screen.KycPersonalDataPage— first/last name, phone, street, house number, postcode, city, country. Same field widgets, validators and layout as the registration address/personal steps.KycPersonalDataCubitsubmits through the genericsetDataPUT already used by the nationality and settings address/name flows, building the body from the existingKycPersonalData/KycAddressmodels.Guards, and why each exists
accountType, and the API explicitly nulls all six organization columns whenever that value isPersonal(user-data.service.ts, theisPersonalAccountbranch) and drops five org-only steps fromrequiredKycSteps. The page reads the account type from the registration payload and refuses to render for anything but personal; the cubit sends exactly the value the page gated on, so the two cannot drift.KycLinkWalletPage's defensive refresh surface, and has its own golden.CountryFieldissue independentGET /v1/countrys and the service does not de-dupe in-flight calls, so either can win.The generic dead end
_mapStepNamerenders 6 of the 24 step names the API can return. Every other one producedKycUnsupportedStepFailure, which rendered the generic failure page —actions: const [], a true dead end — with the step's raw wire identifier printed into the message. The user had nothing to do and nothing useful to tell support.KycUnsupportedStepPagereplaces it with a retry and a route to support, and names no step. The retry is not decorative: for a step under internal review or one the API advances by itself, re-reading is the only way the user finds out. For a genuinely unrenderable step it re-emits the same state, which is honest — the copy and the support CTA are what move that case forward.The identifier is gone deliberately. It is an internal enum value, and DFX support reads the same step server-side, so nothing diagnosable was lost. The personal-data organization refusal renders this same page, so there is one answer to "this step cannot be shown here" instead of two.
This covers
Recommendation,ResidencePermitand every future step name at once. Prod data shows no RealUnit account currently behindResidencePermit, and only a very small latent population behindRecommendation, so dedicated forms for them would have been the more expensive way to fix less.Shared-widget fix
PhoneNumberFieldleftprefixnull whenever a seeded value did not start with a dial code it offers. Its prefix dropdown carries no validator, soForm.validate()returned true whileupdatePhoneNumber()silently refused to write — the stale number was submitted instead of what the user typed. It now falls back to the first prefix; the number field starts empty, so the validator still blocks submit until it is re-entered.This also fixes the registration prefill (
kyc_registration_page.dart), which seedsdto.phoneNumberunconditionally and is the path that provably carries arbitrary dial codes.Verification
flutter analyze— no issues.flutter test --exclude-tags golden— 4723 passed.kyc_page_managercase pinning both hops of the payload plumbing, and both new sticky-CTA surfaces registered in the responsive catalog with full device × text-scale matrix coverage.PhoneNumberFieldfallback, the handoff page, its retry, its support route, or moving the support CTA out of the sticky block each turns the suite red.golden-regenerate.yamlon the self-hosted runner, never locally. The last regeneration after the shared-widget change committed no baseline, confirming it is behaviour-neutral for every state under test.Two existing tests were updated, not worked around
kyc_cubit_test.dartusedpersonalDataas its stand-in for "a step name with no UI mapping" — that premise is now false, so it usesstatutes.kyc_bitbox_create_wallet_states_test.dartpinned theKycStepenum at ten variants; it now pins eleven.Not in scope
Reporting an unmapped step to telemetry. The app has no runtime SDK —
sentry_dart_pluginis a dev-dependency that only uploads symbols, and there is noSentry.capture*anywhere inlib/. Adding one is a product decision, not something to fold in here. Worth doing: this class of failure is invisible in monitoring today, because every response involved is a 200.