[HOLD for payment 2024-10-30] QBD - Empty link in RHP when network is disconnected while QBD setup is loading

[IuliiaHerets]

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.47-1
Reproducible in staging?: Y
Reproducible in production?: N/A - new feature, doesn't exist in prod
Issue was found when executing this PR: #50216
Email or phone of affected tester (no customers): applausetester+pso@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Accounting.
3. Click Connect next to Quickbooks desktop
4. While RHP is loading, quickly disconnect network.

Expected Result:

RHP will show that the page is offline.

Actual Result:

RHP is loaded with empty link.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6630106_1728513620793.20241010_063732.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to @strepanier03 (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]

Triggered auto assignment to @techievivek (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[IuliiaHerets]

We think that this bug might be related to #wave-control

[hoangzinh]

The project is still in beta so I don't think it's a DB.

[hoangzinh]

cc @ZhenjaHorbach @lakchote ^

[ZhenjaHorbach]

It's not really clear what the expected result is
Should we show infinite loading
Or show separate screen to not show empty link

[lakchote]

The project is still in beta so I don't think it's a DB.

It's not a blocker as it's behind beta indeed.

[lakchote]

It's not really clear what the expected result is Should we show infinite loading Or show separate screen to not show empty link

I'd prefer to not show infinite loading when offline, as it would mistakenly make the user think something is going to happen and the link will show. We should find a way to notify him he's offline, and as such, cannot get the setup link.

Let's tag @Expensify/design to see if we already had to handle this situation before, and to have their point of view.

[dannymcclain]

Yeah I agree with not showing a loader forever—that doesn't seem super helpful.

Here are a couple ideas about how to handle this:

1) Replace the link with an offline indicator

If we can automatically update the link box with the link when it's available, this feels like the smoothest option.

2) Blast 'em with our blocking offline pattern

If we can't automatically update the link/we actually need them to just try again once they come back online, this pattern might work best.

cc @dubielzyk-expensify for thoughts and preferences.

[hoangzinh]

2nd idea looks promising to me 👀

[dubielzyk-expensify]

I think the 2nd idea just for the simplicity and consistency 👍

[strepanier03]

I think the 2nd makes it really clear but the 1st one looks more friendly.

[lakchote]

Thanks for the proposals @dannymcclain!

If the user comes back online, we can fetch the setup link again, thus making option #1 viable.

Code will need to be adjusted either way to take into account going offline cc @hoangzinh :

Example for option #1:

    const {isOffline} = useNetwork();
    useEffect(() => {
        if (isOffline) {
            setCodatSetupLink('');
            setIsLoading(false);
            return;
        }
        const fetchSetupLink = () => {
             //...existing logic
        };
        fetchSetupLink();
    }, [isOffline]);

I'm with @strepanier03, option #1 looks more friendly, but I don't have any hard preference.

If the option #2 is more consistent in terms of what we're doing in our design patterns, let's do #2 as consistency is key.

[daledah]

Proposal

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

RHP is loaded with empty link.

What is the root cause of that problem?

QuickBooksDesktopSetupPage currently doesn't have logics to handle offline case

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

Regarding #50548 (comment)

1. Replace the link with an offline indicator

Update Logics inside this useEffect to handle offline cases

    const {isOffline} = useNetwork();
    useEffect(() => {
        if (isOffline) {
            setCodatSetupLink('');
            setIsLoading(false);
            return;
        }
        const fetchSetupLink = () => {
            // eslint-disable-next-line rulesdir/no-thenable-actions-in-views
            QuickBooksDesktop.getQuickbooksDesktopCodatSetupLink(policyID).then((response) => {
                setCodatSetupLink(String(response?.setupUrl ?? ''));
                setIsLoading(false);
            });
        };
        fetchSetupLink();
    }, [isOffline]);

Add a new text You appear to be offline. Your setup link will be available when you're back online and update offline indicator to this part:

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

Lines 66 to 71 in abdfb72

	<View style={[styles.qbdSetupLinkBox, styles.mt5]}>
	<CopyTextToClipboard
	text={codatSetupLink}
	textStyles={[styles.textSupporting]}
	/>
	</View>

<View style={[styles.qbdSetupLinkBox, styles.mt5]}>
                    {!isOffline && (
                        <CopyTextToClipboard
                            text={codatSetupLink}
                            textStyles={[styles.textSupporting]}
                        />
                    )}
                    {isOffline && (
                        <View style={[styles.flexRow, styles.alignItemsCenter]}>
                            <Icon
                                fill={theme.icon}
                                src={Expensicons.OfflineCloud}
                                width={variables.iconSizeSmall}
                                height={variables.iconSizeSmall}
                            />
                            <Text style={[styles.ml3, styles.chatItemComposeSecondaryRowSubText]}>You appear to be offline. Your setup link will be available when you're back online</Text>
                        </View>
                    )}
                </View>

2. Blast 'em with our blocking offline pattern

Wrap this part inside FullPageOfflineBlockingView.

What alternative solutions did you explore? (Optional)

[dannymcclain]

I think the 2nd idea just for the simplicity and consistency 👍

Totally down to go with the full offline screen to keep things consistent across the app. 👍

[hoangzinh]

@lakchote Given this is an edge case, can I put this issue at the end of the queue and focus on other QBD issues?

[lakchote]

@lakchote Given this is an edge case, can I put this issue at the end of the queue and focus on other QBD issues?

Sure!

[ZhenjaHorbach]

@lakchote @dannymcclain
About this issue

If we already have setupLink
Do we need to show OfflineBlockingView on full screen when user is offline ?

[dannymcclain]

🤔 Hmm, I don't think we do if we already have the link. Obviously the user might run into problems opening the link if they're truly offline, but IMO that's not really on us. Open to other perspectives though!

[hoangzinh]

I don't think we do if we already have the link

Yes, I have same thought.

[lakchote]

@ZhenjaHorbach, I agree with @dannymcclain and @hoangzinh.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.52-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [QBD] Offline mode for RequireQuickBooksDesktopModal #50915

If no regressions arise, payment will be issued on 2024-10-30. 🎊

For reference, here are some details about the assignees on this issue:

• @hoangzinh requires payment (Needs manual offer from BZ)
• @ZhenjaHorbach requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@hoangzinh / @ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:
• [@hoangzinh / @ZhenjaHorbach] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@hoangzinh / @ZhenjaHorbach] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@hoangzinh / @ZhenjaHorbach] Determine if we should create a regression test for this bug.
• [@hoangzinh / @ZhenjaHorbach] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: