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

Jump to bottom

Update display name front-end validation to match back-end #14873

Merged
merged 19 commits into from
Feb 14, 2023
Merged

Update display name front-end validation to match back-end #14873

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
afa0442
Add display name constants
puneetlath Feb 6, 2023
29dc612
Add maxLength on display name inputs
puneetlath Feb 6, 2023
7e8f90c
Refactor display name validation to remove assignErrors function
puneetlath Feb 6, 2023
863a0d1
Refactor findInvalidSymbols to be doesContainCommaOrSemicolon
puneetlath Feb 6, 2023
b4ad412
Update copy for display name errorsg
puneetlath Feb 6, 2023
2fc2349
Add reserved word in first name check
puneetlath Feb 6, 2023
411b2bf
Add copy for reserved word error
puneetlath Feb 6, 2023
c281fe0
Use const on request call page
puneetlath Feb 6, 2023
33b015e
Remove no longer used imports
puneetlath Feb 6, 2023
0c1546d
Remove unncessary max length validation
puneetlath Feb 6, 2023
eeb046c
Use constants for error copy
puneetlath Feb 6, 2023
4c539ea
Merge main into branch
puneetlath Feb 7, 2023
9b8ae39
Add localize import
puneetlath Feb 7, 2023
841d8ae
Use this.props.translate instead of Localize
puneetlath Feb 8, 2023
5ea2df6
Remove unused privatePersonalDetails.error.hasInvalidCharacter
puneetlath Feb 8, 2023
05fc161
Merge branch 'main' into reserved-first-names
puneetlath Feb 8, 2023
745867c
Merge main into branch
puneetlath Feb 13, 2023
00ae704
Fully merge es.js
puneetlath Feb 13, 2023
b31fb63
Change doesContainCommaOrSemicolon to isValidDisplayName
puneetlath Feb 14, 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
5 changes: 5 additions & 0 deletions src/CONST.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ const CONST = {
DEFAULT_AVATAR_COUNT: 24,
OLD_DEFAULT_AVATAR_COUNT: 8,

DISPLAY_NAME: {
MAX_LENGTH: 50,
RESERVED_FIRST_NAMES: ['Expensify', 'Concierge'],
},

// Sizes needed for report empty state background image handling
EMPTY_STATE_BACKGROUND: {
SMALL_SCREEN: {
Expand Down
10 changes: 4 additions & 6 deletions src/languages/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,10 @@ export default {
},
personalDetails: {
error: {
firstNameLength: 'First name shouldn\'t be longer than 50 characters',
lastNameLength: 'Last name shouldn\'t be longer than 50 characters',
hasInvalidCharacter: ({invalidCharacter}) => `Please remove the ${invalidCharacter} from the name field.`,
firstNameLength: `First name cannot be longer than ${CONST.DISPLAY_NAME.MAX_LENGTH} characters`,
lastNameLength: `Last name cannot be longer than ${CONST.DISPLAY_NAME.MAX_LENGTH} characters`,
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
containsReservedWord: 'First name cannot contain the words Expensify or Concierge',
hasInvalidCharacter: 'Name cannot contain a comma or semicolon',
},
},
privatePersonalDetails: {
Expand All @@ -603,7 +604,6 @@ export default {
legalLastName: 'Legal last name',
homeAddress: 'Home address',
error: {
hasInvalidCharacter: ({invalidCharacter}) => `Please remove the ${invalidCharacter} from the field above.`,
dateShouldBeBefore: ({dateString}) => `Date should be before ${dateString}.`,
dateShouldBeAfter: ({dateString}) => `Date should be after ${dateString}.`,
},
Expand Down Expand Up @@ -1057,8 +1057,6 @@ export default {
phoneNumberExtension: 'Please enter a valid phone extension number',
firstName: 'Please provide your first name',
lastName: 'Please provide your last name',
firstNameLength: 'First name shouldn\'t be longer than 50 characters',
lastNameLength: 'Last name shouldn\'t be longer than 50 characters',
},
},
requestCallConfirmationScreen: {
Expand Down
10 changes: 4 additions & 6 deletions src/languages/es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,10 @@ export default {
},
personalDetails: {
error: {
firstNameLength: 'El nombre no debe tener más de 50 caracteres',
lastNameLength: 'El apellido no debe tener más de 50 caracteres',
hasInvalidCharacter: ({invalidCharacter}) => `Por favor elimina ${invalidCharacter} del campo nombre.`,
firstNameLength: `El nombre no puede tener más de ${CONST.DISPLAY_NAME.MAX_LENGTH} caracteres`,
lastNameLength: `El apellido no puede tener más de ${CONST.DISPLAY_NAME.MAX_LENGTH} caracteres`,
containsReservedWord: 'El nombre no puede contener las palabras Expensify o Concierge',
hasInvalidCharacter: 'El nombre no puede contener una coma o un punto y coma',
},
},
privatePersonalDetails: {
Expand All @@ -603,7 +604,6 @@ export default {
legalLastName: 'Apellidos',
homeAddress: 'Domicilio',
error: {
hasInvalidCharacter: ({invalidCharacter}) => `Por favor elimina ${invalidCharacter}`,
dateShouldBeBefore: ({dateString}) => `La fecha debe ser anterior a ${dateString}.`,
dateShouldBeAfter: ({dateString}) => `La fecha debe ser posterior a ${dateString}.`,
},
Expand Down Expand Up @@ -1059,8 +1059,6 @@ export default {
phoneNumberExtension: 'Por favor, introduce una extensión telefónica válida',
firstName: 'Por favor, ingresa tu nombre',
lastName: 'Por favor, ingresa tu apellido',
firstNameLength: 'El nombre no debe tener más de 50 caracteres',
lastNameLength: 'El apellido no debe tener más de 50 caracteres',
},
},
requestCallConfirmationScreen: {
Expand Down
46 changes: 12 additions & 34 deletions src/libs/ValidationUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,46 +357,25 @@ function isValidRoutingNumber(number) {
}

/**
* Checks if each string in array is of valid length and then returns true
* for each string which exceeds the limit.
* Checks if each provided string includes any commas or semicolons
*
* @param {Number} maxLength
* @param {String[]} valuesToBeValidated
* @returns {Boolean[]}
*/
function doesFailCharacterLimit(maxLength, valuesToBeValidated) {
return _.map(valuesToBeValidated, value => value && value.length > maxLength);
function doesContainCommaOrSemicolon(valuesToBeValidated) {
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
return _.map(valuesToBeValidated, value => (value.includes(',') || value.includes(';')));
}

/**
* Checks if each string in array is of valid length and then returns true
* for each string which exceeds the limit. The function trims the passed values.
* Checks if the provided string includes any of the provided reserved words
*
* @param {Number} maxLength
* @param {String[]} valuesToBeValidated
* @returns {Boolean[]}
*/
function doesFailCharacterLimitAfterTrim(maxLength, valuesToBeValidated) {
return _.map(valuesToBeValidated, value => value && value.trim().length > maxLength);
}

/**
* Checks if input value includes comma or semicolon which are not accepted
*
* @param {String[]} valuesToBeValidated
* @returns {String[]}
* @param {String} value
* @param {String[]} reservedWords
* @returns {Boolean}
*/
function findInvalidSymbols(valuesToBeValidated) {
return _.map(valuesToBeValidated, (value) => {
if (!value) {
return '';
}
let inValidSymbol = value.replace(/[,]+/g, '') !== value ? Localize.translateLocal('common.comma') : '';
if (_.isEmpty(inValidSymbol)) {
inValidSymbol = value.replace(/[;]+/g, '') !== value ? Localize.translateLocal('common.semicolon') : '';
}
return inValidSymbol;
});
function doesContainReservedWord(value, reservedWords) {
const valueToCheck = value.trim().toLowerCase();
return _.some(reservedWords, reservedWord => valueToCheck.includes(reservedWord.toLowerCase()));
}

/**
Expand Down Expand Up @@ -473,12 +452,11 @@ export {
isValidRoutingNumber,
isValidSSNLastFour,
isValidSSNFullNine,
doesFailCharacterLimit,
doesFailCharacterLimitAfterTrim,
isReservedRoomName,
isExistingRoomName,
isValidRoomName,
isValidTaxID,
isValidValidateCode,
findInvalidSymbols,
doesContainCommaOrSemicolon,
doesContainReservedWord,
};
12 changes: 2 additions & 10 deletions src/pages/RequestCallPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,16 +219,6 @@ class RequestCallPage extends Component {
errors.lastName = this.props.translate('requestCallPage.error.lastName');
}

const [firstNameLengthError, lastNameLengthError] = ValidationUtils.doesFailCharacterLimit(50, [values.firstName, values.lastName]);

if (firstNameLengthError) {
errors.firstName = this.props.translate('requestCallPage.error.firstNameLength');
}

if (lastNameLengthError) {
errors.lastName = this.props.translate('requestCallPage.error.lastNameLength');
}

const phoneNumber = LoginUtils.getPhoneNumberWithoutSpecialChars(values.phoneNumber);
if (_.isEmpty(values.phoneNumber.trim()) || !Str.isValidPhone(phoneNumber)) {
errors.phoneNumber = this.props.translate('common.error.phoneNumber');
Expand Down Expand Up @@ -285,6 +275,7 @@ class RequestCallPage extends Component {
name="fname"
placeholder={this.props.translate('profilePage.john')}
containerStyles={[styles.mt4]}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
/>
<TextInput
inputID="lastName"
Expand All @@ -293,6 +284,7 @@ class RequestCallPage extends Component {
name="lname"
placeholder={this.props.translate('profilePage.doe')}
containerStyles={[styles.mt4]}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
/>
<TextInput
inputID="phoneNumber"
Expand Down
61 changes: 13 additions & 48 deletions src/pages/settings/Profile/DisplayNamePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import lodashGet from 'lodash/get';
import _ from 'underscore';
import React, {Component} from 'react';
import {View} from 'react-native';
import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsPropTypes, withCurrentUserPersonalDetailsDefaultProps} from '../../../components/withCurrentUserPersonalDetails';
import ScreenWrapper from '../../../components/ScreenWrapper';
import HeaderWithCloseButton from '../../../components/HeaderWithCloseButton';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import * as Localize from '../../../libs/Localize';
import ROUTES from '../../../ROUTES';
import Form from '../../../components/Form';
import ONYXKEYS from '../../../ONYXKEYS';
Expand Down Expand Up @@ -57,60 +55,25 @@ class DisplayNamePage extends Component {
*/
validate(values) {
const errors = {};

// Check for invalid characters in first and last name
const [firstNameInvalidCharacter, lastNameInvalidCharacter] = ValidationUtils.findInvalidSymbols(
const [doesFirstNameHaveInvalidCharacters, doesLastNameHaveInvalidCharacters] = ValidationUtils.doesContainCommaOrSemicolon(
[values.firstName, values.lastName],
);
this.assignError(
errors,
'firstName',
!_.isEmpty(firstNameInvalidCharacter),
Localize.translateLocal(
'personalDetails.error.hasInvalidCharacter',
{invalidCharacter: firstNameInvalidCharacter},
),
);
this.assignError(
errors,
'lastName',
!_.isEmpty(lastNameInvalidCharacter),
Localize.translateLocal(
'personalDetails.error.hasInvalidCharacter',
{invalidCharacter: lastNameInvalidCharacter},
),
);
if (!_.isEmpty(errors)) {
return errors;

// First we validate the first name field
if (doesFirstNameHaveInvalidCharacters) {
errors.firstName = this.props.translate('personalDetails.error.hasInvalidCharacter');
} else if (ValidationUtils.doesContainReservedWord(values.firstName, CONST.DISPLAY_NAME.RESERVED_FIRST_NAMES)) {
errors.firstName = this.props.translate('personalDetails.error.containsReservedWord');
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

:issue We should display multiple error messages at the same time if multiple validations fail, as we state in the form guidelines. For example, if I set my first name to Concierge; I should see First name cannot contain the words Expensify or Concierge. Name cannot contain a comma or semicolon..

suggestion: We can easily join the error messages together.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: We can easily join the error messages together.

I'm not sure we do this in any of our other forms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, could you give an example of a form that does this? Because that would require copy variations and conditions for each combination of errors. Or I guess appending the error to errors.firstName instead of replacing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we currently do this in any of our forms. I'll ask in Slack if we want to do this. If not then I'm happy with what we have and we should update the form guidelines.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the Slack discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll make this a NAB comment. If we decide to make the change I'll create a refactor issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright sounds good. I'm going to go ahead and merge then. Thanks for starting the slack discussion @neil-marcellini.

}

// Check the character limit for first and last name
const characterLimitError = Localize.translateLocal('common.error.characterLimit', {limit: CONST.FORM_CHARACTER_LIMIT});
const [hasFirstNameError, hasLastNameError] = ValidationUtils.doesFailCharacterLimitAfterTrim(
CONST.FORM_CHARACTER_LIMIT,
[values.firstName, values.lastName],
);
this.assignError(errors, 'firstName', hasFirstNameError, characterLimitError);
this.assignError(errors, 'lastName', hasLastNameError, characterLimitError);
// Then we validate the last name field
if (doesLastNameHaveInvalidCharacters) {
errors.lastName = this.props.translate('personalDetails.error.hasInvalidCharacter');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We missed out a case here, we shouldn't had allowed the user to submit empty first name, this caused #44167.

Though i agree this is not actually a bug from this PR, but rather a missed logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing it out!


return errors;
}

/**
* @param {Object} errors
* @param {String} errorKey
* @param {Boolean} hasError
* @param {String} errorCopy
* @returns {Object} - An object containing the errors for each inputID
*/
assignError(errors, errorKey, hasError, errorCopy) {
const validateErrors = errors;
if (hasError) {
validateErrors[errorKey] = errorCopy;
}
return validateErrors;
}

render() {
const currentUserDetails = this.props.currentUserPersonalDetails || {};

Expand Down Expand Up @@ -140,6 +103,7 @@ class DisplayNamePage extends Component {
label={this.props.translate('common.firstName')}
defaultValue={lodashGet(currentUserDetails, 'firstName', '')}
placeholder={this.props.translate('displayNamePage.john')}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
/>
</View>
<View>
Expand All @@ -149,6 +113,7 @@ class DisplayNamePage extends Component {
label={this.props.translate('common.lastName')}
defaultValue={lodashGet(currentUserDetails, 'lastName', '')}
placeholder={this.props.translate('displayNamePage.doe')}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
/>
</View>
</Form>
Expand Down
31 changes: 9 additions & 22 deletions src/pages/settings/Profile/PersonalDetails/LegalNamePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {withOnyx} from 'react-native-onyx';
import ScreenWrapper from '../../../../components/ScreenWrapper';
import HeaderWithCloseButton from '../../../../components/HeaderWithCloseButton';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import * as Localize from '../../../../libs/Localize';
import ROUTES from '../../../../ROUTES';
import Form from '../../../../components/Form';
import ONYXKEYS from '../../../../ONYXKEYS';
Expand Down Expand Up @@ -68,34 +67,20 @@ class LegalNamePage extends Component {
const errors = {};

// Check for invalid characters in legal first and last name
const [invalidLegalFirstNameCharacter, invalidLegalLastNameCharacter] = ValidationUtils.findInvalidSymbols(
[values.legalFirstName, values.legalLastName],
);
const [hasLegalFirstNameLengthError, hasLegalLastNameLengthError] = ValidationUtils.doesFailCharacterLimitAfterTrim(
CONST.LEGAL_NAMES_CHARACTER_LIMIT,
const [doesLegalFirstNameHaveInvalidCharacters, doesLegalLastNameHaveInvalidCharacters] = ValidationUtils.doesContainCommaOrSemicolon(
[values.legalFirstName, values.legalLastName],
);

if (!_.isEmpty(invalidLegalFirstNameCharacter)) {
errors.legalFirstName = Localize.translateLocal(
'privatePersonalDetails.error.hasInvalidCharacter',
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
{invalidCharacter: invalidLegalFirstNameCharacter},
);
if (doesLegalFirstNameHaveInvalidCharacters) {
errors.legalFirstName = this.props.translate('personalDetails.error.hasInvalidCharacter');
} else if (_.isEmpty(values.legalFirstName)) {
errors.legalFirstName = Localize.translateLocal('common.error.fieldRequired');
} else if (hasLegalFirstNameLengthError) {
errors.legalFirstName = Localize.translateLocal('common.error.characterLimit', {limit: CONST.LEGAL_NAMES_CHARACTER_LIMIT});
errors.legalFirstName = this.props.translate('common.error.fieldRequired');
}

if (!_.isEmpty(invalidLegalLastNameCharacter)) {
errors.legalLastName = Localize.translateLocal(
'privatePersonalDetails.error.hasInvalidCharacter',
{invalidCharacter: invalidLegalLastNameCharacter},
);
if (doesLegalLastNameHaveInvalidCharacters) {
errors.legalLastName = this.props.translate('personalDetails.error.hasInvalidCharacter');
} else if (_.isEmpty(values.legalLastName)) {
errors.legalLastName = Localize.translateLocal('common.error.fieldRequired');
} else if (hasLegalLastNameLengthError) {
errors.legalLastName = Localize.translateLocal('common.error.characterLimit', {limit: CONST.LEGAL_NAMES_CHARACTER_LIMIT});
errors.legalLastName = this.props.translate('common.error.fieldRequired');
}

return errors;
Expand Down Expand Up @@ -126,6 +111,7 @@ class LegalNamePage extends Component {
name="lfname"
label={this.props.translate('privatePersonalDetails.legalFirstName')}
defaultValue={privateDetails.legalFirstName || ''}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
/>
</View>
<View>
Expand All @@ -134,6 +120,7 @@ class LegalNamePage extends Component {
name="llname"
label={this.props.translate('privatePersonalDetails.legalLastName')}
defaultValue={privateDetails.legalLastName || ''}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
/>
</View>
</Form>
Expand Down