Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.91k forks source link

[$250] [CRITICAL] Multiple workspaces get created when someone either refreshes the page, or drops off and returns later. #52894

Open trjExpensify opened 18 hours ago

trjExpensify commented 18 hours ago

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: v9.0.65-1 Reproducible in staging?: Y Reproducible in production?:Y 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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @trjExpensify Slack conversation : #convert thread

Action Performed:

  1. Sign-up on expensify.com
  2. Choose the “1-9” option on the home page (signUpQualifier: vsb)
  3. Get redirected to new.expensify.com
  4. Observe a workspace is created
  5. Don’t complete the onboarding modal steps (hasCompletedGuidedSetupFlow: false)
  6. Refresh the page
  7. Observe another workspace is created
  8. Type in expensify.com in the browser URL and click enter
  9. Get redirected to new.expensify.com (classicRedirect.dismissed: false)
  10. Another workspace is created

Expected Result:

If we’ve created a workspace for the new sign-up already, don’t create another one when they refresh or return.

Actual Result:

We create a workspace every time they refresh or get redirected to NewDot and haven’t completed the onboarding modal steps yet.

Workaround:

N/A

Platforms:

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

Screenshots/Videos

Refreshing the page

https://github.com/user-attachments/assets/2587d4c0-7191-4ab6-847b-960ca1c30785

Returning to expensify.com

https://github.com/user-attachments/assets/9525a801-2360-46d3-b663-512c919403da

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859580489556912069
  • Upwork Job ID: 1859580489556912069
  • Last Price Increase: 2024-11-21
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @FitseTLT
melvin-bot[bot] commented 18 hours ago

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

mountiny commented 17 hours ago

@nkdengineer This is coming from your PR here https://github.com/Expensify/App/pull/51839/files

We should not be creating the workspace on each mount forever. Only if there is no group policy the user is admin of yet

FitseTLT commented 17 hours ago

Proposal

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

Multiple workspaces get created when someone either refreshes the page, or drops off and returns later.

What is the root cause of that problem?

We are already returning early in case there is onboarding policy id set but on refresh the useOnyx will not make the data available and the effect runs too early https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx#L61-L62

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

We should first check that onBoarding Policy ID data is loaded not loading https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/pages/OnboardingAccounting/BaseOnboardingAccounting.tsx#L48

    const [onboardingPolicyID, onboardingPolicyIDResults] = useOnyx(ONYXKEYS.ONBOARDING_POLICY_ID);
if (!isVsb || !!onboardingPolicyID || isLoadingOnyxValue(onboardingPolicyIDResults)) {
            return;
        }

What alternative solutions did you explore? (Optional)

FitseTLT commented 17 hours ago

@mountiny The code is already doing that but the effect runs too early for the useEffect. I have posted a fix I can immediately raise a PR if you want 👍

melvin-bot[bot] commented 17 hours ago

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

melvin-bot[bot] commented 17 hours ago

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

mountiny commented 17 hours ago

@jayeshmangwani I see the offending PR is just on the fence of the 1 week regression period and it was paid out. I think its fair if you will review this PR as part of regression still. Let us know if that sounds fine with you

@FitseTLT I think your solution makes sense, you also need to add the isLoading to the useEffect dependency but sounds good.

FitseTLT commented 17 hours ago

@jayeshmangwani I see the offending PR is just on the fence of the 1 week regression period and it was paid out. I think its fair if you will review this PR as part of regression still. Let us know if that sounds fine with you

@FitseTLT I think your solution makes sense, you also need to add the isLoading to the useEffect dependency but sounds good.

Yep I only missed it when commenting the proposal 👍

myspace20 commented 17 hours ago

@trjExpensify I requested to become a contributor through email, but I have not received feedback for a few days. Is there another way to do this?

melvin-bot[bot] commented 17 hours ago

📣 @myspace20! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
jayeshmangwani commented 17 hours ago

@jayeshmangwani I see the offending PR is just on the fence of the 1 week regression period and it was paid out. I think its fair if you will review this PR as part of regression still. Let us know if that sounds fine with you

Yes, I am definitely fine with it. We missed the case during testing

jayeshmangwani commented 17 hours ago

@mountiny, @FitseTLT 's Suggestion looks good to me here. Should I raise a PR quickly, or will @FitseTLT do it?

FitseTLT commented 17 hours ago

@jayeshmangwani I am already preparing 👍

jayeshmangwani commented 17 hours ago

Cool, please do it

jayeshmangwani commented 17 hours ago

offending PR is just on the fence of the 1 week regression period and it was paid out

@mountiny BTW, the payment for the original issue that this one is coming from hasn't been completed yet for me(I've requested on ND today). We can hold the payment until this issue is resolved. I can delete the payment request for that issue, please let me know.

mountiny commented 17 hours ago

@jayeshmangwani that is fine, we would just not process any payment here

mountiny commented 17 hours ago

Thanks!

trjExpensify commented 17 hours ago

@trjExpensify I requested to become a contributor through email, but I have not received feedback for a few days. Is there another way to do this?

Welcome! We have a problem with Slack invites at the moment, so we can't give you access to the Slack channel atm. That's okay though, you submit proposals and pick up available jobs in GitHub. We then hire you via Upwork to make the payment. Good place to start looking is here: https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3A%22Help+Wanted%22

myspace20 commented 17 hours ago

Contributor details Your Expensify account email: rogersatsi98@outlook.com Upwork Profile Link: https://www.upwork.com/freelancers/~016c50704f37520a88

melvin-bot[bot] commented 17 hours ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

nkdengineer commented 17 hours ago

We should not be creating the workspace on each mount forever. Only if there is no group policy the user is admin of yet

I already did it but missed the refresh case. The reason is useOnyx return the onboardingPolicyID is undefined at the first time if we don't have cache data.

FitseTLT commented 16 hours ago

PR ready for review 👍