From 59617f86924ae28a7edc27a7c1a89e6d20db1e3c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 2 Sep 2021 12:33:56 -1000 Subject: [PATCH 1/8] Move ExistingOwners view into new component. Reuse confirm modal. --- src/languages/en.js | 1 - .../ReimbursementAccount/BankAccountStep.js | 61 ---------------- .../ReimbursementAccount/ExistingOwners.js | 71 +++++++++++++++++++ .../ReimbursementAccountPage.js | 11 ++- 4 files changed, 81 insertions(+), 63 deletions(-) create mode 100644 src/pages/ReimbursementAccount/ExistingOwners.js diff --git a/src/languages/en.js b/src/languages/en.js index 1b694a772750..bc95c77099a0 100755 --- a/src/languages/en.js +++ b/src/languages/en.js @@ -398,7 +398,6 @@ export default { ssnLast4: 'Please enter valid last 4 digits of SSN', noDefaultDepositAccountOrDebitCardAvailable: 'Please add a default deposit bank account or debit card', existingOwners: { - unableToAddBankAccount: 'Unable to add bank account', alreadyInUse: 'This bank account is already in use by ', pleaseAskThemToShare: 'Please ask them to share it with you.', alternatively: 'Alternatively, you can ', diff --git a/src/pages/ReimbursementAccount/BankAccountStep.js b/src/pages/ReimbursementAccount/BankAccountStep.js index cfad8fccd180..cc441d45533b 100644 --- a/src/pages/ReimbursementAccount/BankAccountStep.js +++ b/src/pages/ReimbursementAccount/BankAccountStep.js @@ -22,11 +22,9 @@ import exampleCheckImage from '../../../assets/images/example-check-image.png'; import Text from '../../components/Text'; import ExpensiTextInput from '../../components/ExpensiTextInput'; import { - goToWithdrawalAccountSetupStep, hideBankAccountErrors, setupWithdrawalAccount, } from '../../libs/actions/BankAccounts'; -import ConfirmModal from '../../components/ConfirmModal'; import ONYXKEYS from '../../ONYXKEYS'; import compose from '../../libs/compose'; @@ -35,9 +33,6 @@ const propTypes = { reimbursementAccount: PropTypes.shape({ /** Error set when handling the API response */ error: PropTypes.string, - - /** The existing owners for if the bank account is already owned */ - existingOwners: PropTypes.arrayOf(PropTypes.string), }).isRequired, ...withLocalizePropTypes, @@ -127,9 +122,7 @@ class BankAccountStep extends React.Component { // Disable bank account fields once they've been added in db so they can't be changed 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 isExistingOwnersErrorVisible = Boolean(error && existingOwners); return ( )} - - - - - {this.props.translate('bankAccount.error.existingOwners.alreadyInUse')} - - {existingOwners && existingOwners.map((existingOwner, i) => { - let separator = ', '; - if (i === 0) { - separator = ''; - } else if (i === existingOwners.length - 1) { - separator = ` ${this.props.translate('common.and')} `; - } - return ( - <> - {separator} - {existingOwner} - {i === existingOwners.length - 1 && .} - - ); - })} - - - {this.props.translate('bankAccount.error.existingOwners.pleaseAskThemToShare')} - - - - {this.props.translate('bankAccount.error.existingOwners.alternatively')} - - goToWithdrawalAccountSetupStep( - CONST.BANK_ACCOUNT.STEP.COMPANY, - this.props.achData, - )} - > - {this.props.translate( - 'bankAccount.error.existingOwners.setUpThisAccountByYourself', - )} - - - {this.props.translate('bankAccount.error.existingOwners.validationProcessAgain')} - - - - )} - confirmText={this.props.translate('common.ok')} - /> ); } diff --git a/src/pages/ReimbursementAccount/ExistingOwners.js b/src/pages/ReimbursementAccount/ExistingOwners.js new file mode 100644 index 000000000000..cd9614abbe98 --- /dev/null +++ b/src/pages/ReimbursementAccount/ExistingOwners.js @@ -0,0 +1,71 @@ +import _ from 'underscore'; +import React from 'react'; +import PropTypes from 'prop-types'; +import {View} from 'react-native'; +import Text from '../../components/Text'; +import styles from '../../styles/styles'; +import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; +import CONST from '../../CONST'; +import {goToWithdrawalAccountSetupStep} from '../../libs/actions/BankAccounts'; + +const propTypes = { + /** Reimbursement account state with list of existing owners */ + reimbursementAccount: PropTypes.shape({ + /** The existing owners for if the bank account is already owned */ + existingOwners: PropTypes.arrayOf(PropTypes.string), + }).isRequired, + ...withLocalizePropTypes, +}; + +const ExistingOwners = (props) => { + const existingOwners = props.reimbursementAccount.existingOwners; + return ( + + + + {props.translate('bankAccount.error.existingOwners.alreadyInUse')} + + {existingOwners && _.map(existingOwners, (existingOwner, i) => { + let separator = ', '; + if (i === 0) { + separator = ''; + } else if (i === existingOwners.length - 1) { + separator = ` ${props.translate('common.and')} `; + } + return ( + <> + {separator} + {existingOwner} + {i === existingOwners.length - 1 && .} + + ); + })} + + + {props.translate('bankAccount.error.existingOwners.pleaseAskThemToShare')} + + + + {props.translate('bankAccount.error.existingOwners.alternatively')} + + goToWithdrawalAccountSetupStep( + CONST.BANK_ACCOUNT.STEP.COMPANY, + props.achData, + )} + > + {props.translate( + 'bankAccount.error.existingOwners.setUpThisAccountByYourself', + )} + + + {props.translate('bankAccount.error.existingOwners.validationProcessAgain')} + + + + ); +}; + +ExistingOwners.propTypes = propTypes; +export default withLocalize(ExistingOwners); diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.js b/src/pages/ReimbursementAccount/ReimbursementAccountPage.js index 5a4216d9f971..a6ee53c4a73b 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.js +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.js @@ -32,6 +32,7 @@ import EnableStep from './EnableStep'; import ROUTES from '../../ROUTES'; import HeaderWithCloseButton from '../../components/HeaderWithCloseButton'; import ConfirmModal from '../../components/ConfirmModal'; +import ExistingOwners from './ExistingOwners'; const propTypes = { /** List of betas */ @@ -154,6 +155,14 @@ class ReimbursementAccountPage extends React.Component { } } + getErrorModalPrompt() { + if (this.props.reimbursementAccount.error === CONST.BANK_ACCOUNT.ERROR.EXISTING_OWNERS) { + return ; + } + + return this.props.reimbursementAccount.errorModalMessage || this.props.translate('bankAccount.confirmModalPrompt'); + } + render() { if (!Permissions.canUseFreePlan(this.props.betas)) { console.debug('Not showing new bank account page because user is not on free plan beta'); @@ -247,7 +256,7 @@ class ReimbursementAccountPage extends React.Component { Date: Thu, 2 Sep 2021 12:36:50 -1000 Subject: [PATCH 2/8] add return type to jsdoc --- src/pages/ReimbursementAccount/ReimbursementAccountPage.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.js b/src/pages/ReimbursementAccount/ReimbursementAccountPage.js index a6ee53c4a73b..f23bef568db3 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.js +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.js @@ -155,6 +155,9 @@ class ReimbursementAccountPage extends React.Component { } } + /** + * @returns {React.Component|string} + */ getErrorModalPrompt() { if (this.props.reimbursementAccount.error === CONST.BANK_ACCOUNT.ERROR.EXISTING_OWNERS) { return ; From b65885847ec88c94d97197eec0bf629a5a2974e7 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 2 Sep 2021 13:13:24 -1000 Subject: [PATCH 3/8] Clean up some mistakes --- src/components/AddPlaidBankAccount.js | 2 +- src/libs/actions/BankAccounts.js | 4 +- .../ReimbursementAccount/ExistingOwners.js | 100 ++++++++++-------- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/src/components/AddPlaidBankAccount.js b/src/components/AddPlaidBankAccount.js index f1b6cf200756..37e1c8453841 100644 --- a/src/components/AddPlaidBankAccount.js +++ b/src/components/AddPlaidBankAccount.js @@ -132,7 +132,7 @@ class AddPlaidBankAccount extends React.Component { )} - {!_.isEmpty(this.props.plaidLinkToken) && _.isEmpty(this.state.institution) && ( + {!_.isEmpty(this.props.plaidLinkToken) && ( { diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index 786f20aa160f..e52cbd7edc93 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -704,12 +704,12 @@ function setupWithdrawalAccount(data) { // Show warning if another account already set up this bank account and promote share if (response.existingOwners) { - console.error('Cannot set up withdrawal account due to existing owners', response); Onyx.merge( ONYXKEYS.REIMBURSEMENT_ACCOUNT, { existingOwners: response.existingOwners, error: CONST.BANK_ACCOUNT.ERROR.EXISTING_OWNERS, + isErrorModalVisible: true, }, ); return; @@ -822,7 +822,7 @@ function setupWithdrawalAccount(data) { } function hideBankAccountErrors() { - Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {error: '', existingOwnersList: ''}); + Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {error: '', existingOwners: []}); } function setWorkspaceIDForReimbursementAccount(workspaceID) { diff --git a/src/pages/ReimbursementAccount/ExistingOwners.js b/src/pages/ReimbursementAccount/ExistingOwners.js index cd9614abbe98..f99d5870f29f 100644 --- a/src/pages/ReimbursementAccount/ExistingOwners.js +++ b/src/pages/ReimbursementAccount/ExistingOwners.js @@ -2,11 +2,14 @@ import _ from 'underscore'; import React from 'react'; import PropTypes from 'prop-types'; import {View} from 'react-native'; +import {withOnyx} from 'react-native-onyx'; import Text from '../../components/Text'; import styles from '../../styles/styles'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import CONST from '../../CONST'; import {goToWithdrawalAccountSetupStep} from '../../libs/actions/BankAccounts'; +import compose from '../../libs/compose'; +import ONYXKEYS from '../../ONYXKEYS'; const propTypes = { /** Reimbursement account state with list of existing owners */ @@ -17,55 +20,60 @@ const propTypes = { ...withLocalizePropTypes, }; -const ExistingOwners = (props) => { - const existingOwners = props.reimbursementAccount.existingOwners; - return ( - - - - {props.translate('bankAccount.error.existingOwners.alreadyInUse')} - - {existingOwners && _.map(existingOwners, (existingOwner, i) => { - let separator = ', '; - if (i === 0) { - separator = ''; - } else if (i === existingOwners.length - 1) { - separator = ` ${props.translate('common.and')} `; - } - return ( - <> - {separator} - {existingOwner} - {i === existingOwners.length - 1 && .} - - ); - })} +const ExistingOwners = ({reimbursementAccount, translate}) => ( + + + + {translate('bankAccount.error.existingOwners.alreadyInUse')} + + {reimbursementAccount.existingOwners && _.map(reimbursementAccount.existingOwners, (existingOwner, i) => { + let separator = ', '; + if (i === 0) { + separator = ''; + } else if (i === reimbursementAccount.existingOwners.length - 1) { + separator = ` ${translate('common.and')} `; + } + return ( + <> + {separator} + {existingOwner} + {i === reimbursementAccount.existingOwners.length - 1 && .} + + ); + })} + + + {translate('bankAccount.error.existingOwners.pleaseAskThemToShare')} + + + + {translate('bankAccount.error.existingOwners.alternatively')} - - {props.translate('bankAccount.error.existingOwners.pleaseAskThemToShare')} + goToWithdrawalAccountSetupStep( + CONST.BANK_ACCOUNT.STEP.COMPANY, + reimbursementAccount.achData || {}, + )} + > + {translate( + 'bankAccount.error.existingOwners.setUpThisAccountByYourself', + )} - - {props.translate('bankAccount.error.existingOwners.alternatively')} - - goToWithdrawalAccountSetupStep( - CONST.BANK_ACCOUNT.STEP.COMPANY, - props.achData, - )} - > - {props.translate( - 'bankAccount.error.existingOwners.setUpThisAccountByYourself', - )} - - - {props.translate('bankAccount.error.existingOwners.validationProcessAgain')} - + {translate('bankAccount.error.existingOwners.validationProcessAgain')} - - ); -}; + + +); + ExistingOwners.propTypes = propTypes; -export default withLocalize(ExistingOwners); +export default compose( + withLocalize, + withOnyx({ + reimbursementAccount: { + key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, + }, + }), +)(ExistingOwners); From b6f1843021e37a72e257ed09b11592b0e36cd9a9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 2 Sep 2021 13:22:14 -1000 Subject: [PATCH 4/8] fix React warning by using keyed fragment --- src/pages/ReimbursementAccount/ExistingOwners.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/ReimbursementAccount/ExistingOwners.js b/src/pages/ReimbursementAccount/ExistingOwners.js index f99d5870f29f..1dc29426ca99 100644 --- a/src/pages/ReimbursementAccount/ExistingOwners.js +++ b/src/pages/ReimbursementAccount/ExistingOwners.js @@ -34,11 +34,11 @@ const ExistingOwners = ({reimbursementAccount, translate}) => ( separator = ` ${translate('common.and')} `; } return ( - <> + {separator} {existingOwner} {i === reimbursementAccount.existingOwners.length - 1 && .} - + ); })} From 642faf5f1a0df2a842475578c9b4d6e227373457 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 3 Sep 2021 09:53:38 -1000 Subject: [PATCH 5/8] remove unused translation --- src/languages/es.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/languages/es.js b/src/languages/es.js index 729d0786f7d8..4cc1b906263c 100644 --- a/src/languages/es.js +++ b/src/languages/es.js @@ -398,7 +398,6 @@ export default { ssnLast4: 'Ingrese los últimos 4 dígitos del número de seguro social', noDefaultDepositAccountOrDebitCardAvailable: 'Por favor agregue una cuenta bancaria para depósitos o una tarjeta de débito', existingOwners: { - unableToAddBankAccount: 'No ha sido posible añadir la cuenta bancaria', alreadyInUse: 'La cuenta bancaria ya se encuentra en uso por ', pleaseAskThemToShare: 'Por favor, solicita que la compartan contigo.', alternatively: 'En su defecto, puedes ', From 4b02a80af389dc977615e02ced7bdb94844aa32f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 3 Sep 2021 13:02:01 -1000 Subject: [PATCH 6/8] remove unused import --- src/pages/ReimbursementAccount/BankAccountStep.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/ReimbursementAccount/BankAccountStep.js b/src/pages/ReimbursementAccount/BankAccountStep.js index 9f1d78657e3f..93458d8a7cae 100644 --- a/src/pages/ReimbursementAccount/BankAccountStep.js +++ b/src/pages/ReimbursementAccount/BankAccountStep.js @@ -23,7 +23,6 @@ import exampleCheckImage from '../../../assets/images/example-check-image.png'; import Text from '../../components/Text'; import ExpensiTextInput from '../../components/ExpensiTextInput'; import { - hideBankAccountErrors, setBankAccountFormValidationErrors, setupWithdrawalAccount, showBankAccountErrorModal, From 5fb61a534a17b94f7fcf128649b09aa2c71c0aec Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 3 Sep 2021 13:07:14 -1000 Subject: [PATCH 7/8] remove error for existing owners --- src/CONST.js | 1 - src/libs/actions/BankAccounts.js | 1 - src/pages/ReimbursementAccount/ReimbursementAccountPage.js | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/CONST.js b/src/CONST.js index e8c1c270fc4b..3cadfc7f8243 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -26,7 +26,6 @@ const CONST = { MAX_ROUTING_NUMBER: '402 Maximum Size Exceeded routingNumber', MISSING_INCORPORATION_STATE: '402 Missing incorporationState in additionalData', MISSING_INCORPORATION_TYPE: '402 Missing incorporationType in additionalData', - EXISTING_OWNERS: '402 Existing Owners', }, STEP: { // In the order they appear in the VBA flow diff --git a/src/libs/actions/BankAccounts.js b/src/libs/actions/BankAccounts.js index 308cb226238e..1704d1f19d04 100644 --- a/src/libs/actions/BankAccounts.js +++ b/src/libs/actions/BankAccounts.js @@ -717,7 +717,6 @@ function setupWithdrawalAccount(data) { ONYXKEYS.REIMBURSEMENT_ACCOUNT, { existingOwners: response.existingOwners, - error: CONST.BANK_ACCOUNT.ERROR.EXISTING_OWNERS, isErrorModalVisible: true, }, ); diff --git a/src/pages/ReimbursementAccount/ReimbursementAccountPage.js b/src/pages/ReimbursementAccount/ReimbursementAccountPage.js index f23bef568db3..81d1f3880230 100644 --- a/src/pages/ReimbursementAccount/ReimbursementAccountPage.js +++ b/src/pages/ReimbursementAccount/ReimbursementAccountPage.js @@ -159,7 +159,7 @@ class ReimbursementAccountPage extends React.Component { * @returns {React.Component|string} */ getErrorModalPrompt() { - if (this.props.reimbursementAccount.error === CONST.BANK_ACCOUNT.ERROR.EXISTING_OWNERS) { + if (lodashGet(this.props.reimbursementAccount, 'existingOwners', []).length > 0) { return ; } From 573dc95d0239d93746ba83a30c9fb2730a288448 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 7 Sep 2021 09:45:42 -1000 Subject: [PATCH 8/8] hideBankAccountErrorModal when tapping link --- src/pages/ReimbursementAccount/ExistingOwners.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/pages/ReimbursementAccount/ExistingOwners.js b/src/pages/ReimbursementAccount/ExistingOwners.js index 1dc29426ca99..c3fea5a860a7 100644 --- a/src/pages/ReimbursementAccount/ExistingOwners.js +++ b/src/pages/ReimbursementAccount/ExistingOwners.js @@ -7,7 +7,7 @@ import Text from '../../components/Text'; import styles from '../../styles/styles'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import CONST from '../../CONST'; -import {goToWithdrawalAccountSetupStep} from '../../libs/actions/BankAccounts'; +import {goToWithdrawalAccountSetupStep, hideBankAccountErrorModal} from '../../libs/actions/BankAccounts'; import compose from '../../libs/compose'; import ONYXKEYS from '../../ONYXKEYS'; @@ -51,10 +51,13 @@ const ExistingOwners = ({reimbursementAccount, translate}) => ( goToWithdrawalAccountSetupStep( - CONST.BANK_ACCOUNT.STEP.COMPANY, - reimbursementAccount.achData || {}, - )} + onPress={() => { + goToWithdrawalAccountSetupStep( + CONST.BANK_ACCOUNT.STEP.COMPANY, + reimbursementAccount.achData || {}, + ); + hideBankAccountErrorModal(); + }} > {translate( 'bankAccount.error.existingOwners.setUpThisAccountByYourself',