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

Highlight all errors together in BankAccount step #5041

Merged
merged 7 commits into from
Sep 3, 2021
1 change: 1 addition & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ export default {
industryCode: 'Please enter a valid industry classification code. Must be 6 digits.',
restrictedBusiness: 'Please confirm company is not on the list of restricted businesses',
routingNumber: 'Please enter a valid Routing Number',
accountNumber: 'Please enter a valid Account Number',
companyType: 'Please enter a valid Company Type',
tooManyAttempts: 'Due to a high number of login attempts, this option has been temporarily disabled for 24 hours. Please try again later or manually enter details instead.',
address: 'Please enter a valid address',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ export default {
industryCode: 'Ingrese un código de clasificación de industria válido',
restrictedBusiness: 'Confirme que la empresa no está en la lista de negocios restringidos',
routingNumber: 'Ingrese un número de ruta válido',
accountNumber: 'Ingrese un número de cuenta válido',
companyType: 'Ingrese un tipo de compañía válido',
tooManyAttempts: 'Debido a la gran cantidad de intentos de inicio de sesión, esta opción se ha desactivado temporalmente durante 24 horas. Vuelva a intentarlo más tarde o introduzca los detalles manualmente.',
address: 'Ingrese una dirección válida',
Expand Down
21 changes: 19 additions & 2 deletions src/libs/actions/BankAccounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,17 @@ function hideBankAccountErrorModal() {
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {isErrorModalVisible: false});
}

/**
* Set the current fields with errors.
*
* @param {String} errors
*/
function setBankAccountFormValidationErrors(errors) {
// We set 'errors' to null first because we don't have a way yet to replace a specific property like 'errors' without merging it
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {errors: null});
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {errors});
marcaaron marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Set the current error message.
*
Expand Down Expand Up @@ -690,6 +701,7 @@ function setupWithdrawalAccount(data) {
const currentStep = newACHData.currentStep;
let achData = lodashGet(response, 'achData', {});
let error = lodashGet(achData, CONST.BANK_ACCOUNT.VERIFICATIONS.ERROR_MESSAGE);
const errors = {};

if (response.jsonCode === 200 && !error) {
// Save an NVP with the bankAccountID for this account. This is temporary since we are not showing lists
Expand Down Expand Up @@ -787,7 +799,7 @@ function setupWithdrawalAccount(data) {
if (response.message === CONST.BANK_ACCOUNT.ERROR.MISSING_ROUTING_NUMBER
|| response.message === CONST.BANK_ACCOUNT.ERROR.MAX_ROUTING_NUMBER
) {
error = translateLocal('bankAccount.error.routingNumber');
errors.routingNumber = true;
achData.subStep = CONST.BANK_ACCOUNT.SETUP_TYPE.MANUAL;
} else if (response.message === CONST.BANK_ACCOUNT.ERROR.MISSING_INCORPORATION_STATE) {
error = translateLocal('bankAccount.error.incorporationState');
Expand All @@ -806,6 +818,10 @@ function setupWithdrawalAccount(data) {
// Go to next step
goToWithdrawalAccountSetupStep(nextStep, achData);

if (_.size(errors)) {
setBankAccountFormValidationErrors(errors);
showBankAccountErrorModal();
}
if (error) {
showBankAccountFormValidationError(error);
showBankAccountErrorModal(error);
Expand All @@ -819,7 +835,7 @@ function setupWithdrawalAccount(data) {
}

function hideBankAccountErrors() {
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {error: '', existingOwnersList: ''});
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {error: '', existingOwners: [], errors: null});
}

function setWorkspaceIDForReimbursementAccount(workspaceID) {
Expand All @@ -842,5 +858,6 @@ export {
hideBankAccountErrorModal,
showBankAccountErrorModal,
showBankAccountFormValidationError,
setBankAccountFormValidationErrors,
setWorkspaceIDForReimbursementAccount,
};
85 changes: 72 additions & 13 deletions src/pages/ReimbursementAccount/BankAccountStep.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import _ from 'underscore';
import lodashGet from 'lodash/get';
import React from 'react';
import {View, Image} from 'react-native';
import PropTypes from 'prop-types';
Expand All @@ -24,7 +25,9 @@ import ExpensiTextInput from '../../components/ExpensiTextInput';
import {
goToWithdrawalAccountSetupStep,
hideBankAccountErrors,
setBankAccountFormValidationErrors,
setupWithdrawalAccount,
showBankAccountErrorModal,
} from '../../libs/actions/BankAccounts';
import ConfirmModal from '../../components/ConfirmModal';
import ONYXKEYS from '../../ONYXKEYS';
Expand Down Expand Up @@ -57,6 +60,28 @@ class BankAccountStep extends React.Component {
routingNumber: props.achData.routingNumber || '',
accountNumber: props.achData.accountNumber || '',
};

// Keys in this.errorTranslationKeys are associated to inputs, they are a subset of the keys found in this.state
this.errorTranslationKeys = {
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved
routingNumber: 'bankAccount.error.routingNumber',
accountNumber: 'bankAccount.error.accountNumber',
};
}

/**
* @returns {Object}
*/
getErrors() {
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved
return lodashGet(this.props, ['reimbursementAccount', 'errors'], {});
}

/**
* @param {String} inputKey
* @returns {string}
*/
getErrorText(inputKey) {
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved
const errors = this.getErrors();
return errors[inputKey] ? this.props.translate(this.errorTranslationKeys[inputKey]) : '';
}

toggleTerms() {
Expand All @@ -70,13 +95,52 @@ class BankAccountStep extends React.Component {
*/
canSubmitManually() {
return this.state.hasAcceptedTerms
&& this.state.accountNumber.trim()
&& this.state.routingNumber.trim();
}

// These are taken from BankCountry.js in Web-Secure
&& CONST.BANK_ACCOUNT.REGEX.IBAN.test(this.state.accountNumber.trim())
&& CONST.BANK_ACCOUNT.REGEX.SWIFT_BIC.test(this.state.routingNumber.trim());
/**
* @returns {Boolean}
*/
validate() {
const errors = {};

// These are taken from BankCountry.js in Web-Secure
if (!CONST.BANK_ACCOUNT.REGEX.IBAN.test(this.state.accountNumber.trim())) {
errors.accountNumber = true;
}
if (!CONST.BANK_ACCOUNT.REGEX.SWIFT_BIC.test(this.state.routingNumber.trim())) {
errors.routingNumber = true;
}
setBankAccountFormValidationErrors(errors);
return _.size(errors) === 0;
}

/**
* Clear the error associated to inputKey if found and store the inputKey new value in the state.
*
* @param {String} inputKey
* @param {String} value
*/
clearErrorAndSetValue(inputKey, value) {
this.setState({[inputKey]: value});
const errors = this.getErrors();
if (!errors[inputKey]) {
// No error found for this inputKey
return;
}
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved

// Clear the existing error for this inputKey
const newErrors = {...errors};
delete newErrors[inputKey];
setBankAccountFormValidationErrors(newErrors);
}

addManualAccount() {
if (!this.validate()) {
showBankAccountErrorModal();
return;
}
setupWithdrawalAccount({
acceptTerms: this.state.hasAcceptedTerms,
accountNumber: this.state.accountNumber,
Expand Down Expand Up @@ -128,7 +192,7 @@ class BankAccountStep extends React.Component {
const isFromPlaid = this.props.achData.setupType === CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID;
const shouldDisableInputs = Boolean(this.props.achData.bankAccountID) || isFromPlaid;
const existingOwners = this.props.reimbursementAccount.existingOwners;
const error = this.props.reimbursementAccount.error;
const error = this.getErrors();
const isExistingOwnersErrorVisible = Boolean(error && existingOwners);
return (
<View style={[styles.flex1, styles.justifyContentBetween]}>
Expand Down Expand Up @@ -209,23 +273,18 @@ class BankAccountStep extends React.Component {
placeholder={this.props.translate('bankAccount.routingNumber')}
keyboardType="number-pad"
value={this.state.routingNumber}
onChangeText={(routingNumber) => {
if (error === this.props.translate('bankAccount.error.routingNumber')) {
hideBankAccountErrors();
}
this.setState({routingNumber});
}}
onChangeText={value => this.clearErrorAndSetValue('routingNumber', value)}
disabled={shouldDisableInputs}
errorText={error === this.props.translate('bankAccount.error.routingNumber')
? error : ''}
errorText={this.getErrorText('routingNumber')}
/>
<ExpensiTextInput
containerStyles={[styles.mt4]}
placeholder={this.props.translate('bankAccount.accountNumber')}
keyboardType="number-pad"
value={this.state.accountNumber}
onChangeText={accountNumber => this.setState({accountNumber})}
onChangeText={value => this.clearErrorAndSetValue('accountNumber', value)}
disabled={shouldDisableInputs}
errorText={this.getErrorText('accountNumber')}
/>
<CheckboxWithLabel
style={[styles.mb4, styles.mt5]}
Expand Down