[$250] IOU - Web - Loading spinner appears under expense every time open the expense page

[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: 9.0.6-0
Reproducible in staging?: Y
Reproducible in production?: Y
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. Go to FAB> Submit expense> Complete the flow
2. Go to thread transaction page
3. Go back to request page ( switch chat if needed)

Expected Result:

There should be no loading spinner when open the request page

Actual Result:

Loading spinner appears under expense every time open the request page

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

Bug6538463_1720653760599.Recording__3488.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018c260c51c3955b65
• Upwork Job ID: 1813342162639120406
• Last Price Increase: 2024-07-16
• Automatic offers:
• paultsimura | Reviewer | 103347808
• ishpaul777 | Contributor | 103347809

Issue Owner

Current Issue Owner: @ishpaul777

[melvin-bot]

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

[lanitochka17]

@mallenexpensify 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

[bernhardoj]

Proposal

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

Loading indicator shows when we go back from a thread of the last message of a chat.

What is the root cause of that problem?

When we open a thread of the last message of a chat, then go back usign the parent link (on the header), a ReadNewestAction request is called which will show the loading indicator.

App/src/pages/home/report/ListBoundaryLoader.tsx

Lines 99 to 109 in 978b833

	if (type === CONST.LIST_COMPONENTS.HEADER && isLoadingNewerReportActions) {
	// applied for a header of the list, i.e. when you scroll to the bottom of the list
	// the styles for android and the rest components are different that's why we use two different components
	return (
	<View style={[styles.alignItemsCenter, styles.justifyContentCenter, styles.listBoundaryLoader]}>
	<ActivityIndicator
	color={theme.spinner}
	size="small"
	/>
	</View>
	);

This happens after #39454. The read newest action request is triggered from this block.

App/src/pages/home/report/ReportActionsView.tsx

Lines 431 to 433 in 978b833

	if ((reportActionID && indexOfLinkedAction > -1 && !isLoadingOlderReportsFirstNeeded) || (!reportActionID && !isLoadingOlderReportsFirstNeeded)) {
	handleReportActionPagination({firstReportActionID: newestReportAction?.reportActionID});
	}

App/src/pages/home/report/ReportActionsView.tsx

Lines 297 to 308 in 978b833

	const handleReportActionPagination = useCallback(
	({firstReportActionID}: {firstReportActionID: string}) => {
	// This function is a placeholder as the actual pagination is handled by visibleReportActions
	if (!hasMoreCached) {
	isFirstLinkedActionRender.current = false;
	fetchNewerAction(newestReportAction);
	}
	if (isFirstLinkedActionRender.current) {
	isFirstLinkedActionRender.current = false;
	}
	setCurrentReportActionID(firstReportActionID);
	},

Before the linked PR, we have a hasNewestReportAction in the condition. hasNewestReportAction tells us whether the last report action in the list is the newest one or not. If it's the newest one, then we don't request for read newest action. But the PR removed it because of this issue, so the read newest action request is always triggered.

Removing hasNewestReportAction works, but incomplete. We still need to prevent the read newest action request if hasNewestReportAction is true, but we need to place it correctly. To know where to put it, we need to understand what the issue is about first that caused hasNewestReportAction to be removed. To summarize it:

The issue that I linked above is that, when we link to the last action/message, sending another message won't show in the chat list. That's because isFirstLinkedActionRender is "stuck" at true, so the report actions is sliced only up to the linked action.

App/src/pages/home/report/ReportActionsView.tsx

Lines 251 to 255 in 978b833

	if (isFirstLinkedActionRender.current) {
	return combinedReportActions.slice(indexOfLinkedAction);
	}
	const paginationSize = getInitialPaginationSize;
	return combinedReportActions.slice(Math.max(indexOfLinkedAction - paginationSize, 0));

The only way to make it false is to trigger handleReportActionPagination, but it's never triggered because hasNewestReportAction is already true (remember, we linked to the last action/message).

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

The solution is to only request for read newest action if the hasNewestReportAction is false in here.

App/src/pages/home/report/ReportActionsView.tsx

Lines 300 to 303 in 978b833

	if (!hasMoreCached) {
	isFirstLinkedActionRender.current = false;
	fetchNewerAction(newestReportAction);
	}

This way, handleReportActionPagination will still be called and set isFirstLinkedActionRender to false while preventing read newest action request to be fired.

[bernhardoj]

Added more explanation to the proposal

[mallenexpensify]

@bernhardoj is this the loading spinner that's referenced in the OP?

[bernhardoj]

I believe it's this one, the read new action loading indicator.

[melvin-bot]

@mallenexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018c260c51c3955b65

[melvin-bot]

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

[mallenexpensify]

@paultsimura can you attempt reproduction? If you're able to reproduce and the steps in the OP need to be updated, please outline them and I'll update the OP. I tried to reproduce yesterday and today with no luck (I think I might not have been holding it right). thx

[paultsimura]

Hey @mallenexpensify – for me, using the steps from OP was sufficient.
The mentioned spinner is the one at the bottom of the chat as @bernhardoj outlined here.

I can reproduce it constantly:

2024-07-17.-.09.16.-.Screen.Recording.2024-07-17.at.09.13.14.mp4

If you think it's a relevant bug, I can proceed with the review.

[mallenexpensify]

If it's a reproducible bug and we can fix it, we likely should. That said... might this be occurring anywhere else in the app? Or.. would the fix here fix/improve anything else?

[bernhardoj]

It will fix #45387 too, which is easier to reproduce.

[mallenexpensify]

Thanks @bernhardoj, @paultsimura plz proceed with reviewing @bernhardoj 's proposal above

[melvin-bot]

@paultsimura, @mallenexpensify Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

@trjExpensify , added to Wave Collect cuz the bug occurs on the expense page. It's def polish and/or low priority.

[trjExpensify]

We should for sure get eyes on this RCA and proposal from the crew that worked on this comment linking initiative, as it was a complex project. So, CC'ing @puneetlath @jasperhuangg @roryabraham @rayane-djouah @rushatgabhane as a few peeps mentioned in the linked issues I saw above.

[melvin-bot]

@paultsimura, @mallenexpensify Huh... This is 4 days overdue. Who can take care of this?

[ishpaul777]

@mallenexpensify would be great if you can assign me here, so this is on my k2

[melvin-bot]

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

📣 @ishpaul777 🎉 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 📖

[paultsimura]

Unassigning myself here as @ishpaul777 will be the C+.

[ishpaul777]

lets roll with @bernhardoj's proposal to keep this moving forward..

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[bernhardoj]

PR is ready

cc: @ishpaul777

[ishpaul777]

This is ready for payment! 🎉

[mallenexpensify]

Contributor: @bernhardoj owed $250 via NewDot
Contributor+: @ishpaul777 paid $250 via Upwork

Thx @ishpaul777, can you fill out the BZ checklist plz?

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:

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

[bernhardoj]

Requested in ND.

[mallenexpensify]

Forgot to tag @bernhardoj in the payment summary, updated.

@ishpaul777 BZ checklist plzzzzzzzz

[ishpaul777]

On it, Sorry missed the ping before

[ishpaul777]

• The PR that introduced the bug has been identified. Link to the PR: Update thread headers and ancestry to deep link back to the original comment using comment linking #39454
• 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: https://github.com/Expensify/App/pull/39454/files#r1739242426
• 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 required
• Determine if we should create a regression test for this bug. - Yes
• 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.

Regression Test Proposal

Steps:

1. Go to FAB> Submit expense> Complete the flow
2. Go to thread transaction page
3. Go back to request page.

Expected Result:
There should be no loading spinner when open the request page

Test steps 2:

1. copy link to comment in a report
2. visit the link.
3. Send a new message in report.

Expected Result:
No loading indicator at the bottom

Do we agree 👍 or 👎 ?

[mallenexpensify]

Thanks @ishpaul777 , closing, test case is here

• https://github.com/Expensify/Expensify/issues/424780

[ishpaul777]

i think This is good to close ?

[JmillsExpensify]

$250 approved for @bernhardoj