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.

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

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

Merged
merged 19 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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
Original file line number Diff line number Diff line change
Expand Up @@ -581,12 +581,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',
characterLimit: ({limit}) => `Exceeds the max length of ${limit} characters`,
hasInvalidCharacter: ({invalidCharacter}) => `Please remove the ${invalidCharacter} from the name field.`,
comma: 'comma',
semicolon: 'semicolon',
firstNameLength: 'First name cannot be longer than 50 characters',
lastNameLength: 'Last name cannot be longer than 50 characters',
containsReservedWord: 'First name cannot contain the words Expensify or Concierge',
hasInvalidCharacter: 'Name cannot contain a comma or semicolon',
},
},
resendValidationForm: {
Expand Down
10 changes: 4 additions & 6 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,12 +581,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',
characterLimit: ({limit}) => `Supera el límite de ${limit} caracteres`,
hasInvalidCharacter: ({invalidCharacter}) => `Por favor elimina ${invalidCharacter} del campo nombre.`,
comma: 'la coma',
semicolon: 'el punto y coma',
firstNameLength: 'El nombre no puede tener más de 50 caracteres',
lastNameLength: 'El apellido no puede tener más de 50 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',
},
},
resendValidationForm: {
Expand Down
33 changes: 18 additions & 15 deletions src/libs/ValidationUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import _ from 'underscore';
import CONST from '../CONST';
import * as CardUtils from './CardUtils';
import * as LoginUtils from './LoginUtils';
import * as Localize from './Localize';

/**
* Implements the Luhn Algorithm, a checksum formula used to validate credit card
Expand Down Expand Up @@ -357,22 +356,25 @@ function doesFailCharacterLimitAfterTrim(maxLength, valuesToBeValidated) {
}

/**
* Checks if input value includes comma or semicolon which are not accepted
* Checks if each provided string includes any commas or semicolons
*
* @param {String[]} valuesToBeValidated
* @returns {String[]}
* @returns {Boolean[]}
*/
function findInvalidSymbols(valuesToBeValidated) {
return _.map(valuesToBeValidated, (value) => {
if (!value) {
return '';
}
let inValidSymbol = value.replace(/[,]+/g, '') !== value ? Localize.translateLocal('personalDetails.error.comma') : '';
if (_.isEmpty(inValidSymbol)) {
inValidSymbol = value.replace(/[;]+/g, '') !== value ? Localize.translateLocal('personalDetails.error.semicolon') : '';
}
return inValidSymbol;
});
function doesContainCommaOrSemicolon(valuesToBeValidated) {
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
return _.map(valuesToBeValidated, value => (value.includes(',') || value.includes(';')));
}

/**
* Checks if the provided string includes any of the provided reserved words
*
* @param {String} value
* @param {String[]} reservedWords
* @returns {Boolean}
*/
function doesContainReservedWord(value, reservedWords) {
const valueToCheck = value.trim().toLowerCase();
return _.some(reservedWords, reservedWord => valueToCheck.includes(reservedWord.toLowerCase()));
}

/**
Expand Down Expand Up @@ -455,5 +457,6 @@ export {
isValidRoomName,
isValidTaxID,
isValidValidateCode,
findInvalidSymbols,
doesContainCommaOrSemicolon,
doesContainReservedWord,
};
4 changes: 3 additions & 1 deletion src/pages/RequestCallPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class RequestCallPage extends Component {
errors.lastName = this.props.translate('requestCallPage.error.lastName');
}

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

if (firstNameLengthError) {
errors.firstName = this.props.translate('requestCallPage.error.firstNameLength');
Expand Down Expand Up @@ -285,6 +285,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 +294,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
67 changes: 20 additions & 47 deletions src/pages/settings/Profile/DisplayNamePage.js
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,58 +55,31 @@ 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;
}

// Check the character limit for first and last name
const characterLimitError = Localize.translateLocal('personalDetails.error.characterLimit', {limit: CONST.FORM_CHARACTER_LIMIT});
const [hasFirstNameError, hasLastNameError] = ValidationUtils.doesFailCharacterLimitAfterTrim(
CONST.FORM_CHARACTER_LIMIT,
const [isFirstNameTooLong, isLastNameTooLong] = ValidationUtils.doesFailCharacterLimitAfterTrim(
CONST.DISPLAY_NAME.MAX_LENGTH,
[values.firstName, values.lastName],
);
this.assignError(errors, 'firstName', hasFirstNameError, characterLimitError);
this.assignError(errors, 'lastName', hasLastNameError, characterLimitError);

return errors;
}
// First we validate the first name field
if (doesFirstNameHaveInvalidCharacters) {
errors.firstName = this.props.translate('personalDetails.error.hasInvalidCharacter');
} else if (isFirstNameTooLong) {
errors.firstName = this.props.translate('personalDetails.error.firstNameLength');
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
} 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
}

/**
* @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;
// Then we validate the last name field
if (doesLastNameHaveInvalidCharacters) {
errors.lastName = this.props.translate('personalDetails.error.hasInvalidCharacter');
} else if (isLastNameTooLong) {
errors.lastName = this.props.translate('personalDetails.error.lastNameLength');
}
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 validateErrors;

return errors;
}

render() {
Expand Down Expand Up @@ -140,6 +111,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 +121,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