[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] [Wave Control - NetSuite] Have Accounting Connection Sync Spinner Timeout After 20 minutes

[yuwenmemon]

Part of the #Wave-Control NetSuite project

Main issue: https://github.com/Expensify/Expensify/issues/377671
Doc section: https://docs.google.com/document/d/1WubNv_VAv78IxG4FKsi9aS0pWESfWvUYbqBAAy5b4bc/edit#heading=h.tb9yro1dmj3r
Project: https://github.com/orgs/Expensify/projects/130

Feature Description

One issue with the Pusher ProgressManager updates for syncs that come from the IS to NewDot is that they are sometimes unreliable. If the jobDone sync update is not received by the client, then the sync spinner will hang forever, even though the sync has finished:

Kapture.2024-05-17.at.15.00.36.mp4

To fix this, let's have the spinner timeout to stop 20 min after a sync is started and no jobDone sync update is received. We landed one 20 minutes because, in recent measurements, our longest NetSuite jobs take around 14 minutes.

Manual Test Steps

1. Initiate a QBO sync from your workspace that's connected to QBO
2. Go offline
3. Wait for 10 min
4. Go back online
5. Make sure that after 10 more minutes, the spinner has stopped.

Automated Tests

N/A

cc @aldo-expensify @francoisl @lakchote @marcaaron @tgolen

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c42a63242268a2f0
• Upwork Job ID: 1792660966817673216
• Last Price Increase: 2024-05-29

Issue Owner

Current Issue Owner: @twisterdotcom

[melvin-bot]

Triggered auto assignment to @twisterdotcom (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.

[twisterdotcom]

I think this is New Feature, not Bug right?

[melvin-bot]

Current assignee @twisterdotcom is eligible for the NewFeature assigner, not assigning anyone new.

[yuwenmemon]

Yeah that seems like a fair characterization as well.

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

Spinner hasn't stopped even though the sync has finished

What is the root cause of that problem?

This is a new feature

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

1. In QuickBooksOnline, create a function like syncConnectionAfterTimeout that will update stageInProgress to jobDone

function syncConnectionAfterTimeout(policyID: string) {
    Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS}${policyID}`,  {
        stageInProgress: CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE,
        connectionName: CONST.POLICY.CONNECTIONS.NAME.QBO,
    })
}

2. In PolicyAccountingPage, we will create a useEffect that will be triggered when connectionSyncProgress?.stageInProgress is changed and in this we will have a setTimeout 20 minutes to update connectionSyncProgress?.stageInProgress to jobDone so if a sync is started and no jobDone sync update is received

useEffect(() => {
    if (!connectionSyncProgress?.stageInProgress || connectionSyncProgress?.stageInProgress === CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.JOB_DONE) {
        return;
    }
    spinnerTimeoutRef.current = setTimeout(() => {
        syncConnectionAfterTimeout(policy.id);
    }, 20*60*1000);

    return () => {
        if (!spinnerTimeoutRef.current) {
            return;
        }

        clearTimeout(spinnerTimeoutRef.current)
    }
}, [connectionSyncProgress?.stageInProgress])

What alternative solutions did you explore? (Optional)

We can create a state to control whenever we should show spinner and then update it in the setTimeout of point 2

[aldo-expensify]

@yuwenmemon for this, instead of adding a setTimeout in the front end, I think it can be more reliably implemented like:

1. The IS server should include a timestamp in the progress data (PolicyConnectionSyncProgress)
2. isSyncInProgress should ignore the progress data if it is older than 20 minutes

What do you think?

[twisterdotcom]

Removed the /app job and automation hopefully.

[twisterdotcom]

Ah crap, wait I see @ahmedGaber93 reviewed #42498. Damn, thought that was an internal PR. Okay, will pay that out when it's due.

[melvin-bot]

Upwork job price has been updated to $250

[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 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Make sure that the connection sync progress times out after 20 minutes using timestamp #42498

If no regressions arise, payment will be issued on 2024-06-18. 🎊

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

• @ahmedGaber93 requires payment (Needs manual offer from BZ)

[melvin-bot]

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

• [@ahmedGaber93] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[melvin-bot]

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

• Make sure that the connection sync progress times out after 20 minutes using timestamp #42498

If no regressions arise, payment will be issued on 2024-06-20. 🎊

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

• @ahmedGaber93 requires payment (Needs manual offer from BZ)

[melvin-bot]

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

• [@ahmedGaber93] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[twisterdotcom]

Payment Summary:

• @ahmedGaber93 paid $250 here (C+)

NewFeature, not sure we need a regression test right?

[ahmedGaber93]

@twisterdotcom offer accepted, please please complete the payment when you are available. Thanks!