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

[CRITICAL] Confirm if migration members will see onboarding modal when they shouldn't #53981

Open
jamesdeanexpensify opened this issue Dec 12, 2024 · 15 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Overdue Weekly KSv2

Comments

@jamesdeanexpensify
Copy link
Contributor

Background

Slack convo

There have been numerous instances of Expensify members who already belong to a workspace being taken through the onboarding modal and creating additional, erroneous workspaces. We should not be showing the onboarding modal to those members at all, and we're concerned this may turn into an even bigger issue once migration begins.

Examples

  • [email protected] (clip) - looking in Supportal, they're already a member of a group Control workspace, so they shouldn't be shown the onboarding modal - but they're taken through the onboarding modal and end up creating a new workspace erroneously
  • [email protected] (clip) - looking in Supportal, they're already a member of a group Control workspace (they also experience a bug where they get stuck in an infinite onboarding modal loop, but that's been logged separately here)
  • [email protected] (clip) - ignore the looping onboarding modal, that bug has already been logged. But this member already belongs to a group Control workspace so they shouldn't be taken through the onboarding modal.

Potential solution

If someone is already a workspace member and does not have the introSelected NVP, then we should not show the onboarding modal.

cc @danielrvidal @francoisl @deetergp @puneetlath @flaviadefaria

@jamesdeanexpensify jamesdeanexpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 12, 2024
Copy link

melvin-bot bot commented Dec 12, 2024

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

@jamesdeanexpensify jamesdeanexpensify added the Internal Requires API changes or must be handled by Expensify staff label Dec 12, 2024
@jamesdeanexpensify
Copy link
Contributor Author

@slafortune I'm going to grab an internal engineer for this one so I'll remove you!

Copy link

melvin-bot bot commented Dec 16, 2024

@deetergp Whoops! This issue is 2 days overdue. Let's get this updated quick!

@deetergp
Copy link
Contributor

Digging into this today!

@deetergp
Copy link
Contributor

We decide whether or not to show the onboarding modal to users here and it mostly seems to do with what's in nvp_onboarding in the Onyx store for a given user. The weird thing is that for invited/non-organic users, we never set an onboarding NVP for these users on the back end, but we do send a value for it from the back end in OpenApp here. And now it looks like we are setting a value for that NVP somewhere in the front end. This users was invited to be a workspace member and this is what their nvp_onboarding looks like after signing in the first time.
Screenshot 2024-12-16 at 4 25 34 PM

At one point we were getting a type error in the JS console in NewDot and so we added a check to the back make sure we are only setting that NVP for organic users. But it looks like we should set it. And after testing it locally, it looks like we no longer get a console error when we do.

@deetergp deetergp removed the Reviewing Has a PR in review label Dec 17, 2024
@flaviadefaria flaviadefaria moved this to First Cohort - CRITICAL in [#whatsnext] #migrate Dec 17, 2024
@deetergp
Copy link
Contributor

Now that [email protected] has completed the onboarding modal, we have set an onboarding NVP for them and now they should not ever see it again.

[email protected]:~$ getacct.sh [email protected]
16925390
[email protected]:~$ sudo readdb.sh -noheader "SELECT value FROM nameValuePairs WHERE name IN ('introSelected', 'onboarding') AND accountID = 16925390;" | jq
{
  "addExpenseApprovals": "6737201472724418",
  "choice": "newDotManageTeam",
  "companySize": "1-10",
  "createWorkspace": "5470938429703158",
  "inviteTeam": "3057217234069852",
  "meetGuide": "33590672114107",
  "setupCategories": "979258812033352",
  "setupTags": "3804047747710502",
  "userReportedIntegration": "null",
  "viewTour": "5353997149835316"
}
{
  "chatReportID": "5167728788916060",
  "hasCompletedGuidedSetupFlow": true
}

@deetergp
Copy link
Contributor

The situation for [email protected] is interesting. They were invited to be participate in a workspace, which means their choice should be either newDotSubmit or newDotAdmin but instead it is newDotPersonalSpend. I wonder if that is what is causing the loop. 🤔

[email protected]:~$ getacct.sh [email protected]
18712274
[email protected]:~$ sudo readdb.sh -noheader "SELECT value FROM nameValuePairs WHERE name IN ('introSelected', 'onboarding') AND accountID = 18712274;" | jq
{
  "addBankAccount": "3741717047670832",
  "choice": "newDotPersonalSpend",
  "inviteType": "workspace",
  "isInviteOnboardingComplete": true,
  "submitExpense": "2173775929746749",
  "trackExpense": "6445281929762540",
  "viewTour": "3109713135132156"
}

@deetergp
Copy link
Contributor

Similarly, [email protected] is a workspace invite but their choice is an empty string. Not sure how that is possible, but that inconsistency may be the cause of the loop.

[email protected]:~$ getacct.sh [email protected]
18712262
[email protected]:~$ sudo readdb.sh -noheader "SELECT value FROM nameValuePairs WHERE name IN ('introSelected', 'onboarding') AND accountID = 18712262;" | jq
{
  "addBankAccount": "4279390238470194",
  "choice": "",
  "inviteType": "workspace",
  "isInviteOnboardingComplete": true,
  "submitExpense": "3422160313480434",
  "viewTour": "4261786267636786"
}

@deetergp
Copy link
Contributor

Oh something is broken with the workspace invite flow. With one of my local test accounts, I invited a new user to join a workspace. This is what their introSelected NVP looked like right after I sent the invite, but before I attempted to log in as that user:

sqlite> SELECT accountID, name, value FROM nameValuePairs WHERE name = 'introSelected' AND accountID = 1248;

That is exactly what it should look like.

But then I waited for the invite email to show up and tried to click on the link, which took me to a blank screen, when it should log me straight in. (That's another bug for sure!) Since the invite link didn't work, I logged in with a magic code and this is what it looked like afterwards:

sqlite> SELECT accountID, name, value FROM nameValuePairs WHERE name = 'introSelected' AND accountID = 1248;
accountID  name           value
---------  -------------  ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1248       introSelected  {"addBankAccount":"2980170708312072","choice":"","inviteType":"workspace","isInviteOnboardingComplete":true,"previousChoices":["newDotSubmit"],"submitExpense":"7282012460907978","viewTour":"9006654587872538"}

That is another bug though it may be unrelated to this one.

@deetergp
Copy link
Contributor

Oh, that is happening in this block but I'm not sure why. (cc @francoisl)

@deetergp
Copy link
Contributor

And every time you point a finger, you point three back at yourself I'm the one that introduced the empty string bug 😅

@francoisl
Copy link
Contributor

Oh huh good catch, any idea of the flow where createTasksAndMessages() is called with a blank choice?

@deetergp
Copy link
Contributor

deetergp commented Dec 19, 2024

I do indeed! It was here where we were mistakenly trying to get the previous choice from personalDetails instead of introSelected.

@anmurali
Copy link

https://fsty.io/v/CjEZ6Zbv

Here is a session that I think is hitting this same issue.

Copy link

melvin-bot bot commented Dec 26, 2024

@deetergp this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Dec 27, 2024
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. Internal Requires API changes or must be handled by Expensify staff Overdue Weekly KSv2
Projects
Status: First Cohort - CRITICAL
Development

No branches or pull requests

5 participants