[HOLD for payment 2024-07-10] [$250] [CRITICAL] [UX Reliability] When deeplinking to expense report from email empty screen is shown

[mountiny]

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

---

Version Number:
Reproducible in staging?: Y
Reproducible in production?: Y
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
Expensify/Expensify Issue URL:
Issue reported by: @danielrvidal
Slack conversation:

Action Performed:

1. [email protected] submitted [email protected] an IOU.
2. [email protected] got the email and clicked Pay , is taken to the sign in page, gets the magic code and logs in
3. [email protected] is taken to directly to the expense.
4. The subheader is not loading right away and LHN chats are not loading right away.
5. After letting it load, I click into the DM and there is nothing showing.
   a. I let it load for a bit, nothing.
   b. I refresh and see the expense show up.

This makes for a confusing experience. As a side note, we’ll be updating the flow so we don’t send users to the expense directly but rather the DM (GH) but I still experienced this bug so it felt like we should update it.

Expected Result:

The DM chat should load immediately as well as the chats should be available in LHN first, we should wait for the openApp response and show the skeleton if its not available

Actual Result:

• Subheader not showing when I’m brought into a report for the first time as a new user, and the DM the IOU sent to me is not loading.
• The DM chat is showing empty screen

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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

In slack here https://expensify.slack.com/archives/C05LX9D6E07/p1716940223060999?thread_ts=1716939884.285849&cid=C05LX9D6E07

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0167ada75a878e8b10
• Upwork Job ID: 1795785341708906496
• Last Price Increase: 2024-05-29
• Automatic offers:
• c3024 | Reviewer | 102542821

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0167ada75a878e8b10

[melvin-bot]

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

[Skakruk]

Proposal

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

Subheader not showing when user brought into a report for the first time as a new user, and the DM the IOU sent to me is not loading.
Also reproducible for any user, who received IOU link, on the first app load (after clearing DB) when the user navigates to IOU report directly.

What is the root cause of that problem?

On the first app load, we're making OpenApp request which blocks any other updates to Onyx, until it finishes.

App/src/libs/actions/OnyxUpdateManager/index.ts

Lines 74 to 80 in 15600b0

	if (isLoadingApp) {
	// When ONYX_UPDATES_FROM_SERVER is set, we pause the queue. Let's unpause
	// it so the app is not stuck forever without processing requests.
	SequentialQueue.unpause();
	console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`);
	return;
	}

Since OpenReport request is established before the OpenApp, the Report data is skipped and not saved into IndexedDB.

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

We need to delay OpenReport request until OpenApp is made.

In ReportScreen.tsx we add

const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);

and add checks for isLoadingApp in a few places:

1. don't make fetchReportIfNeeded() call

   App/src/pages/home/ReportScreen.tsx

   Lines 472 to 480 in 21d7884

   	useEffect(() => {
   	// Call OpenReport only if we are not linking to a message or the report is not available yet
   	if (isLoadingReportOnyx || (reportActionIDFromRoute && report.reportID)) {
   	return;
   	}
   	fetchReportIfNeeded();
   	// eslint-disable-next-line react-hooks/exhaustive-deps
   	}, [isLoadingReportOnyx]);

2. for showing skeleton while the app is still loading

   App/src/pages/home/ReportScreen.tsx

   Line 377 in 21d7884

   const isLoading = !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty();

3. to condition shouldShowNotFoundPage for not showing "Report not found"

   App/src/pages/home/ReportScreen.tsx

   Line 388 in 21d7884

   const shouldShowNotFoundPage = useMemo(

Also add condition

if (report.reportID === reportIDFromRoute) {
    return;
}

to

App/src/pages/home/ReportScreen.tsx

Line 417 in 21d7884

const fetchReportIfNeeded = useCallback(() => {

to make sure we don't make a duplicate request for already loaded report.

load-report.mp4

[arosiclair]

I can manage this. Looks like we're having @kacper-mikolajczak take a look

[kacper-mikolajczak]

Hi! While trying to reproduce the issue I've gotten the infinite loading spinner on LHN with error message for the report:

Screen.Recording.2024-05-31.at.13.41.18.mp4

It hangs even after OpenApp call has ended. Is it a known issue?

[Skakruk]

@kacper-mikolajczak I got the same issue, and it also related to requests made prior the OpenApp

[kacper-mikolajczak]

Understandable, I've checked your approach and it looks fine. One thing that we could improve is making a OpenReport request in parallel to OpenApp but as you said, such change would not be easy with current architecture.

@mountiny What do you think about @Skakruk taking over the PR as he already proposed a solid solution?

[kacper-mikolajczak]

What also I've noticed while testing, we are making an OpenReport request even for user who was not logged in. Is it intended behaviour?

Screen.Recording.2024-05-31.at.15.21.19.mp4

[Skakruk]

appears to be:

App/src/libs/actions/Report.ts

Lines 2487 to 2489 in b4c7110

	if (reportID && !isAuthenticated) {
	// Call the OpenReport command to check in the server if it's a public room. If so, we'll open it as an anonymous user
	openReport(reportID, '', [], {}, '0', true);

[arosiclair]

@mountiny What do you think about @Skakruk taking over the PR as he already proposed a solid solution?

@Skakruk's proposal looks promising to me as well. Let's implement it 👍🏽

[melvin-bot]

📣 @c3024 🎉 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]

📣 @Skakruk You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[arosiclair]

@Skakruk actually this is still up for discussion please hold.

[arosiclair]

Alright, upon some further discussion we realized this requires a backend fix to address. The root problem is that the subtitle in the header needs data from the parent report to be populated (that's done here) and OpenReport does not return that data. So we just need to include parent report data in the OpenReport response.

I'll take this internal and post a PR to implement that. @Skakruk appreciate your proposal but it actually won't be necessary anymore.

[johncschuster]

Pre-emptive bump while @arosiclair is OOO 👍

[johncschuster]

And one more before the weekend 👍

[melvin-bot]

@johncschuster, @arosiclair, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[arosiclair]

Since this is critical functionality and #43307 does work, I'm moving forward with it so we can get the fix out ASAP. I'll create a follow up to look into some of the better ideas I mentioned later. ETA on the PR is EOW.

[johncschuster]

@arosiclair how's this one going?

[arosiclair]

PR just hit staging yesterday.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[johncschuster]

How's this one going, @arosiclair?

[arosiclair]

We're just waiting on the prod deploy

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

• Fix deep linking to reports after signing in #43307

If no regressions arise, payment will be issued on 2024-07-10. 🎊

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

• @c3024 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:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR:
• [@c3024] 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:
• [@c3024] 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:
• [@c3024] Determine if we should create a regression test for this bug.
• [@c3024] 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:

[johncschuster]

Acknowledged! I will issue payment after the regression window 👍

[johncschuster]

@c3024 can you complete the BZ Checklist above when you get a moment? Thank you!

[c3024]

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:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR: No specific PR can be held responsible for this bug.
• [@c3024] 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: NA
• [@c3024] 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: Not started as this bug cannot be caught earlier.
• [@c3024] Determine if we should create a regression test for this bug. Yes
• [@c3024] 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. Test here in the author checklist Tests

1. Log into NewDot as user A
2. Verify the app and the initial report load without issue
3. Open a DM to user B
4. Submit an expense in the DM
5. Open the report and copy the link
6. Sign out
7. Open the link and sign in as user B
8. Verify

• the app loads
• the expense report loads
• the header includes a link to the parent report

[johncschuster]

Payment has been issued. Thanks for your contributions! 🎉