[Due for payment 2025-04-17] [$500][HIGH] Navigating to a thread from the Search page doesn’t take you to the chat

[m-natarajan]

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.1.11-3
Reproducible in staging?: Yes
Reproducible in production?: yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @NikkiWines
Slack conversation (hyperlinked to channel name): Expensify-bugs

Action Performed:

1. Click the Search icon from the Inbox page
2. Search for text you know exists in a thread by searching for type:chat "", replacing with your search value
3. Click on the result in the search page
4. Click on the top chat in the thread to try and navigate to that thread in your inbox

Expected Result:

You’re routed to the message that you clicked on

Actual Result:

You’re routed to the most recent message in the channel that has the thread you clicked on

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

Screen.Recording.2025-03-11.at.10.48.46.mov

Recording.1044.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021900185525361353185
• Upwork Job ID: 1900185525361353185
• Last Price Increase: 2025-03-13
• Automatic offers:
• thelullabyy | Contributor | 106560190

Issue Owner

Current Issue Owner: @eVoloshchak

[melvin-bot]

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

[thelullabyy]

🚨 Edited by proposal-police: This proposal was edited at 2025-03-12 01:12:20 UTC.

Proposal

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

• You’re routed to the most recent message in the channel that has the thread you clicked on

What is the root cause of that problem?

• When clicking on the parent message, the logic in:
  

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

  Lines 133 to 137 in de07682

  	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID));
  	if (isVisibleAction && !isOffline) {
  	// Pop the chat report screen before navigating to the linked report action.
  	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));
  	}

executes without verifying whether the click originates from the report RHP. As a result, calling:

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

Line 136 in de07682

Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));

triggers the navigation logic in:

App/src/libs/Navigation/Navigation.ts

Line 301 in de07682

navigationRef.current.dispatch({...StackActions.pop(distanceToPop), target: targetState.key});

which only performs a pop action. Since this does not append reportActionID to the parameters, it results in navigating to the most recent message in the channel containing the thread instead of the specific message that was clicked.

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

• When clicking on the parent message, we need to handle cases where the click originates from the report RHP separately.

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

Line 131 in de07682

const isVisibleAction = shouldReportActionBeVisible(ancestor.reportAction, ancestor.reportAction.reportActionID, canUserPerformWriteAction);

                                          const isVisibleAction = shouldReportActionBeVisible(ancestor.reportAction, ancestor.reportAction.reportActionID, canUserPerformWriteAction);
                                          if (isInNarrowPaneModal) {
                                              Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: ancestor.report.reportID, reportActionID: ancestor.reportAction.reportActionID}));
                                              return;
                                          }

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

• Implement a utility function, navigateToParent.
  Verify that when isInNarrowPaneModal is true, the navigate function is called with the correct URL, ensuring it includes both reportID and reportActionID.

What alternative solutions did you explore? (Optional)

• An alternative option is to update:

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

Lines 133 to 137 in de07682

	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID));
	if (isVisibleAction && !isOffline) {
	// Pop the chat report screen before navigating to the linked report action.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));
	}

                                          if (isVisibleAction && !isOffline) {
                                              // Pop the chat report screen before navigating to the linked report action.
                                              Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));
                                          } else {
                                              Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID));
                                          }

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

@eVoloshchak 👀 on the proposal above.
Adding status of HIGH cuz search is a feature that most people will use and if they can't find their way to a chat then search isn't really working. cc @luacmartins in case you have thoughts/ideas/tips here. (also.. while here, are there any others to tag for 👀 on search-related bugs? Don't wanna just blast you all the time.)

[melvin-bot]

Upwork job price has been updated to $500

[mallenexpensify]

Bumping to $500 cuz it's HIGH and has Help Wanted (I'm doing to most HIGH and Help Wanted issues, I realize @thelullabyy already submitted a proposal that @eVoloshchak needs to review)

[eVoloshchak]

@thelullabyy, thank you for the proposal!

Since this does not append reportActionID to the parameters

We aren't passing reportActionID as a navigation param, we're passing it to getRoute as a second argument here. In this case, both .navigate and .goBack receive only one argument - route

The proposal doesn't resolve the issue for me, could you please double-check this on your end?

[thelullabyy]

• @eVoloshchak I tested my solution and it works for me, could you help check the video below:

Screen.Recording.2025-03-14.at.07.56.40.mov

We aren't passing reportActionID as a navigation param, we're passing it to getRoute as a second argument here. In this case, both .navigate and .goBack receive only one argument - route

• I noticed that we’re already passing the second argument to the getRoute function here:

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

Line 136 in de07682

Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));

Inside Navigation.goBack, it calls:

App/src/libs/Navigation/Navigation.ts

Line 301 in de07682

navigationRef.current.dispatch({...StackActions.pop(distanceToPop), target: targetState.key});

It is only a pop action, and no reportActionID parameter is passed along with that action.

[eVoloshchak]

Aha, I see what the problem is. We have separate logic for when you click on a thread message vs when you click on a 'Thread' text (top of the thread chat)

Your proposal does highlight the correct thread, but only for the first case (clicking on the thread message itself). But even then it doesn't scroll the chat to the highlighted message

Screen.Recording.2025-03-14.at.14.27.28.mov

[thelullabyy]

@eVoloshchak Thanks, you are correct, my original solution does not scroll to the linked message position.

I just updated my main solution, and here is the result, it works well:

Screen.Recording.2025-03-14.at.21.53.13.mov

[mallenexpensify]

@eVoloshchak 👀 on @thelullabyy 's comment above and updated proposal. Thx

[eVoloshchak]

@thelullabyy's updated proposal looks good to me!

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[melvin-bot]

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

[eVoloshchak]

@mallenexpensify, @Beamanator, should there be any difference in behavior between clicking on a thread message

and clicking on a thread title/icon?

We have two separate handlers for that right now, but they're the same (both have this issue). I suspect there is no need for this and behavior should be the same?

[mallenexpensify]

I suspect there is no need for this and behavior should be the same?

This would be my guess but I'm uncertain. @Beamanator ?

[Beamanator]

Yeah I'm seeing the same result for both, so it makes sense for only 1 handler... Is there any code diff in each handler?

[eVoloshchak]

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

Lines 135 to 141 in cf0a826

	const isVisibleAction = shouldReportActionBeVisible(ancestor.reportAction, ancestor.reportAction.reportActionID, canUserPerformWriteAction);
	// Pop the thread report screen before navigating to the chat report.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID));
	if (isVisibleAction && !isOffline) {
	// Pop the chat report screen before navigating to the linked report action.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));
	}

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

Lines 49 to 55 in cf0a826

	const isVisibleAction = shouldReportActionBeVisible(ancestor.reportAction, ancestor.reportAction.reportActionID, canUserPerformWriteAction(ancestor.report));
	// Pop the thread report screen before navigating to the chat report.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID));
	if (isVisibleAction) {
	// Pop the chat report screen before navigating to the linked report action.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(ancestor.report.reportID, ancestor.reportAction.reportActionID));
	}

Is there any code diff in each handler?

@Beamanator, they're the same (canUserPerformWriteAction is calculated in an upper scope for the first one, but they're effectively the same). I reckon we can combine them into one?

[Beamanator]

Wow yeah that would be great to combine them into 1, they look insanely similar 👍 👍 thanks for finding that!

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

• Navigating to a thread from the Search page doesn’t take you to the chat #58628

If no regressions arise, payment will be issued on 2025-04-17. 🎊

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

• @eVoloshchak requires payment through NewDot Manual Requests
• @thelullabyy requires payment automatic offer (Contributor)

[melvin-bot]

@eVoloshchak @NicMendonca @eVoloshchak 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]

[NicMendonca]

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 (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 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 👎

[NicMendonca]

@eVoloshchak bump on the BZ checklist ^^

[eVoloshchak]

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 (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 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: Unable to pinpoint the exact PR that has caused this, Git blame is not working correctly. According to git blame, Fixes for redesign thread ancestry feature #39343 is the PR to introduce the new (invalid) logic, but that is not the case

• [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: regression wasn't critical

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

Test:

1. Click the Search icon from the Inbox page
2. Search for text you know exists in a thread by searching for type:chat "", replacing with your search value
3. Click on the result in the search page
4. Click on the top chat in the thread to try and navigate to that thread in your inbox
5. Verify you’re routed to the message that you clicked on

Do we agree 👍 or 👎

[melvin-bot]

@eVoloshchak Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mallenexpensify]

Contributor: @thelullabyy paid $500 via Upwork
Contributor+: @eVoloshchak due $500 via NewDot

Test case created

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