[$500] IOU - Request details page loads infinitely when request is created offline

[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.44-0
Reproducible in staging?: Y
Reproducible in production?: N
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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go offline
3. Go to workspace chat
4. Create a manua; request
5. Navigate to request details page

Expected Result:

The request details page will load when the request is created offline (production behavior)

Actual Result:

The request details page loads infinitely when the request is created offline (production behavior).
This issue also occurs when:

• creating IOU with new user offline. The main chat loads infinitely.
• Same issue when opening thread offline

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

Bug6393457_1708992632169.bandicam_2024-02-27_06-12-20-868.mp4

View all open jobs on GitHub

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0106a1c5479d6e9daa

[melvin-bot]

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

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

[melvin-bot]

Auto-assign attempt failed, all eligible assignees are OOO.

[lanitochka17]

We think that this bug might be related to #vip-bills
CC @davidcardoza

[dukenv0307]

Proposal

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

The request details page loads infinitely when the request is created offline (production behavior).
This issue also occurs when:

What is the root cause of that problem?

This is regression of this PR Expensify/react-native-onyx#470 since we always set value to mapping.initialValue without the check mapping.initialValue is undefined or not

And then we have allowStaleData as true for report key here which makes cachedState wrong and then report data in Onyx is wrong.

App/src/pages/home/ReportScreen.js

Line 617 in d2a9c6f

allowStaleData: true,

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

We should add the check mapping.initialValue !== undefined here https://github.com/Expensify/react-native-onyx/blob/c9f22a3f655eb14b1cce7b98282fe21646c5d27d/lib/withOnyx.js#L62

What alternative solutions did you explore? (Optional)

We can remove allowStaleData here and then cachedState will be calculated correctly

App/src/pages/home/ReportScreen.js

Line 617 in d2a9c6f

allowStaleData: true,

[youssef-lr]

cc @bernhardoj I think this is related to your PRs here & here

[bernhardoj]

Checking

[bernhardoj]

Okay, it's indeed reproducible after the Onyx version bump, but here is the real root cause.

What the recent Onyx PR does is to immediately render the report screen by fixing some initialValue that are not applied when they are falsey (false or null). Btw, this PR is the first one that attempted to immediately render the report screen, but because we have the Onyx bug, it's not solved 100%.

1. Now, when we press the money request preview, we create an optimistic transaction thread and navigate to it.
   

   App/src/components/ReportActionItem/MoneyRequestAction.tsx

   Lines 96 to 101 in d2a9c6f

   	if (!childReportID) {
   	const thread = ReportUtils.buildTransactionThread(action, requestReportID);
   	const userLogins = PersonalDetailsUtils.getLoginsByAccountIDs(thread.participantAccountIDs ?? []);
   	Report.openReport(thread.reportID, userLogins, thread, action.reportActionID);
   	Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(thread.reportID));
   	return;

Because the report screen is now immediately rendered, the new report Onyx data is not available yet, so the code below will call openReport.

App/src/pages/home/ReportScreen.js

Lines 355 to 362 in d2a9c6f

	// It possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
	// is not stored locally yet. If report.reportID exists, then the report has been stored locally and nothing more needs to be done.
	// If it doesn't exist, then we fetch the report from the API.
	if (report.reportID && report.reportID === getReportID(route) && !isLoadingInitialReportActions) {
	return;
	}
	Report.openReport(reportIDFromPath);

2. In openReport, we have a logic to use SET method (for the optimistic report) when the report is not in the Onyx yet,
   

   App/src/libs/actions/Report.ts

   Lines 608 to 613 in d2a9c6f

   	const report = ReportUtils.getReport(reportID);
   	// If we open an exist report, but it is not present in Onyx yet, we should change the method to set for this report
   	// and we need data to be available when we navigate to the chat page
   	if (isEmptyObject(report)) {
   	optimisticData[0].onyxMethod = Onyx.METHOD.SET;
   	}

The optimistic report looks like this

App/src/libs/actions/Report.ts

Lines 552 to 556 in d2a9c6f

	const optimisticReport = reportActionsExist(reportID)
	? {}
	: {
	reportName: allReports?.[reportID]?.reportName ?? CONST.REPORT.DEFAULT_REPORT_NAME,
	};

So, the optimistic transaction thread is overwritten with the report above that only contains reportName, thus the infinite loading.

How do we solve this?

I would remove the logic to change the onyx method to SET (in point 2). It was added in this PR to solve #19364. The issue is that the not-found view is shown briefly (instead of skeleton loading) when opening a thread.

If we read the root cause from the proposal, it's because when we don't have the report data yet, the default props of the report will have a value of isLoadingReportActions as false which makes the not found condition becomes true.

App/src/pages/home/ReportScreen.js

Lines 97 to 100 in 2157057

	report: {
	hasOutstandingIOU: false,
	isLoadingReportActions: false,
	},

App/src/pages/home/ReportScreen.js

Lines 266 to 267 in 2157057

	<FullPageNotFoundView
	shouldShow={(!this.props.report.reportID && !this.props.report.isLoadingReportActions && !isLoading) || shouldHideReport}

isLoadingReportActions will become true when the openReport optimistic data is merged.

App/src/libs/actions/Report.js

Lines 334 to 339 in 2157057

	function openReport(reportID, participantList = [], newReportObject = {}, parentReportActionID = '0', isFromDeepLink = false) {
	const optimisticReportData = {
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
	value: {
	isLoadingReportActions: true,

Instead of using SET, we should just set the default value of isLoadingReportActions to true which is already the case today.
isLoadingReportActions is isLoadingInitialReportActions in the latest code.

App/src/pages/home/ReportScreen.js

Lines 620 to 623 in d2a9c6f

	reportMetadata: {
	key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`,
	initialValue: {
	isLoadingInitialReportActions: true,

App/src/pages/home/ReportScreen.js

Lines 495 to 501 in d2a9c6f

	const shouldShowNotFoundPage = useMemo(
	() =>
	(!wasReportAccessibleRef.current &&
	!firstRenderRef.current &&
	!report.reportID &&
	!isOptimisticDelete &&
	!reportMetadata.isLoadingInitialReportActions &&

@youssef-lr should the solution be reviewed first or should I just immediately create the follow-up PR to fix this?

[youssef-lr]

You can start on the PR, thanks!

[paultsimura]

Hey, @bernhardoj just a note: a part of the root cause is that the money report actions have no childReportID when created offline – this is being fixed in #37232

[bernhardoj]

@paultsimura thanks, yep I noticed about that too, but not really sure whether it's intentional or not.

Here is the PR to fix this issue #37269

[puneetlath]

Looks like this revert fixed it. Closing, but feel free to reopen if I'm mistaken.