[HOLD for payment 2024-06-11] [$250] Android - Login - While login android app, a blank page slightly opens and closes

[lanitochka17]

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: 1.4.68
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch app
2. Enter email id and tap continue
3. Tap magic link to login

Expected Result:

Question While login android app, a blank page must not slightly open and close

Actual Result:

While login android app, a blank page slightly opens and closes

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6466605_1714483792772.screenrecorder-2024-04-30-17-49-07-211.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0182c092cd87c0b915
• Upwork Job ID: 1785327999234457600
• Last Price Increase: 2024-05-07
• Automatic offers:
• rayane-djouah | Reviewer | 0
• bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @johnmlee101 (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

@johnmlee101 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[johnmlee101]

I don't think this should necessarily be a blocker, but I do think that it should be External since it has to deal with some rendering order most likely.

[johnmlee101]

Thanks for holding to confirm!

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@johnmlee101, @rayane-djouah Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rayane-djouah]

@johnmlee101 please add Bug label to assign a BZ member, Thanks!

[johnmlee101]

My bad, thanks for catching that!

[melvin-bot]

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

[bernhardoj]

Sorry, I missed it.

but when I test removing this part of the code the deep linking still working correctly

May I know the steps you took?

Below is the video without the code:

Screen.Recording.2024-05-11.at.12.56.51.mov

and below is the video with the code:

Screen.Recording.2024-05-11.at.12.58.29.mov

Also, can you identify what PR from #41266 (comment) caused this bug?

Hmm, I think the issue has been there for a long time since the code is old. Usually, QA posts a video comparison on prod, but I don't see it here. So I don't think it's a regression from recent changes. I also already noticed this issue when working on this PR (see the native recordings).

[rayane-djouah]

@bernhardoj

For signing without navigating to an exitTo path:
npx uri-scheme open 'new-expensify://v/14826403/632926' --android

Below is the video with the code:

Screen.Recording.2024-05-11.at.3.15.25.PM.mov

The bug is reproducible in this case, see (a blank page slightly opens and closes):

and below is the video without the code:

Screen.Recording.2024-05-11.at.3.20.28.PM.mov

Removing the code fixed the bug. but I don't understand the role of this code and the root cause of this bug.

[rayane-djouah]

For navigation to an exitTo path after signing (npx uri-scheme open 'new-expensify://v/14826403/665502?exitTo=/settings/preferences' --android) (in this case, there is no blank page slightly opens and closes):

Below is the video with the code:

Screen.Recording.2024-05-11.at.3.18.51.PM.mov

and below is the video with the code:

Screen.Recording.2024-05-11.at.3.23.21.PM.mov

The navigation is done correctly in both cases.

[rayane-djouah]

@bernhardoj, Nevermind, I understand now that this code is handling deep link navigation that was made while logged out.

I inspected the debug console and confirmed that we are navigating again to "v/:accountID/:validateCode" route after the login is successful. the root cause in your proposal makes sense.

For the solution, I don't think we should call Navigation.navigate for "v/:accountID/:validateCode" route because the react navigation handled the navigation to it already, so instead of resolving the promise, can we early return for this route?

I see that in #33225 we introduced shouldSkipDeepLinkNavigation to early return in case of transition route, can we use a similar approach / include more routes in shouldSkipDeepLinkNavigation?

Also, do we need to apply the solution for all public screens or only to VALIDATE_LOGIN? can you explain why?

[bernhardoj]

so instead of resolving the promise, can we early return for this route?

Good idea, it would simplify the code too. I guess it would work fine. We can test it more for all platforms in the PR. I have updated the proposal to include the early return and also mention to apply that public screen check only while logged out, so it's more predictable I guess.

Also, do we need to apply the solution for all public screens or only to VALIDATE_LOGIN? can you explain why?

Yes, I don't think we want to navigate before and after sign-in, right? I don't see a reason to limit it to VALIDATE_LOGIN only.

[rayane-djouah]

@bernhardoj Let's go with the early return solution by including the public screens in shouldSkipDeepLinkNavigation. provided that the navigation to public screens is done before sign-in and we should not navigate to them another time after sign-in.

@bernhardoj's propoal looks good to me.

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @johnmlee101 is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[bernhardoj]

PR is ready

cc: @rayane-djouah

[rayane-djouah]

Heads up, I'll be mostly offline until June 5th, 2024. I can still review this issue, but my response might be slower. If there is something urgent, please reassign it. Thanks!

[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.4.78-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Don't navigate to public screen deep link after sign in #42130

If no regressions arise, payment will be issued on 2024-06-11. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment automatic offer (Contributor)
• @rayane-djouah requires payment automatic offer (Reviewer)

[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:

• [@johnmlee101] The PR that introduced the bug has been identified. Link to the PR:
• [@johnmlee101] 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:
• [@johnmlee101] 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:
• [@bernhardoj / @rayane-djouah] Determine if we should create a regression test for this bug.
• [@bernhardoj / @rayane-djouah] 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.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[lschurr]

Payment summary:

Contributor: $250 @bernhardoj (Paid in Upwork)
Contributor+ $250 @rayane-djouah (Paid in Upwork)