Sign in, out, or navigate on transition mount #8257

neil-marcellini · 2022-03-21T17:35:29Z

Details

Fix endless loading on the transition page by navigating at the end of componentDidMount.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/201705

Tests

Below are tests for all of the transition flows from Web-Expensify to NewDot. I have outlined all of the flows here, but I have only included tests for the Web. The mobile flows haven't worked for a while, as far as I can tell, so I think those should be fixed in a follow up issue.

A. Set up my company for free

Make sure you are signed out of NewDot
Go to OldDot with a mobile screen size and sign up with a new account
Click join
Click "Set up my company for free".
You should be directed to NewDot and have a new workspace created.
Repeat steps 2-5 to test the case where you are signed into a different account on NewDot

B. Select a free workspace

Make sure you are signed out of NewDot
Go to OldDot and sign in to an account if needed
Go to Settings > Policies > Group and click New Policy in the top right
Click select on the free plan
You should be directed to NewDot and have a new workspace created.
Repeat steps 2-5 to test the case where you are signed in to the same account on NewDot
Sign in to a different account on OldDot
Repeat steps 3-5 to test the case where you are signed into a different account on NewDot

C. Clicking on the name of a free policy

Make sure you are signed out of NewDot
Go to OldDot and sign in (if needed) to an account that has a free workspace / group policy
Go to Settings > Policies > Group and click on the name of a free workspace
You should be directed to NewDot and the workspace settings should open
Repeat steps 2-4 to test the case where you are signed in to the same account on NewDot
Sign in to a different account on OldDot that has a free workspace / group policy
Repeat steps 3-4 to test the case where you are signed into a different account on NewDot

This flow is a bit slow and will sometimes show a blank page for a little while.

D. Get Started Inbox Task

Make sure you are signed out of NewDot
Go to OldDot and sign up for a new account with an @gmail.com address
Click on the "Get started" on the inbox task that says "Would you like to get started with our free plan?".
You should be directed to NewDot and have a new workspace created, and the workspace settings should open.
Go back to OldDot and go to Settings > Policies > Group
Refresh the page and verify that 1 free workspace policy has been created
Repeat steps 2-6 to test the case where you are signed into a different account on NewDot

E. Pricing select free

Make sure you are signed out of NewDot
Go to OldDot and sign in with any account
Go to expensify.com.dev/pricing and click "Select" on the free plan
You should be directed to NewDot and have a new workspace created, and the workspace settings should open.
Repeat steps 3-4 to test the case where you are signed into the same account
Go to OldDot and sign in with a different account
Repeat steps 3-4 to test the case where you are signed into a different account.

F. Continue setup Inbox task

Go to OldDot and sign up for a new account with an @gmail.com address
Click on the "Get started" on the inbox task that says "Would you like to get started with our free plan?".
You should be directed to NewDot and have a new workspace created, and the workspace settings should open.
Sign out of NewDot
Go back to the OldDot inbox and refresh the page
Click "Continue setup" on the inbox task that says "Finish setting up your bank account".
The Connect bank account page should open in the right sidebar
Sign out of NewDot
Go back to OldDot
Repeat steps 6-7 to test the case where you are signed out of NewDot
Sign out of NewDot and sign in with a different account
Repeat steps 6-7 to test the case where you are signed into a different account on NewDot

Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Same as above but on staging.

Verify that no new errors appear in the JS console

Screenshots

Web

Mobile Web

Desktop

iOS

Android

iwiznia · 2022-03-21T19:37:04Z

(sorry was doing a test of puller bear, removing @AndrewGable as reviewer)

marcaaron

Interesting.. so no componentDidUpdate() at all? Does it work? If so - I like it.

marcaaron

Just asking some questions and trying to understand the changes for now. Will do a proper review / test after.

src/libs/Navigation/AppNavigator/MainDrawerNavigator.js

marcaaron · 2022-03-22T22:37:56Z

src/libs/actions/Session/index.js

@@ -188,6 +189,7 @@ function createTemporaryLogin(authToken, email) {
        partnerUserSecret: autoGeneratedPassword,
        shouldRetry: false,
        forceNetworkRequest: true,
+        shouldProcessImmediately,


Same here - I'm having trouble seeing how this is connected to the transition stuff.

Removed this code and it doesn't seem to impact anything.

src/libs/Growl.js

marcaaron · 2022-03-23T21:40:14Z

Gonna jump in and add some commits here and do some testing @roryabraham

marcaaron · 2022-03-23T21:50:22Z

Looks like test steps are failing for me when logged in with a pre-existing different account for the first set of steps "A" "Setup my workspace from OldDot".

marcaaron · 2022-03-23T21:59:54Z

Oh actually it's because of a dev quirk. A call to Push_Authenticate makes the API return 404 because of the Referrer-Policy header.

…mall screen sizes

marcaaron · 2022-03-23T22:23:13Z

For "Continue Setup" steps there's a super annoying bug where if you dismiss the bank account modal you get redirected to OldDot. I don't think that was introduced here though, but it's kinda of 🥔

marcaaron · 2022-03-23T22:33:24Z

marcaaron · 2022-03-23T22:34:47Z

I think we should also test all three cases here...

Logged in as same user in NewDot
Logged in as different user in NewDot
Not logged in at all on NewDot

As they are all slightly different.

marcaaron · 2022-03-23T22:38:56Z

Adding some logs to make testing/debugging easier.

marcaaron · 2022-03-23T22:52:17Z

src/libs/actions/WelcomeActions.js

@@ -68,7 +68,9 @@ function show({routes, toggleCreateMenu}) {
        // If we are rendering the SidebarScreen at the same time as a workspace route that means we've already created a workspace via workspace/new and should not open the global
        // create menu right now.
        const topRouteName = lodashGet(_.last(routes), 'name', '');
-        const isDisplayingWorkspaceRoute = topRouteName.toLowerCase().includes('workspace');
+        const loginWithShortLivedTokenRoute = _.find(routes, route => route.name === 'LogInWithShortLivedToken');
+        const exitingToWorkspaceRoute = lodashGet(loginWithShortLivedTokenRoute, 'params.exitTo', '') === 'workspace/new';


Added this otherwise we would pop open the create menu in some contexts. I guess the top route is not always "workspace" and sometimes still the LoginWithShortLivedToken page.

src/pages/LogInWithShortLivedTokenPage.js

roryabraham · 2022-03-23T23:10:47Z

I'm up-to-date on this branch and just tested:

Sign into account A on NewDot
Sign into account B on OldDot, where account B has an existing workspace
Go to Settings -> Policies and click on the workspace

And it correctly logged out account A and logged in account B, but it did not open the existing workspace

marcaaron · 2022-03-23T23:13:36Z

Cool. Lemme re-test that flow. Share logs if can.

roryabraham · 2022-03-23T23:15:16Z

marcaaron · 2022-03-23T23:18:31Z

It's working for me 😄

2022-03-23_13-17-18.mp4

marcaaron · 2022-03-23T23:20:27Z

Seems good to go from my end. Gonna do another round of tests in a bit.

roryabraham · 2022-03-23T23:23:53Z

Screen.Recording.2022-03-23.at.4.21.45.PM.mov

Maybe a race condition because it doesn't seem to be working for me 😕

roryabraham

Thought about it some more and if this inconsistently reproducible bug is the only one left, then we should just merge this and address it with a follow-up. I'm going to be offline until tomorrow but going to approve this so it's mergeable.

marcaaron · 2022-03-23T23:53:05Z

Cool. Seems like this is blocking the deploy. If so we can add a CP staging to it (though the flow breaking on the Growl might still be affected) will retest once it hits and try to check it off.

github-actions · 2022-03-23T23:53:24Z

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

Sign in, out, or navigate on transition mount (cherry picked from commit cea6cf0)

…8257 🍒 Cherry pick PR #8257 to staging 🍒

OSBotify · 2022-03-24T01:20:44Z

🚀 Cherry-picked to staging by @marcaaron in version: 1.1.44-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

Sign in, out, or navigate on transition mount

acbdf28

neil-marcellini requested review from TomatoToaster and marcaaron March 21, 2022 17:35

neil-marcellini self-assigned this Mar 21, 2022

iwiznia requested a review from a team March 21, 2022 19:35

melvin-bot bot requested review from AndrewGable and removed request for a team March 21, 2022 19:35

iwiznia removed the request for review from AndrewGable March 21, 2022 19:37

marcaaron reviewed Mar 21, 2022

View reviewed changes

neil-marcellini mentioned this pull request Mar 22, 2022

Deploy Checklist: New Expensify 2022-03-17 #8210

Closed

31 tasks

neil-marcellini added 2 commits March 22, 2022 11:03

Queue requests to sign in with a short lived token

a873630

Retry growls when the ref is not available

191c02f

neil-marcellini force-pushed the neil-setup-transition branch from e198054 to 191c02f Compare March 22, 2022 18:04

Show a loading screen when the betas are loading

e527f2e

neil-marcellini marked this pull request as ready for review March 22, 2022 21:33

neil-marcellini requested a review from a team as a code owner March 22, 2022 21:33

neil-marcellini requested a review from marcaaron March 22, 2022 21:33

melvin-bot bot requested review from sketchydroide and removed request for a team March 22, 2022 21:33

marcaaron reviewed Mar 22, 2022

View reviewed changes

Remove setTimeout on Growl

2d63d2b

marcaaron added 2 commits March 23, 2022 12:17

Fix CreateMenu from popping up when navigating to workspace page on s…

6e375bb

…mall screen sizes

do it for large screens too

151571b

Merge remote-tracking branch 'origin/main' into neil-setup-transition

e1e594f

revert growl changes improve log

7e65708

marcaaron reviewed Mar 23, 2022

View reviewed changes

marcaaron added 2 commits March 23, 2022 13:02

Remove IS_LOADING_BETAS

38c4b8a

remove isLoadingBetas

282a9d1

roryabraham reviewed Mar 23, 2022

View reviewed changes

src/pages/LogInWithShortLivedTokenPage.js Show resolved Hide resolved

Remove shouldProcessImmediately

25bb9c4

remove parseInt

2996c88

marcaaron removed request for TomatoToaster and sketchydroide March 23, 2022 23:20

roryabraham approved these changes Mar 23, 2022

View reviewed changes

marcaaron added the CP Staging label Mar 23, 2022

marcaaron merged commit cea6cf0 into main Mar 23, 2022

marcaaron deleted the neil-setup-transition branch March 23, 2022 23:53

OSBotify pushed a commit that referenced this pull request Mar 24, 2022

Merge pull request #8257 from Expensify/neil-setup-transition

b443e09

Sign in, out, or navigate on transition mount (cherry picked from commit cea6cf0)

OSBotify mentioned this pull request Mar 24, 2022

🍒 Cherry pick PR #8257 to staging 🍒 #8278

Merged

OSBotify added a commit that referenced this pull request Mar 24, 2022

Merge pull request #8278 from Expensify/OSBotify-cherry-pick-staging-…

6a3d1a4

…8257 🍒 Cherry pick PR #8257 to staging 🍒

kavimuru mentioned this pull request Mar 24, 2022

Web - Inbox - User has to click "Continue setup" CTA twice in olddot to display "Connect bank account" page in the right sidebar #8295

Closed

OSBotify mentioned this pull request Mar 24, 2022

Deploy Checklist: New Expensify 2022-03-24 #8305

Closed

69 tasks

OSBotify mentioned this pull request Aug 16, 2022

Deploy Checklist: New Expensify 2022-08-04 #10253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sign in, out, or navigate on transition mount #8257

Sign in, out, or navigate on transition mount #8257

neil-marcellini commented Mar 21, 2022 •

edited by marcaaron

Loading

iwiznia commented Mar 21, 2022

marcaaron left a comment

marcaaron left a comment

marcaaron Mar 22, 2022

marcaaron Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022 •

edited

Loading

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron Mar 23, 2022

roryabraham commented Mar 23, 2022

marcaaron commented Mar 23, 2022

roryabraham commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

roryabraham commented Mar 23, 2022

roryabraham left a comment

marcaaron commented Mar 23, 2022

github-actions bot commented Mar 23, 2022

OSBotify commented Mar 24, 2022

Sign in, out, or navigate on transition mount #8257

Sign in, out, or navigate on transition mount #8257

Conversation

neil-marcellini commented Mar 21, 2022 • edited by marcaaron Loading

Details

Fixed Issues

Tests

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Screenshots

Web

Mobile Web

Desktop

iOS

Android

iwiznia commented Mar 21, 2022

marcaaron left a comment

Choose a reason for hiding this comment

marcaaron left a comment

Choose a reason for hiding this comment

marcaaron Mar 22, 2022

Choose a reason for hiding this comment

marcaaron Mar 23, 2022

Choose a reason for hiding this comment

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022 • edited Loading

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron Mar 23, 2022

Choose a reason for hiding this comment

roryabraham commented Mar 23, 2022

marcaaron commented Mar 23, 2022

roryabraham commented Mar 23, 2022

marcaaron commented Mar 23, 2022

marcaaron commented Mar 23, 2022

roryabraham commented Mar 23, 2022

roryabraham left a comment

Choose a reason for hiding this comment

marcaaron commented Mar 23, 2022

github-actions bot commented Mar 23, 2022

OSBotify commented Mar 24, 2022

neil-marcellini commented Mar 21, 2022 •

edited by marcaaron

Loading

marcaaron commented Mar 23, 2022 •

edited

Loading