[HOLD for payment 2023-07-06] [$1000] Login - Page freezes when navigating to secondary login verification link

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Log in to staging.new.expensify.com
2. Log in to staging.expensify.com with the same account as Step 1
3. Add a secondary login via Settings > Account details > Secondary logins
4. Go to verification email and copy the link
5. Change the URL to staging.new.expensify.com/.....
6. Navigate to the link

Expected Result:

Page does not freeze

Actual Result:

Page is freezing

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.30.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6101771_bandicam_2023-06-22_02-56-38-516.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017f461734f63ec998
• Upwork Job ID: 1672271611349622784
• Last Price Increase: 2023-06-23

[melvin-bot]

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

[johncschuster]

@lanitochka17 is this test meant to be part of the Add Secondary Login flow? I don't see a flow in new.expensify.com to add a Secondary Login, so I'm wondering why this was tested in this way.

[lanitochka17]

@johncschuster According to the steps with Test Rail https://expensify.testrail.io/index.php?/tests/view/3501015

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

[ddrzaic]

Proposal

ThreePaneView.js file, line 72
return ( <View key={route.key} style={[styles.pAbsolute, styles.t0, styles.l0, styles.w100, styles.h100]} > {props.descriptors[route.key].render()} </View> );

this returns a div which overlays the whole page, and that looks like the page is frozen, but in fact, there is a div which is above the page, and if you delete that div, the page is responsive.

Solution

Add ValidateLogin screen to SCREENS constant, and add that case into ThreePaneView.js

[melvin-bot]

📣 @ddrzaic! 📣
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. 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.
2. 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.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[ddrzaic]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01fd9f4fb7e646df10

[melvin-bot]

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

[s77rt]

@ddrzaic Thanks for the proposal. I don't think your RCA is correct/complete. I think this issue is more about ValidateLoginPage.

I see you are new here, so welcome! 🎉 I recommand that you read our contributing guide and to follow the proposal template.

BTW The expected behaviour here is not clear (asking for clarification in next comment).

[s77rt]

@johncschuster This issue can be easily reproducible if you just navigate to a link in this format: http://localhost:8080/v/123/123. This will render <ValidateLoginPage />but apparently it returns nothing if you are already signed in. It usually returns the magic code (see screenshot below). Can we get a clarification on the expected behaviour when navigating to this page while logged in? (Asked on Slack)

[hoangzinh]

Proposal

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

Login - Page freezes when navigating to secondary login verification link

What is the root cause of that problem?

• This is current logic how we render ValidateLoginPage in Web

  App/src/pages/ValidateLoginPage/index.website.js

  Lines 124 to 135 in 84a2048

  	<>
  	{currentAuthState === CONST.AUTO_AUTH_STATE.FAILED && <ExpiredValidateCodeModal />}
  	{currentAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && is2FARequired && !isSignedIn && <JustSignedInModal is2FARequired />}
  	{currentAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && isSignedIn && <JustSignedInModal is2FARequired={false} />}
  	{currentAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED && !isSignedIn && (
  	<ValidateCodeModal
  	accountID={this.getAccountID()}
  	code={this.getValidateCode()}
  	/>
  	)}
  	{currentAuthState === CONST.AUTO_AUTH_STATE.SIGNING_IN && <FullScreenLoadingIndicator />}
  	</>

  
  In this case, because we already signed in the 1st tab, it won't sign-in/verify with the code but keep the initial authState which is CONST.AUTO_AUTH_STATE.NOT_STARTED. According to above render conditions, it will render nothing in component ValidateLoginPage

• Next question is: why it shows Page freezes? It because of those LOC

  App/src/libs/Navigation/AppNavigator/createResponsiveStackNavigator/ThreePaneView.js

  Lines 73 to 78 in 057a182

  	<View
  	key={route.key}
  	style={[styles.pAbsolute, styles.t0, styles.l0, styles.w100, styles.h100]}
  	>
  	{props.descriptors[route.key].render()}
  	</View>

  In ThreePaneView, if the route is not neither HOME nor CENTRAL_PANE_NAVIGATOR nor RHN, we will render a full screen view. But in case, we show nothing in props.descriptors, so there is a full screen backdrop that makes us could not click anything in the page

Click to see screenshot of props and state of `ThreePaneView` in 2nd tab

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

We can twist a bit how it render in the ValidateLoginPage in those LOC

App/src/pages/ValidateLoginPage/index.website.js

Lines 128 to 133 in 057a182

	{currentAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED && !isSignedIn && (
	<ValidateCodeModal
	accountID={this.getAccountID()}
	code={this.getValidateCode()}
	/>
	)}

so that, if currentAuthState is CONST.AUTO_AUTH_STATE.NOT_STARTED we should render ValidateCodeModal regardless current session is signed or not

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[s77rt]

@hoangzinh Thanks for the proposal. Your RCA makes sense but I'm not sure about the solution (in terms of the final outcome). I think it's better to align on a clearer expected results first. #21258 (comment)

[s77rt]

I have asked on Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1687788368985859

[s77rt]

As per the Slack discussion we should still show the magic code even if logged in but without the sign in here link (this is already handled by <ValidateCodeModal />).

@hoangzinh Provided the correct fix but it may re-introduce bugs that were caught on this PR #19260. I have asked here #19260 (comment) for confirmation. @hoangzinh It would be great if you investigate that as well.

[s77rt]

We got a clarification. It's safe to remove the extra condition now. Let's go with @hoangzinh's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Li357, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[HezekielT]

@s77rt Do you think this is a dupe of #20410 ?

[hoangzinh]

@HezekielT In my opinion, it's not dup because it has different expectation and also different context

[hoangzinh]

but I agree they have almost same root cause, actually.

[s77rt]

@HezekielT I don't think it's a dupe yet they are similar

[Li357]

Proposal looks good! 🚀

[melvin-bot]

📣 @hoangzinh You have been assigned to this job by @Li357!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[hoangzinh]

@s77rt @Li357 Thanks for accepting my proposal. The PR is ready #21653. Please help me review it. Thanks ❤️

[melvin-bot]

🎯 ⚡️ Woah @s77rt / @hoangzinh, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @hoangzinh got assigned: 2023-06-26 16:53:24 Z
• when the PR got merged: 2023-06-27 15:09:41 UTC

On to the next one 🚀

[johncschuster]

@s77rt / @hoangzinh I've sent invites to you both on Upwork. Please let me know when you've accepted!

[hoangzinh]

Accepted. Thanks @johncschuster

[s77rt]

@johncschuster Accepted!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix Page freezes when user is logged in and click on secondary login verification link #21653

If no regressions arise, payment will be issued on 2023-07-06. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter - Applause
• Contributor that fixed the issue - @hoangzinh
• Contributor+ that helped on the issue and/or PR - @s77rt

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus - Eligible
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@johncschuster] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: fix: 2FA bug fixes #19260
• The offending PR has been commented on: fix: 2FA bug fixes #19260 (comment)
• A discussion in #expensify-bugs has been started: I don't think this is needed based on the discussion in the linked PR above
• Determine if we should create a regression test for this bug: Yes

---

Regression Test Proposal

1. Logout of NewDot if logged in
2. In the login screen enter your email
3. Navigate to email and locate the magic link and the magic code
4. Enter the magic code manually
5. Verify you are logged in
6. Navigate to the magic link - change to staging if needed
7. Verify the "Here's your magic code" page is displayed

[johncschuster]

Payments have been issued! I think we're good to close this out! 💃