[$250] Accounting - No warning modal when connecting to NetSuite/Intacct while having existing connection #48269

lanitochka17 · 2024-08-29T13:46:44Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 9.0.26-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

Workspace is connected to QBO
Workspace is under Collect plan

Go to staging.new.expensify.com
Go to workspace settings > Accounting
Click Other integrations
Click Connect next to Xero
Note that it shows warning modal
Close the modal
Click Connect next to NetSuite or Intacct
Click Upgrade
Click Got it, thanks
Note that it proceeds to NetSuite login without the warning modal

Expected Result:

This is how we want the flow to work:

Click Connect alongside NetSuite or Intacct.
Upgrade RHP opens
Click Upgrade
Click Got it, thanks (or <)
RHP closes, centered warning modal appears
Click Connect
Existing integration is disconnected, RHP opens to connect.

Actual Result:

app does not throw warning pop-up when user attempts to connect to NetSuite while connecting to QBO
This issue happens with NetSuite and Sage Intacct which require Control plan

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

Bug6585993_1724893015428.20240829_085101.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~014b053b91eb324667
Upwork Job ID: 1829168357156360660
Last Price Increase: 2024-08-29
Automatic offers:

suneox | Reviewer | 103753753
nyomanjyotisa | Contributor | 103753756

Issue Owner

Current Issue Owner: @nyomanjyotisa

melvin-bot · 2024-08-29T13:46:53Z

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

lanitochka17 · 2024-08-29T13:47:32Z

@joekaufmanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

joekaufmanexpensify · 2024-08-29T14:38:08Z

Reproduced. This only happens when upgrading the workspace. If it's already a control workspace and you try and connect with NetSuite or Intacct, you are warned that the connection will be wiped.

Another issue is that the modal is saying Connect to QuickBooks online when you click to connect to Xero, which seems odd...

Nodebrute · 2024-08-29T14:39:25Z

@joekaufmanexpensify Here's an open issue for wrong modal name #48265

joekaufmanexpensify · 2024-08-29T14:42:43Z

Got it. TY!

joekaufmanexpensify · 2024-08-29T14:44:49Z

Okay, we agreed here that the flow should be try to upgrade to NetSuite/Intacct > warning > upgrade path.

We should fix this for both NetSuite/Intacct here.

melvin-bot · 2024-08-29T14:44:56Z

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

melvin-bot · 2024-08-29T14:45:01Z

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

Nodebrute · 2024-08-29T14:46:42Z

Edited by proposal-police: This proposal was edited at 2024-08-29 14:48:06 UTC.

Proposal

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

No warning modal when connecting to NetSuite while having existing connection

What is the root cause of that problem?

Here we are passing backToAfterWorkspaceUpgradeRoute

App/src/pages/workspace/accounting/AccountingContext.tsx

Line 65 in e77f204

    
           ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, workspaceUpgradeNavigationDetails.integrationAlias, workspaceUpgradeNavigationDetails.backToAfterWorkspaceUpgradeRoute),

which will be

App/src/pages/workspace/accounting/utils.tsx

Line 129 in e77f204

    
           backToAfterWorkspaceUpgradeRoute: ROUTES.POLICY_ACCOUNTING_SAGE_INTACCT_PREREQUISITES.getRoute(policyID),

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

We can remove these upgrade routes from here and here
and here In backTo route we should navigate back to settings accounting page ROUTES.WORKSPACE_ACCOUNTING.getRoute(policyID)

App/src/pages/workspace/accounting/AccountingContext.tsx

Line 65 in e77f204

    
           ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, workspaceUpgradeNavigationDetails.integrationAlias, workspaceUpgradeNavigationDetails.backToAfterWorkspaceUpgradeRoute),

What alternative solutions did you explore? (Optional)

nkdengineer · 2024-08-29T14:58:02Z

Edited by proposal-police: This proposal was edited at 2023-10-04T10:15:00Z.

Proposal

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

In Step 7, app does not throw warning pop-up when user attempts to connect to NetSuite while connecting to QBO
This issue happens with NetSuite and Sage Intacct which require Control plan

What is the root cause of that problem?

When we startIntegrationFlow here for NetSuite/Intacct because these connections require a control policy then we navigate to upgrade page without showing warning modal

App/src/pages/workspace/accounting/AccountingContext.tsx

Lines 59 to 75 in e77f204

    
           const startIntegrationFlow = React.useCallback( 
        
               (newActiveIntegration: ActiveIntegration) => { 
        
                   const accountingIntegrationData = getAccountingIntegrationData(newActiveIntegration.name, policyID, translate); 
        
                   const workspaceUpgradeNavigationDetails = accountingIntegrationData?.workspaceUpgradeNavigationDetails; 
        
                   if (workspaceUpgradeNavigationDetails && !isControlPolicy(policy)) { 
        
                       Navigation.navigate( 
        
                           ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, workspaceUpgradeNavigationDetails.integrationAlias, workspaceUpgradeNavigationDetails.backToAfterWorkspaceUpgradeRoute), 
        
                       ); 
        
                       return; 
        
                   } 
        
                   setActiveIntegration({ 
        
                       ...newActiveIntegration, 
        
                       key: Math.random(), 
        
                   }); 
        
               }, 
        
               [policy, policyID, translate], 
        
           );

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

We can display the warning modal first then if we click on confirm button we will navigate to the upgrade page

To do that we can follow these steps in AccountingContextProvider

Create a ref onModalHideRef to store the callback that we need to do when clicking on confirm button

const onModalHideRef = useRef<(() => void) | undefined>(undefined);

We will assign onModalHideRef as the navigate function here and not return early if newActiveIntegration.integrationToDisconnect is not undefined

if (workspaceUpgradeNavigationDetails && !isControlPolicy(policy)) {
    if (newActiveIntegration?.integrationToDisconnect) {
        onModalHideRef.current = () =>
            Navigation.navigate(
                ROUTES.WORKSPACE_UPGRADE.getRoute(
                    policyID,
                    workspaceUpgradeNavigationDetails.integrationAlias,
                    workspaceUpgradeNavigationDetails.backToAfterWorkspaceUpgradeRoute,
                ),
            );
    } else {
        Navigation.navigate(
            ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, workspaceUpgradeNavigationDetails.integrationAlias, workspaceUpgradeNavigationDetails.backToAfterWorkspaceUpgradeRoute),
        );
        return;
    }
} else {
    onModalHideRef.current = undefined;
}

App/src/pages/workspace/accounting/AccountingContext.tsx

Lines 63 to 66 in e77f204

    
           if (workspaceUpgradeNavigationDetails && !isControlPolicy(policy)) { 
        
               Navigation.navigate( 
        
                   ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, workspaceUpgradeNavigationDetails.integrationAlias, workspaceUpgradeNavigationDetails.backToAfterWorkspaceUpgradeRoute), 
        
               );

Call the onModalHideRef callback when we click on confirm button here

onConfirm={() => {
    if (!activeIntegration?.integrationToDisconnect) {
        return;
    }
    removePolicyConnection(policyID, activeIntegration?.integrationToDisconnect);
    closeConfirmationModal();
    onModalHideRef.current?.();
}}

App/src/pages/workspace/accounting/AccountingContext.tsx

Line 115 in e77f204

onConfirm={() => {

We will also need to reset the onModalHideRef to undefined when we close the confirm modal and after we trigger this callback

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2024-08-29.at.21.57.16.mov

nkdengineer · 2024-08-29T15:05:06Z

Updated proposal

Update implementation

nyomanjyotisa · 2024-08-29T15:13:48Z

Edited by proposal-police: This proposal was edited at 2023-10-05T12:01:00Z.

Proposal

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

No warning modal when connecting to NetSuite/Intacct while having existing connection

What is the root cause of that problem?

On click "Got it, thanks" on workspace upgrade navigate directly to POLICY_ACCOUNTING_NETSUITE_TOKEN_INPUT due to this code and this

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

we should pass newActiveIntegration.integrationToDisconnect to getAccountingIntegrationData here

then we should navigate to POLICY_ACCOUNTING if there is integration to disconnect
change this and this to the following

backToAfterWorkspaceUpgradeRoute: !!integrationToDisconnect ? ROUTES.POLICY_ACCOUNTING.getRoute(policyID) : ROUTES.POLICY_ACCOUNTING_NETSUITE_TOKEN_INPUT.getRoute(policyID),

Once we navigate back to POLICY_ACCOUNTING after the workspace update, it don't automatically display the warning modal. So we can do either of the following to automatically display it after navigate from the workspace upgrade:

Pass connectionName, integrationToDisconnect, shouldDisconnectIntegrationBeforeConnecting to POLICY_ACCOUNTING route, then use these values to startIntegrationFlow once the page opened
Save connectionName, integrationToDisconnect, shouldDisconnectIntegrationBeforeConnecting on onyx, and do the same

What alternative solutions did you explore? (Optional)

joekaufmanexpensify · 2024-08-29T15:49:24Z

We revised the plan in Slack. Here's how we want it to work:

Click Connect alongside NetSuite or Intacct.
Upgrade RHP opens
Click Upgrade
Click Got it, thanks (or <)
RHP closes, centered warning modal appears
Click Connect
Existing integration is disconnected, RHP opens to connect.

Updated the OP to reflect this!

Nodebrute · 2024-08-29T15:58:50Z

@joekaufmanexpensify Could you please review this flow?

Screen.Recording.2024-08-29.at.8.55.09.PM.mov

nyomanjyotisa · 2024-08-29T17:15:46Z

Proposal updated

joekaufmanexpensify · 2024-08-30T12:52:30Z

Pending proposal review by @suneox

suneox · 2024-08-30T13:00:20Z

Thanks for all the proposals. I’ll start the review within a few hours

suneox · 2024-08-30T15:59:51Z

Based on the latest OP, the RCA from @nyomanjyotisa proposal isn't clearly & solution doesn't match with expected result

Solution from @nkdengineer proposal also doesn't match with expected result

Screen.Recording.2024-08-30.at.22.14.57.mp4

After click on Got it, thanks at step5: RHP closes then centered warning modal appears instead of display the warning modal first

RCA from @nyomanjyotisa proposal is correct & solution add more parameter to handle re-trigger setActiveIntegration flow to show warning modal after navigation back is LGTM, so I think we can go ahead with this proposal

🎀 👀 🎀 C+ reviewed

melvin-bot · 2024-08-30T16:00:04Z

Triggered auto assignment to @thienlnam, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot · 2024-08-30T17:48:01Z

📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

melvin-bot · 2024-08-30T17:48:05Z

📣 @nyomanjyotisa 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

joekaufmanexpensify · 2024-09-02T15:15:44Z

@nyomanjyotisa is there an ETA for PR here?

melvin-bot · 2024-09-02T18:41:03Z

@suneox, @thienlnam, @joekaufmanexpensify, @nyomanjyotisa Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

joekaufmanexpensify · 2024-09-03T13:33:58Z

PR is WIP. @nyomanjyotisa will this be in review today?

melvin-bot · 2024-09-06T09:11:16Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

eh2077 · 2024-09-06T11:35:42Z

@suneox Are you available to review this PR #48698 to fix the DB?

suneox · 2024-09-06T12:04:06Z

@suneox Are you available to review this PR #48698 to fix the DB?

@eh2077 I'll start review now

joekaufmanexpensify · 2024-09-09T13:33:11Z

PR on staging. It seems like there was a regression too that we had to fix, is that right?

suneox · 2024-09-09T14:02:44Z

PR on staging. It seems like there was a regression too that we had to fix, is that right?

Yes, the PR for this issue raised regression #48686 & it has been fixed.

joekaufmanexpensify · 2024-09-09T14:03:53Z

Sweet. TY!

joekaufmanexpensify · 2024-09-16T14:05:27Z

Looks like automation failed here, and this PR went to prod on 09/09. Payment is due today.

joekaufmanexpensify · 2024-09-16T14:06:24Z

We need to pay:

@nyomanjyotisa - $125 for PR via Upwork (50% reduction for regression).
@suneox - $125 for C+ via Upwork (50% reduction for regression).

joekaufmanexpensify · 2024-09-16T14:06:33Z

All set to pay!

joekaufmanexpensify · 2024-09-16T14:08:53Z

@nyomanjyotisa $125 sent and contract ended!

joekaufmanexpensify · 2024-09-16T14:09:03Z

@suneox $125 sent and contract ended!

joekaufmanexpensify · 2024-09-16T14:09:07Z

Upwork job closed.

joekaufmanexpensify · 2024-09-16T14:09:13Z

All set. TY everyone!

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 29, 2024

melvin-bot bot assigned joekaufmanexpensify Aug 29, 2024

joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Aug 29, 2024

melvin-bot bot changed the title ~~Accounting - No warning modal when connecting to NetSuite while having existing connection~~ [$250] Accounting - No warning modal when connecting to NetSuite while having existing connection Aug 29, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 29, 2024

melvin-bot bot assigned suneox Aug 29, 2024

joekaufmanexpensify changed the title ~~[$250] Accounting - No warning modal when connecting to NetSuite while having existing connection~~ [$250] Accounting - No warning modal when connecting to NetSuite/Intacct while having existing connection Aug 29, 2024

melvin-bot bot assigned thienlnam Aug 30, 2024

thienlnam assigned nyomanjyotisa Aug 30, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 30, 2024

melvin-bot bot added the Overdue label Sep 2, 2024

nyomanjyotisa mentioned this issue Sep 2, 2024

Fix: Show warning modal on POLICY_ACCOUNTING after workspace upgrade #48419

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Sep 3, 2024

trjExpensify mentioned this issue Sep 10, 2024

Accounting-App proceeds to Netsuite Setup when trying to connect to Sage Intacct #48686

Closed

6 tasks

joekaufmanexpensify closed this as completed Sep 16, 2024

[$250] Accounting - No warning modal when connecting to NetSuite/Intacct while having existing connection #48269

[$250] Accounting - No warning modal when connecting to NetSuite/Intacct while having existing connection #48269

Comments

lanitochka17 commented Aug 29, 2024 • edited by joekaufmanexpensify Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Aug 29, 2024

lanitochka17 commented Aug 29, 2024

joekaufmanexpensify commented Aug 29, 2024

Nodebrute commented Aug 29, 2024

joekaufmanexpensify commented Aug 29, 2024

joekaufmanexpensify commented Aug 29, 2024

melvin-bot bot commented Aug 29, 2024

melvin-bot bot commented Aug 29, 2024

Nodebrute commented Aug 29, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

nkdengineer commented Aug 29, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Result

nkdengineer commented Aug 29, 2024

nyomanjyotisa commented Aug 29, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

joekaufmanexpensify commented Aug 29, 2024

Nodebrute commented Aug 29, 2024

nyomanjyotisa commented Aug 29, 2024

joekaufmanexpensify commented Aug 30, 2024

suneox commented Aug 30, 2024

suneox commented Aug 30, 2024

melvin-bot bot commented Aug 30, 2024

melvin-bot bot commented Aug 30, 2024

melvin-bot bot commented Aug 30, 2024

joekaufmanexpensify commented Sep 2, 2024

melvin-bot bot commented Sep 2, 2024

joekaufmanexpensify commented Sep 3, 2024

melvin-bot bot commented Sep 6, 2024

eh2077 commented Sep 6, 2024

suneox commented Sep 6, 2024 • edited Loading

joekaufmanexpensify commented Sep 9, 2024

suneox commented Sep 9, 2024

joekaufmanexpensify commented Sep 9, 2024

joekaufmanexpensify commented Sep 16, 2024

joekaufmanexpensify commented Sep 16, 2024

joekaufmanexpensify commented Sep 16, 2024

joekaufmanexpensify commented Sep 16, 2024

joekaufmanexpensify commented Sep 16, 2024

joekaufmanexpensify commented Sep 16, 2024

joekaufmanexpensify commented Sep 16, 2024

lanitochka17 commented Aug 29, 2024 •

edited by joekaufmanexpensify

Loading

Nodebrute commented Aug 29, 2024 •

edited by github-actions bot

Loading

nkdengineer commented Aug 29, 2024 •

edited

Loading

nyomanjyotisa commented Aug 29, 2024 •

edited by github-actions bot

Loading

suneox commented Sep 6, 2024 •

edited

Loading