Skip to content

Commit

Permalink
Merge pull request Expensify#33605 from barros001/patch-parse-phone-n…
Browse files Browse the repository at this point in the history
…umber2

fix: treat certain US numbers as invalid
  • Loading branch information
madmax330 authored Jan 10, 2024
2 parents b4a9e28 + 1ebf097 commit eeb428e
Show file tree
Hide file tree
Showing 13 changed files with 104 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ const restrictedImportPaths = [
importNames: ['TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight'],
message: "Please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from 'src/components/Pressable' instead.",
},
{
name: 'awesome-phonenumber',
importNames: ['parsePhoneNumber'],
message: "Please use '@libs/PhoneNumber' instead.",
},
{
name: 'react-native-safe-area-context',
importNames: ['useSafeAreaInsets', 'SafeAreaConsumer', 'SafeAreaInsetsContext'],
Expand Down
2 changes: 1 addition & 1 deletion src/libs/LocalePhoneNumber.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';
import {parsePhoneNumber} from './PhoneNumber';

let countryCodeByIP: number;
Onyx.connect({
Expand Down
2 changes: 1 addition & 1 deletion src/libs/LoginUtils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import {PUBLIC_DOMAINS} from 'expensify-common/lib/CONST';
import Str from 'expensify-common/lib/str';
import Onyx from 'react-native-onyx';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {parsePhoneNumber} from './PhoneNumber';

let countryCodeByIP: number;
Onyx.connect({
Expand Down
8 changes: 4 additions & 4 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable no-continue */
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import lodashGet from 'lodash/get';
import lodashOrderBy from 'lodash/orderBy';
Expand All @@ -17,6 +16,7 @@ import ModifiedExpenseMessage from './ModifiedExpenseMessage';
import Navigation from './Navigation/Navigation';
import Permissions from './Permissions';
import * as PersonalDetailsUtils from './PersonalDetailsUtils';
import * as PhoneNumber from './PhoneNumber';
import * as ReportActionUtils from './ReportActionsUtils';
import * as ReportUtils from './ReportUtils';
import * as TaskUtils from './TaskUtils';
Expand Down Expand Up @@ -129,7 +129,7 @@ Onyx.connect({
* @return {String}
*/
function addSMSDomainIfPhoneNumber(login) {
const parsedPhoneNumber = parsePhoneNumber(login);
const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(login);
if (parsedPhoneNumber.possible && !Str.isValidEmail(login)) {
return parsedPhoneNumber.number.e164 + CONST.SMS.DOMAIN;
}
Expand Down Expand Up @@ -1333,7 +1333,7 @@ function getOptions(
let recentReportOptions = [];
let personalDetailsOptions = [];
const reportMapForAccountIDs = {};
const parsedPhoneNumber = parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue)));
const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue)));
const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase();

// Filter out all the reports that shouldn't be displayed
Expand Down Expand Up @@ -1843,7 +1843,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 isValidPhone = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(searchValue)).possible;

const isValidEmail = Str.isValidEmail(searchValue);

Expand Down
43 changes: 43 additions & 0 deletions src/libs/PhoneNumber.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// eslint-disable-next-line no-restricted-imports
import {parsePhoneNumber as originalParsePhoneNumber} from 'awesome-phonenumber';
import type {ParsedPhoneNumber, ParsedPhoneNumberInvalid, PhoneNumberParseOptions} from 'awesome-phonenumber';
import CONST from '@src/CONST';

/**
* Wraps awesome-phonenumber's parsePhoneNumber function to handle the case where we want to treat
* a US phone number that's technically valid as invalid. eg: +115005550009.
* See https://github.com/Expensify/App/issues/28492
*/
function parsePhoneNumber(phoneNumber: string, options?: PhoneNumberParseOptions): ParsedPhoneNumber {
const parsedPhoneNumber = originalParsePhoneNumber(phoneNumber, options);
if (!parsedPhoneNumber.possible) {
return parsedPhoneNumber;
}

const phoneNumberWithoutSpecialChars = phoneNumber.replace(CONST.REGEX.SPECIAL_CHARS_WITHOUT_NEWLINE, '');
if (!/^\+11[0-9]{10}$/.test(phoneNumberWithoutSpecialChars)) {
return parsedPhoneNumber;
}

const countryCode = phoneNumberWithoutSpecialChars.substring(0, 2);
const phoneNumberWithoutCountryCode = phoneNumberWithoutSpecialChars.substring(2);

return {
...parsedPhoneNumber,
valid: false,
possible: false,
number: {
...parsedPhoneNumber.number,

// mimic the behavior of awesome-phonenumber
e164: phoneNumberWithoutSpecialChars,
international: `${countryCode} ${phoneNumberWithoutCountryCode}`,
national: phoneNumberWithoutCountryCode,
rfc3966: `tel:${countryCode}-${phoneNumberWithoutCountryCode}`,
significant: phoneNumberWithoutCountryCode,
},
} as ParsedPhoneNumberInvalid;
}

// eslint-disable-next-line import/prefer-default-export
export {parsePhoneNumber};
2 changes: 1 addition & 1 deletion src/libs/ValidationUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import {addYears, endOfMonth, format, isAfter, isBefore, isSameDay, isValid, isWithinInterval, parse, parseISO, startOfDay, subYears} from 'date-fns';
import {URL_REGEX_WITH_REQUIRED_PROTOCOL} from 'expensify-common/lib/Url';
import isDate from 'lodash/isDate';
Expand All @@ -10,6 +9,7 @@ import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import * as CardUtils from './CardUtils';
import DateUtils from './DateUtils';
import * as LoginUtils from './LoginUtils';
import {parsePhoneNumber} from './PhoneNumber';
import StringUtils from './StringUtils';

/**
Expand Down
2 changes: 1 addition & 1 deletion src/pages/DetailsPage.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
Expand All @@ -23,6 +22,7 @@ import withLocalize, {withLocalizePropTypes} from '@components/withLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import {parsePhoneNumber} from '@libs/PhoneNumber';
import * as ReportUtils from '@libs/ReportUtils';
import * as UserUtils from '@libs/UserUtils';
import * as Report from '@userActions/Report';
Expand Down
2 changes: 1 addition & 1 deletion src/pages/EnablePayments/AdditionalDetailsStep.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import {subYears} from 'date-fns';
import PropTypes from 'prop-types';
import React from 'react';
Expand All @@ -17,6 +16,7 @@ import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsDefaultPro
import withLocalize, {withLocalizePropTypes} from '@components/withLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import {parsePhoneNumber} from '@libs/PhoneNumber';
import * as ValidationUtils from '@libs/ValidationUtils';
import AddressForm from '@pages/ReimbursementAccount/AddressForm';
import * as PersonalDetails from '@userActions/PersonalDetails';
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ProfilePage.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
Expand Down Expand Up @@ -27,6 +26,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import Navigation from '@libs/Navigation/Navigation';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import {parsePhoneNumber} from '@libs/PhoneNumber';
import * as ReportUtils from '@libs/ReportUtils';
import * as UserUtils from '@libs/UserUtils';
import * as ValidationUtils from '@libs/ValidationUtils';
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ReimbursementAccount/CompanyStep.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
Expand All @@ -20,6 +19,7 @@ import TextLink from '@components/TextLink';
import withLocalize from '@components/withLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import compose from '@libs/compose';
import {parsePhoneNumber} from '@libs/PhoneNumber';
import * as ValidationUtils from '@libs/ValidationUtils';
import * as BankAccounts from '@userActions/BankAccounts';
import CONST from '@src/CONST';
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Wallet/Card/GetPhysicalCardPhone.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import PropTypes from 'prop-types';
import React from 'react';
Expand All @@ -9,6 +8,7 @@ import TextInput from '@components/TextInput';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import FormUtils from '@libs/FormUtils';
import {parsePhoneNumber} from '@libs/PhoneNumber';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down
2 changes: 1 addition & 1 deletion src/pages/signin/LoginForm/BaseLoginForm.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {useIsFocused} from '@react-navigation/native';
import {parsePhoneNumber} from 'awesome-phonenumber';
import Str from 'expensify-common/lib/str';
import PropTypes from 'prop-types';
import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
Expand All @@ -25,6 +24,7 @@ import * as ErrorUtils from '@libs/ErrorUtils';
import isInputAutoFilled from '@libs/isInputAutoFilled';
import Log from '@libs/Log';
import * as LoginUtils from '@libs/LoginUtils';
import {parsePhoneNumber} from '@libs/PhoneNumber';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ValidationUtils from '@libs/ValidationUtils';
import Visibility from '@libs/Visibility';
Expand Down
43 changes: 43 additions & 0 deletions tests/unit/PhoneNumberTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import {parsePhoneNumber} from '@libs/PhoneNumber';

describe('PhoneNumber', () => {
describe('parsePhoneNumber', () => {
it('Should return valid phone number', () => {
const validNumbers = [
'+1 (234) 567-8901',
'+12345678901',
'+54 11 8765-4321',
'+49 30 123456',
'+44 20 8759 9036',
'+34 606 49 95 99',
' + 1 2 3 4 5 6 7 8 9 0 1',
'+ 4 4 2 0 8 7 5 9 9 0 3 6',
'+1 ( 2 3 4 ) 5 6 7 - 8 9 0 1',
];

validNumbers.forEach((givenPhone) => {
const parsedPhone = parsePhoneNumber(givenPhone);
expect(parsedPhone.valid).toBe(true);
expect(parsedPhone.possible).toBe(true);
});
});
it('Should return invalid phone number if US number has extra 1 after country code', () => {
const validNumbers = ['+1 1 (234) 567-8901', '+112345678901', '+115550123355', '+ 1 1 5 5 5 0 1 2 3 3 5 5'];

validNumbers.forEach((givenPhone) => {
const parsedPhone = parsePhoneNumber(givenPhone);
expect(parsedPhone.valid).toBe(false);
expect(parsedPhone.possible).toBe(false);
});
});
it('Should return invalid phone number', () => {
const invalidNumbers = ['+165025300001', 'John Doe', '123', '[email protected]'];

invalidNumbers.forEach((givenPhone) => {
const parsedPhone = parsePhoneNumber(givenPhone);
expect(parsedPhone.valid).toBe(false);
expect(parsedPhone.possible).toBe(false);
});
});
});
});

0 comments on commit eeb428e

Please sign in to comment.