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

Handle error in workspace's bank account page #15394

Merged
merged 32 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
99df6d7
Show error in workspace's bank account page
aldo-expensify Feb 23, 2023
426694b
Disable disconnect button on pending action
aldo-expensify Feb 23, 2023
7fb58c1
Merge branch 'main' of github.com:Expensify/App into aldo_fix-disconn…
aldo-expensify Feb 23, 2023
b64bfc3
Move error to reimbursementAccount.errors
aldo-expensify Feb 24, 2023
814df41
Don't clear errors when reloading VBA
aldo-expensify Feb 24, 2023
5a2fb89
Add red dot in avatar when error in reimbursementAccount.errors
aldo-expensify Feb 24, 2023
c502f90
Add red dot in Settings when error in reimbursementAccount.errors
aldo-expensify Feb 24, 2023
5ef05fc
Add RBR error indicator in initial workspace page
aldo-expensify Feb 24, 2023
623e3a7
Add red dot to workspaces list
aldo-expensify Feb 27, 2023
8aea1c2
Handle errors when restarting a VBBA setup
aldo-expensify Feb 27, 2023
654d582
Remove unused prop
aldo-expensify Feb 27, 2023
3e15034
Remove unused import
aldo-expensify Feb 27, 2023
0bd46f8
Resolve conflicts / merge with main
aldo-expensify Mar 4, 2023
a93784b
Merge branch 'main' of github.com:Expensify/App into aldo_fix-disconn…
aldo-expensify Mar 10, 2023
5218695
Remove `pendingAction` on failure
aldo-expensify Mar 10, 2023
ed12f80
Recalculate state.shouldHideContinueSetupButton after pendingAction=d…
aldo-expensify Mar 10, 2023
c1b12d0
Update misspell
aldo-expensify Mar 11, 2023
f9a0e34
Disable button if there are errors
aldo-expensify Mar 11, 2023
5425565
Merge branch 'main' of github.com:Expensify/App into aldo_fix-disconn…
aldo-expensify Mar 15, 2023
2e0bc5d
Update continue with setup state
aldo-expensify Mar 15, 2023
777f086
Update route even when showing continue/starto over button
aldo-expensify Mar 15, 2023
332d464
Fix initial step param
aldo-expensify Mar 15, 2023
81ab0f4
Remove unused import
aldo-expensify Mar 15, 2023
f51f032
Keep url /bank-account is we get choice to continue
aldo-expensify Mar 15, 2023
6914f4c
Update logic clearing errors
aldo-expensify Mar 17, 2023
c957618
Fix edge cases in relation to showing options continue/start over
aldo-expensify Mar 18, 2023
42dac27
Resolve conflicts
aldo-expensify Mar 18, 2023
7e36149
Style
aldo-expensify Mar 18, 2023
85888a7
Remove unused variable
aldo-expensify Mar 20, 2023
48585ef
Merge main / Resolve conflicts
aldo-expensify Apr 14, 2023
bd11b4c
Merge branch 'aldo_fix-disconnect-workspace-bankaccount' of github.co…
aldo-expensify Apr 14, 2023
270dc00
Add new line at end of file (style)
aldo-expensify Apr 14, 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
9 changes: 9 additions & 0 deletions src/components/AvatarWithIndicator.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {policyPropTypes} from '../pages/workspace/withPolicy';
import walletTermsPropTypes from '../pages/EnablePayments/walletTermsPropTypes';
import * as PolicyUtils from '../libs/PolicyUtils';
import * as PaymentMethods from '../libs/actions/PaymentMethods';
import * as ReimbursementAccountProps from '../pages/ReimbursementAccount/reimbursementAccountPropTypes';

const propTypes = {
/** URL for the avatar */
Expand All @@ -41,13 +42,17 @@ const propTypes = {
/** The user's wallet (coming from Onyx) */
userWallet: userWalletPropTypes,

/** Bank account attached to free plan */
reimbursementAccount: ReimbursementAccountProps.reimbursementAccountPropTypes,

/** Information about the user accepting the terms for payments */
walletTerms: walletTermsPropTypes,
};

const defaultProps = {
size: 'default',
tooltipText: '',
reimbursementAccount: {},
policiesMemberList: {},
policies: {},
bankAccountList: {},
Expand Down Expand Up @@ -78,6 +83,7 @@ const AvatarWithIndicator = (props) => {
() => _.some(cleanPolicies, PolicyUtils.hasPolicyError),
() => _.some(cleanPolicies, PolicyUtils.hasCustomUnitsError),
() => _.some(cleanPolicyMembers, PolicyUtils.hasPolicyMemberError),
() => !_.isEmpty(props.reimbursementAccount.errors),

// Wallet term errors that are not caused by an IOU (we show the red brick indicator for those in the LHN instead)
() => !_.isEmpty(props.walletTerms.errors) && !props.walletTerms.chatReportID,
Expand Down Expand Up @@ -114,6 +120,9 @@ export default withOnyx({
bankAccountList: {
key: ONYXKEYS.BANK_ACCOUNT_LIST,
},
reimbursementAccount: {
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
},
cardList: {
key: ONYXKEYS.CARD_LIST,
},
Expand Down
1 change: 0 additions & 1 deletion src/libs/actions/BankAccounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ function openReimbursementAccountPage(stepToOpen, subStep, localCurrentStep) {
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: {
errors: null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We re-load the reimbursementAccount data each time we enter the "Connect bank account" page of a workspace. If there is an error, errors: null, was clearing the errors, not allowing the user to clear it with the X and not giving time to read it.

isLoading: true,
},
},
Expand Down
5 changes: 4 additions & 1 deletion src/libs/actions/ReimbursementAccount/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ function setBankAccountFormValidationErrors(errorFields) {
*/
function resetReimbursementAccount() {
setBankAccountFormValidationErrors({});
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {errors: null});
Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {
errors: null,
pendingAction: null,
});
}

/**
Expand Down
23 changes: 11 additions & 12 deletions src/libs/actions/ReimbursementAccount/resetFreePlanBankAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ function resetFreePlanBankAccount(bankAccountID) {
},
{
optimisticData: [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: {
shouldShowResetModal: false,
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved
isLoading: true,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved
},
},
],
successData: [
{
onyxMethod: CONST.ONYX.METHOD.SET,
key: ONYXKEYS.ONFIDO_TOKEN,
Expand All @@ -44,24 +55,12 @@ function resetFreePlanBankAccount(bankAccountID) {
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: ReimbursementAccountProps.reimbursementAccountDefaultProps,
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: {isLoading: true},
},
{
onyxMethod: CONST.ONYX.METHOD.SET,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT_DRAFT,
value: {},
},
],
successData: [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: {isLoading: false},
},
],
failureData: [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
Expand Down
112 changes: 61 additions & 51 deletions src/pages/ReimbursementAccount/ContinueBankAccountSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import withPolicy from '../workspace/withPolicy';
import * as ReimbursementAccountProps from './reimbursementAccountPropTypes';
import WorkspaceResetBankAccountModal from '../workspace/WorkspaceResetBankAccountModal';
import * as BankAccounts from '../../libs/actions/BankAccounts';
import OfflineWithFeedback from '../../components/OfflineWithFeedback';

const propTypes = {
/** Bank account currently in setup */
Expand All @@ -29,9 +30,6 @@ const propTypes = {
/** Callback to continue to the next step of the setup */
continue: PropTypes.func.isRequired,

/** Callback to reset the bank account */
startOver: PropTypes.func.isRequired,

/** Policy values needed in the component */
policy: PropTypes.shape({
name: PropTypes.string,
Expand All @@ -40,55 +38,67 @@ const propTypes = {
...withLocalizePropTypes,
};

const ContinueBankAccountSetup = props => (
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<FullPageNotFoundView shouldShow={_.isEmpty(props.policy)}>
<HeaderWithCloseButton
title={props.translate('workspace.common.bankAccount')}
subtitle={lodashGet(props.policy, 'name')}
onCloseButtonPress={Navigation.dismissModal}
onBackButtonPress={Navigation.goBack}
shouldShowGetAssistanceButton
guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_BANK_ACCOUNT}
shouldShowBackButton
/>
<ScrollView style={styles.flex1}>
<Section
title={props.translate('workspace.bankAccount.almostDone')}
icon={Illustrations.BankArrow}
>
<Text>
{props.translate('workspace.bankAccount.youreAlmostDone')}
</Text>
<Button
text={props.translate('workspace.bankAccount.continueWithSetup')}
onPress={props.continue}
icon={Expensicons.Bank}
style={[styles.mv4]}
iconStyles={[styles.buttonCTAIcon]}
shouldShowRightIcon
large
success
/>
<MenuItem
title={props.translate('workspace.bankAccount.startOver')}
icon={Expensicons.RotateLeft}
onPress={() => BankAccounts.requestResetFreePlanBankAccount()}
shouldShowRightIcon
wrapperStyle={[styles.cardMenuItem]}
/>
</Section>
</ScrollView>
</FullPageNotFoundView>
const ContinueBankAccountSetup = (props) => {
const errors = lodashGet(props.reimbursementAccount, 'errors', {});
const pendingAction = lodashGet(props.reimbursementAccount, 'pendingAction', null);
return (
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<FullPageNotFoundView shouldShow={_.isEmpty(props.policy)}>
<HeaderWithCloseButton
title={props.translate('workspace.common.bankAccount')}
subtitle={lodashGet(props.policy, 'name')}
onCloseButtonPress={Navigation.dismissModal}
onBackButtonPress={Navigation.goBack}
shouldShowGetAssistanceButton
guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_BANK_ACCOUNT}
shouldShowBackButton
/>
<ScrollView style={styles.flex1}>
<Section
title={props.translate('workspace.bankAccount.almostDone')}
icon={Illustrations.BankArrow}
>
<OfflineWithFeedback
Copy link
Member

Choose a reason for hiding this comment

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

Hi all! Just dropping a note that this PR caused issue - #18517

Wrapping with OfflineWithFeedback crosses out all text. In bank account page, it doesn't makes sense to have the strikethrough because the component is static, that is not showing a user input (chat message, workspace etc).

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch, but with removing the pending action we may lose some other intended strikethough styles?

https://github.com/Expensify/App/pull/15394/files#r1119420530

pendingAction={pendingAction}
errors={errors}
shouldShowErrorMessage
onClose={BankAccounts.resetReimbursementAccount}
Comment on lines +66 to +70
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrapped in OfflineWithFeedback to:

  • Show error
  • Add strikethrough if offline and pendingAction = 'delete'

>
<Text>
{props.translate('workspace.bankAccount.youreAlmostDone')}
</Text>
<Button
text={props.translate('workspace.bankAccount.continueWithSetup')}
onPress={props.continue}
icon={Expensicons.Bank}
style={[styles.mv4]}
iconStyles={[styles.buttonCTAIcon]}
shouldShowRightIcon
large
success
isDisabled={Boolean(pendingAction)}
/>
<MenuItem
title={props.translate('workspace.bankAccount.startOver')}
icon={Expensicons.RotateLeft}
onPress={() => BankAccounts.requestResetFreePlanBankAccount()}
shouldShowRightIcon
wrapperStyle={[styles.cardMenuItem]}
disabled={Boolean(pendingAction)}
/>
</OfflineWithFeedback>
</Section>
</ScrollView>
</FullPageNotFoundView>

{props.reimbursementAccount.shouldShowResetModal && (
<WorkspaceResetBankAccountModal
reimbursementAccount={props.reimbursementAccount}
onConfirm={props.startOver}
/>
)}
</ScreenWrapper>
);
{props.reimbursementAccount.shouldShowResetModal && (
<WorkspaceResetBankAccountModal
reimbursementAccount={props.reimbursementAccount}
/>
)}
</ScreenWrapper>
);
};

ContinueBankAccountSetup.propTypes = propTypes;
ContinueBankAccountSetup.displayName = 'ContinueBankAccountSetup';
Expand Down
83 changes: 48 additions & 35 deletions src/pages/ReimbursementAccount/EnableStep.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import PropTypes from 'prop-types';
import {ScrollView} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import lodashGet from 'lodash/get';
Expand All @@ -13,6 +14,7 @@ import CONST from '../../CONST';
import Button from '../../components/Button';
import * as Expensicons from '../../components/Icon/Expensicons';
import MenuItem from '../../components/MenuItem';
import OfflineWithFeedback from '../../components/OfflineWithFeedback';
import getBankIcon from '../../components/Icon/BankIcons';
import * as ReimbursementAccountProps from './reimbursementAccountPropTypes';
import userPropTypes from '../settings/userPropTypes';
Expand Down Expand Up @@ -45,6 +47,8 @@ const EnableStep = (props) => {
: '';
const bankName = achData.addressName;

const errors = lodashGet(props.reimbursementAccount, 'errors', {});
const pendingAction = lodashGet(props.reimbursementAccount, 'pendingAction', null);
return (
<ScreenWrapper style={[styles.flex1, styles.justifyContentBetween]} includeSafeAreaPaddingBottom={false}>
<HeaderWithCloseButton
Expand All @@ -60,42 +64,51 @@ const EnableStep = (props) => {
title={!isUsingExpensifyCard ? props.translate('workspace.bankAccount.oneMoreThing') : props.translate('workspace.bankAccount.allSet')}
icon={!isUsingExpensifyCard ? Illustrations.ConciergeNew : Illustrations.ThumbsUpStars}
>
<MenuItem
title={bankName}
description={formattedBankAccountNumber}
icon={icon}
iconWidth={iconSize}
iconHeight={iconSize}
disabled
interactive={false}
wrapperStyle={[styles.cardMenuItem, styles.mv3]}
/>
<Text style={[styles.mv3]}>
{!isUsingExpensifyCard
? props.translate('workspace.bankAccount.accountDescriptionNoCards')
: props.translate('workspace.bankAccount.accountDescriptionWithCards')}
</Text>
{!isUsingExpensifyCard && (
<Button
text={props.translate('workspace.bankAccount.addWorkEmail')}
onPress={() => {
Link.openOldDotLink(CONST.ADD_SECONDARY_LOGIN_URL);
User.subscribeToExpensifyCardUpdates();
}}
icon={Expensicons.Mail}
style={[styles.mt4]}
iconStyles={[styles.buttonCTAIcon]}
shouldShowRightIcon
large
success

<OfflineWithFeedback
pendingAction={pendingAction}
errors={errors}
shouldShowErrorMessage
onClose={BankAccounts.resetReimbursementAccount}
>
<MenuItem
title={bankName}
description={formattedBankAccountNumber}
icon={icon}
iconWidth={iconSize}
iconHeight={iconSize}
disabled
interactive={false}
wrapperStyle={[styles.cardMenuItem, styles.mv3]}
/>
<Text style={[styles.mv3]}>
{!isUsingExpensifyCard
? props.translate('workspace.bankAccount.accountDescriptionNoCards')
: props.translate('workspace.bankAccount.accountDescriptionWithCards')}
</Text>
{!isUsingExpensifyCard && (
<Button
text={props.translate('workspace.bankAccount.addWorkEmail')}
onPress={() => {
Link.openOldDotLink(CONST.ADD_SECONDARY_LOGIN_URL);
User.subscribeToExpensifyCardUpdates();
}}
icon={Expensicons.Mail}
style={[styles.mt4]}
iconStyles={[styles.buttonCTAIcon]}
shouldShowRightIcon
large
success
/>
)}
<MenuItem
title={props.translate('workspace.bankAccount.disconnectBankAccount')}
icon={Expensicons.Close}
onPress={BankAccounts.requestResetFreePlanBankAccount}
wrapperStyle={[styles.cardMenuItem, styles.mv3]}
disabled={Boolean(pendingAction)}
/>
)}
<MenuItem
title={props.translate('workspace.bankAccount.disconnectBankAccount')}
icon={Expensicons.Close}
onPress={BankAccounts.requestResetFreePlanBankAccount}
wrapperStyle={[styles.cardMenuItem, styles.mv3]}
/>
</OfflineWithFeedback>
</Section>
{props.user.isCheckingDomain && (
<Text style={[styles.formError, styles.mh5]}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ class ReimbursementAccountPage extends React.Component {
<ContinueBankAccountSetup
reimbursementAccount={this.props.reimbursementAccount}
continue={this.continue}
startOver={() => this.setState({shouldHideContinueSetupButton: true})}
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved
/>
);
}
Expand Down
Loading