[HOLD for payment 2024-04-05] [$500] Skeleton is displayed when navigating to a transaction thread created offline

[roryabraham]

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: 1.4.55-2
Reproducible in staging?: no
Reproducible in production?: no
Issue reported by: @youssef-lr
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1711054985550349

Action Performed:

1. Create a chat with someone online
2. Request money from them then navigate to the money report (click on the preview).
3. Go offline and request again from the money report.
4. Try to navigate to the transaction thread of the request created offline.

Expected Result:

it should display the transaction thread where we can edit the fields

Actual Result:

It shows an infinite skeleton loader

Workaround:

n/a

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

Screen.Recording.2024-03-21.at.20.56.26.1.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01694ea7e3c07bf01d
• Upwork Job ID: 1771247582016843776
• Last Price Increase: 2024-03-22
• Automatic offers:
• situchan | Reviewer | 0

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[roryabraham]

this is reproducible on main but not staging, so will be a deploy blocker once we ship the current checklist. For that reason, I'm not marking it as external, but going to ask @perunt to investigate a fix, as there seems to be consensus in slack that this is a regression from #30269

[situchan]

Happy to review this if C+ is needed

[roryabraham]

I found that the optimistic IOU action has a childReportID field, that does point to an optimistic chat report. That optimistic chat report has a single optimistic CREATED action So that's all as expected

[roryabraham]

shouldShowSkeleton is getting stuck as true here

isLoadingInitialReportActions appears to be the problem, but interestingly it's wrong in the reportMetadata as well

[roryabraham]

ok, it appears that the reportMetadata for the report in question is somehow getting deleted, and since isLoadingInitialReportActions defaults to true in ReportScreen, it's getting stuck there

edit: It appears the debug util Onyx.log just randomly stopped working for me...
edit 2: nope, it works fine I just had a filter on 🤦

[roryabraham]

Did git checkout b3068864afeaedd448cb59107629fc5cb67fe009~1 and verified this is not reproducible pre-comment-linking

[roryabraham]

Interestingly isLoadingInitialReportActions was getting stuck on true before, it just didn't matter because we weren't relying on it to be correct:

[roryabraham]

and, I suppose it kind of makes sense that it would get stuck as true when we're offline, because OpenReport sets it to true in its optimisticData, then to false in its successData + failureData. So it's stuck as true, even though we're not really loading anything while we're offline

[youssef-lr]

Interestingly isLoadingInitialReportActions was getting stuck on true before, it just didn't matter because we weren't relying on it to be correct

What has changed now?

[roryabraham]

It's used in ReportScreen as part of shouldShowSkeleton:

App/src/pages/home/ReportScreen.tsx

Lines 334 to 335 in 7e4a438

	const shouldShowSkeleton =
	isLinkingToMessage || !isReportReadyForDisplay || isLoadingInitialReportActions || isLoading || (!!reportActionIDFromRoute && reportMetadata?.isLoadingInitialReportActions);

[nkdengineer]

Proposal

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

It shows an infinite skeleton loader

What is the root cause of that problem?

We're getting reportActions to display by ReportActionsUtils.getContinuousReportActionChain function.

App/src/pages/home/ReportScreen.tsx

Line 242 in 408a894

const currentRangeOfReportActions = ReportActionsUtils.getContinuousReportActionChain(sortedAllReportActions, reportActionIDFromRoute);

When we create a money request, report actions of transaction thread report only have a created action with pendingAction is add. It leads index is -1 and we return an empty array.

App/src/libs/ReportActionsUtils.ts

Lines 265 to 278 in 408a894

	if (id) {
	index = sortedReportActions.findIndex((obj) => obj.reportActionID === id);
	} else {
	index = sortedReportActions.findIndex((obj) => obj.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
	}
	if (index === -1) {
	return [];
	}
	let startIndex = index;
	let endIndex = index;
	// Iterate forwards through the array, starting from endIndex. This loop checks the continuity of actions by:

That makes isLoadingInitialReportActions is true and then the skeleton display.

App/src/pages/home/ReportScreen.tsx

Line 265 in 408a894

const isLoadingInitialReportActions = reportActions.length === 0 && !!reportMetadata?.isLoadingInitialReportActions;

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

In here I think we should update to accept the created action, the created action is in the last of sortedReportActions so if we can't find any action it will return the created action if it exists.

index = sortedReportActions.findIndex((obj) => obj.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD || obj.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED);

App/src/libs/ReportActionsUtils.ts

Line 268 in 408a894

index = sortedReportActions.findIndex((obj) => obj.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);

What alternative solutions did you explore? (Optional)

Or in this case if index is -1 and id is empty we will return sortedReportActions itself instead of return an empty array

App/src/libs/ReportActionsUtils.ts

Lines 271 to 273 in 408a894

	if (index === -1) {
	return [];
	}

Result

Screen.Recording.2024-03-22.at.16.10.45.mov

[nkdengineer]

@roryabraham What do you think about my proposal?

[roryabraham]

thanks @nkdengineer, good investigation. Your proposal LGTM 👍🏼

[roryabraham]

I did a bit of extra refactoring to rename some variables and improve some comments in https://github.com/Expensify/App/pull/38825/files. If possible can I request that you include these changes in your PRs as well? 🙇🏼

[nkdengineer]

If possible can I request that you include these changes in your PRs as well? 🙇🏼

@roryabraham Sure.

[roryabraham]

since #30269 is now on staging, I'm marking this as a deploy blocker

[melvin-bot]

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

📣 @nkdengineer 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 📖

[nkdengineer]

@roryabraham @situchan the PR is ready for review.

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

[roryabraham]

Verified this is fixed:

web.mov

so now all that's needed is payment:

• $500 to @nkdengineer for the PR
• $500 to @situchan for the review

[lschurr]

Great! PR is here: #38829 - We'll issue payments 7 days after it's deployed to Prod.

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

• Fix skeleton display on transaction thread report in offline #38829

If no regressions arise, payment will be issued on 2024-04-05. 🎊

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

• @situchan requires payment automatic offer (Reviewer)
• @nkdengineer requires payment (Needs manual offer from BZ)

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

• [@situchan] The PR that introduced the bug has been identified. Link to the PR:
• [@situchan] 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:
• [@situchan] 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:
• [@situchan] Determine if we should create a regression test for this bug.
• [@situchan] 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: @nkdengineer $500 (paid in Upwork)
• Contributor+: @situchan $500 (paid in Upwork)

[lschurr]

@nkdengineer - Can you send me your Upwork profile or apply to the job directly here? https://www.upwork.com/ab/applicants/1771247582016843776

[nkdengineer]

@lschurr Below's my Upwork profile!

https://www.upwork.com/freelancers/~0134d0b74faef1a5ac

[lschurr]

Great! I sent the offer in Upwork @nkdengineer - https://www.upwork.com/nx/wm/offer/101804520

[lschurr]

This is complete.