[PhoneNumber Verification] Display only valid numbers in user search. #17578
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import Onyx from 'react-native-onyx'; | |
| import lodashOrderBy from 'lodash/orderBy'; | ||
| import lodashGet from 'lodash/get'; | ||
| import Str from 'expensify-common/lib/str'; | ||
| import {parsePhoneNumber} from 'awesome-phonenumber'; | ||
| import ONYXKEYS from '../ONYXKEYS'; | ||
| import CONST from '../CONST'; | ||
| import * as ReportUtils from './ReportUtils'; | ||
|
|
@@ -32,12 +33,6 @@ Onyx.connect({ | |
| callback: val => loginList = _.isEmpty(val) ? {} : val, | ||
| }); | ||
|
|
||
| let preferredLocale; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.NVP_PREFERRED_LOCALE, | ||
|
|
@@ -134,9 +129,9 @@ function getPolicyExpenseReportOptions(report) { | |
| * @return {String} | ||
| */ | ||
| function addSMSDomainIfPhoneNumber(login) { | ||
| const parsedPhoneNumber = parsePhoneNumber(login); | ||
| if (parsedPhoneNumber.possible && !Str.isValidEmail(login)) { | ||
| return parsedPhoneNumber.number.e164 + CONST.SMS.DOMAIN; | ||
| } | ||
| return login; | ||
| } | ||
|
|
@@ -532,8 +527,8 @@ function getOptions(reports, personalDetails, { | |
| let recentReportOptions = []; | ||
| let personalDetailsOptions = []; | ||
| const reportMapForLogins = {}; | ||
| const parsedPhoneNumber = parsePhoneNumber(LoginUtils.appendCountryCode(searchInputValue)); | ||
| const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue; | ||
|
|
||
| // Filter out all the reports that shouldn't be displayed | ||
| const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList( | ||
|
|
@@ -676,25 +671,21 @@ function getOptions(reports, personalDetails, { | |
| const noOptions = (recentReportOptions.length + personalDetailsOptions.length) === 0; | ||
| const noOptionsMatchExactly = !_.find(personalDetailsOptions.concat(recentReportOptions), option => option.login === searchValue.toLowerCase()); | ||
|
|
||
|
Contributor
There was a problem hiding this comment.
@s77rt Reverting this PR fixes the issue. I believe this line is the root cause, with old code we were appending the country code after we validate the phone number. So if the country code is const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue;
Member
There was a problem hiding this comment. Ah so the bug is searching for
Contributor
There was a problem hiding this comment.
The bug is that when you are logged in using a phone number
Member
There was a problem hiding this comment. @fedirjh Yeah I got it, previously we used to append the country code to every input, but now we only do it for valid numbers.
Contributor
There was a problem hiding this comment. Interesting case. I would also think that we don't need to support |
||
| if (searchValue && (noOptions || noOptionsMatchExactly) | ||
| && !isCurrentUser({login: searchValue}) | ||
| && _.every(selectedOptions, option => option.login !== searchValue) | ||
| && ((Str.isValidEmail(searchValue) && !Str.isDomainEmail(searchValue)) || parsedPhoneNumber.possible) | ||
| && (!_.find(loginOptionsToExclude, loginOptionToExclude => loginOptionToExclude.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase())) | ||
| && (searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas)) | ||
| ) { | ||
| userToInvite = createOption([searchValue], personalDetails, null, reportActions, { | ||
| showChatPreviewLine, | ||
| }); | ||
|
|
||
| // If user doesn't exist, use a default avatar | ||
| userToInvite.icons = [{ | ||
| source: ReportUtils.getAvatar('', searchValue), | ||
| name: searchValue, | ||
| type: CONST.ICON_TYPE_AVATAR, | ||
| }]; | ||
| } | ||
|
|
@@ -864,7 +855,7 @@ function getHeaderMessage(hasSelectableOptions, hasUserToInvite, searchValue, ma | |
| return Localize.translate(preferredLocale, 'common.maxParticipantsReached', {count: CONST.REPORT.MAXIMUM_PARTICIPANTS}); | ||
| } | ||
|
|
||
| const isValidPhone = parsePhoneNumber(LoginUtils.appendCountryCode(searchValue)).possible; | ||
|
|
||
| const isValidEmail = Str.isValidEmail(searchValue); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import moment from 'moment'; | ||
| import _ from 'underscore'; | ||
| import {URL_REGEX_WITH_REQUIRED_PROTOCOL} from 'expensify-common/lib/Url'; | ||
| import {parsePhoneNumber} from 'awesome-phonenumber'; | ||
| import CONST from '../CONST'; | ||
| import * as CardUtils from './CardUtils'; | ||
| import * as LoginUtils from './LoginUtils'; | ||
|
|
@@ -297,12 +298,11 @@ function validateIdentity(identity) { | |
| * @returns {Boolean} | ||
| */ | ||
| function isValidUSPhone(phoneNumber = '', isCountryCodeOptional) { | ||
|
Member
There was a problem hiding this comment. To be consist with the recent change, I think it would be better if we benefit from the second param of function isValidUSPhone(phoneNumber = '', isCountryCodeOptional) {
const phone = phoneNumber || '';
const regionCode = isCountryCodeOptional ? CONST.COUNTRY.US : null;
const parsedPhoneNumber = parsePhoneNumber(phone, {regionCode});
return parsedPhoneNumber.possible && parsedPhoneNumber.regionCode === CONST.COUNTRY.US;
}
Contributor
Author
There was a problem hiding this comment. @s77rt Noticed an issue here. We only allow US phone numbers, right? So the number
Member
There was a problem hiding this comment. Where is the error originating from?
Contributor
Author
There was a problem hiding this comment. Isn't a console error, it's the form error. It's on the Step 2 of the VBA Flow, input phone number.
Member
There was a problem hiding this comment. Seems unrelated to this PR. Let this be handled somewhere else.
Contributor
Author
There was a problem hiding this comment. sounds good. I'll raise a bug report in slack to see if we want to tackle this.
Contributor
There was a problem hiding this comment.
That is not technically true. There are some others that are accepted, however currently we are not releasing a public list of them for some internal reasons. We should be opening up to all countries again shortly 😄 |
||
| const phone = phoneNumber || ''; | ||
| const regionCode = isCountryCodeOptional ? CONST.COUNTRY.US : null; | ||
|
|
||
| const parsedPhoneNumber = parsePhoneNumber(phone, {regionCode}); | ||
| return parsedPhoneNumber.possible && parsedPhoneNumber.regionCode === CONST.COUNTRY.US; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import {withOnyx} from 'react-native-onyx'; | |
| import {compose} from 'underscore'; | ||
| import lodashGet from 'lodash/get'; | ||
| import Str from 'expensify-common/lib/str'; | ||
| import {parsePhoneNumber} from 'awesome-phonenumber'; | ||
| import Button from '../../../../components/Button'; | ||
| import FixedFooter from '../../../../components/FixedFooter'; | ||
| import HeaderWithCloseButton from '../../../../components/HeaderWithCloseButton'; | ||
|
|
@@ -68,10 +69,10 @@ function NewContactMethodPage(props) { | |
| }, []); | ||
|
|
||
| const isFormValid = useMemo(() => { | ||
| const phoneLogin = LoginUtils.appendCountryCode(LoginUtils.getPhoneNumberWithoutSpecialChars(login)); | ||
|
Member
There was a problem hiding this comment. We need to fix the bug here as mentioned in this comment #17578 (comment).
Contributor
Author
There was a problem hiding this comment. @s77rt Right now, I've added the check and conversion the the
Member
There was a problem hiding this comment. I think that's enough since the bug is technically on staging too. |
||
|
|
||
| return (Permissions.canUsePasswordlessLogins(props.betas) || password) | ||
| && (Str.isValidEmail(login) || parsePhoneNumber(phoneLogin).possible); | ||
| }, [login, password, props.betas]); | ||
|
|
||
| const submitForm = useCallback(() => { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.