[PAY 8/19] Fix how validation displays in UI

[Beamanator]

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

---

Expected Result:

Front-end validation should catch obviously invalid phone numbers, and display the error message "Please enter a phone number including the country code e.g +447814266907" instead of a clickable OptionRow. Like this:

Actual Result:

No visual feedback given to user when OptionRow with invalid phone number is clicked, only an error is shown in the console:

Action Performed:

1. Click search icon
2. Enter an invalid phone number (example: 7777)
3. Click on the OptionRow that shows up:

Workaround:

No workaround (other than making sure you enter valid phone numbers), but not a deploy-blocking issue.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.0.46.0
Notes/Photos/Videos:
Enhancements that can we worked on concurrently by the assigned engineer of this issue or worked on a week after this closes by someone else #2936

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/159892

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @alexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @mallenexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[Beamanator]

Unassigning @alexpensify , I don't believe triage is needed :D

[MelvinBot]

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

[alev374]

Hello Client!
It's one day job for me. I can add the validation function and display error message like you want instead of clickable option row. I will catch the onChangeText event with the correct Debounce value, so that I can validate the phone number each input.
Then error message will be shown if any error has occurred. This can be done in each component or customized component.
It will not affect to the redux action, so it will not require to change the busines logic.
That's all.
I am looking forward your message. I can start to work right now.
Best.
Athur.

[pranshuchittora]

Proposal 📄

Front-end validation should catch obviously invalid phone numbers, and display the error message "Please enter a phone number including the country code e.g +447814266907" instead of a clickable OptionRow. Like this:

Approach 👨🏼‍💻

File of concern : /src/components/OptionsSelector.js

1. We need to validate each input from the user.
   If it’s invalid then you will get this

UUID: "66998340-d01b-49a5-8e6e-92fefad0b531"
code: 666
data: []
htmlMessage: ""
jsonCode: 402
message: "I couldn't validate the phone number, please try again with the country code (e.g. +15005550006)."
requestID: "0a0e4344a600003c13200a6000000001"
title: ""
type: "Expensify\\Error\\User\\InvalidPhoneNumber"

If it’s valid then we can show the option to start the chat
Like this

2. This will increase the number of call to the BE, to fix this we need to debounce the user input which will help in reducing the number of calls.
2. The error state of the input can be handled easily by showing a relevant message to the user based on the API response. Here we are required to show a message which is different from the API response.
For this, we can create an entry in our string store en.js and then use the translate method to resolve the required string.

Illustrative purpose only 👇🏼

// en.js 
error {
message: "Please enter a phone number including the country code e.g +447814266907"
}

// usage
translate("error.message")

Testing Strategy🧪

We can mock the call and purposely fail/invalidate the input and then check for the relevant error message text to have been rendered.

Expected Delivery Time

Approx 2 Days.

Previous Experience

• [Hold - for payment on Wed 26th] Add an About and App download links page to Expensify.cash #2811

[Beamanator]

@alev374 Thanks for your proposal!

Although I believe I understand where you're going with your proposal, I can't quite tell how you plan to solve this issue. Next time, please clarify exactly how you plan to implement the fix. More specifically, "This can be done in each component or customized component" is not clear enough.

Additionally, "It will not affect to the redux action, so it will not require to change the business logic" makes me wonder if you've looked through our code much, as we currently don't use the redux library. Instead, we use the home-grown https://github.com/Expensify/react-native-onyx.

Next time, please try to be more specific with your approach, and maybe think about formatting your thoughts in an easily-consumable format like Pranshu.

[Beamanator]

@pranshuchittora Thanks for your proposal as well!

Thanks for clearly defining the file(s) you plan to change, and the steps you'll take. As you may have noticed, we split the debouncing enhancement into a new issue (found here: #2936) so if you could please adjust your proposal to remove that plan.

Additionally, if an invalid phone number is entered, see in the original post that we want to see the error message "Please enter a phone number including the country code e.g +447814266907", even though the API returns a slightly different error message.

Please let me know when you have adjusted your proposal :)

[pranshuchittora]

Thanks for the feedback @Beamanator. I have updated my proposal.
Good Day:)

[Beamanator]

Thanks for the quick update @pranshuchittora ! Your proposal looks great, please make sure you also submitted a proposal in Upwork so @mallenexpensify can assign you the job, then please submit a PR when you have time :)

[Beamanator]

Contrib linked PR (draft for now), changing to Weekly

[rdjuric]

@Beamanator Pullerbear didn't assign you to review the PR and I think I might need some help/inputs from you there.

Can you take a look at it when you have time?

[Beamanator]

PR was merged but @rdjuric agreed there's one final fix needed, he said he'll open up a PR soon (see this comment)

[rdjuric]

Follow-up PR is ready for review 😃

[Beamanator]

Aaand merged 💪

[mallenexpensify]

Will pay onn 8/19 if no regressions

[Santhosh-Sellavel]

@rdjuric or @Beamanator I'm still able to reproduce this issue.

Untitled.mov

DIGITS_AND_PLUS: /^\+?[0-9]*$/,
In above expression still + is still optional.

E164_REGEX: /^\+?[1-9]\d{1,14}$/ which is used by Str.isValidPhone method,+is also optional here

Also, in the below expression Str.isValidPhone(searchValue) will always be true for just numbers, and by using ! over it makes the below expression always false, so no error message will be returned.

[Santhosh-Sellavel]

@Beamanator
Regex should be updated for DIGITS_AND_PLUS as /^\+(?:[0-9]*)$/

Validation expression should be like below,
if(searchValue && Str.isValidPhone(searchValue) && !CONST.REGEX.DIGITS_AND_PLUS.test(searchValue))

Similar to what I proposed earlier here #2934 (comment), where I suggested E.123 standard regex, instead we could use this /^\+(?:[0-9]*)$/ for just phone number it should have at least one + occurrences.

The reason I suggest E.123 it's future proof

[Beamanator]

@Santhosh-Sellavel did you try clicking on the "new chat" option (+911223233) at the end of your video? If so, you should have seen the "I couldn't validate the phone number, please try again" error -> that's the main fix for this issue. Improving the regex is part of a different issue that I plan to write the requirements for early next week

[Santhosh-Sellavel]

Yes, I'm seeing that error on click,

Sorry, I am unaware of the requirement change here!
There were two PR's that caused me the confusion, where one looks like solving validation message, the initial requirement.

Thanks for the clarity mate! @Beamanator

[Beamanator]

Wrong button 😅 glad we're on the same page now @Santhosh-Sellavel , sorry for the confusion 👍

[rdjuric]

Hi @mallenexpensify! Is there any job in Upwork for this issue? I wasn't hired yet and can't find one

[mallenexpensify]

@rdjuric lotta action in here, let's assume I messed something up.

1. The most recent job post is here https://www.upwork.com/jobs/~018f3274b947f5d7b2
2. Is this you? If so, confirm and I'll hire
   

[rdjuric]

@mallenexpensify That's me! Thanks.

[mallenexpensify]

Hired @rdjuric in Upwork!

[mallenexpensify]

Paid in Upwork!

[Beamanator]

Great! Looks like it's time to close this out :D