[HOLD for payment 2024-12-05] [HOLD for payment 2024-12-03] [$250] mweb/Chrome - Thread - Not here page when open multi level threads

[lanitochka17]

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: 9.0.59-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5193203&group_by=cases:section_id&group_order=asc&group_id=291935
Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to any conversation> Send a message
   2 Reply in the thread of this message
2. Create a thread within the thread
3. Repeat steps until you have 5 different levels of threads
4. Click on the header link to navigate to the parent thread
5. Go through the threads again

Expected Result:

All thread should open and no error page should appear

Actual Result:

Error page opens when open the last multi level thread/ same behavior occurs while creating multi level threads too

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6657771_1730995176084.Recording__4483.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021855829217876432826
• Upwork Job ID: 1855829217876432826
• Last Price Increase: 2024-11-11

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot]

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

[Christinadobrzyn]

I'm not getting this error. Asking QA for more information on the error

screenRecording-8-11-2024-13-49.mp4

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[bernhardoj]

This is most likely the same as #51825 (comment) which is easier to repro while offline. Can we reopen that issue or should I move my proposal here?

[izarutskaya]

Tester still can reproducible the issue with the same steps
Build v9.0.59-0

922ca273-f50f-4268-9c24-775b8645aec8.mp4

[melvin-bot]

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

[melvin-bot]

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

[Christinadobrzyn]

Thanks, @bernhardoj. Let's move forward with this GH, feel free to move your proposal here. Also adding External so @hoangzinh can review the proposal.

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-11-13 15:49:30 UTC.

Proposal

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

Not found page is shown after going to a new thread. It's easier to repro while offline.

What is the root cause of that problem?

When we open the new thread while offline, there will be 3 open report request (but only the last one sent). The first one when we optimistically create the thread and then send the parent report action ID to the request (along with other data).

App/src/libs/actions/Report.ts

Lines 1099 to 1101 in 6763f1a

	const participantLogins = PersonalDetailsUtils.getLoginsByAccountIDs(Object.keys(newChat.participants ?? {}).map(Number));
	openReport(newChat.reportID, '', participantLogins, newChat, parentReportAction.reportActionID);
	Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(newChat.reportID));

The 2nd one is when the report screen is mounted.

App/src/pages/home/ReportScreen.tsx

Lines 489 to 496 in 6763f1a

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

And the 3rd one is in this effect. (there is an issue to prevent this to be called)

App/src/pages/home/ReportScreen.tsx

Lines 580 to 587 in 6763f1a

	// before deciding that we shouldn't call OpenReport.
	if (reportIDFromRoute === lastReportIDFromRoute && (!onyxReportID || onyxReportID === reportIDFromRoute)) {
	return;
	}
	fetchReportIfNeeded();
	ComposerActions.setShouldShowComposeInput(true);
	// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps

The 2nd and 3rd calls shouldn't be made because it's an optimistic report.

App/src/pages/home/ReportScreen.tsx

Lines 441 to 446 in 6763f1a

	if (!shouldFetchReport(report) && (isInitialPageReady || isLinkedMessagePageReady)) {
	return;
	}
	fetchReport();
	}, [report, fetchReport, reportIDFromRoute, isLoadingApp, isInitialPageReady, isLinkedMessagePageReady]);

shouldFetchReport will return false for optimistic report and report that has not found error.

App/src/libs/shouldFetchReport.ts

Lines 4 to 8 in 6763f1a

	export default function shouldFetchReport(report: OnyxEntry<Report>) {
	// If the report is optimistic, there's no need to fetch it. The original action should create it.
	// If there is an error for creating the chat, there's no need to fetch it since it doesn't exist
	return !report?.isOptimisticReport && !report?.errorFields?.createChat;
	}

However, the && (isInitialPageReady || isLinkedMessagePageReady) was added in #44488 which causes the OpenReport to be called. isInitialPageReady checks for the report actions length.

App/src/pages/home/ReportScreen.tsx

Lines 337 to 339 in 6763f1a

	const isInitialPageReady = isOffline
	? reportActions.length > 0
	: reportActions.length >= CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT || isPendingActionExist || (doesCreatedActionExists() && reportActions.length > 0);

But when we open report, the report actions is still loading, so isInitialPageReady return false and an OpenReport request is made with a parentReportActionID as -1, and BE will return a not found report error when we create a new thread.

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

I think it doesn't make sense to tie shouldFetchReport with isInitialPageReady. If shouldFetchReport is false, then don't fetch the report. No need for the report actions to be available. shouldFetchReport only checks for the report object, so it doesn't depend on report actions. So, the solution is to remove, && (isInitialPageReady || isLinkedMessagePageReady)

App/src/pages/home/ReportScreen.tsx

Lines 441 to 443 in 6763f1a

	if (!shouldFetchReport(report) && (isInitialPageReady || isLinkedMessagePageReady)) {
	return;
	}

[hoangzinh]

Thanks for the proposal @bernhardoj, Can you help me understand why OpenReport to be called 3 times will cause "Not here page" error? Thank you.

[bernhardoj]

It's not that the "multiple OpenReport" causes the issue, but rather that the 2nd and 3rd OpenReport doesn't have parentReportActionID which is required when we create a thread for the first time, and only the 3rd one is sent because we now resolved duplicates request in this PR. We can compare for the parentReportActionID params, but I think it's better to fix the root cause where the OpenReport is called 3 times.

[hoangzinh]

It's clearer. Thanks @bernhardoj. Can you further help me understand why 3rd (and 2nd) OpenReport doesn't have parentReportActionID would lead to showing "Not here page"? Can you link me to the logic that will show the Not here page in the report screen in this case? Thank you.

[bernhardoj]

It's the BE response that returns not found when the parentReportActionID is missing (or -1 in this case because of the fallback) when we create the thread for the first time.

[hoangzinh]

I see. @bernhardoj can you put above comment into your RCA section? It would help the internal engineer review better.

[bernhardoj]

Updated.

[hoangzinh]

Thanks @bernhardoj. His proposal looks good to me

Link to proposal #52204 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[bernhardoj]

Another PR to fix #53013 is up.

Now I fully understand why not found view shows when linking to a comment (including thread parent action). When we are linking to a comment but the reportActionPages is empty, then the report actions will be empty.

App/src/libs/PaginationUtils.ts

Lines 174 to 177 in ba65563

	): {data: TResource[]; hasNextPage: boolean; hasPreviousPage: boolean} {
	if (pages.length === 0) {
	return {data: id ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false};
	}

Before #44488, we always trigger OpenReport when linking to a comment to fetch the reportActionPages. We did that inside ReportActionsView.

But that PR moved it to ReportScreen and use the fetchReportIfNeeded logic.

The reason was explained here. The new PR I raise fix it by making it work like before. I have made sure that there won't be double OpenReport when comment linking.

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

• Fix not found page shows when open a new thread #52815
• [CP Staging] Fix optimistic report status is not set correctly #53050
• [CP Staging] Fix not found view when linking to a message #53064

If no regressions arise, payment will be issued on 2024-12-03. 🎊

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

• @hoangzinh requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@hoangzinh @Christinadobrzyn @hoangzinh The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.67-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] Fix optimistic report status is not set correctly #53050
• [CP Staging] Fix not found view when linking to a message #53064

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

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

• @hoangzinh requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@hoangzinh @Christinadobrzyn @hoangzinh The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[Christinadobrzyn]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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:

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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:

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[Christinadobrzyn]

whoops! sorry for posting ^ @hoangzinh- Let us know about a regression test.

[hoangzinh]

@Christinadobrzyn I think we won't need a regression test for this issue. It's an edge-case issue when user creates a new thread while offline, in online case it's hard to reproduce. Btw, I think we already have a regression test somewhere to cover a scenario that creates a new thread.

[Christinadobrzyn]

Awesome! thanks @hoangzinh!

Contributor: @bernhardoj paid $250 via NewDot
Contributor+: @hoangzinh  owed $250 via NewDot

Payment day is tomorrow

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @hoangzinh owed $250 via NewDot
• Reviewer: @bernhardoj owed $250 via NewDot

BugZero Checklist (@Christinadobrzyn)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1855829217876432826/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[Christinadobrzyn]

@hoangzinh can you please submit a request for payment through NewDot? TY!

[hoangzinh]

ah yes. Thanks @Christinadobrzyn. Requested in ND

[Christinadobrzyn]

Awesome! TY! Closing!

[garrettmknight]

$250 approved for @hoangzinh