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

Refactor the way of displaying errors in several forms #15978

Merged
15 changes: 7 additions & 8 deletions src/libs/ErrorUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,17 @@ function getLatestErrorMessage(onyxData) {
* @param {Object} errors - An object containing current errors in the form
* @param {String} inputID
* @param {String} message - Message to assign to the inputID errors
* @returns {Object} - An object containing the errors for each inputID
*
*/
function addErrorMessage(errors, inputID, message) {
const errorList = errors;

if (_.isEmpty(errorList[inputID])) {
errorList[inputID] = message;
} else {
errorList[inputID] = `${errorList[inputID]}\n${message}`;
if (message && inputID) {
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
if (_.isEmpty(errorList[inputID])) {
errorList[inputID] = message;
} else {
errorList[inputID] = `${errorList[inputID]}\n${message}`;
}
}

return errorList;
}

export {
Expand Down
5 changes: 3 additions & 2 deletions src/pages/EnablePayments/AdditionalDetailsStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import TextInput from '../../components/TextInput';
import * as Wallet from '../../libs/actions/Wallet';
import * as ValidationUtils from '../../libs/ValidationUtils';
import * as LoginUtils from '../../libs/LoginUtils';
import * as ErrorUtils from '../../libs/ErrorUtils';
import AddressForm from '../ReimbursementAccount/AddressForm';
import DatePicker from '../../components/DatePicker';
import Form from '../../components/Form';
Expand Down Expand Up @@ -125,11 +126,11 @@ class AdditionalDetailsStep extends React.Component {
}

if (!ValidationUtils.isValidPastDate(values[INPUT_IDS.DOB])) {
errors[INPUT_IDS.DOB] = this.props.translate(this.errorTranslationKeys.dob);
ErrorUtils.addErrorMessage(errors, INPUT_IDS.DOB, this.props.translate(this.errorTranslationKeys.dob));
}

if (!ValidationUtils.meetsAgeRequirements(values[INPUT_IDS.DOB])) {
errors[INPUT_IDS.DOB] = this.props.translate(this.errorTranslationKeys.age);
ErrorUtils.addErrorMessage(errors, INPUT_IDS.DOB, this.props.translate(this.errorTranslationKeys.age));
}

if (!ValidationUtils.isValidAddress(values[INPUT_IDS.ADDRESS.street]) || _.isEmpty(values[INPUT_IDS.ADDRESS.street])) {
Expand Down
23 changes: 11 additions & 12 deletions src/pages/ReportSettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import Text from '../components/Text';
import RoomNameInput from '../components/RoomNameInput';
import Picker from '../components/Picker';
import * as ValidationUtils from '../libs/ValidationUtils';
import * as ErrorUtils from '../libs/ErrorUtils';
import OfflineWithFeedback from '../components/OfflineWithFeedback';
import reportPropTypes from './reportPropTypes';
import withReportOrNotFound from './home/report/withReportOrNotFound';
Expand Down Expand Up @@ -116,19 +117,17 @@ class ReportSettingsPage extends Component {
return errors;
}

// The following validations are ordered by precedence.
// First priority: We error if the user doesn't enter a room name or left blank
if (!values.newRoomName || values.newRoomName === CONST.POLICY.ROOM_PREFIX) {
errors.newRoomName = this.props.translate('newRoomPage.pleaseEnterRoomName');
} else if (ValidationUtils.isReservedRoomName(values.newRoomName)) {
// Second priority: Certain names are reserved for default rooms and should not be used for policy rooms.
errors.newRoomName = this.props.translate('newRoomPage.roomNameReservedError');
} else if (ValidationUtils.isExistingRoomName(values.newRoomName, this.props.reports, this.props.report.policyID)) {
// Third priority: Show error if the room name already exists
errors.newRoomName = this.props.translate('newRoomPage.roomAlreadyExistsError');
} else if (!ValidationUtils.isValidRoomName(values.newRoomName)) {
// Fourth priority: We error if the room name has invalid characters
errors.newRoomName = this.props.translate('newRoomPage.roomNameInvalidError');
// We error if the user doesn't enter a room name or left blank
ErrorUtils.addErrorMessage(errors, 'newRoomName', this.props.translate('newRoomPage.pleaseEnterRoomName'));
} else if (values.newRoomName !== CONST.POLICY.ROOM_PREFIX && !ValidationUtils.isValidRoomName(values.newRoomName)) {
// We error if the room name has invalid characters
ErrorUtils.addErrorMessage(errors, 'newRoomName', this.props.translate('newRoomPage.roomNameInvalidError'));
}

if (ValidationUtils.isReservedRoomName(values.newRoomName) || ValidationUtils.isExistingRoomName(values.newRoomName, this.props.reports, values.policyID)) {
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
// Certain names are reserved for default rooms and should not be used for policy rooms.
ErrorUtils.addErrorMessage(errors, 'newRoomName', this.props.translate('newRoomPage.roomNameReservedError'));
}

return errors;
Expand Down
6 changes: 3 additions & 3 deletions src/pages/settings/Profile/DisplayNamePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ class DisplayNamePage extends Component {
* @returns {Object} - An object containing the errors for each inputID
*/
validate(values) {
let errors = {};
const errors = {};

// First we validate the first name field
if (!ValidationUtils.isValidDisplayName(values.firstName)) {
errors = ErrorUtils.addErrorMessage(errors, 'firstName', this.props.translate('personalDetails.error.hasInvalidCharacter'));
ErrorUtils.addErrorMessage(errors, 'firstName', this.props.translate('personalDetails.error.hasInvalidCharacter'));
}
if (ValidationUtils.doesContainReservedWord(values.firstName, CONST.DISPLAY_NAME.RESERVED_FIRST_NAMES)) {
errors = ErrorUtils.addErrorMessage(errors, 'firstName', this.props.translate('personalDetails.error.containsReservedWord'));
ErrorUtils.addErrorMessage(errors, 'firstName', this.props.translate('personalDetails.error.containsReservedWord'));
}

// Then we validate the last name field
Expand Down
23 changes: 11 additions & 12 deletions src/pages/workspace/WorkspaceNewRoomPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import CONST from '../../CONST';
import Text from '../../components/Text';
import Permissions from '../../libs/Permissions';
import Log from '../../libs/Log';
import * as ErrorUtils from '../../libs/ErrorUtils';
import * as ValidationUtils from '../../libs/ValidationUtils';
import Form from '../../components/Form';
import shouldDelayFocus from '../../libs/shouldDelayFocus';
Expand Down Expand Up @@ -82,19 +83,17 @@ class WorkspaceNewRoomPage extends React.Component {
validate(values) {
const errors = {};

// The following validations are ordered by precedence.
// First priority: We error if the user doesn't enter a room name or left blank
if (!values.roomName || values.roomName === CONST.POLICY.ROOM_PREFIX) {
errors.roomName = this.props.translate('newRoomPage.pleaseEnterRoomName');
} else if (ValidationUtils.isReservedRoomName(values.roomName)) {
// Second priority: Certain names are reserved for default rooms and should not be used for policy rooms.
errors.roomName = this.props.translate('newRoomPage.roomNameReservedError');
} else if (ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
// Third priority: We error if the room name already exists.
errors.roomName = this.props.translate('newRoomPage.roomAlreadyExistsError');
} else if (!ValidationUtils.isValidRoomName(values.roomName)) {
// Fourth priority: We error if the room name has invalid characters
errors.roomName = this.props.translate('newRoomPage.roomNameInvalidError');
// We error if the user doesn't enter a room name or left blank
ErrorUtils.addErrorMessage(errors, 'roomName', this.props.translate('newRoomPage.pleaseEnterRoomName'));
} else if (values.roomName !== CONST.POLICY.ROOM_PREFIX && !ValidationUtils.isValidRoomName(values.roomName)) {
// We error if the room name has invalid characters
ErrorUtils.addErrorMessage(errors, 'roomName', this.props.translate('newRoomPage.roomNameInvalidError'));
}

if (ValidationUtils.isReservedRoomName(values.roomName) || ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
// Certain names are reserved for default rooms and should not be used for policy rooms.
ErrorUtils.addErrorMessage(errors, 'roomName', this.props.translate('newRoomPage.roomNameReservedError'));
}

if (!values.policyID) {
Expand Down
64 changes: 64 additions & 0 deletions tests/unit/ErrorUtilsTest.js
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import * as ErrorUtils from '../../src/libs/ErrorUtils';

describe('ErrorUtils', () => {
test('should add a new error message for a given inputID', () => {
const errors = {};
ErrorUtils.addErrorMessage(errors, 'username', 'Username cannot be empty');

expect(errors).toEqual({username: 'Username cannot be empty'});
});

test('should append an error message to an existing error message for a given inputID', () => {
const errors = {username: 'Username cannot be empty'};
ErrorUtils.addErrorMessage(errors, 'username', 'Username must be at least 6 characters long');

expect(errors).toEqual({username: 'Username cannot be empty\nUsername must be at least 6 characters long'});
});

test('should add an error to input which does not contain any errors yet', () => {
const errors = {username: 'Username cannot be empty'};
ErrorUtils.addErrorMessage(errors, 'password', 'Password cannot be empty');

expect(errors).toEqual({username: 'Username cannot be empty', password: 'Password cannot be empty'});
});

test('should not mutate the errors object when message is empty', () => {
const errors = {username: 'Username cannot be empty'};
ErrorUtils.addErrorMessage(errors, 'username', '');

expect(errors).toEqual({username: 'Username cannot be empty'});
});

test('should not mutate the errors object when inputID is null', () => {
const errors = {username: 'Username cannot be empty'};
ErrorUtils.addErrorMessage(errors, null, 'InputID cannot be null');

expect(errors).toEqual({username: 'Username cannot be empty'});
});

test('should not mutate the errors object when message is null', () => {
const errors = {username: 'Username cannot be empty'};
ErrorUtils.addErrorMessage(errors, 'username', null);

expect(errors).toEqual({username: 'Username cannot be empty'});
});

test('should add multiple error messages for the same inputID', () => {
const errors = {};
ErrorUtils.addErrorMessage(errors, 'username', 'Username cannot be empty');
ErrorUtils.addErrorMessage(errors, 'username', 'Username must be at least 6 characters long');
ErrorUtils.addErrorMessage(errors, 'username', 'Username must contain at least one letter');

expect(errors).toEqual({username: 'Username cannot be empty\nUsername must be at least 6 characters long\nUsername must contain at least one letter'});
});

test('should append multiple error messages to an existing error message for the same inputID', () => {
const errors = {username: 'Username cannot be empty\nUsername must be at least 6 characters long'};
ErrorUtils.addErrorMessage(errors, 'username', 'Username must contain at least one letter');
ErrorUtils.addErrorMessage(errors, 'username', 'Username must not contain special characters');
TMisiukiewicz marked this conversation as resolved.
Show resolved Hide resolved

expect(errors).toEqual(
{username: 'Username cannot be empty\nUsername must be at least 6 characters long\nUsername must contain at least one letter\nUsername must not contain special characters'},
);
});
});