[Payment due @mkhutornyi] Fix thread RHP not closing when parent message deleted from search pages by WojtekBoman · Pull Request #94293 · Expensify/App

WojtekBoman · 2026-06-23T10:22:34Z

Explanation of Change

When a parent message with a thread is deleted, the thread RHP should auto-close. This worked when the thread was opened from a regular report view, but not from search pages (SEARCH_REPORT route). Added a check for the search report route in deleteReportComment so Navigation.goBack() fires in both cases.

Screen.Recording.2026-06-23.at.12.28.55.mov

Fixed Issues

$ #92543 (comment)
PROPOSAL:

Tests

Verify that no errors appear in the JS console

Open the Search page and find a message that has a thread (reply in conversation).
Tap the message to open the thread RHP from search.
Delete the parent message (e.g. from the message action menu).
Verify that the thread RHP closes automatically and you return to the search results.
Open a report directly (non-search) and open a thread RHP for a message.
Delete the parent message.
Verify the thread RHP still closes correctly (regression check for the existing path).

Offline tests

N/A

QA Steps

Same as tests.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

codecov · 2026-06-23T10:27:19Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/libs/actions/Report/index.ts	`70.77% <100.00%> (+0.25%)`	⬆️
... and 22 files with indirect coverage changes

melvin-bot · 2026-06-23T10:33:05Z

@grgia Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1afc6a8d8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-06-23T10:38:02Z

-    } else if (Navigation.isActiveRoute(ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID)) && !isDeletedParentAction) {
+    } else if (
+        (Navigation.isActiveRoute(ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID)) ||
+            Navigation.isActiveRoute(ROUTES.SEARCH_REPORT.getRoute({reportID: reportAction.childReportID}))) &&


Handle search thread routes with action IDs

When a thread is opened from a search result that deep-links to a specific action, the active path is search/view/:reportID/:reportActionID (Search builds those routes with reportActionID). Navigation.isActiveRoute() compares cleaned paths exactly, so comparing only SEARCH_REPORT.getRoute({reportID: reportAction.childReportID}) does not match search/view/<child>/<action>, and deleting the parent message leaves that search-opened thread RHP on screen instead of calling goBack().

Useful? React with 👍 / 👎.

WojtekBoman · 2026-06-23T10:38:50Z

It's a followup to #92543
@mkhutornyi it's ready for your review :)

trjExpensify

makes sense 👍

mkhutornyi · 2026-06-24T16:19:53Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android.mov

Android: mWeb Chrome

iOS: HybridApp

ios.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

web.mov

mkhutornyi · 2026-06-24T16:26:15Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0de97ceb7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-06-24T16:32:31Z

        Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(reportID));
-    } else if (Navigation.isActiveRoute(ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID)) && !isDeletedParentAction) {
-        Navigation.goBack(undefined);
+    } else if (!isDeletedParentAction) {


Allow search thread cleanup for deleted parent actions

When the deleted message is the thread parent that this change is trying to handle, isDeletedParentAction is true because deleteReportComment() sets it from ReportActionsUtils.isThreadParentMessage() for actions with visible child replies or whose childReportID matches the current thread report. This guard therefore skips all of the new SEARCH_REPORT route checks, so deleting a parent message from a search-opened thread still never calls Navigation.goBack() and leaves the RHP open in that scenario.

Useful? React with 👍 / 👎.

Thanks for the note, but this is intentional behavior. isDeletedParentAction is true only when the deleted message has visible child replies (childVisibleActionCount > 0), meaning the thread still exists and remains accessible — navigating away in that case would be wrong. The new SEARCH_REPORT checks only need to fire when the thread is fully gone (isDeletedParentAction = false), which is exactly what this code does

MelvinBot · 2026-06-24T16:32:50Z

🤖 Review — the navigation fix is logically correct, but it ships without a test for the new path.

Verdict: The change correctly closes the thread RHP for search-opened threads. One blocking gap (missing test) and one optional cleanup.

1. No unit test for the new search-route behavior 🔴

The only test change is adding the getTopmostSearchReportRouteParams mock that returns undefined (tests/actions/ReportTest.ts:132). Because the isActiveRoute mock defaults to false, no test actually exercises the new SEARCH_REPORT branch — the bug being fixed would not be caught by a regression test. The author checklist claims unit tests were added for the fix; please add a case that makes isActiveRoute return true for a SEARCH_REPORT route and asserts goBack is called (mirroring how the existing goBack assertions are written).

2. `isOnSearchChildReportWithAction` is more convoluted than it needs to be 🟡 (optional)

src/libs/actions/Report/index.ts:3160-3162 builds a route from the current topmost search route's reportActionID and then string-compares it via isActiveRoute. Since the comparison path is built with childReportID, this is equivalent to simply checking that the active search route's reportID is the child report:

const isOnSearchChildReport = Navigation.getTopmostSearchReportRouteParams()?.reportID === reportAction.childReportID;

That single check covers both the search/view/:reportID and search/view/:reportID/:reportActionID cases (it ignores the action segment entirely, which is what you want), and drops isOnSearchChildReportWithAction. Functionally a no-op — just easier to read.

Why the current logic is correct (for the record)

getActiveRouteWithoutParams strips only the ?query string, so the path-level :reportActionID segment is significant in isActiveRoute comparisons. That's why both a with-action and without-action variant are required to match search/view/<child> vs search/view/<child>/<action>.
isOnSearchChildReportWithAction can't produce a false positive because the comparison route is always built with childReportID in the path — it only matches when the active route's reportID already equals childReportID.
Navigation.goBack() is identical to the old Navigation.goBack(undefined), and refactoring !isDeletedParentAction into the else if guard preserves the original behavior.

Next Steps: Reply with @MelvinBot <feedback> to refine this review.

mkhutornyi

Please check AI reviews above. Otherwise LGTM

melvin-bot · 2026-06-24T16:39:23Z

🎯 @mkhutornyi, thanks for reviewing and testing this PR! 🎉

A payment issue will be created for your review once this PR is deployed to production.
E/E issue linked to the PR - no internal issue.

If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created.

mkhutornyi · 2026-06-25T07:01:44Z

tests failing

WojtekBoman · 2026-06-25T07:33:23Z

@mkhutornyi It should be ok now :)

mkhutornyi · 2026-06-25T07:50:14Z

@codex review

MelvinBot · 2026-06-25T07:53:15Z

🤖 Review — both items from my previous review are resolved. LGTM.

Verdict: The refactor and the new tests address the earlier feedback. No blocking issues.

✅ Test coverage added — tests/actions/ReportTest.ts:2540-2592 now exercises the SEARCH_REPORT branch with getTopmostSearchReportRouteParams returning the child reportID (and isActiveRoute left false), asserting goBack fires. Covers both the search/view/<childReportID> and .../<reportActionID> variants — the bug being fixed would now be caught by a regression test.
✅ Logic simplified — src/libs/actions/Report/index.ts:3157-3164 replaced the brittle with-action / without-action isActiveRoute pair with a direct getTopmostSearchReportRouteParams()?.reportID === reportAction.childReportID comparison, which ignores the action segment as intended.

Type-safety note (not a concern)

The comparison getTopmostSearchReportRouteParams()?.reportID === reportAction.childReportID compares string | undefined on both sides. It can't produce a false-positive from undefined === undefined, because the enclosing else if (!isDeletedParentAction && reportAction.childReportID) guard already guarantees childReportID is a defined string — so when search params are absent, the left side is undefined and the comparison is correctly false.

Next Steps: Reply with @MelvinBot <feedback> to refine this review.

mkhutornyi

LGTM

chatgpt-codex-connector · 2026-06-25T07:56:02Z

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 7c58ac9abe

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

MelvinBot · 2026-06-25T07:56:46Z

🤖 Regression scan — no real regression risk found. The change strictly adds a navigation path; the existing one is preserved.

Verdict: Safe. The new isOnSearchChildReport branch can only fire goBack() when the Search thread RHP is the genuinely active screen, so it can't trigger navigation from unrelated screens.

I checked the three ways this could have regressed:

Could getTopmostSearchReportRouteParams() return stale params and fire goBack() from the wrong screen? No. It reads only the topmost root route and bails to undefined unless RIGHT_MODAL_NAVIGATOR is on top (Navigation.ts:1202-1205). It doesn't surface a search route buried under another modal.
deleteReportComment is reachable from any screen (context menu). Confirmed — it's called from PopoverReportActionContextMenu.tsx:407, which fires regardless of where you are. But because of the topmost-RHP guard above, deleting a parent comment from a normal report / list / search results (without the thread open) leaves isOnSearchChildReport === false, so no unexpected goBack().
goBack() vs the old goBack(undefined) — behaviorally identical (Navigation.ts:511-533).

One low-priority edge worth a reviewer's eye

getTopmostSearchReportRouteParams selects the last SEARCH_REPORT within the RHP stack via .reverse().find(...) (Navigation.ts:1207-1208) rather than the strictly-focused nested index. In normal flows the last nested route in an RHP is the focused one, so this holds. It would only matter if multiple SEARCH_REPORT screens could stack within a single RHP instance with a non-focused one matching childReportID — not a scenario this PR introduces, and pre-existing behavior of that helper regardless. No action needed unless you know of such a flow.

Next Steps: Reply with @MelvinBot <feedback> to dig deeper.

github-actions · 2026-06-26T10:58:02Z

🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here.

OSBotify · 2026-06-26T10:59:42Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-06-26T11:05:40Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #94293.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-36535316aa4-27a47d6-06048e2c665b2703fba72549cab52a8fe26622c0/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-3653531-27a47d6-ee4d785f142bddcbfd4fe470126bb5db5e50b773/index.html

Web 🕸️
https://94293.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-06-26T20:00:48Z

🚀 Deployed to staging by https://github.com/grgia in version: 9.4.21-2 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

OSBotify · 2026-06-29T12:37:34Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.21-9 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android

melvin-bot · 2026-06-29T12:38:42Z

🤖 Payment issue created: #94833

mkhutornyi · 2026-07-07T02:40:37Z

-    } else if (Navigation.isActiveRoute(ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID)) && !isDeletedParentAction) {
-        Navigation.goBack(undefined);


@MelvinBot Which PR first introduced this logic? Trying to find offending PR

The else if branch that navigates back when the deleted action's childReportID is the active route was first introduced in #58889 ("fix: header shows skeleton, message is empty" by nkdengineer, merged 2025-03-23, fixing #58134).

Added the branch: ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID) check + goBack (commit eb215dab1aa)

Same PR then tweaked the goBack args: goBack(undefined, {shouldPopToTop: true}) → goBack(undefined) (commit d2cea8ba7aa)

Before that PR the block only had the first if (navigate back from REPORT_WITH_ID.getRoute(reportID, reportActionID)), so #58889 is where the child-report goBack logic originated. This PR (#94293) later refactored that same branch to also handle the SEARCH_REPORT route.

Blame evidence

Blame of the pre-#94293 version of src/libs/actions/Report/index.ts:3157-3158:

eb215dab1aae (nkdengineer 2025-03-21) } else if (Navigation.isActiveRoute(ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID)) && !isDeletedParentAction) { d2cea8ba7aa6 (nkdengineer 2025-03-21) Navigation.goBack(undefined);

Both commits merged via #58889.

melvin-bot Bot assigned WojtekBoman Jun 23, 2026

WojtekBoman marked this pull request as ready for review June 23, 2026 10:32

WojtekBoman requested review from a team as code owners June 23, 2026 10:32

melvin-bot Bot requested review from grgia and trjExpensify and removed request for a team June 23, 2026 10:32

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

WojtekBoman force-pushed the fix/deleting-reply-in-rhp branch 2 times, most recently from 2634fa7 to b0de97c Compare June 23, 2026 12:33

trjExpensify previously approved these changes Jun 24, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jun 24, 2026

View reviewed changes

mkhutornyi approved these changes Jun 24, 2026

View reviewed changes

melvin-bot Bot changed the title ~~Fix thread RHP not closing when parent message deleted from search pages~~ [Payment due @mkhutornyi] Fix thread RHP not closing when parent message deleted from search pages Jun 24, 2026

WojtekBoman dismissed trjExpensify’s stale review via e789567 June 25, 2026 06:48

WojtekBoman force-pushed the fix/deleting-reply-in-rhp branch 2 times, most recently from e789567 to a19e28e Compare June 25, 2026 06:53

Fix thread RHP not closing when parent message deleted from search pages

7c58ac9

WojtekBoman force-pushed the fix/deleting-reply-in-rhp branch from a19e28e to 7c58ac9 Compare June 25, 2026 07:27

mkhutornyi reviewed Jun 25, 2026

View reviewed changes

grgia approved these changes Jun 26, 2026

View reviewed changes

grgia merged commit 3653531 into Expensify:main Jun 26, 2026
34 checks passed

OSBotify mentioned this pull request Jun 26, 2026

Deploy Checklist: New Expensify 2026-06-26 #94730

Closed

55 tasks

melvin-bot Bot mentioned this pull request Jun 29, 2026

[Due for payment 2026-07-06] Payment issue for E/App PR#94293 #94833

Closed

mkhutornyi reviewed Jul 7, 2026

View reviewed changes

mkhutornyi mentioned this pull request Jul 7, 2026

fix: header shows skeleton, message is empty #58889

Merged

50 tasks

		} else if (Navigation.isActiveRoute(ROUTES.REPORT_WITH_ID.getRoute(reportAction.childReportID)) && !isDeletedParentAction) {
		Navigation.goBack(undefined);

Uh oh!

Conversation

WojtekBoman commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Jun 23, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

melvin-bot Bot commented Jun 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

WojtekBoman commented Jun 23, 2026

Uh oh!

trjExpensify left a comment

Choose a reason for hiding this comment

Uh oh!

mkhutornyi commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

mkhutornyi commented Jun 24, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

WojtekBoman Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Jun 24, 2026

1. No unit test for the new search-route behavior 🔴

2. isOnSearchChildReportWithAction is more convoluted than it needs to be 🟡 (optional)

Uh oh!

mkhutornyi left a comment

Choose a reason for hiding this comment

Uh oh!

melvin-bot Bot commented Jun 24, 2026

Uh oh!

mkhutornyi commented Jun 25, 2026

Uh oh!

WojtekBoman commented Jun 25, 2026

Uh oh!

mkhutornyi commented Jun 25, 2026

Uh oh!

MelvinBot commented Jun 25, 2026

Uh oh!

mkhutornyi left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Jun 25, 2026

Uh oh!

MelvinBot commented Jun 25, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jun 26, 2026

Uh oh!

OSBotify commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026

Uh oh!

OSBotify commented Jun 26, 2026

Uh oh!

WojtekBoman commented Jun 23, 2026 •

edited

Loading

codecov Bot commented Jun 23, 2026 •

edited by github-actions Bot

Loading

mkhutornyi commented Jun 24, 2026 •

edited

Loading

2. `isOnSearchChildReportWithAction` is more convoluted than it needs to be 🟡 (optional)