Only check for valid phone number in the deposit bank account flow - #97801
Only check for valid phone number in the deposit bank account flow#97801mountiny wants to merge 2 commits into
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Review — LGTM ✅The change is clean, minimal, and correct. It strictly loosens validation: the phone step previously required a number to pass both Verified:
Two things worth confirming (neither a blocker)
Optional: no unit test was added for the widened behavior (the removed tests covered NANP). A one-line assertion that In-app verification: I attempted to exercise the flow on web, but the tester was blocked before the phone step — EvidenceWallet page with Add bank account buttonValidate your account security-code wall blocking the flow |


Explanation of Change
The phone number step in the deposit bank account flow validated the number twice. It ran
isValidPhoneNumber()on the number with the country code appended, and it also ranisValidNANPPhone()on the E.164 formatted number.isValidNANPPhone()only accepted numbers that parse to a North American Numbering Plan region:US,PR,GU,VI,AS,MP, andCA. This is the international deposit account flow, so the account holder can live anywhere. A user with a valid number such as+44 20 7123 4567sawPlease enter a valid phone numberand could not finish the step.This PR keeps
isValidPhoneNumber()as the only phone check in the step. It uses libphonenumber'spossiblecheck, so it accepts a valid number from any country and still rejects malformed input.isValidNANPPhone()is removed fromValidationUtils, along with its unit tests, because this step was its only caller. The genuinely US only callers, in the EnablePayments wallet flows and the USDReimbursementAccountflow, useisValidUSPhone()and are unchanged.Changed files:
src/pages/AddPersonalBankAccountPage/substeps/PhoneNumberStep.tsx— drop theisValidNANPPhone()check and the E.164 formatting it needed.src/libs/ValidationUtils.ts— removeisValidNANPPhone()and its export.tests/unit/ValidationUtilsTest.ts— remove theisValidNANPPhonedescribe block.Fixed Issues
$ #97806
PROPOSAL:
Tests
+44 20 7123 4567.+1 201 867 5309.+1 416 555 1234.123.Please enter a valid phone numberand the step does not continue.abcdefg.Please enter a valid phone numberand the step does not continue.npm run test -- tests/unit/ValidationUtilsTest.tsand verify that the suite passes.Offline tests
+44 20 7123 4567.123.Please enter a valid phone number.QA Steps
+44 20 7123 4567.+61 2 5550 1234.+1 201 867 5309.123.Please enter a valid phone numberand the step does not continue.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Not yet tested — needs manual QA.
Android: mWeb Chrome
Not yet tested — needs manual QA.
iOS: Native
Not yet tested — needs manual QA.
iOS: mWeb Safari
Not yet tested — needs manual QA.
MacOS: Chrome / Safari
Not yet tested — needs manual QA.