[HOLD for payment 2023-11-30] Chat not loading after redirection from OldDot

[francoisl]

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: v1.4.0-3
Reproducible in staging?: Yes
Reproducible in production?: Yes
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): N/A
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

Context

We're testing a feature to redirect users that sign up on Expensify Classic (OldDot) to New Expensify. Sometimes, upon being redirected to NewDot, the app doesn't fully load, and the chat view shows the loading animation.

Action Performed:

1. Open staging.expensify.com
2. Click on "Email" > enter any email address from a public domain that +classicredirect (that's what enables the redirection for now, while we test). For example: your.name+classiredirect123@gmail.com > click Next > Join
3. At that point you'll be redirected to NewDot, and the app should load on the Concierge chat. Sometimes, the Concierge chat doesn't get displayed

The issue doesn't always happen (maybe 1 in 10 times in my personal experience), so you might have to test multiple times in a row with different accounts.

Expected Result:

The Concierge chat should be displayed.

Actual Result:

The Concierge chat is not displayed, there's a loading animation in the chat view instead.

Workaround:

Platforms:

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

Screenshots/Videos

redirect1.mov

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @kevinksullivan

[melvin-bot]

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[koko57]

I can help with this issue 🙂

[danielrvidal]

@koko57 let us know how we can help diagnose! We're trying to get this one cleared asap.

[roryabraham]

The problem appears to be LogOutPreviousUserPage. If the user wasn't already signed in with a different account, and the page is missing the shouldForceLogin route param, it will just display a spinner forever. Unless I'm missing something this seems like the problem?

[roryabraham]

There are two places in OldDot PHP where this redirection happens AFAICT:

• for new users
• here

And in neither case is shouldForceLogin included in the URL

[roryabraham]

This is very consistently reproducible for me if you sign in as a new user on OldDot without being signed in to an account on NewDot

[roryabraham]

Ok, I think I've figured out the race condition at play here...

"Happy" path (haven't observed yet)

1. Redirect from OldDot
2. PublicScreens mounts.
3. LoginWithShortLivedAuthToken page renders:

   App/src/libs/Navigation/AppNavigator/PublicScreens.js

   Lines 23 to 27 in 5d45f0f

   	<RootStack.Screen
   	name={SCREENS.TRANSITION_BETWEEN_APPS}
   	options={defaultScreenOptions}
   	component={LogInWithShortLivedAuthTokenPage}
   	/>

4. Session.signInWithShortLivedAuthToken happens
5. Sign in completes and LoginWithShortLivedAuthToken page renders again, the useEffect runs, and exitTo navigation happens. Note: I think there are potentially multiple reasons this might not happen:
   • As soon as sign-in completes, the PublicScreens will unmount and AuthScreens will mount. I think that consistently happens before LoginWithShortLivedAuthToken re-renders, because it's further up the tree
   • There's nothing in the props.route that will change as a result of calling Session.signInWithShortLivedAuthToken. So even if the component re-renders, the useEffect won't

What actually happens 100% of the time AFAICT

1. Redirect from OldDot
2. PublicScreens mounts
3. LoginWithShortLivedAuthToken page renders:

   App/src/libs/Navigation/AppNavigator/PublicScreens.js

   Lines 23 to 27 in 5d45f0f

   	<RootStack.Screen
   	name={SCREENS.TRANSITION_BETWEEN_APPS}
   	options={defaultScreenOptions}
   	component={LogInWithShortLivedAuthTokenPage}
   	/>

4. Session.signInWithShortLivedAuthToken happens
5. PublicScreens unmounts and AuthScreens mounts.
6. LogOutPreviousUserPage renders
   1. [this redundant call to Session.signOutAndRedirectToSignIn() never happens (which, we don't want it to happen anyways...)
   2. shouldForceLogin is false.
   3. So literally nothing else happens and we get an infinite spinner

[roryabraham]

Quick and dirty fix: #31522

However, I think that some work to clean up and simplify this flow would be valuable. We can have only one transition page that works if a user is:

1. Already signed in
2. Signed in to the wrong account
3. Provided an exitTo, or not
4. etc...

and then we could create E2E tests for all those flows. I noticed a number of redundancies and seemingly unnecessary complexities while looking through this code.

[quinthar]

If this issue only happens when you're already signed in, I think this isn't a blocker