[$125] Web - Expensify Card - Inconsistent behavior of phone number validation for shipping details

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.53-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): nathanmulugetatesting+1943@gmail.com
Issue reported by: Applause Internal Team

Action Performed:

Pre-conditions:

• Workspace is connected with a bank account
• Workspace has a member invited
• Workspace has enabled Expensify card

1. Issue a physical card for the member
2. Sign in as the member
3. Go to the workspace chat and click on add shipping detail page
4. Go through the flow until phone number input page
5. Input this: 8005555555 phone number and click enter
6. Observe the behavior of the error when 1 is prepended
7. Go to settings > Wallet > click on the assigned card under the Assigned cards section
8. Go through the flow until you get to the phone number input page
9. Input 8005555555 and click enter
10. Observe the behavior of the error when you prepend 1
11. Prepend "+" again and observe the behavior of the error

Expected Result:

The phone number validation works the same on both pages

Actual Result:

The phone number validation works inconsistently, when going through the form from the workspace chat page prepending "1" only works fine but when going through the form from the wallet page user has to additionally prepend "+" before "1" in order for the phone number to be accepted.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6643815_1729722869946.bandicam_2024-10-24_01-24-08-489.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021850956027322769046
• Upwork Job ID: 1850956027322769046
• Last Price Increase: 2024-12-30
• Automatic offers:
• NJ-2020 | Contributor | 104756183

Issue Owner

Current Issue Owner: @sobitneupane

[melvin-bot]

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

We think that this bug might be related to #wave-collect - Release 2

[IuliiaHerets]

@garrettmknight FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[NJ-2020]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web - Expensify Card - Inconsistent behavior of phone number validation for shipping details

What is the root cause of that problem?

When we fill the phone number inside the shipping details, we check if the user adds country code then we'll return the current phone number else we will add from the IP

App/src/pages/MissingPersonalDetails/substeps/PhoneNumber.tsx

Lines 38 to 42 in 9508b17

	const phoneNumber = LoginUtils.appendCountryCode(values[INPUT_IDS.PHONE_NUMBER]);
	const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumber);
	if (!parsedPhoneNumber.possible || !Str.isValidE164Phone(phoneNumber.slice(0))) {
	errors[INPUT_IDS.PHONE_NUMBER] = translate('bankAccount.error.phoneNumber');
	}

But inside the wallet page we only check if the user have not yet add the country code, we will return an error

App/src/pages/settings/Wallet/Card/GetPhysicalCardPhone.tsx

Lines 44 to 45 in 9508b17

	if (!LoginUtils.validateNumber(phoneNumberToValidate)) {
	errors.phoneNumber = translate('common.error.phoneNumber');

What changes do you think we should make in order to solve the problem?

We can do the same way like we do inside shipping details, we will check if the user has not yet add the country code then we will the country code form the IP else return the current phone number

const onValidate = (values: OnyxEntry<GetPhysicalCardForm>): OnValidateResult => {
    const {phoneNumber: phoneNumberToValidate = ''} = values ?? {};

    const errors: OnValidateResult = {};
    if (!ValidationUtils.isRequiredFulfilled(phoneNumberToValidate)) {
        errors.phoneNumber = translate('common.error.fieldRequired');
    }

    const phoneNumberWithCountryCode = LoginUtils.appendCountryCode(phoneNumberToValidate);
    const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumberWithCountryCode);

    if (!parsedPhoneNumber.possible || !Str.isValidE164Phone(phoneNumberWithCountryCode.slice(0))) {
        errors.phoneNumber = translate('bankAccount.error.phoneNumber');
    }

    return errors;
};

And we also need to fix the getCurrentRoute function inside GetPhysicalCardUtils, because it will redirect to the phone number page if it's invalid phone number

App/src/libs/GetPhysicalCardUtils.ts

Lines 18 to 20 in 9508b17

	if (!phoneNumber || !LoginUtils.validateNumber(phoneNumber)) {
	return ROUTES.SETTINGS_WALLET_CARD_GET_PHYSICAL_PHONE.getRoute(domain);
	}

to:

const phoneNumberWithCountryCode = LoginUtils.appendCountryCode(phoneNumber ?? '');
const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumberWithCountryCode);

if (!phoneNumber || !parsedPhoneNumber.possible || !Str.isValidE164Phone(phoneNumberWithCountryCode.slice(0))) {
    return ROUTES.SETTINGS_WALLET_CARD_GET_PHYSICAL_PHONE.getRoute(domain);
}

What alternative solutions did you explore? (Optional)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021850956027322769046

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

[twilight2294]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Inconsistent behavior of phone number validation for shipping details

What is the root cause of that problem?

We have different validation function on the get physical cards page:

App/src/pages/settings/Wallet/Card/GetPhysicalCardPhone.tsx

Lines 39 to 45 in 9508b17

	const onValidate = (values: OnyxEntry<GetPhysicalCardForm>): OnValidateResult => {
	const {phoneNumber: phoneNumberToValidate = ''} = values ?? {};
	const errors: OnValidateResult = {};
	if (!LoginUtils.validateNumber(phoneNumberToValidate)) {
	errors.phoneNumber = translate('common.error.phoneNumber');

This doesn't match with the existing logic to verify on shipping page and hence we have this bug.

What changes do you think we should make in order to solve the problem?

I found one more page where we use phone number validation:

App/src/pages/settings/Profile/PersonalDetails/PhoneNumberPage.tsx

Lines 49 to 53 in 444633c

	const validate = useCallback(
	(values: FormOnyxValues<typeof ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM>): FormInputErrors<typeof ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM> => {
	const errors: FormInputErrors<typeof ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM> = {};
	if (!ValidationUtils.isRequiredFulfilled(values[INPUT_IDS.PHONE_NUMBER])) {
	errors[INPUT_IDS.PHONE_NUMBER] = translate('common.error.fieldRequired');

In settings profile page.

This inconsistency is going to increase over time and will be difficult to maintain overtime as we will be needing to change logic as we need in future. So to not cause inconsistency again and to be more streamlined, I propose that we create a common util function common to all the pages where we verify mobile number for shipping and then validate against that util.:

function validatePhoneNumber(phoneNumber: string): string {
    // Check if the phone number field is empty
    if (!ValidationUtils.isRequiredFulfilled(phoneNumber)) {
        return translate('common.error.fieldRequired');
    }

    // Append country code if necessary
    const formattedPhoneNumber = LoginUtils.appendCountryCode(phoneNumber);

    // Parse and validate the phone number
    const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(formattedPhoneNumber);
    if (!parsedPhoneNumber.possible || !Str.isValidE164Phone(formattedPhoneNumber)) {
        return translate('bankAccount.error.phoneNumber');
    }

    // Return an empty string to indicate no error
    return '';
}

and then we can use it consistently in all of our pages:

const validate = useCallback(
    (values: FormOnyxValues<typeof ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM>): FormInputErrors<typeof ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM> => {
        const errors: FormInputErrors<typeof ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM> = {};

        const phoneError = validatePhoneNumber(values[INPUT_IDS.PHONE_NUMBER]);
        if (phoneError) {
            errors[INPUT_IDS.PHONE_NUMBER] = phoneError;
        }

        return errors;
    },
    [translate],
);

This way we are consistent with the shipping number validation flow

What alternative solutions did you explore? (Optional)

[sobitneupane]

Thanks for the proposal, @NJ-2020 and @twilight2294

Were you able to reproduce the issue?

There are several components in the app that use phone numbers, so I think it would be good to review all of them to ensure consistent behavior.

[NJ-2020]

@sobitneupane Yes i was able to reproduce the issue

After checking again, i can confirm that the issue only occurring on wallet get physical card page only

[twilight2294]

There are several components in the app that use phone numbers, so I think it would be good to review all of them to ensure consistent behavior.

Yes i checked, actually there is one place where we use different validation, and that is for the login/signup page, there we shouldn't club the logic as there we allow, non-US numbers, we are good here

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sobitneupane]

Thanks for the proposal @NJ-2020

Proposal from @NJ-2020 looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @chiragsalian, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[chiragsalian]

Proposal LGTM, feel free to create the PR @NJ-2020

[NJ-2020]

PR ready

cc: @sobitneupane

[melvin-bot]

This issue has not been updated in over 15 days. @garrettmknight, @chiragsalian, @sobitneupane, @NJ-2020 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[sobitneupane]

Waiting on @NJ-2020 to make a few changes in the PR.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[jasperhuangg]

We are reverting this PR

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.78-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix wallet phone validation page #52082

If no regressions arise, payment will be issued on 2025-01-02. 🎊

For reference, here are some details about the assignees on this issue:

• @sobitneupane requires payment through NewDot Manual Requests
• @NJ-2020 requires payment automatic offer (Contributor)

[melvin-bot]

@sobitneupane @garrettmknight @sobitneupane The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[NJ-2020]

@sobitneupane Let's discuss the regression issue here since the issue here has been closed

Btw for this issue you mentioned here is a known issue(similar) and also I think this issue is coming not from my PR because this issue is created after my PR was reverted
#54487 (comment)

And also I think we can go for this PR+(reimplement the reverted changes), wdyt?

Thanks

[melvin-bot]

Upwork job price has been updated to $125

[NJ-2020]

Friendly bump @sobitneupane

[sobitneupane]

@NJ-2020 You are right, the issue I mentioned is probably not related to the PR. However, you pointed out here that the original issue is coming from the PR. Were you able to reproduce the issue?

[NJ-2020]

@sobitneupane Based on my comment here #54487 (comment), I couldn't reproduce the issue, But I can see that we're not passing the IP country code(if the user does not provide) inside onSubmit function here

[sobitneupane]

But I can see that we're not passing the IP country code(if the user does not provide) inside onSubmit function #54487 (comment)

@NJ-2020 do you think this could be causing the issue? Could you please provide more details on why you believe it might be responsible for the problem?

[NJ-2020]

@sobitneupane Okay, I will try to reproduce the issue again and find the root cause

[NJ-2020]

@sobitneupane I couldn't reproduce the issue, I've tried using 2 accounts without filling the personal details info

The first account is verified by still I couldn't reproduce the issue (using staging server):

Screen.Recording.2025-01-08.at.17.32.42.2.mov

The second account is not verified but I got another error which is Card data is missing in response(using staging server)

Screen.Recording.2025-01-08.at.17.47.06.1.mov

And also even I reverted my changes the error still happening