Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: wrap contact method validate function inside a callback #21281

Merged
merged 4 commits into from
Jun 24, 2023
Merged

feat: wrap contact method validate function inside a callback #21281

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0fd20ae
feat: wrap contact method validate function inside a callback
allroundexperts Jun 21, 2023
5013992
feat: fix lint errors
allroundexperts Jun 21, 2023
66f1c15
feat: add comment for excluding loginList
allroundexperts Jun 22, 2023
a455bfd
feat: move static methods out of the NewContactMethodPage component
allroundexperts Jun 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 46 additions & 39 deletions src/pages/settings/Profile/Contacts/NewContactMethodPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,59 +54,66 @@ const defaultProps = {
loginList: {},
};

function NewContactMethodPage(props) {
const loginInputRef = useRef(null);
const getPhoneLogin = (phoneOrEmail) => {
if (_.isEmpty(phoneOrEmail)) {
return '';
}

const getPhoneLogin = (phoneOrEmail) => {
if (_.isEmpty(phoneOrEmail)) {
return '';
}
return LoginUtils.appendCountryCode(LoginUtils.getPhoneNumberWithoutSpecialChars(phoneOrEmail));
};

return LoginUtils.appendCountryCode(LoginUtils.getPhoneNumberWithoutSpecialChars(phoneOrEmail));
};
const validateNumber = (values) => {
const parsedPhoneNumber = parsePhoneNumber(values);

const validateNumber = (values) => {
const parsedPhoneNumber = parsePhoneNumber(values);
if (parsedPhoneNumber.possible) {
return parsedPhoneNumber.number.e164 + CONST.SMS.DOMAIN;
}

if (parsedPhoneNumber.possible) {
return parsedPhoneNumber.number.e164 + CONST.SMS.DOMAIN;
}
return '';
};

return '';
};
const addNewContactMethod = (values) => {
const phoneLogin = getPhoneLogin(values.phoneOrEmail);
const validateIfnumber = validateNumber(phoneLogin);
const submitDetail = (validateIfnumber || values.phoneOrEmail).trim().toLowerCase();

const validate = (values) => {
const phoneLogin = getPhoneLogin(values.phoneOrEmail);
const validateIfnumber = validateNumber(phoneLogin);
User.addNewContactMethodAndNavigate(submitDetail, values.password);
};

const errors = {};
function NewContactMethodPage(props) {
const loginInputRef = useRef(null);

if (_.isEmpty(values.phoneOrEmail)) {
ErrorUtils.addErrorMessage(errors, 'phoneOrEmail', 'contacts.genericFailureMessages.contactMethodRequired');
}
const validate = React.useCallback(
(values) => {
const phoneLogin = getPhoneLogin(values.phoneOrEmail);
const validateIfnumber = validateNumber(phoneLogin);

if (!_.isEmpty(values.phoneOrEmail) && !(parsePhoneNumber(phoneLogin).possible || Str.isValidEmail(values.phoneOrEmail))) {
ErrorUtils.addErrorMessage(errors, 'phoneOrEmail', 'contacts.genericFailureMessages.invalidContactMethod');
}
const errors = {};

if (!_.isEmpty(values.phoneOrEmail) && lodashGet(props.loginList, validateIfnumber || values.phoneOrEmail.toLowerCase())) {
ErrorUtils.addErrorMessage(errors, 'phoneOrEmail', 'contacts.genericFailureMessages.enteredMethodIsAlreadySubmited');
}
if (_.isEmpty(values.phoneOrEmail)) {
ErrorUtils.addErrorMessage(errors, 'phoneOrEmail', 'contacts.genericFailureMessages.contactMethodRequired');
}

if (!Permissions.canUsePasswordlessLogins(props.betas) && _.isEmpty(values.password)) {
errors.password = 'contacts.genericFailureMessages.passwordRequired';
}
if (!_.isEmpty(values.phoneOrEmail) && !(parsePhoneNumber(phoneLogin).possible || Str.isValidEmail(values.phoneOrEmail))) {
ErrorUtils.addErrorMessage(errors, 'phoneOrEmail', 'contacts.genericFailureMessages.invalidContactMethod');
}

return errors;
};
if (!_.isEmpty(values.phoneOrEmail) && lodashGet(props.loginList, validateIfnumber || values.phoneOrEmail.toLowerCase())) {
ErrorUtils.addErrorMessage(errors, 'phoneOrEmail', 'contacts.genericFailureMessages.enteredMethodIsAlreadySubmited');
}

const addNewContactMethod = (values) => {
const phoneLogin = getPhoneLogin(values.phoneOrEmail);
const validateIfnumber = validateNumber(phoneLogin);
const submitDetail = (validateIfnumber || values.phoneOrEmail).trim().toLowerCase();
if (!Permissions.canUsePasswordlessLogins(props.betas) && _.isEmpty(values.password)) {
errors.password = 'contacts.genericFailureMessages.passwordRequired';
}

User.addNewContactMethodAndNavigate(submitDetail, values.password);
};
return errors;
},
// We don't need `props.loginList` because when submitting this form
// the props.loginList gets updated, causing this function to run again.
Comment on lines +111 to +112
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the props.loginList gets updated? is there a way to prevent this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's from api response.

Screenshot 2023-06-23 at 8 39 40 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.. 😬

@allroundexperts can we move validateNumber, getPhoneLogin and addNewContactMethod out the component please, they are unnecessarily recreating on each render and the first two are used by validate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allroundexperts let's do that and then I think all good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// https://github.com/Expensify/App/issues/20610
// eslint-disable-next-line react-hooks/exhaustive-deps
[],
);

return (
<ScreenWrapper
Expand Down