Fix IOU request sent to new phone number account #3651
Conversation
|
@HorusGoul looks like you got assigned by PullerBear when @francoisl was already assigned 🤷 feel free to unassign if you want, or add an extra review! 👍 |
| function addSMSDomainIfPhoneNumber(login) { | ||
| if (Str.isValidPhone(login) && !Str.isValidEmail(login)) { | ||
| return login + CONST.SMS.DOMAIN; | ||
| } | ||
| return login; | ||
| } |
There was a problem hiding this comment.
This could go nicely in expensify-common, but I wanted to keep this in one PR since it's a deploy blocker
| if (Str.isValidPhone(login) && !Str.isValidEmail(login)) { | ||
| return login + CONST.SMS.DOMAIN; | ||
| } | ||
| return login; |
There was a problem hiding this comment.
We might prefer to return early, but NAB
There was a problem hiding this comment.
Or even make it a ternary, but I'm not sure either is more legible than this. I'd say let's keep it like it is now.
|
Tests well for me. I saw the infinite spinner when entering an incorrect mobile number, so I'll create a new issue to handle the Invalid number response. |
Aah good point, I didn't worry too much about that b/c we shouldn't be able to create new users w/ invalid phone numbers after #3124 |
| if (Str.isValidPhone(login) && !Str.isValidEmail(login)) { | ||
| return login + CONST.SMS.DOMAIN; | ||
| } | ||
| return login; |
There was a problem hiding this comment.
Or even make it a ternary, but I'm not sure either is more legible than this. I'd say let's keep it like it is now.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging in version: 1.0.71-1🚀
|
|
🚀 Deployed to production in version: 1.0.73-3🚀
|
Details
In Web-E, the
debtorEmailparam is using the$_WAFDEF['_EMAIL'];rule, which doesn't pass for phone numbers. Therefore, if we pass a basic phone number (ex: +17203334444) to a command using that param (ex:CreateIOUTransaction),debtorEmailwill be wiped.This is currently happening because when we merge personal details with the searchValue (in
getPersonalDetailsForLogins),loginmay not have an SMS domain (for new accounts, there's nopersonalDetails` to merge, so we have to manually add the SMS domain there.Video showing the issue:
Screen.Recording.2021-06-17.at.8.48.42.PM.mov
Fixed Issues
Fixes #3614
Tests
+button@expensify.smsat the endQA Steps
+button@expensify.smsat the endTested On
Screenshots
Web
Screen.Recording.2021-06-17.at.8.33.34.PM.mov
Mobile Web
Screen.Recording.2021-06-17.at.8.35.56.PM.mov
Desktop
Screen.Recording.2021-06-17.at.8.47.31.PM.mov
iOS
Screen.Recording.2021-06-17.at.8.38.34.PM.mov
Android
split-android.mov