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.

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

[Due payment][$250] Netsuite - "Reuse existing connection" is an option when there is no existing workspace #46680

Closed
1 of 6 tasks
lanitochka17 opened this issue Aug 1, 2024 · 30 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 1, 2024

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.15-4
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
Issue reported by: Applause - Internal Team

Action Performed:

Precondition
An already signed-in account with a workspace connected to NetSuite integration

  1. Sign out of the account with the workspace connected to NetSuite integration (Important)
  2. Sign in with a new account
  3. Navigate to settings > Workspace > Create new Workspace
  4. Navigate to more features > Enable accounting
  5. Go to Accounting > Connect to NetSuite
  6. Upgrade the workspace to Control > Navigate back to Accounting > Click on "Connect" next to NetSuite
  7. Here notice that two options appear "Create new connection" and "Reuse existing connection"
  8. Click on "Reuse existing connection" > The workspace from the signed-out account appears > Click on the workspace

Expected Result:

Step 7. There will be no option for "Reuse existing connection" since there is no existing workspace connected to NetSuite in the account

Actual Result:

Step 7. "Reuse existing connection" is an option even if there is no existing workspace
Step 8. When clicking on the workspace from the signed-out account, Infinite loading is displayed and there is no way to cancel it

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6558884_1722491297311.2024-08-01_08_10_35.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b773ba1a34cd80ac
  • Upwork Job ID: 1823791114589945944
  • Last Price Increase: 2024-08-14
Issue OwnerCurrent Issue Owner: @
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

Triggered auto assignment to @kadiealexander (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
Copy link
Author

@kadiealexander 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

@lanitochka17
Copy link
Author

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

@melvin-bot melvin-bot bot added the Overdue label Aug 5, 2024
@kadiealexander kadiealexander added the Internal Requires API changes or must be handled by Expensify staff label Aug 6, 2024
@kadiealexander
Copy link
Contributor

Not overdue, looking for someone internal to pick it up.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 6, 2024
@kadiealexander
Copy link
Contributor

@yuwenmemon @mananjadhav is this on your radar at all?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 9, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

@kadiealexander Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@kadiealexander
Copy link
Contributor

@yuwenmemon @mananjadhav bump!

@melvin-bot melvin-bot bot removed the Overdue label Aug 14, 2024
@mananjadhav
Copy link
Collaborator

Sorry I was OOO past few days. I'll take a look at this one.

@yuwenmemon yuwenmemon added External Added to denote the issue can be worked on by a contributor and removed Internal Requires API changes or must be handled by Expensify staff labels Aug 14, 2024
Copy link

melvin-bot bot commented Aug 14, 2024

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

@melvin-bot melvin-bot bot changed the title Netsuite - "Reuse existing connection" is an option when there is no existing workspace [$250] Netsuite - "Reuse existing connection" is an option when there is no existing workspace Aug 14, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 14, 2024
Copy link

melvin-bot bot commented Aug 14, 2024

Current assignee @mananjadhav is eligible for the External assigner, not assigning anyone new.

@daledah
Copy link
Contributor

daledah commented Aug 15, 2024

Proposal

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

Netsuite - "Reuse existing connection" is an option when there is no existing workspace

What is the root cause of that problem?

  • When clicking on "Connect" button, we show the reuse connection popover:

    setIsReuseConnectionsPopoverOpen(true);

    if hasPoliciesConnectedToNetSuite:
    const hasPoliciesConnectedToNetSuite = !!getAdminPoliciesConnectedToNetSuite()?.length;

    is true.

  • The getAdminPoliciesConnectedToNetSuite gets data from allPolicies:

    function getAdminPoliciesConnectedToNetSuite(): Policy[] {
    return Object.values(allPolicies ?? {}).filter<Policy>((policy): policy is Policy => !!policy && policy.role === CONST.POLICY.ROLE.ADMIN && !!policy?.connections?.netsuite);
    }

    which is not cleared when signing out, leads to it is using the allPolicies data of the previous session.

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

  • We should create a clean-up function:
const clearAllPolicies = () => {
    allPolicies = {};
};

and call it once we sign out to.

What alternative solutions did you explore? (Optional)

    const hasPoliciesConnectedToNetSuite = !!getAdminPoliciesConnectedToNetSuite(allPolicies)?.length;
  • In above, we need to modify the getAdminPoliciesConnectedToNetSuite so that it can accept a allPolicies param as well.

Copy link

melvin-bot bot commented Aug 15, 2024

@mananjadhav @yuwenmemon @kadiealexander this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@mananjadhav
Copy link
Collaborator

Thanks for the proposal @daledah. But I think we clear all the keys on Signout.

I didn't get time to take a look at this. I will check this on the weekend.

@melvin-bot melvin-bot bot added the Overdue label Aug 16, 2024
@daledah
Copy link
Contributor

daledah commented Aug 17, 2024

But I think we clear all the keys on Signout

Yes, we cleared all the onyx keys, but the allPolicies variable is not cleared on Signout. It is just cleared when we refreshing the page.

@kadiealexander
Copy link
Contributor

@mananjadhav any updates?

@melvin-bot melvin-bot bot removed the Overdue label Aug 19, 2024
@mananjadhav
Copy link
Collaborator

I tried this and I couldn't reproduce this.

@daledah I am still not convinced as the policies need be explicitly cleared out on sign out. This is especially when you see a new workspace option. @lanitochka17 are you getting the same option for Sage Intacct? the code for reuse existing connection is the same.

I still need help with the reproduction here.

@daledah
Copy link
Contributor

daledah commented Aug 20, 2024

@mananjadhav

I am still not convinced as the policies need be explicitly cleared out on sign out.

This is especially when you see a new workspace option

I still need help with the reproduction here

  • Based on my RCA, to reproduce the issue, you need to make sure from step 1 to step 8, you don't refresh the page.

@mananjadhav
Copy link
Collaborator

Was just coming back to this. Yeah I think it makes sense to clear allPolicies. @yuwenmemon I think @daledah's proposal would work. Can we make this external assign them and I can review?

🎀 👀 🎀

Copy link

melvin-bot bot commented Aug 20, 2024

Current assignee @yuwenmemon is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

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

melvin-bot bot commented Aug 20, 2024

📣 @daledah You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 21, 2024
@daledah
Copy link
Contributor

daledah commented Aug 21, 2024

@mananjadhav This PR is ready for review.

@mananjadhav
Copy link
Collaborator

@yuwenmemon I think this is deployed on production right? I can see the linked deploy checklist is completed.

@yuwenmemon
Copy link
Contributor

Yep @kadiealexander can we process payment and then close this out?

@garrettmknight
Copy link
Contributor

@mananjadhav can you complete the checklist when you get a chance?
@kadiealexander please post the payment summary if this is ready to go!

@mananjadhav
Copy link
Collaborator

The allPolicies has been existing since long and I think this was an edge case which we couldn't trace during any of the NetSuite work. These could be reproducible for other connections as well. Hence, I don't think we have any offending PR.

Considering this is an edge case I don't think we need a regression test for this one.

@kadiealexander All yours for payment summary.

@kadiealexander
Copy link
Contributor

kadiealexander commented Sep 24, 2024

Payouts due:

  • Contributor: $250 @daledah via Upwork (please apply!)
  • Contributor+: $250 @mananjadhav via Manual Requests

Upwork job is here.

@garrettmknight
Copy link
Contributor

$250 approved for @mananjadhav

@daledah
Copy link
Contributor

daledah commented Oct 1, 2024

@daledah via Upwork (please apply!)

@kadiealexander Sorry I don't have Upwork Connects so could not apply. Could you send the offer at https://www.upwork.com/freelancers/~0138d999529f34d33f? Thx

@kadiealexander kadiealexander added Daily KSv2 and removed Weekly KSv2 labels Oct 15, 2024
@kadiealexander
Copy link
Contributor

@daledah I've sent you a contract.

@kadiealexander kadiealexander changed the title [$250] Netsuite - "Reuse existing connection" is an option when there is no existing workspace [Due payment][$250] Netsuite - "Reuse existing connection" is an option when there is no existing workspace Oct 15, 2024
@daledah
Copy link
Contributor

daledah commented Oct 15, 2024

@kadiealexander I accepted the contract

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

6 participants