[HOLD for payment 2024-08-29] [$250] iOS - Thread - Back button in the main chat reopens main chat after leaving thread

[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.11-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4760347
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify app
2. Go to DM
3. Send a message
4. Long press on the message > Reply in thread
5. Send a reply in thread
6. Tap on the report header
7. Tap Leave
8. Tap back button on the top left in the main chat

Expected Result:

App will return to LHN

Actual Result:

App reopens the main chat

Workaround:

Unknwon

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

Bug6551503_1721825710340.ScreenRecording_07-24-2024_20-48-43_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014c369a294716b5b5
• Upwork Job ID: 1816166555115213621
• Last Price Increase: 2024-07-31

Issue Owner

Current Issue Owner: @garrettmknight / @jliexpensify

[melvin-bot]

Triggered auto assignment to @garrettmknight (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]

We think that this bug might be related to #wvip-vsp

[lanitochka17]

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

[FitseTLT]

Proposal

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

Thread - Back button in the main chat reopens main chat after leaving thread

What is the root cause of that problem?

We are navigating to last accessed report after leaving room here

App/src/libs/actions/Report.ts

Lines 2790 to 2791 in a79189f

	navigateToMostRecentReport(report);
	}

Which is the main chat from which the thread was created. Therefore, when we navigate back we navigate to main chat once again as it existed in the navigation route state even before leaving thread.

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

This behaviour is expected for leaving room but for leaving chat thread we should only goBack, so change

App/src/libs/actions/Report.ts

Lines 2790 to 2791 in a79189f

	navigateToMostRecentReport(report);
	}

if (isChatThread) {
        Navigation.goBack();
    } else {
        navigateToMostRecentReport(report);
    }

What alternative solutions did you explore? (Optional)

[dominictb]

Proposal

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

App reopens the main chat

What is the root cause of that problem?

After leaving the thread, we call navigateToMostRecentReport

App/src/libs/actions/Report.ts

Line 2790 in bf69d5d

navigateToMostRecentReport(report);

In the detail implementation of navigateToMostRecentReport, we trigger goBack with fallbackRoute.

App/src/libs/actions/Report.ts

Line 2663 in bf69d5d

Navigation.goBack(lastAccessedReportRoute);

In this case, fallbackRoute is the last route, so it's duplicated.

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

We should call goBack to remove the current route, then go to the correspond lastAccessedReportRoute like what we already did in here

if (lastAccessedReportID) {
        const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? '-1');
        Navigation.goBack();
        Navigation.navigate(lastAccessedReportRoute)
    } else {

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

Pressing the back button after leaving a thread shows the main chat again.

What is the root cause of that problem?

When we leave a thread, we already call dismiss modal to close the RHP first,

App/src/pages/ReportDetailsPage.tsx

Lines 219 to 227 in 98eb1bb

	const leaveChat = useCallback(() => {
	Navigation.dismissModal();
	if (isRootGroupChat) {
	Report.leaveGroupChat(report.reportID);
	return;
	}
	const isWorkspaceMemberLeavingWorkspaceRoom = (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || isPolicyExpenseChat) && isPolicyEmployee;
	Report.leaveRoom(report.reportID, isWorkspaceMemberLeavingWorkspaceRoom);
	}, [isPolicyEmployee, isPolicyExpenseChat, isRootGroupChat, report.reportID, report.visibility]);

the leaveChat then calls navigateToMostRecentReport to perform goBack with a fallback route of the main chat report.

App/src/libs/actions/Report.ts

Lines 2660 to 2663 in 98eb1bb

	if (lastAccessedReportID) {
	const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? '-1');
	Navigation.goBack(lastAccessedReportRoute);
	} else {

In an ideal case, the RHP is already closed and the goBack with the fallback route will pop every report screen until it's arrived at the fallback route.

App/src/libs/Navigation/Navigation.ts

Lines 230 to 239 in 98eb1bb

	if (isCentralPaneFocused && fallbackRoute) {
	// Allow CentralPane to use UP with fallback route if the path is not found in root navigator.
	if (distanceFromPathInRootNavigator === -1) {
	navigate(fallbackRoute, CONST.NAVIGATION.TYPE.UP);
	return;
	}
	// Add possibility to go back more than one screen in root navigator if that screen is on the stack.
	if (distanceFromPathInRootNavigator > 0) {
	navigationRef.current.dispatch(StackActions.pop(distanceFromPathInRootNavigator));

However, in this case, the RHP is still in the nav state stack, so the goBack will replace the RHP with the main chat report.

App/src/libs/Navigation/Navigation.ts

Lines 222 to 225 in 98eb1bb

	if (shouldEnforceFallback || (isFirstRouteInNavigator && fallbackRoute)) {
	navigate(fallbackRoute, CONST.NAVIGATION.TYPE.UP);
	return;
	}

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

We need to delay the navigation like we did in #44537.

Navigation.dismissModal();
Navigation.isNavigationReady().then(() => // leaveRoom / leaveGroupChat);

or use setTimeout/InteractionManager/useWaitForNavigate hook

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[garrettmknight]

@jliexpensify I'm headed out until August 5th. I'll pick this back up when I'm back. Thanks for your help in the meantime!

[jliexpensify]

No worries! Bumping @eVoloshchak for reviews please.

[melvin-bot]

@garrettmknight, @eVoloshchak, @jliexpensify Huh... This is 4 days overdue. Who can take care of this?

[jliexpensify]

Bumping @eVoloshchak for reviews! Also bumping on Slack.

[eVoloshchak]

I think we should proceed with @bernhardoj's proposal, this is the easiest and most reliable approach

🎀👀🎀 C+ reviewed!

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

• Fix back button in the main chat reopens main chat after leaving thread #46861

If no regressions arise, payment will be issued on 2024-08-29. 🎊

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

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

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

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

[jliexpensify]

Garrett is back! Unassigning myself

[melvin-bot]

Payment Summary

Upwork Job

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

BugZero Checklist (@garrettmknight)

• 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/1816166555115213621/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[garrettmknight]

Payment summary looks right to me:

• @eVoloshchak can you finish the checklist before you request payment?
• @bernhardoj please request payment when you're ready.

[bernhardoj]

Requested in ND.

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: I'm unable to pinpoint the PR that has caused this, looks like this is the original implementation
• 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: N/A
• 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: this is a pretty straightforward bug, I don't think an additional discussion is needed

Regression Test Proposal

1. Open any chat
2. Send a message
3. Reply in the thread of the message
4. Press the chat header and press Leave
5. Verify you are navigated back to the main chat
6. Press the go back button
7. Verify you are navigated back to LHN

Do we agree 👍 or 👎

[garrettmknight]

Dropping to weekly while @eVoloshchak request payment.

[JmillsExpensify]

$250 approved for @eVoloshchak

[JmillsExpensify]

$250 approved for @bernhardoj

[garrettmknight]

@JmillsExpensify did you get the request from @bernhardoj too?

[garrettmknight]

@JmillsExpensify bump

[JmillsExpensify]

Yes, I did. I updated my comment above.