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

Fix and refactor VBBA setup flow #13236

Merged
merged 38 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
687cac6
Refactor VBBA setup flow
nkuoch Dec 1, 2022
2af34d4
Dry common props and remove local code
nkuoch Dec 18, 2022
4f01183
Update src/pages/ReimbursementAccount/plaidDataPropTypes.js
nkuoch Dec 19, 2022
51b3bbd
Update src/components/ReimbursementAccountLoadingIndicator.js
nkuoch Dec 19, 2022
027802b
Update src/libs/actions/ReimbursementAccount/resetFreePlanBankAccount.js
nkuoch Dec 19, 2022
9110d5c
Update src/libs/actions/ReimbursementAccount/resetFreePlanBankAccount.js
nkuoch Dec 19, 2022
e9ffd6f
Update src/libs/actions/ReimbursementAccount/resetFreePlanBankAccount.js
nkuoch Dec 19, 2022
f88f7f9
Update src/libs/actions/ReimbursementAccount/resetFreePlanBankAccount.js
nkuoch Dec 19, 2022
8f6c6ea
Add comment and reset plaidData properly
nkuoch Dec 19, 2022
a18a5af
Other plaid fixes and reset fix
nkuoch Dec 19, 2022
a90b064
Update src/libs/actions/ReimbursementAccount/index.js
nkuoch Dec 20, 2022
101175c
Merge branch 'main' into nat-back
nkuoch Dec 20, 2022
65f61de
Merge branch 'nat-back' of github.com:Expensify/App into nat-back
nkuoch Dec 20, 2022
7590db5
Merge branch 'main' into nat-back
nkuoch Jan 4, 2023
eeb4ec7
Pass onBackButtonPress to ValidationStep
nkuoch Jan 4, 2023
23f96df
newDot always useOnfido
nkuoch Jan 4, 2023
99a780b
Merge branch 'main' into nat-back
nkuoch Jan 5, 2023
36303c4
Revert local only needed changes
nkuoch Jan 5, 2023
687612f
Update src/libs/actions/ReimbursementAccount/index.js
nkuoch Jan 9, 2023
193a858
Update src/libs/actions/ReimbursementAccount/index.js
nkuoch Jan 9, 2023
a216a65
Merge branch 'main' into nat-back
nkuoch Jan 9, 2023
d581526
Merge branch 'main' into nat-back
nkuoch Jan 9, 2023
6cfea8e
Merge branch 'main' into nat-back
nkuoch Jan 10, 2023
1190b6a
Add comments on stepToOpen
nkuoch Jan 11, 2023
153a928
Dry code by moving getDefaultStateForField to parent, allow to differ…
nkuoch Jan 12, 2023
87b322d
Make sure Plaid is reset to a valid format when relaunching bank acco…
nkuoch Jan 12, 2023
1bbd590
Fix ESLint
nkuoch Jan 12, 2023
4fa052c
Fix this
nkuoch Jan 12, 2023
5515fdb
Revert step change
nkuoch Jan 13, 2023
2df31d6
Remove extra plaidData rerendering causing ios crash when viewing pla…
nkuoch Jan 13, 2023
c55f574
bankAccountID doesnt need to be in draft
nkuoch Jan 13, 2023
4a1ff1f
Merge branch 'main' into nat-back
nkuoch Jan 16, 2023
a2536ad
Fix bankAccountID location
nkuoch Jan 16, 2023
ab5417a
Merge branch 'main' into nat-back
nkuoch Jan 17, 2023
6f0af2c
Steps should only be in reimbursementAccount, not the draft
nkuoch Jan 17, 2023
bcf8248
Use lodashget to make things safer
nkuoch Jan 17, 2023
c680b84
Merge branch 'main' into nat-back
nkuoch Jan 17, 2023
cba528a
Make sure we dont show continueSetup button after we startOver and re…
nkuoch Jan 19, 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
3 changes: 3 additions & 0 deletions src/ONYXKEYS.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ export default {
// Plaid data (access tokens, bank accounts ...)
PLAID_DATA: 'plaidData',

// If we disabled Plaid because of too many attempts
IS_PLAID_DISABLED: 'isPlaidDisabled',

// Token needed to initialize Plaid link
PLAID_LINK_TOKEN: 'plaidLinkToken',

Expand Down
2 changes: 0 additions & 2 deletions src/components/AddPaymentMethodMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import withWindowDimensions from './withWindowDimensions';
import Permissions from '../libs/Permissions';
import PopoverMenu from './PopoverMenu';
import paypalMeDataPropTypes from './paypalMeDataPropTypes';
import * as BankAccounts from '../libs/actions/BankAccounts';

const propTypes = {
isVisible: PropTypes.bool.isRequired,
Expand Down Expand Up @@ -50,7 +49,6 @@ const AddPaymentMethodMenu = props => (
text: props.translate('common.bankAccount'),
icon: Expensicons.Bank,
onSelected: () => {
BankAccounts.clearPlaid();
Comment on lines 51 to -53
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #32906, this change caused a regression, after this change the plaid token is not cleared.

props.onItemSelected(CONST.PAYMENT_METHODS.BANK_ACCOUNT);
},
},
Expand Down
34 changes: 13 additions & 21 deletions src/components/AddPlaidBankAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {
View,
} from 'react-native';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import {withOnyx} from 'react-native-onyx';
import lodashGet from 'lodash/get';
import Log from '../libs/Log';
import PlaidLink from './PlaidLink';
import * as BankAccounts from '../libs/actions/BankAccounts';
Expand All @@ -16,15 +16,15 @@ import themeColors from '../styles/themes/default';
import compose from '../libs/compose';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import Picker from './Picker';
import plaidDataPropTypes from '../pages/ReimbursementAccount/plaidDataPropTypes';
import {plaidDataPropTypes} from '../pages/ReimbursementAccount/plaidDataPropTypes';
import Text from './Text';
import getBankIcon from './Icon/BankIcons';
import Icon from './Icon';
import FullPageOfflineBlockingView from './BlockingViews/FullPageOfflineBlockingView';

const propTypes = {
/** Contains plaid data */
plaidData: plaidDataPropTypes,
plaidData: plaidDataPropTypes.isRequired,
Copy link
Contributor

Choose a reason for hiding this comment

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

Getting a strange error related to the plaidDataPropTypes when on desktop:

Screenshot 2023-01-11 at 2 47 27 PM

But it doesn't seem to happen on web, strangely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you console.log('this.props.plaidData', JSON.stringify(this.props.plaidData)); in the render of BankAccountPlaidStep?
When does this error occur? When clicking on the Plaid option?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, can't seem to reproduce this error anymore but it was when clicking on Connect back account

Copy link
Contributor Author

@nkuoch nkuoch Jan 12, 2023

Choose a reason for hiding this comment

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

During my multiple testings, I think I also got it once, but then it disappeared.
So, I can't reproduce, but I feel like it might be due to stale data or Onyx update sync issue. So that might fix it (making sure PlaidData is reset to the right format before trying to display the view)?


/** Selected account ID from the Picker associated with the end of the Plaid flow */
selectedPlaidAccountID: PropTypes.string,
Expand Down Expand Up @@ -57,14 +57,6 @@ const propTypes = {
};

const defaultProps = {
plaidData: {
bankName: '',
plaidAccessToken: '',
bankAccounts: [],
isLoading: false,
error: '',
errors: {},
},
selectedPlaidAccountID: '',
plaidLinkToken: '',
onExitPlaid: () => {},
Expand All @@ -85,7 +77,9 @@ class AddPlaidBankAccount extends React.Component {

componentDidMount() {
// If we're coming from Plaid OAuth flow then we need to reuse the existing plaidLinkToken
if ((this.props.receivedRedirectURI && this.props.plaidLinkOAuthToken) || !_.isEmpty(this.props.plaidData)) {
if ((this.props.receivedRedirectURI && this.props.plaidLinkOAuthToken)
|| !_.isEmpty(lodashGet(this.props.plaidData, 'bankAccounts'))
|| !_.isEmpty(lodashGet(this.props.plaidData, 'errors'))) {
return;
}

Expand All @@ -106,20 +100,22 @@ class AddPlaidBankAccount extends React.Component {
}

render() {
const plaidBankAccounts = lodashGet(this.props.plaidData, 'bankAccounts', []);
const plaidBankAccounts = lodashGet(this.props.plaidData, 'bankAccounts') || [];
const token = this.getPlaidLinkToken();
const options = _.map(plaidBankAccounts, account => ({
value: account.plaidAccountID,
label: `${account.addressName} ${account.mask}`,
}));
const {icon, iconSize} = getBankIcon();
const plaidDataErrorMessage = !_.isEmpty(this.props.plaidData.errors) ? _.chain(this.props.plaidData.errors).values().first().value() : this.props.plaidData.error;
const plaidErrors = lodashGet(this.props.plaidData, 'errors');
const plaidDataErrorMessage = !_.isEmpty(plaidErrors) ? _.chain(plaidErrors).values().first().value() : '';
const bankName = lodashGet(this.props.plaidData, 'bankName');

// Plaid Link view
if (!plaidBankAccounts.length) {
return (
<FullPageOfflineBlockingView>
{this.props.plaidData.isLoading && (
{lodashGet(this.props.plaidData, 'isLoading') && (
<View style={[styles.flex1, styles.alignItemsCenter, styles.justifyContentCenter]}>
<ActivityIndicator color={themeColors.spinner} size="large" />
</View>
Expand All @@ -129,13 +125,12 @@ class AddPlaidBankAccount extends React.Component {
{plaidDataErrorMessage}
</Text>
)}
{Boolean(token) && (
{Boolean(token) && !bankName && (
<PlaidLink
token={token}
onSuccess={({publicToken, metadata}) => {
Log.info('[PlaidLink] Success!');
BankAccounts.openPlaidBankAccountSelector(publicToken, metadata.institution.name, this.props.allowDebit);
BankAccounts.updatePlaidData({institution: metadata.institution});
}}
onError={(error) => {
Log.hmmm('[PlaidLink] Error: ', error.message);
Expand Down Expand Up @@ -163,7 +158,7 @@ class AddPlaidBankAccount extends React.Component {
height={iconSize}
width={iconSize}
/>
<Text style={[styles.ml3, styles.textStrong]}>{this.props.plaidData.bankName}</Text>
<Text style={[styles.ml3, styles.textStrong]}>{bankName}</Text>
</View>
<View style={[styles.mb5]}>
<Picker
Expand All @@ -188,9 +183,6 @@ AddPlaidBankAccount.defaultProps = defaultProps;
export default compose(
withLocalize,
withOnyx({
plaidData: {
key: ONYXKEYS.PLAID_DATA,
},
plaidLinkToken: {
key: ONYXKEYS.PLAID_LINK_TOKEN,
initWithStoredValues: false,
Expand Down
12 changes: 11 additions & 1 deletion src/components/ReimbursementAccountLoadingIndicator.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ import Navigation from '../libs/Navigation/Navigation';
import ScreenWrapper from './ScreenWrapper';
import FullScreenLoadingIndicator from './FullscreenLoadingIndicator';
import FullPageOfflineBlockingView from './BlockingViews/FullPageOfflineBlockingView';
import compose from '../libs/compose';
import {withNetwork} from './OnyxProvider';

const propTypes = {
/** Whether the user is submitting verifications data */
isSubmittingVerificationsData: PropTypes.bool.isRequired,

/** Method to trigger when pressing back button of the header */
onBackButtonPress: PropTypes.func.isRequired,
nkuoch marked this conversation as resolved.
Show resolved Hide resolved
...withLocalizePropTypes,
};

Expand All @@ -23,6 +27,8 @@ const ReimbursementAccountLoadingIndicator = props => (
<HeaderWithCloseButton
title={props.translate('reimbursementAccountLoadingAnimation.oneMoment')}
onCloseButtonPress={Navigation.dismissModal}
shouldShowBackButton={props.network.isOffline}
onBackButtonPress={props.onBackButtonPress}
/>
<FullPageOfflineBlockingView>
{props.isSubmittingVerificationsData ? (
Expand All @@ -49,4 +55,8 @@ const ReimbursementAccountLoadingIndicator = props => (
ReimbursementAccountLoadingIndicator.propTypes = propTypes;
ReimbursementAccountLoadingIndicator.displayName = 'ReimbursementAccountLoadingIndicator';

export default withLocalize(ReimbursementAccountLoadingIndicator);
export default compose(
withLocalize,
withNetwork(),
)(ReimbursementAccountLoadingIndicator);

24 changes: 5 additions & 19 deletions src/libs/ReimbursementAccountUtils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import _ from 'underscore';
import lodashGet from 'lodash/get';
import * as BankAccounts from './actions/BankAccounts';
import FormHelper from './FormHelper';
Expand All @@ -15,28 +14,16 @@ const clearErrors = (props, paths) => formHelper.clearErrors(props, paths);
/**
* Get the default state for input fields in the VBA flow
*
* @param {Object} props
* @param {Object} reimbursementAccountDraft
* @param {Object} reimbursementAccount
* @param {String} fieldName
* @param {*} defaultValue
*
* @returns {*}
*/
function getDefaultStateForField(props, fieldName, defaultValue = '') {
return lodashGet(props, ['reimbursementAccountDraft', fieldName])
|| lodashGet(props, ['reimbursementAccount', 'achData', fieldName], defaultValue);
}

/**
* @param {Object} props
* @param {Array} fieldNames
*
* @returns {*}
*/
function getBankAccountFields(props, fieldNames) {
return {
..._.pick(lodashGet(props, 'reimbursementAccount.achData'), ...fieldNames),
..._.pick(props.reimbursementAccountDraft, ...fieldNames),
};
function getDefaultStateForField(reimbursementAccountDraft, reimbursementAccount, fieldName, defaultValue = '') {
Comment on lines -35 to -39
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious why this function was moved to CompanyStep.js instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it's only used there

return lodashGet(reimbursementAccountDraft, fieldName)
|| lodashGet(reimbursementAccount, ['achData', fieldName], defaultValue);
}

/**
Expand All @@ -56,5 +43,4 @@ export {
clearError,
clearErrors,
getErrorText,
getBankAccountFields,
};
28 changes: 19 additions & 9 deletions src/libs/actions/BankAccounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import * as API from '../API';
import ONYXKEYS from '../../ONYXKEYS';
import * as Localize from '../Localize';
import DateUtils from '../DateUtils';
import * as PlaidDataProps from '../../pages/ReimbursementAccount/plaidDataPropTypes';
import Navigation from '../Navigation/Navigation';
import ROUTES from '../../ROUTES';

export {
goToWithdrawalAccountSetupStep,
Expand All @@ -28,17 +31,23 @@ export {
acceptWalletTerms,
} from './Wallet';

function clearPersonalBankAccount() {
Onyx.set(ONYXKEYS.PERSONAL_BANK_ACCOUNT, {});
}

function clearPlaid() {
Onyx.set(ONYXKEYS.PLAID_DATA, {});
Onyx.set(ONYXKEYS.PLAID_LINK_TOKEN, '');

return Onyx.set(ONYXKEYS.PLAID_DATA, PlaidDataProps.plaidDataDefaultProps);
}

function updatePlaidData(plaidData) {
Onyx.merge(ONYXKEYS.PLAID_DATA, plaidData);
function openPlaidView() {
clearPlaid().then(() => this.setBankAccountSubStep(CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID));
}

function openPersonalBankAccountSetupView() {
clearPlaid().then(() => Navigation.navigate(ROUTES.SETTINGS_ADD_BANK_ACCOUNT));
}

function clearPersonalBankAccount() {
clearPlaid();
Onyx.set(ONYXKEYS.PERSONAL_BANK_ACCOUNT, {});
}

function clearOnfidoToken() {
Expand Down Expand Up @@ -341,7 +350,7 @@ function updateBeneficialOwnersForBankAccount(params) {
/**
* Create the bank account with manually entered data.
*
* @param {String} [bankAccountID]
* @param {number} [bankAccountID]
* @param {String} [accountNumber]
* @param {String} [routingNumber]
* @param {String} [plaidMask]
Expand Down Expand Up @@ -387,14 +396,15 @@ export {
clearOnfidoToken,
clearPersonalBankAccount,
clearPlaid,
openPlaidView,
connectBankAccountManually,
connectBankAccountWithPlaid,
deletePaymentBankAccount,
openPersonalBankAccountSetupView,
openReimbursementAccountPage,
updateBeneficialOwnersForBankAccount,
updateCompanyInformationForBankAccount,
updatePersonalInformationForBankAccount,
updatePlaidData,
openWorkspaceView,
validateBankAccount,
verifyIdentityForBankAccount,
Expand Down
16 changes: 0 additions & 16 deletions src/libs/actions/PaymentMethods.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import _ from 'underscore';
import {createRef} from 'react';
import lodashGet from 'lodash/get';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '../../ONYXKEYS';
import * as API from '../API';
Expand All @@ -10,7 +9,6 @@ import * as Localize from '../Localize';
import Navigation from '../Navigation/Navigation';
import * as CardUtils from '../CardUtils';
import * as User from './User';
import * as store from './ReimbursementAccount/store';
import ROUTES from '../../ROUTES';

function deletePayPalMe() {
Expand All @@ -37,19 +35,6 @@ function continueSetup() {
kycWallRef.current.continue();
}

/**
* Clears local reimbursement account if it doesn't exist in bankAccounts
* @param {Object[]} bankAccounts
*/
function cleanLocalReimbursementData(bankAccounts) {
const bankAccountID = lodashGet(store.getReimbursementAccountInSetup(), 'bankAccountID');

// We check if the bank account list doesn't have the reimbursementAccount
if (!_.find(bankAccounts, bankAccount => bankAccount.bankAccountID === bankAccountID)) {
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {achData: null, shouldShowResetModal: false});
}
}

function openPaymentsPage() {
const onyxData = {
optimisticData: [
Expand Down Expand Up @@ -359,7 +344,6 @@ export {
resetWalletTransferData,
saveWalletTransferAccountTypeAndID,
saveWalletTransferMethodType,
cleanLocalReimbursementData,
hasPaymentMethodError,
clearDeletePaymentMethodError,
clearAddPaymentMethodError,
Expand Down
19 changes: 18 additions & 1 deletion src/libs/actions/Plaid.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import getPlaidLinkTokenParameters from '../getPlaidLinkTokenParameters';
import ONYXKEYS from '../../ONYXKEYS';
import * as API from '../API';
import CONST from '../../CONST';
import * as PlaidDataProps from '../../pages/ReimbursementAccount/plaidDataPropTypes';

/**
* Gets the Plaid Link token used to initialize the Plaid SDK
Expand All @@ -12,7 +13,23 @@ function openPlaidBankLogin(allowDebit, bankAccountID) {
const params = getPlaidLinkTokenParameters();
params.allowDebit = allowDebit;
params.bankAccountID = bankAccountID;
API.read('OpenPlaidBankLogin', params);
const optimisticData = [{
onyxMethod: CONST.ONYX.METHOD.SET,
key: ONYXKEYS.PLAID_DATA,
value: {...PlaidDataProps.plaidDataDefaultProps, isLoading: true},
}, {
onyxMethod: CONST.ONYX.METHOD.SET,
key: ONYXKEYS.PLAID_LINK_TOKEN,
value: '',
}, {
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT_DRAFT,
value: {
plaidAccountID: '',
},
}];

API.read('OpenPlaidBankLogin', params, {optimisticData});
}

/**
Expand Down
6 changes: 2 additions & 4 deletions src/libs/actions/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -911,10 +911,8 @@ function createWorkspace(ownerEmail = '', makeMeAdmin = false, policyName = '',
/**
*
* @param {string} policyID
* @param {string} subStep The sub step in first step of adding withdrawal bank account
* @param {*} localCurrentStep The locally stored current step of adding a withdrawal bank account
*/
function openWorkspaceReimburseView(policyID, subStep, localCurrentStep) {
function openWorkspaceReimburseView(policyID) {
if (!policyID) {
Log.warn('openWorkspaceReimburseView invalid params', {policyID});
return;
Expand All @@ -940,7 +938,7 @@ function openWorkspaceReimburseView(policyID, subStep, localCurrentStep) {
],
};

API.read('OpenWorkspaceReimburseView', {policyID, subStep, localCurrentStep}, onyxData);
API.read('OpenWorkspaceReimburseView', {policyID}, onyxData);
}

function openWorkspaceMembersPage(policyID, clientMemberEmails) {
Expand Down
7 changes: 5 additions & 2 deletions src/libs/actions/ReimbursementAccount/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ export {
} from './errors';

/**
* Set the current sub step in first step of adding withdrawal bank account
* Set the current sub step in first step of adding withdrawal bank account:
* - `null` if we want to go back to the view where the user selects between connecting via Plaid or connecting manually
* - CONST.BANK_ACCOUNT.SETUP_TYPE.MANUAL to ask them to enter their accountNumber and routingNumber
* - CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID to ask them to login to their bank via Plaid
*
* @param {String} subStep - One of {CONST.BANK_ACCOUNT.SETUP_TYPE.MANUAL, CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID, null}
* @param {String} subStep
*/
function setBankAccountSubStep(subStep) {
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {achData: {subStep}});
Expand Down
Loading
Loading