feat(kyc): open the phone country prefix field to any 1-3 digit dial code - #909
Conversation
The registration and personal-data phone field was hard-limited to a closed +41/+49 dropdown, blocking customers with other prefixes (e.g. Austria, Swiss citizens abroad) from registering with their real number. Replace the dropdown with a free-text prefix field, prefilled +41, capped at exactly 2 digits with a fixed non-deletable + prefix (InputDecoration.prefixText). Update the twin test assertions that counted or matched the old dropdown, and drop the now-orphaned phone-prefix-dropdown golden case and its handbook references.
1cac041 to
9d8a61a
Compare
Exactly 2 digits excluded real dial codes (e.g. +1, +423, +420, +7)
and duplicated an existence check that belongs to the API per
CONTRIBUTING ("the API decides") -- the national-number field right
next to it already follows that rule. Widen the format check to 1-3
digits and let the backend decide whether a given code exists.
Also restores +423 to the seeded-value decomposition list (it was
dropped when the field was 2-digit-only) with longest-first matching,
fixing a case where a stored +423... number would silently fall back
to the +41 default on re-entry.
joshuakrueger-dfx
changed the title
The number field already had this test; the prefix field was missing the equivalent, so an empty prefix path was untested.
86557d2 to
f3b5461
Compare
Removing the phone-prefix-dropdown handbook slot in the previous commit dropped the handbook screenshot count by one; the CI check and docs still expected the old count of 279.
|
@marassteiner could you take a look when you have a moment? |
|
EN: Working on this now — job |
marassteiner
left a comment
marassteiner
left a comment
There was a problem hiding this comment.
EN: Approved: no merge-blockers; leftover handbook copy, seed-decomposition, and prefix-test gaps filed as follow-ups.
DE: Freigegeben: keine Merge-Blocker; Handbook-Reste, Prefill-Zerlegung und Prefix-Tests als Folgeissues erfasst.
Details
Recommendation
Approve. Nothing here should stop this PR from merging today. The customer-facing change (any 1–3 digit dial code instead of a closed +41/+49 dropdown) is complete, CI is green, and the remaining notes are documentation, a pre-existing seed-path limitation, and test-coverage gaps.
Merge-blockers
None.
- No production correctness bug introduced by this diff.
- No security / PII issue.
- Format-only client checks; dial-code existence is left to the API (CONTRIBUTING: “the API decides”).
- Handbook mapping + count guard moved 279 → 278 in lockstep;
scripts/assemble-handbook-screenshots.shproduces 278 PNGs and no leftover slot164. - Required checks on this SHA are green: Analyze & Test, Visual Regression, Coverage Floor Gate, BitBox quirks audit, Handbook Build Check. Maestro handbook flows is an expected skip (no
tier3:fulllabel).
Follow-up findings
These do not block merge.
- Handbook leftovers after deleting slot 164 —
docs/screens.md:95still lists164;docs/handbook/de/index.html:5528still says “12 Screens”; three handbook blurbs (:7176,:7935,:7952) still describe a Vorwahl-Dropdown. → #913 - Seed decomposition is still a four-code list — input is free-form, but a stored
+33/+1/+7/+420number still falls back to displayed41and an empty national field (lib/widgets/form/phone_number_field.dart:18). Pre-existing path; this PR already fixed+43/+423. → #914 - Prefix widget tests miss the 1-digit case and the digits-only filter on the prefix field — 2- and 3-digit paths are covered;
_enterAndValidatestill writes only to the national field. → #915 - Semantic interaction with the parallel trunk-0 fix — #910 still strips a leading national
0for every prefix. Harmless on today’s+41/+49surface; wrong if combined with this open prefix for countries where a leading0is significant (e.g.+39). Already tracked. → #912
Local run
Documented start path is flutter run (README “Getting started”, after flutter pub get / localization / build_runner).
This review host has no Flutter/Dart SDK:
$ command -v flutter
flutter: not found
$ command -v dart
dart: not found
Homebrew, ~/.local/bin, and the active package bins also have no flutter/dart. Failure happens before any project file is read, so it cannot come from this diff. The PR does not change pubspec.yaml, the SDK constraint, or the documented start path.
Why this is environmental (not a merge-blocker): the same SHA already passed Analyze & Test in CI (run 31809820535). The host is missing the toolchain; the PR does not use or introduce that toolchain.
What could be exercised locally, was:
$ bash scripts/assemble-handbook-screenshots.sh /tmp/handbook-shots-909
assembled 278 handbook screenshots into /tmp/handbook-shots-909
COUNT=278
No leftover 164 / phone-prefix file in the output. Matches the workflow guard in .github/workflows/handbook-build-check.yaml.
Scope checked
Full diff of #909 vs staging (25cb745 … 29efd85). Two independent passes: conformity/quality against CONTRIBUTING.md, and logic/correctness (format, composition, +423 longest-first, callers, handbook count).
Since #909 the country prefix is a free 1-3 digit field, so the prefix-agnostic strip reaches dial codes whose leading 0 is significant. Measured against libphonenumber-js, the library the API validates with: CH, DE, AT and FR drop a national trunk 0, IT does not -- all seven tested Italian landlines turn invalid once it is stripped. An Italian registration that succeeds today would start failing, which is the same class of break as #905 rather than a fix for it. Limit the strip to +41/+49/+43. +423 deliberately stays out: the dial code is in the seeded-value decomposition list, but Liechtenstein has no national trunk 0. Also repairs a test that still drove the dropdown #909 replaced, and covers each list entry so removing one fails a named test.
3 participants
Summary
hard-limited to a closed
+41/+49dropdown, blocking customers withother prefixes (e.g. Austria, Swiss citizens living abroad) from
registering with their real number.
+41,with a fixed non-deletable
+prefix (InputDecoration.prefixText).The client checks format only (1 to 3 digits) and defers whether a given
dial code actually exists to the API, per CONTRIBUTING ("the API
decides") -- the national-number field right next to it already follows
that rule.
+423(Liechtenstein) to the seeded-value decomposition listwith longest-first matching, so a stored
+423...number is no longersilently misread as
+41on re-entry.(widget count,
find.text) and drops the now-orphanedphone-prefix-dropdown golden case and its handbook references.
Symptom (verbatim): Product owner forwarded a customer-facing report:
Austrian customers and Swiss citizens living abroad cannot register with
their real phone number, because the country-prefix dropdown only offers
+41and+49.Scale: At least one concrete customer report. The affected class is
every registration outside CH/DE -- explicitly named: Austria and Swiss
citizens abroad (potentially any country). The prior dropdown covered 2
of roughly 200 possible international dial codes.
Smaller fix considered: Adding a few more hardcoded entries to the
closed dropdown list (e.g.
+43,+423) was rejected -- it would stillbe a closed, arbitrarily-bounded list needing a follow-up PR for the next
missing country, repeating the same complaint. A full country-code picker
package (search UI, flags) was also rejected as unnecessarily large for
this fix: new dependency, and a second country-list data source parallel
to the existing DFX-API-backed one already used for nationality.
Test plan
flutter test test/widgets/form/phone_number_field_test.dart test/screens/kyc/steps/kyc_personal_data_page_test.dart-- 25/25 green(12 + 13)
without those changes
phone_number_field_test.dartalone without that specific changevalidator's empty check is mutated out
flutter analyzeon all touched Dart files -- no issues+41by default,accepts any prefix from 1 to 3 digits (e.g.
+43,+423),+cannotbe deleted, a 4th digit is rejected
golden-regenerateworkflow(11 PNGs, bytes verified against the bot commit before taking over as a
signed commit) -- all CI checks green: Analyze & Test, Visual Regression,
Build handbook image + container smoke, BitBox quirks audit, Coverage
Floor Gate.
Final pass (25cb745):
Coherent: All 5 commits serve the one title -- open the phone-prefix
field. Commit 1 is the feature itself; commits 2-3 are direct fixes to
findings from two independent reviews of that same feature (dial-code
range, missing required-prefix test); commits 4-5 are the necessary
consequences of the UI change (golden baselines, handbook screenshot
count) -- no unrelated file is touched.
Nothing extra: No new dependency, no country-picker package, no
changes to
dropdown_field.dart/country_field.dartor any of the threePhoneNumberFieldcallers. The smallest fix (dropdown to free-text field)was widened from 2 to 1-3 digits only because review found it excluded
real dial codes and violated CONTRIBUTING's "the API decides" -- not for
speculative completeness.
Sources closed: No linked GitHub issue (reported via a forwarded
customer email, not a tracked issue). All three review channels checked
via API (
pulls/909/reviews,pulls/909/comments,issues/909/comments)-- all empty, no external feedback pending. Two independent judgment
passes (TaprootFreak-catalog reviewer + Conformity/Logic pair) each found
one real issue; both are fixed in commits 2 and 3 respectively and
re-verified.