[HOLD for payment 2024-12-30] [$250] QBD - QBD is displayed like an established connection without error after dismissing the RHP

[vincdargento]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.71-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5297104
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team

Action Performed:

1. Go to ND and log in with a new expensifail account
2. Create a new collect workspace
3. Go to "More Features" in workspace settings
4. Enable the Accounting toggle
5. Navigate to Accounting
6. Click on the connect button next to QBD
7. Upgrade your workspace to Control plan
8. Dismiss the RHP before the setup link appears
9. Navigate to any other page in workspace settings
10. Go back to Accounting

Expected Result:

QuickBooks Desktop should not be displayed as if a successful connection was made due to the RHP being dismissed even before the setup link appears. The behavior should be similar to the other integrations.

Actual Result:

QBD is displayed like an established connection without error after dismissing the RHP before the setup link appears.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021865081766703229432
• Upwork Job ID: 1865081766703229432
• Last Price Increase: 2024-12-06
• Automatic offers:
• ikevin127 | Contributor | 105312550

Issue Owner

Current Issue Owner: @bfitzexpensify

[melvin-bot]

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021865081766703229432

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

[ikevin127]

Proposal

Please re-state the problem that we are trying to solve in this issue

QBD is displayed like an established connection without error after dismissing the RHP before the setup link appears.

What is the root cause of that problem?

App/src/pages/workspace/accounting/qbd/QuickBooksDesktopSetupPage.tsx

Lines 62 to 69 in b595f5d

	useEffect(() => {
	// Since QBD doesn't support Taxes, we should disable them from the LHN when connecting to QBD
	PolicyAction.enablePolicyTaxes(policyID, false);
	fetchSetupLink();
	// disabling this rule, as we want this to run only on the first render
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
	}, []);

The code block above calls ConnectPolicyToQuickbooksDesktop when the RHP modal mounts, the modal starting with isLoading state variable as true which leads to the QBD connection to be established even when the RHP modal is closed while still loading because the call is initialized on mount.

The issue is that this component was designed such that the setup link appears only when the loading state becomes false and hasResultOfFetchingSetupLink becomes true as a result of the ConnectPolicyToQuickbooksDesktop call being completed, therefore the appearance of the setup link depends on the loading state which depends on the call being made.

What changes do you think we should make in order to solve the problem?

Given that the setup link being shown depends on the loading state becoming false and hasResultOfFetchingSetupLink being true, which depends on the ConnectPolicyToQuickbooksDesktop call being completed, and the fact that the Expected result requires that the call should only be made once the setup link is visible, this is my suggested approach:

• remove the isLoading state variable completely from the RequireQuickBooksDesktopModal component
• remove the shouldShowLoading variable as well as the FullScreenLoadingIndicator which comes with it

this way the setup link page content is visible right away except for the copy to clipboard link where we will show a small ActivityIndicator until ConnectPolicyToQuickbooksDesktop call is completed - this way there's no issue anymore since we eliminated the full screen loading loading indicator which led to this issue.

This would pose another issue when it comes to the setup link content page because the link only becomes available once ConnectPolicyToQuickbooksDesktop call is completed and hasResultOfFetchingSetupLink becomes true.

For this we have to conditionally show the CopyTextToClipboard component only when hasResultOfFetchingSetupLink is true, if it's false we display a small ActivityIndicator:

{!hasResultOfFetchingSetupLink ? (
    <ActivityIndicator
        color={theme.spinner}
        size="small"
    />
) : (
    <CopyTextToClipboard
        text={codatSetupLink}
        textStyles={[styles.textSupporting]}
    />
)}

Here's the complete DIFF for clarity

diff --git a/src/pages/workspace/accounting/qbd/QuickBooksDesktopSetupPage.tsx b/src/pages/workspace/accounting/qbd/QuickBooksDesktopSetupPage.tsx
index 5e90c3db4ed..02a0744e356 100644
--- a/src/pages/workspace/accounting/qbd/QuickBooksDesktopSetupPage.tsx
+++ b/src/pages/workspace/accounting/qbd/QuickBooksDesktopSetupPage.tsx
@@ -1,11 +1,11 @@
+import {useTheme} from '@react-navigation/native';
 import React, {useCallback, useEffect, useState} from 'react';
-import {View} from 'react-native';
+import {ActivityIndicator, View} from 'react-native';
 import Computer from '@assets/images/laptop-with-second-screen-sync.svg';
 import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
 import Button from '@components/Button';
 import CopyTextToClipboard from '@components/CopyTextToClipboard';
 import FixedFooter from '@components/FixedFooter';
-import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
 import HeaderWithBackButton from '@components/HeaderWithBackButton';
 import Icon from '@components/Icon';
 import * as Illustrations from '@components/Icon/Illustrations';
@@ -31,10 +31,10 @@ type RequireQuickBooksDesktopModalProps = PlatformStackScreenProps<SettingsNavig
 
 function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalProps) {
     const {translate} = useLocalize();
+    const theme = useTheme();
     const styles = useThemeStyles();
     const {environmentURL} = useEnvironment();
     const policyID: string = route.params.policyID;
-    const [isLoading, setIsLoading] = useState(true);
     const [hasError, setHasError] = useState(false);
     const [codatSetupLink, setCodatSetupLink] = useState<string>('');
     const hasResultOfFetchingSetupLink = !!codatSetupLink || hasError;
@@ -42,20 +42,19 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
     const ContentWrapper = hasResultOfFetchingSetupLink ? ({children}: React.PropsWithChildren) => children : FullPageOfflineBlockingView;
 
     const fetchSetupLink = useCallback(() => {
-        setIsLoading(true);
         setHasError(false);
         // eslint-disable-next-line rulesdir/no-thenable-actions-in-views
         QuickbooksDesktop.getQuickbooksDesktopCodatSetupLink(policyID).then((response) => {
-            if (response?.jsonCode) {
-                if (response.jsonCode === CONST.JSON_CODE.SUCCESS) {
-                    setCodatSetupLink(String(response?.setupUrl ?? ''));
-                } else {
-                    setConnectionError(policyID, CONST.POLICY.CONNECTIONS.NAME.QBD, translate('workspace.qbd.setupPage.setupErrorTitle'));
-                    setHasError(true);
-                }
+            if (!response?.jsonCode) {
+                return;
             }
 
-            setIsLoading(false);
+            if (response.jsonCode === CONST.JSON_CODE.SUCCESS) {
+                setCodatSetupLink(String(response?.setupUrl ?? ''));
+            } else {
+                setConnectionError(policyID, CONST.POLICY.CONNECTIONS.NAME.QBD, translate('workspace.qbd.setupPage.setupErrorTitle'));
+                setHasError(true);
+            }
         });
     }, [policyID, translate]);
 
@@ -77,8 +76,7 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
         },
     });
 
-    const shouldShowLoading = isLoading || !hasResultOfFetchingSetupLink;
-    const shouldShowError = !shouldShowLoading && hasError;
+    const shouldShowError = hasError;
 
     return (
         <ScreenWrapper
@@ -92,7 +90,6 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
                 onBackButtonPress={() => Navigation.dismissModal()}
             />
             <ContentWrapper>
-                {shouldShowLoading && <FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />}
                 {shouldShowError && (
                     <View style={[styles.flex1, styles.justifyContentCenter, styles.alignItemsCenter, styles.ph5, styles.mb9]}>
                         <Icon
@@ -113,7 +110,7 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
                         </Text>
                     </View>
                 )}
-                {!shouldShowLoading && !shouldShowError && (
+                {!shouldShowError && (
                     <View style={[styles.flex1, styles.ph5]}>
                         <View style={[styles.alignSelfCenter, styles.computerIllustrationContainer, styles.pv6]}>
                             <ImageSVG src={Computer} />
@@ -122,10 +119,17 @@ function RequireQuickBooksDesktopModal({route}: RequireQuickBooksDesktopModalPro
                         <Text style={[styles.textHeadlineH1, styles.pt5]}>{translate('workspace.qbd.setupPage.title')}</Text>
                         <Text style={[styles.textSupporting, styles.textNormal, styles.pt4]}>{translate('workspace.qbd.setupPage.body')}</Text>
                         <View style={[styles.qbdSetupLinkBox, styles.mt5]}>
-                            <CopyTextToClipboard
-                                text={codatSetupLink}
-                                textStyles={[styles.textSupporting]}
-                            />
+                            {!hasResultOfFetchingSetupLink ? (
+                                <ActivityIndicator
+                                    color={theme.spinner}
+                                    size="small"
+                                />
+                            ) : (
+                                <CopyTextToClipboard
+                                    text={codatSetupLink}
+                                    textStyles={[styles.textSupporting]}
+                                />
+                            )}
                         </View>
                         <FixedFooter style={[styles.mtAuto, styles.ph0]}>
                             <Button

This is how the UI would look with this solution:

MacOS: Web

web.mov

cc @Expensify/design for their take on this approach UI / UX wise

[shawnborton]

That looks pretty good to me. cc @kadiealexander as you worked on QBD too.

[dannymcclain]

Yeah I think it looks good too.