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.

Sign up for GitHub

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

Android-BA-Tapping connect online with plaid shows 2 spin circles loading #32641

Merged
merged 4 commits into from
Dec 22, 2023
Merged

Android-BA-Tapping connect online with plaid shows 2 spin circles loading #32641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de99c1a
Refactor AddPlaidBankAccount
ZhenjaHorbach Dec 7, 2023
0cb834c
Update PlaidLink
ZhenjaHorbach Dec 7, 2023
c5a5997
Update branch
ZhenjaHorbach Dec 19, 2023
5a9f6d7
Fix TS issues
ZhenjaHorbach Dec 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
96 changes: 53 additions & 43 deletions src/components/AddPlaidBankAccount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,52 +181,62 @@ function AddPlaidBankAccount({
);
}

// Plaid Link view
if (!plaidBankAccounts.length) {
return (
<FullPageOfflineBlockingView>
{lodashGet(plaidData, 'isLoading') && (
<View style={[styles.flex1, styles.alignItemsCenter, styles.justifyContentCenter]}>
<ActivityIndicator
color={theme.spinner}
size="large"
/>
</View>
)}
{Boolean(plaidDataErrorMessage) && <Text style={[styles.formError, styles.mh5]}>{plaidDataErrorMessage}</Text>}
{Boolean(token) && !bankName && (
<PlaidLink
token={token}
onSuccess={({publicToken, metadata}) => {
Log.info('[PlaidLink] Success!');
BankAccounts.openPlaidBankAccountSelector(publicToken, metadata.institution.name, allowDebit, bankAccountID);
}}
onError={(error) => {
Log.hmmm('[PlaidLink] Error: ', error.message);
}}
onEvent={(event, metadata) => {
BankAccounts.setPlaidEvent(event);
// Handle Plaid login errors (will potentially reset plaid token and item depending on the error)
if (event === 'ERROR') {
Log.hmmm('[PlaidLink] Error: ', metadata);
if (bankAccountID && metadata && metadata.error_code) {
BankAccounts.handlePlaidError(bankAccountID, metadata.error_code, metadata.error_message, metadata.request_id);
}
const renderPlaidLink = () => {
if (Boolean(token) && !bankName) {
return (
<PlaidLink
token={token}
onSuccess={({publicToken, metadata}) => {
Log.info('[PlaidLink] Success!');
BankAccounts.openPlaidBankAccountSelector(publicToken, metadata.institution.name, allowDebit, bankAccountID);
}}
onError={(error) => {
Log.hmmm('[PlaidLink] Error: ', error.message);
}}
onEvent={(event, metadata) => {
BankAccounts.setPlaidEvent(event);
// Handle Plaid login errors (will potentially reset plaid token and item depending on the error)
if (event === 'ERROR') {
Log.hmmm('[PlaidLink] Error: ', metadata);
if (bankAccountID && metadata && metadata.error_code) {
BankAccounts.handlePlaidError(bankAccountID, metadata.error_code, metadata.error_message, metadata.request_id);
}
}

// Limit the number of times a user can submit Plaid credentials
if (event === 'SUBMIT_CREDENTIALS') {
App.handleRestrictedEvent(event);
}
}}
// User prematurely exited the Plaid flow
// eslint-disable-next-line react/jsx-props-no-multi-spaces
onExit={onExitPlaid}
receivedRedirectURI={receivedRedirectURI}
// Limit the number of times a user can submit Plaid credentials
if (event === 'SUBMIT_CREDENTIALS') {
App.handleRestrictedEvent(event);
}
}}
// User prematurely exited the Plaid flow
// eslint-disable-next-line react/jsx-props-no-multi-spaces
onExit={onExitPlaid}
receivedRedirectURI={receivedRedirectURI}
/>
);
}

if (plaidDataErrorMessage) {
return <Text style={[styles.formError, styles.mh5]}>{plaidDataErrorMessage}</Text>;
}

if (lodashGet(plaidData, 'isLoading')) {
Comment on lines +219 to +223
Copy link
Contributor

Choose a reason for hiding this comment

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

The one thing I'm slightly worried about here... is this way, we'll never be able to show the error message AND a loading indicator at the same time, right? Where previously we could... Can we make sure this doesn't cause any regressions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we expect only one state from the backend
-isLoader (Loader)
-bankName (PlaidLink)
-Error (Text error)
I'm not sure that such cases are possible

App/src/libs/actions/Plaid.ts

Lines 51 to 80 in 990815f

optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.PLAID_DATA,
value: {
isLoading: true,
errors: null,
bankName,
},
},
],
successData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.PLAID_DATA,
value: {
isLoading: false,
errors: null,
},
},
],
failureData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.PLAID_DATA,
value: {
isLoading: false,
},
},
],

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm those are just optimistic loading states though, i could see a situation where an error happens and we want to try again so we set isLoading again, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Beamanator @ZhenjaHorbach that's what I was pointing out, even if we have an error message we will only show the loader because the loader is inside PlaidLink component and we will always show the PlaidLink, and if we show the error then we can't retry because the PlaidLink component will not be rendered.

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 get the state for showing components from plaidData
If we have at the moment optimisticData we show loader
If we have at the moment successData we show plaidData (The loader that is located inside the web plaidLink is associated with that the component we use requires initialization )
If we have the moment failure we show error text

And if we will get error when plaidLink is open we will skip token and hide plaidLink but show error text

Copy link
Contributor

Choose a reason for hiding this comment

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

oh wait ya i see that we clear the error any time we try again so most likely they won't ever be shown at the same time

return (
<View style={[styles.flex1, styles.alignItemsCenter, styles.justifyContentCenter]}>
<ActivityIndicator
color={theme.spinner}
size="large"
/>
)}
</FullPageOfflineBlockingView>
);
</View>
);
}

return <View />;
};

// Plaid Link view
if (!plaidBankAccounts.length) {
return <FullPageOfflineBlockingView>{renderPlaidLink()}</FullPageOfflineBlockingView>;
}

// Plaid bank accounts view
Expand Down
16 changes: 14 additions & 2 deletions src/components/PlaidLink/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import {useCallback, useEffect, useState} from 'react';
import React, {useCallback, useEffect, useState} from 'react';
import {ActivityIndicator, View} from 'react-native';
import {PlaidLinkOnSuccessMetadata, usePlaidLink} from 'react-plaid-link';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import Log from '@libs/Log';
import PlaidLinkProps from './types';

function PlaidLink({token, onSuccess = () => {}, onError = () => {}, onExit = () => {}, onEvent, receivedRedirectURI}: PlaidLinkProps) {
const [isPlaidLoaded, setIsPlaidLoaded] = useState(false);
const theme = useTheme();
const styles = useThemeStyles();
const successCallback = useCallback(
(publicToken: string, metadata: PlaidLinkOnSuccessMetadata) => {
onSuccess({publicToken, metadata});
Expand Down Expand Up @@ -47,7 +52,14 @@ function PlaidLink({token, onSuccess = () => {}, onError = () => {}, onExit = ()
open();
}, [ready, error, isPlaidLoaded, open, onError]);

return null;
return (
<View style={[styles.flex1, styles.alignItemsCenter, styles.justifyContentCenter]}>
<ActivityIndicator
color={theme.spinner}
size="large"
/>
</View>
);
}

PlaidLink.displayName = 'PlaidLink';
Expand Down