Redirect the expense of a single-expense report to the report view by MelvinBot · Pull Request #99811 · Expensify/App

MelvinBot · 2026-08-29T03:01:08Z

Explanation of Change

This PR adds OneTransactionThreadRedirectHandler, a render-nothing handler mounted in ReportScreen alongside the existing route handlers. When a route lands on the transaction thread of a single-expense report, it replaces the route with the report itself.

It also adds useIsOneTransactionThread hook, a reactive counterpart of ReportUtils.isOneTransactionThread, which reads module globals and so can't drive rendering. It's the same definition HeaderView and SidebarUtils use, so the redirect and the thread's own views agree.

It does not redirect when:

the parent's transactionCount isn't 1 — the action-based check can briefly see a multi-expense report that's still paginating in as a single-expense one
the route carries a reportActionID — that deep link points at a message, and replacing the route would drop its anchor
the parent action is a "send money" action — the report and the thread aren't interchangeable there
a sibling set is active (TRANSACTION_THREAD_NAVIGATION_TRANSACTION_IDS) — the prev/next arrows only exist in a thread's header, so Home "Recently added", "Review N flagged expenses" and the duplicate review list would dead-end mid-review
the screen is blurred

Where it redirects to — the route shape the app itself would have used from where we are:

From	To
The inbox report route	`r/<reportID>`, carrying `referrer`
The Search RHP, with Search as the topmost full screen route	`search/view/<reportID>`
The Search RHP, outside Search	`e/<reportID>`

Back navigation. It replaces (forceReplace) rather than pushes, so Back doesn't return to the thread, and it reuses the route's own backTo.

One case needs more: a thread opened from its own report carries that report as backTo, which would point the report at itself. The handler resolves backTo through getStateFromPath + findFocusedRoute and compares the resulting reportID; when it's the parent, it inherits that route's nested backTo instead. Resolving through the navigator rather than comparing paths matters because the same report is reachable as r/, e/, search/view/ and search/r/, with or without a trailing reportActionID.

Automated coverage. tests/navigation/OneTransactionThreadRedirectHandlerTest.tsx mocks the hooks to pin down the decision table above; tests/navigation/OneTransactionThreadRedirectHandlerOnyxTest.tsx feeds the handler real Onyx data, catching a wrong Onyx key or reportID the mocked suite would pass.

Fixed Issues

$ #99356

Tests

Create 2 expenses (E1, E2) in a report
Open the report and tap the E1 row in the table
Copy the URL from the address bar, then press Back
Tap the E2 row → More → Move to report, and move it elsewhere. B now holds one expense
Paste the copied URL into the address bar and load it
Verify that user lands on the report (header shows the report name), not expense view

Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

…ort view Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-08-29T03:32:33Z

Result: pass — all 6 test steps verified on Web (dev NewDot).

Created a workspace, submitted a €25 "Test Merchant" expense into a single-expense report (R00erFLmAaqR / 8921121751637139), then drove each step. Opening the only expense — from the chat preview or from its own thread entry point — always landed on the report. A message-anchored deep link into that thread was left alone and highlighted the message. After adding a second expense, each expense opened its own thread again.

Step	Status	Observation
1. Submit a single expense to a workspace so it lands in an expense report holding exactly one expense	pass	Submitted €25.00 "Test Merchant"; Inbox shows "Expense Report 2026-08-29" with a "1 expense" badge, Report ID `R00erFLmAaqR` / `8921121751637139`.
2. Open that expense from the chat — verify you land on the expense report, not the standalone expense view	pass	Clicking the money-request preview opened a view whose header, breadcrumb, and Report field all read "Expense Report 2026-08-29"; View Details confirmed Report ID `R00erFLmAaqR`, identical to opening the report directly.
3. Open the expense's own transaction thread URL directly — verify redirect to `/r/<expenseReportID>`	pass	Clicking "1 Reply" (the transaction-thread entry point) landed on the full report view. Network trace showed the pre-redirect route `/r/1823360041683454/8047992094615092167` replaced by `/r/8921121751637139`.
4. Verify the report still shows the expense details and that adding a comment works	pass	Report displayed Amount/Merchant/Category/Date rows; sent "Testing comment on the report" and it appeared in the activity feed immediately.
5. Add a second expense, open one of the two — verify it still opens that expense's own thread (no redirect)	pass	Added "Second Merchant" €15 to the same report (2 expenses, €40, "2 Replies"). Clicking the "Test Merchant" row opened "€25.00 for Test Merchant" with breadcrumb "From Expense Report 2026-08-29" — its own thread.
6. Open a link to a specific message inside the transaction thread of a single-expense report — verify the message is still highlighted (no redirect)	pass	While the report held 1 expense, opened `/r/1823360041683454/8047992094615092167`: the target action was highlighted in yellow with no redirect, matching the `reportActionID` bypass.
7. Verify that no errors appear in the JS console	pass (with caveat)	Console capture is unsupported on the web agent-device platform, so this could not be read directly. No error banners, broken screens, or non-200 API calls were observed across the run.

Evidence (5)

Clicking the single expense from chat opens the expense report view

Transaction thread entry point redirects to the full Expense Report view

Expense report showing expense details plus a newly added comment

With 2 expenses, clicking one opens its own transaction thread (no redirect)

Message deep link highlights the message without redirecting

view run

MelvinBot · 2026-08-29T03:54:53Z

Android: Native — Result: pass — all 6 test steps verified on standalone Android NewDot.

Created a €25.00 "Test Merchant" expense on "Melvin Bot's Workspace" and submitted it into "Expense Report 2026-08-29" (report 6593282062794587, thread 8038705863313887). Both the chat tap and a direct transaction-thread link redirected to the report. A message-anchored deep link into that thread was left alone and highlighted the message. After adding a second expense, each expense opened its own thread again.

Step	Status	Observation
1. Submit a single expense to a workspace so it lands in an expense report holding exactly one expense	pass	FAB → Create expense → Manual, €25.00 "Test Merchant" (Other Business Expenses), then "Mark as done" → "Expense Report 2026-08-29", reportID `6593282062794587`.
2. Open that expense from the chat — verify you land on the expense report, not the standalone expense view	pass	Tapping "View" on the transaction row in "Melvin Bot's expenses" opened header "Expense Report 2026-08-29" with breadcrumb "From Melvin Bot's expenses in Melvin Bot's Workspace" — the report view, not a bare transaction title.
3. Open the expense's own transaction thread URL directly — verify redirect to `/r/<expenseReportID>`	pass	Got the thread reportID `8038705863313887` via "Copy link" on a thread-native message, sent `.../r/8038705863313887` as a chat message and tapped it: landed on "Expense Report 2026-08-29", not "€25.00 for Test Merchant".
4. Verify the report still shows the expense details and that adding a comment works	pass	Redirected report showed Receipt / Amount €25.00 / Description / Merchant / Date / Category rows; sent "Verify comment works on redirected report view" and it appeared immediately.
5. Add a second expense, open one of the two — verify it still opens that expense's own thread (no redirect)	pass	Retracted the report, added €15.00 "Second Merchant" (2 expenses, €40.00). Tapping the first row opened "€25.00 for Test Merchant"; the second opened "€15.00 for Second Merchant" — neither redirected.
6. Open a link to a specific message inside the transaction thread of a single-expense report — verify the message is still highlighted (no redirect)	pass	Sent `.../r/8038705863313887/1172905027780974765` and tapped it: opened the standalone thread (breadcrumb "From Expense Report 2026-08-29") with the target message highlighted in yellow — the `reportActionID` bypass held.
7. Verify that no errors appear in the JS console	pass	`adb logcat` ReactNativeJS review showed AddComment, RequestMoney, ReopenReport, OpenReport, and ReadNewestAction all returning `jsonCode:200`. No FATAL/AndroidRuntime crashes or unhandled JS exceptions.

Evidence (7)

Single-expense report view after submission

Tapping expense from chat lands on Expense Report view

Transaction thread root URL redirects to Expense Report view

Report shows expense details plus a working added comment

First expense in a 2-expense report opens its own transaction thread

Second expense in a 2-expense report opens its own transaction thread

Message-specific thread link opens thread and highlights message, no redirect

view run

Evidence (1)

Recordings

- [▶ Session recording of the verification run](https://melvinbot-assets.exops.io/session-33230351756-1-e7d63f40-0919-4bed-a567-d2c798ba13b6.mp4)

aimane-chnaif · 2026-09-07T01:53:40Z

@MelvinBot pull main and fix Oxfmt check

…ExpenseReportThread Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-09-07T02:05:27Z

⚠️ MelvinBot CI Fix Loop Detected

MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR.

Next steps: A human should review the recent commits and resolve the conflicting CI errors manually.

MelvinBot · 2026-09-07T02:06:51Z

Done — merged main and fixed the Oxfmt check. It now passes on CI (run).

What was wrong: two files weren't oxfmt-formatted. Running npm run fmt collapsed a multi-line boolean expression in src/pages/inbox/OneTransactionThreadRedirectHandler.tsx:57 and, in tests/navigation/OneTransactionThreadRedirectHandlerTest.tsx, reordered an import and switched a double-quoted test name to single quotes. Formatting only — no behavior change.

Commits: f828c40 (merge main) and 4101fdf (npm run fmt).

AI tests run locally

Check	Result
`npm run fmt` + `git diff --exit-code` (exact CI step)	✅ clean
`npm run lint-changed`	✅ pass
`npm run typecheck`	✅ pass
`npm test tests/navigation/OneTransactionThreadRedirectHandlerTest.tsx`	✅ 8/8 pass
`npm run react-compiler-compliance-check check-changed`	⚠️ couldn't run locally (needs `GITHUB_BASE_REF`) — passing on CI

The full npm test suite and Storybook smoke test were left to CI.

The only other failing check is Check independent approval, which needs a human reviewer — not something I can resolve.

aimane-chnaif · 2026-09-07T02:07:18Z

@codex review

aimane-chnaif · 2026-09-07T02:07:22Z

@MelvinBot review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4101fdf2aa

ℹ️ 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-09-07T02:13:49Z

+        const backTo = route.params?.backTo;
+        const reportRoute =
+            route.name === SCREENS.RIGHT_MODAL.SEARCH_REPORT
+                ? ROUTES.SEARCH_REPORT.getRoute({reportID: parentReportID, backTo})
+                : ROUTES.REPORT_WITH_ID.getRoute(parentReportID, undefined, undefined, backTo);


Avoid replacing the thread with a duplicate parent route

When the thread is opened from its parent report, getReportRouteForCurrentContext() sets backTo to that parent and pushes the child thread. Reusing that value here and force-replacing the child produces a stack such as parent -> parent?backTo=parent, so the first Back action appears to do nothing and the replacement route has a self-referential fallback. Detect when backTo already targets parentReportID and return to the existing parent instead of replacing the child with another copy.

Useful? React with 👍 / 👎.

Fixed in 2dd1e9e. When backTo already targets parentReportID, the handler now calls Navigation.goBack(backTo) to pop onto the parent that is already in the stack, instead of force-replacing the thread with a second copy of it. Covered by three new tests: inbox, Search RHP, and a backTo that points at a different report (which still takes the replace path).

MelvinBot · 2026-09-07T02:14:10Z

Review

The approach is sound and it does solve #99356 — a route-level backstop is the right shape for "deep link / push notification / stale history" entry points that no call site controls. All checks are green except Check independent approval. I would not merge yet: one duplication issue is worth fixing before this lands, and the PR template is still unfilled.

1. This is the third definition of "one-transaction thread" — reuse the existing one

isOneTransactionThread already computes exactly the condition hand-rolled at OneTransactionThreadRedirectHandler.tsx:57 — report.reportID === oneTransactionThreadReportID && !isSentMoneyReportAction(parentAction) — and getDisplayedReportID is already "the parent report ID if this is a one-transaction thread".

The new copy also diverges: it adds a transactionCount === 1 gate the shared helper doesn't have. So a thread can be a one-transaction thread for HeaderView and SidebarUtils but not for the redirect. Suggest a useIsOneTransactionThread / useDisplayedReportID hook wrapping the shared util (the util reads Onyx via module globals, which is presumably why it wasn't reused — say so in a comment if you keep the copy).

2. The `transactionCount` gate and the view use different predicates

The gate is isOneTransactionReport (report.transactionCount === 1, the server counter), but the thing that makes the redirect correct — shouldDisplayReportTableView — uses the actual transactions array. When they disagree you either redirect onto a report that renders the table view, or fail to redirect at all. I did not verify how often they diverge in practice; worth a thought since the counter is maintained optimistically in the IOU builders.

3. The redirect fires after the thread has already mounted and fetched

The effect runs post-paint and then awaits Navigation.isNavigationReady(). On a cold deep link — the exact case this PR targets — ReportScreen for the thread mounts, ReportFetchHandler fires openReport for it, it renders, and only then does the route get replaced. Expect a visible flash plus a wasted round trip. Please attach a screen recording of a cold deep link so we can judge whether it's acceptable; if it is noticeable, resolving at the link/route layer (as the Search call sites do) avoids both.

4. The queued navigation isn't cancelled

At OneTransactionThreadRedirectHandler.tsx:78, if the user navigates away between the effect and the promise resolving, the forceReplace still runs and replaces whatever is on top. Add an unmounted/blurred guard in the .then().

5. The tests mock away the logic that decides the behavior

useOnyx, useOneTransactionThreadReportID, and useParentReportAction are all mocked, so the suite verifies the boolean expression but not the wiring — it would still pass if the wrong reportID were passed to useOneTransactionThreadReportID or the Onyx key were wrong. One test with real Onyx data covering the actual one-transaction derivation would be worth more than the eight current cases. Nothing covers the redirectedFromReportIDRef guard either.

6. Not ready to mark as ready for review

Tests, Offline tests, and QA Steps are all still // TODO: **The human co-author** must fill out…, the author checklist is entirely unchecked, PROPOSAL: is empty, and there are no screenshots for any platform.

Things I checked that look right

No redirect loop. After the replace, the handler re-runs on the expense report, whose parent is the chat report. getOneTransactionThreadReportAction early-returns for a chat-type report, so oneTransactionThreadReportID is undefined and nothing fires again.
Send money is correctly excluded, and the check is load-bearing. getOneTransactionThreadReportAction returns the send-money action, so oneTransactionThreadReportID does match the thread — only the isSentMoneyReportAction guard stops the redirect. Don't let anyone "simplify" that away.
Route shapes. The tests exercise the real ROUTES module, so r/<id>?backTo=… and the Search RHP search/view/<id> forms are genuinely verified, including the backTo argument position in REPORT_WITH_ID.getRoute.
Selector stability. isOneTransactionReport is a module-level reference, so the useOnyx selector is stable — no re-subscribe churn.
shouldDeferNonEssentials only defers during the dismiss_modal_and_open_report transition, so gating the handler behind it is not a meaningful delay.

Scope of this review

Static review of the diff against main at 4101fdf. I did not exercise the flow in a running app, so the flash in point 3 and the divergence in point 2 are reasoned from the code, not observed.

aimane-chnaif · 2026-09-07T02:33:43Z

@MelvinBot address #99811 (comment) and #99811 (comment).

…cate parent - Add useIsOneTransactionThread, the reactive counterpart of ReportUtils.isOneTransactionThread, and use it in the redirect handler instead of a third hand-rolled copy of the condition. - Go back to the parent report when backTo already points at it, rather than force-replacing the thread with parent?backTo=parent. - Add a real-Onyx test suite covering the derivation the mocked suite mocks away, plus cases for the new backTo behaviour and the redirect-once guard. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-09-07T03:05:44Z

Addressed the Codex comment and points 1 and 5 of the review in 2dd1e9e. Skipped the edge cases as asked. All checks are green except Check independent approval.

Codex: duplicate parent route

Fixed. When backTo already targets the parent, the handler now pops onto the parent that is already in the stack instead of force-replacing the thread with a second copy of it — no more parent -> parent?backTo=parent. See OneTransactionThreadRedirectHandler.tsx:28-36 and :85-94. Covered by three new cases (inbox, Search RHP, and a backTo pointing somewhere else).

1. Third definition of "one-transaction thread"

Fixed. Added useIsOneTransactionThread, the reactive counterpart of ReportUtils.isOneTransactionThread, and the handler now calls it instead of hand-rolling the condition. Both land on the same getOneTransactionThreadReportID derivation and both exclude send money. The hook's docblock says why the util itself can't be called from a component — it resolves the parent report, its actions and the chat report through module globals, so it never re-runs.

The transactionCount gate stays in the handler, not the shared hook, and now says why: it is a redirect-only guard, because navigating on a half-paginated report is the part you can't take back.

5. Tests mocked away the logic

Fixed. Added OneTransactionThreadRedirectHandlerOnyxTest.tsx — same handler, real Onyx data, no hook mocks. I checked it has teeth: changing the hook to pass the thread's own reportID to useOneTransactionThreadReportID instead of the parent's fails the new suite and passes the mocked one, which is exactly the blind spot you called out. Also added a case for the redirectedFromReportIDRef guard. 15 tests across the two files.

Skipped

2 (counter vs shouldDisplayReportTableView) and 4 (queued navigation not cancelled) — per your instruction to skip edge cases. Both are still open questions on this PR.
3 (flash on cold deep link) — I tried to record it, but the web test session came up blank (0 accessibility nodes, unresponsive to every probe) and never reached Home, so I have no observation to report. The dev server did compile the changed files cleanly. Still unverified.
6 (PR template) — Tests, Offline tests, QA Steps, the author checklist and screenshots are for the human co-author to fill in; I won't fill those on someone's behalf.

Local checks

Check	Result
`npm run fmt` + `git diff --exit-code`	✅ clean
`npm run lint-changed`	✅ pass
`npm run typecheck`	✅ pass
`npm run spell-changed`	✅ pass
`npm run react-compiler-compliance-check check <changed files>`	✅ both files compile (babel + oxc)
`npm test tests/navigation/OneTransactionThreadRedirectHandler*`	✅ 15/15 pass

aimane-chnaif · 2026-09-07T03:07:05Z

@codex review

aimane-chnaif · 2026-09-07T03:07:10Z

@MelvinBot review

aimane-chnaif · 2026-09-07T03:11:30Z

@MelvinBot Findings below, roughly in order of how much they matter. Only address valid bugs. Ignore nits/edge cases.

1. The redirect drops `referrer`, which breaks mark-as-read for notification-opened reports

navigateFromNotification opens /r/<reportID>?referrer=notification, and a push notification for a comment on an expense targets the transaction thread — exactly the route this handler rewrites. The rebuilt route drops the param:

ROUTES.REPORT_WITH_ID.getRoute(parentReportID, undefined, undefined, backTo)
//                                              ^^^^^^^^^  referrer

referrer is not decorative. useMarkAsRead reads it:

const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION;
const shouldReadOnReportChange = ((isVisible && Visibility.hasFocus()) || isFromNotification) && !hasNewerActions && isScrolledToEnd;

So opening a notification for a single-expense report and landing on the redirected route can skip the auto-mark-as-read that the un-redirected route would have done, because the isVisible && hasFocus() half is what's left. MoneyRequestReportActionsList reads the same param.

Forwarding it is a one-liner:

-const reportRoute =
-    route.name === SCREENS.RIGHT_MODAL.SEARCH_REPORT
-        ? ROUTES.SEARCH_REPORT.getRoute({reportID: parentReportID, backTo})
-        : ROUTES.REPORT_WITH_ID.getRoute(parentReportID, undefined, undefined, backTo);
+const reportRoute =
+    route.name === SCREENS.RIGHT_MODAL.SEARCH_REPORT
+        ? ROUTES.SEARCH_REPORT.getRoute({reportID: parentReportID, backTo})
+        // The route we replace may have been opened from a notification, and `referrer` is what tells
+        // `useMarkAsRead` to mark it read without waiting on window focus. Carry it across.
+        : ROUTES.REPORT_WITH_ID.getRoute(parentReportID, undefined, route.params?.referrer, backTo);

(referrer only exists on the inbox param list, so the SEARCH_REPORT branch is unaffected.) Worth a test alongside the existing backTo one.

2. Gate the heavy Onyx read behind the cheap counter

useIsOneTransactionThread runs for every report screen that is a thread — plain comment threads in busy chats included — and through useOneTransactionThreadReportID it subscribes to the parent's entire reportActions_ member plus the parent's chat report, re-running the selector on every new message there. getOneTransactionThreadReportAction bails immediately when the parent isn't IOU/EXPENSE/INVOICE, so the derivation itself is cheap, but the subscription and the per-message selector churn are not.

isParentOneTransactionReport is already computed from a single field and is a strict precondition of the result, so it can short-circuit the expensive one:

 const [isParentOneTransactionReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`, {selector: isOneTransactionReport});
-const isOneTransactionThread = useIsOneTransactionThread(report);
+// Only pay for the parent's action-list derivation once the server's own counter says it can matter.
+// Passing `undefined` keeps the hook call unconditional while leaving its subscriptions inert.
+const isOneTransactionThread = useIsOneTransactionThread(isParentOneTransactionReport ? report : undefined);

Same result, no subscription to a chat's full action list from every thread the user opens. (Ordering in shouldRedirectToParentReport can stay as-is.)

3. `isBackToParentReport`'s premise is narrower than the comment claims

backTo is captured from the active route when the thread is opened (see getReportRouteForCurrentContext), so it holds the parent report whenever the thread was opened from that parent — which is the common case.

The main call site that opens a transaction thread, ChatTransactionPreview, goes through getReportRouteForCurrentContext from within the chat, so backTo is /r/<chatReportID> — the thread's parent is the expense report, so the check is false. The other way a thread gets opened from its own parent is the report table view, which only renders for multi-expense reports, and those never reach this code. So this branch is close to unreachable in the flows the redirect actually fires on.

Not asking to remove it — it's harmless and correct where it does apply — but the comment should say so, because as written it implies the forceReplace path is the rare one when it's the opposite. Worth noting too that the [parent, parent] stack it exists to prevent still happens whenever the previous entry is the parent and backTo isn't set, so this isn't a complete fix for that either.

4. `Navigation.isNavigationReady().then(...)` is redundant, and slightly worse than calling directly

Navigation.navigate already handles a not-yet-ready container itself:

if (!canNavigate('navigate', {route})) {
    if (!navigationRef.isReady()) {
        pendingNavigationCall = {route, options};
    }
    return;
}

The wrapper adds a microtask gap after the isFocused check and after the ref is set, so a fast navigation in between can be clobbered by a redirect that already decided it was safe. Dropping the wrapper in both branches is simpler and closes that window.

5. Two defensive guards (neither is a live bug today)

Both are cheap, and both protect against useRoute() not meaning what the handler assumes:

Side panel. SidePanelReport renders an entire ReportScreen against a synthetic NavigationRouteContext of {name: SCREENS.REPORT, params: {reportID}}, and useIsFocused() there reflects the host screen. A report shown in the panel is a passive view, not a destination, so a redirect from it would move the main stack. Today SidePanelContextProvider only ever feeds it the admins room or Concierge, so it can't fire — but nothing in the panel's contract says that stays true.

+const isInSidePanel = useIsInSidePanel();
 ...
-if (!isFocused || !shouldRedirectToParentReport || redirectedFromReportIDRef.current === reportIDFromRoute) {
+if (isInSidePanel || !isFocused || !shouldRedirectToParentReport || redirectedFromReportIDRef.current === reportIDFromRoute) {

Route allowlist. RHPReportScreen also backs SCREENS.RIGHT_MODAL.AGENT_REPORT, which falls into the else of the SEARCH_REPORT ternary and would be rewritten to an inbox r/<id> route, breaking out of the settings RHP. An agent chat is never a transaction thread so it can't trigger, but the ternary reads as if only two routes exist:

+const isSearchReportRoute = route.name === SCREENS.RIGHT_MODAL.SEARCH_REPORT;
+if (!isSearchReportRoute && route.name !== SCREENS.REPORT) {
+    return;
+}

6. Test fixture nit

OneTransactionThreadRedirectHandlerOnyxTest seeds the thread as type: CONST.REPORT.TYPE.EXPENSE, but a real transaction thread is a chat with a parentReportActionID. It doesn't change any outcome here (nothing reads the thread's own type), so this is representativeness rather than a bug — but the Onyx suite's stated value is catching what the mocked suite can't, and a fixture that couldn't exist in production undercuts that a little.

Otherwise the two suites are a good split, and the pagination case in particular is the right thing to have pinned down.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2dd1e9e2ff

ℹ️ 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-09-07T03:12:33Z

+
+    // The active route is captured with a leading slash and may carry query params of its own.
+    const backToPath = backTo.replace(/^\//, '').replace(/\?.*$/, '');
+    return backToPath === ROUTES.REPORT_WITH_ID.getRoute(parentReportID) || backToPath === ROUTES.SEARCH_REPORT.getRoute({reportID: parentReportID});


Recognize anchored parent report routes

When the expense is opened from a parent URL containing a reportActionID (for example r/<parent>/<action> or search/view/<parent>/<action>), getReportRouteForCurrentContext() stores that full URL in backTo, but this equality check only accepts the unanchored route. The handler therefore force-replaces the thread with another parent screen instead of popping to the parent already in the stack, so Back returns to a second view of the same report. Fresh evidence beyond the prior comment is that both route definitions explicitly support the optional :reportActionID? suffix (src/ROUTES.ts), and ChatTransactionPreview opens the child through the context helper while such an anchored parent can be active.

Useful? React with 👍 / 👎.

Skipping this one as an edge case, on aimane-chnaif's instruction.

The mechanism is real — both route definitions do take the optional :reportActionID? suffix, and getReportRouteForCurrentContext stores Navigation.getActiveRoute() verbatim. But it only bites when the parent report was itself opened at a message anchor and the expense is then opened from there, which is narrow enough to leave alone for now.

I did widen isBackToParentReport in 1868bf9 for the unanchored cases it was genuinely missing — search/r/<id> and e/<id> are both the parent expense report and both used to fall through to the stacking branch.

MelvinBot · 2026-09-07T03:14:37Z

Code review

The approach is sound — a route-level backstop is the right shape for this, the transactionCount gate is a good guard against the paginating-in race, and the goBack branch that avoids stacking parent -> parent?backTo=parent shows real care. Both test files pass locally (15/15). Below are the things I'd want addressed before merge, ranked.

1. `AGENT_REPORT` also mounts `ReportScreen`, and it falls through to the inbox route

RHPReportScreen is registered for two RHP screens, not one — RightModalNavigator.tsx:516-523 registers SCREENS.RIGHT_MODAL.AGENT_REPORT with the same loadRHPReportScreen. The handler only special-cases SEARCH_REPORT at OneTransactionThreadRedirectHandler.tsx:96-99, so an agent RHP that lands on a one-transaction thread would forceReplace onto r/<parentReportID> and eject the user out of the RHP into the inbox.

ReportScreenRoute doesn't include that screen either, so route.params?.backTo / reportActionID are typed as present when AGENT_REPORT's param list has neither.

Suggest an explicit allowlist: bail out unless route.name is SCREENS.REPORT or SCREENS.RIGHT_MODAL.SEARCH_REPORT. That's also future-proof against the next screen that reuses ReportScreen.

2. `isBackToParentReport` misses two other shapes of "the parent report"

isBackToParentReport only recognises r/<id> and search/view/<id>. backTo comes from Navigation.getActiveRoute(), which can also produce search/r/<id> (SEARCH_MONEY_REQUEST_REPORT) or e/<id> (EXPENSE_REPORT_RHP) — both are the parent expense report. Opening a thread from either of those leaves you in the navigate branch and stacks search/r/parent -> r/parent, which is the exact duplicate the goBack branch exists to prevent.

3. The one-shot ref latches even when the navigation never happens

redirectedFromReportIDRef.current is set at line 79, before the deferred Navigation.isNavigationReady().then(...). Both Navigation.navigate and Navigation.goBack start with a canNavigate(...) guard that can silently bail. If it does, the effect will never retry for that reportID and the user is stranded on the thread with no recovery. Set the ref inside the .then() callback instead.

4. Focus isn't re-checked after the async hop

isFocused is read at effect time, but the actual navigate runs a microtask (or more) later inside isNavigationReady().then(...). If the screen blurred in between, forceReplace acts on whatever is topmost. A ref-backed focus re-check inside the callback is cheap insurance.

5. `ReportFetchHandler` still fetches the thread you're about to leave

OneTransactionThreadRedirectHandler is mounted before ReportFetchHandler, but it defers its navigation to a promise while ReportFetchHandler calls openReport synchronously in its effect. So every redirected entry costs one wasted OpenReport on the transaction thread. Not a blocker, but worth knowing given deep links / push notifications are the target use case.

6. Perf: new Onyx subscriptions on the hottest screen

useIsOneTransactionThread adds subscriptions to the parent report, the parent's report actions (twice — once via useOneTransactionThreadReportID, once via useParentReportAction), and the grandparent chat report, plus the parent-report subscription in the handler itself. getOneTransactionThreadReportID iterates every parent action on each change. On a large report that selector re-runs on every action update, for every mounted ReportScreen. Worth a sanity check with @frontend-performance given this is unconditional on ReportScreen.

7. Doc comment in `useIsOneTransactionThread` is misleading

useIsOneTransactionThread.ts:17 says the shared derivation excludes send money. It doesn't — getOneTransactionThreadReportAction returns the send-money action early. The exclusion comes only from the explicit !isSentMoneyReportAction(...) guard in the hook. As written, a future reader could delete that guard as redundant and silently break send money.

Tests, conventions, and smaller notes

Tests — 15/15 pass locally. The split between a mocked decision-table suite and an Onyx-backed suite is a good pattern, and the paginating-in case is covered in both. Gaps:

No case for an RHP route name other than SEARCH_REPORT (finding 1).
No direct test for useIsOneTransactionThread; it's only exercised through the handler.
'goes back to the parent report in the search RHP when backTo already points at it' asserts only toHaveBeenCalledTimes(1), not the argument — the sibling inbox test does assert it.
Mocking @hooks/useOnyx wholesale returns a value for any key, so the mocked suite would keep passing if the handler started reading a third key. The Onyx-backed suite mostly covers for that.
The file-level eslint-disable lines could be narrowed to the mock factories.

Conventions — no manual useMemo/useCallback/memo, correct for React Compiler; the displayName and render-nothing handler pattern matches the neighbouring handlers in ReportScreen; getNonEmptyStringOnyxID is used consistently.

Correctness I checked and am happy with — no redirect loop: the parent expense report's own parent is a CHAT, and getOneTransactionThreadReportAction early-returns for non-IOU/EXPENSE/INVOICE types, so the handler can't fire again on the destination. Self-DM / track expenses are excluded for the same reason. ROUTES.REPORT_WITH_ID.getRoute(id) and ROUTES.SEARCH_REPORT.getRoute({reportID}) both return bare paths with no query string, so the string comparison in isBackToParentReport is valid. useIsOneTransactionThread is a faithful reactive mirror of ReportUtils.isOneTransactionThread.

Not verified — I reviewed code only; I didn't run the app, so the actual back-navigation feel and any thread-view flash before the redirect still need manual testing.

PR body

The Tests, Offline tests, and QA Steps sections still contain the // TODO: **The human co-author** must fill out... placeholders. Those need filling in before this goes ready for review.

…ent action read Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-09-07T03:32:13Z

Fixed #1 and #2 in c51a9f792f5. Skipped #3–#6, and #4's proposal as written would introduce a bug — details below.

Fixed

1. Dropped referrer — valid bug. Confirmed: ROUTES.ts:2503 takes referrer as the third arg, navigateFromNotification/index.ts:7 sets it, and useMarkAsRead.ts:117-118 is the only thing that lets an unfocused window mark the report read. Now forwarded.

I hoisted it to a const in the component body rather than reading route.params?.referrer inline: referrer only exists on the inbox param list, so on the ReportScreenRoute union it needs the route.name === SCREENS.REPORT narrowing, and the effect's dep array can't carry that expression. Added two tests — referrer alone, and referrer + backTo together (that second one pins the & vs ? join in getUrlWithBackToParam).

2. Gating the parent action read — taking it as a perf fix. Not a bug, but the reasoning holds and the change is free: useOneTransactionThreadReportID.tsx:12-14 subscribes to reportActions_<parentReportID> with a selector, so every plain comment thread was re-deriving on each new chat message. isOneTransactionReport is just transactionCount === 1, and it's already an && term in shouldRedirectToParentReport, so gating on it is semantically identical. Reordered the two hooks and applied it.

Skipped

4. Not a simplification — dropping the wrapper breaks the goBack branch. You're right that Navigation.navigate queues into pendingNavigationCall when the ref isn't ready, so the wrapper is redundant there. But goBack has no such queue — it returns on !canNavigate and drops the call. Since redirectedFromReportIDRef is already set by then, that redirect would never retry: the user would be stranded on the thread. So "drop it in both branches" trades a microtask race for a silently-lost redirect. Left both as-is for symmetry rather than splitting the two branches over a race I can't demonstrate.

3. Your reading checks out — getReportRouteForCurrentContext.ts:17 captures Navigation.getActiveRoute(), so from ChatTransactionPreview backTo is the chat and the parent is the expense report, making the check false. But it's a comment-accuracy point, and you said to skip nits.

5, 6. Both self-described as not live bugs (5) and representativeness rather than a bug (6).

Verification

typecheck, lint-changed, react-compiler-compliance-check, and spell-changed all pass. Both suites pass: 17 tests, up from 15.

I could not smoke-test the redirect in the browser — the prestarted web session never got past the account-creation "Join" screen and the page went blank (0 DOM nodes, no network traffic) for the whole run, with the dev server itself responding 200. So the perf gate in #2 is covered by the unit suites only, including the real-Onyx one, and not by a live flow.

aimane-chnaif · 2026-09-07T03:35:22Z

@MelvinBot address only valid items in #99811 (comment), #99811 (comment). Ignore edge cases

melvin-bot · 2026-09-13T09:44:04Z

@aimane-chnaif 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]

aimane-chnaif · 2026-09-13T09:45:42Z

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

melvin-bot · 2026-09-13T09:46:03Z

We did not find an internal engineer to review this PR, trying to assign a random engineer to #99356 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 53750646c9

ℹ️ 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-09-13T09:47:40Z

+
+        // Replacing rather than pushing keeps the thread we are leaving out of the history, so going back doesn't
+        // land on it again.
+        Navigation.navigate(getExpenseReportRoute({routeName: route.name, reportID: parentReportID, referrer, backTo: resolvedBackTo}), {forceReplace: true});


Pop to the existing parent instead of replacing the thread

When the thread was opened from its own parent report, that parent route is already immediately beneath it—the ChatTransactionPreview flow navigates through getReportRouteForCurrentContext(), which records the active parent in backTo and pushes the child. Stripping the self-referencing backTo and then force-replacing the child still leaves a parent -> parent stack, so the first Back action appears to do nothing. Fresh evidence beyond the prior comment is that the current implementation no longer calls Navigation.goBack() when isBackToParentReport is true; it unconditionally executes this replacement.

Useful? React with 👍 / 👎.

In a workspace chat, click the report preview card for a report holding two expenses. It opens in the wide right-hand pane.

Click the first expense. It opens on its own, still in the pane.

From a second session, open the second expense → More → Move to report, and move it elsewhere.

The pane switches by itself to the expense report — the feature working.

Need to press Back twice to actually leave.

This also happens on production so out of scope

MelvinBot · 2026-09-13T09:52:28Z

PR body set to your content and the PR is now ready for review.

🤖 Android test steps verification results: ✅ pass

Created a 2-expense workspace report, confirmed E1 opened its own transaction thread while the report had 2 expenses, then after moving E2 elsewhere, navigating to E1's expense view redirected to the parent report (header shows report name) instead of the transaction thread, with no JS console errors.

Step	Status
VERIFY 1: Create 2 expenses (E1, E2) in a report Created workspace "Tester Tester's Workspace" and a draft "Expense Report 2026-09-13" via Create report, then added two manual expenses: E1 Merchant $10.00 and E2 Merchant $20.00. Report table view showed both rows with Total $30.00.	✅
VERIFY 2/3: Tap the E1 row in the report table, note the resulting expense view/route, then go Back to the report Tapping the E1 row navigated to a standalone transaction-thread screen with header "$10.00 for E1 Merchant" (distinct from the report screen). Pressing Back returned to the report table showing both expenses again.	✅
VERIFY 4: Tap the E2 row → More → Move to report, moving E2 elsewhere so the report holds one expense Opened E2's transaction thread, tapped More > Move to report > Create report, which moved E2 into a brand-new draft report. The original report screen then auto-rendered E1 inline (single-transaction report view) with disabled prev/next arrows.	✅
VERIFY 5/6: Navigate to the E1 route recorded in step 3 and confirm it lands on the report (header = report name), not the expense/transaction-thread view Since the report table no longer exposes a per-row link once single-transaction (renders inline), re-navigated via Search > Expenses tab and tapped the same E1 Merchant $10.00 entry - the same underlying transaction-thread route previously reached in step 2. Landed on the report screen with header/back-button title reading "Expense Report 2026-09-13" (report name), not a transaction-thread header like "$10.00 for E1 Merchant". Reproduced twice for consistency; read-back via `snapshot -i` confirmed the header node text as "Expense Report 2026-09-13" both times.	✅
VERIFY 7: No errors appear in the JS console Captured app.log via `agent-device logs start/stop` spanning the whole session. Grepped for error/exception/FATAL/Unhandled/TypeError/ReferenceError/console.error/redbox - no genuine JS exceptions or crash traces found. All "errors:" matches were Onyx network-middleware debug lines logging a request's predefined optimistic failureData (with `response: undefined`, i.e. never actually applied), not real failures.	✅

view run · no recording available

aimane-chnaif · 2026-09-13T14:58:49Z

@MelvinBot pull main

…ExpenseReportThread Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

melvin-bot · 2026-09-13T15:09:53Z

@aimane-chnaif 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]

MelvinBot · 2026-09-13T15:10:21Z

Merged main into the branch and pushed as 3ffdfd2. No conflicts — the branch was 41 commits behind; GitHub now reports it MERGEABLE.

Re-ran the checks on the merged tree, all green:

OneTransactionThreadRedirectHandlerTest.tsx — 20/20
OneTransactionThreadRedirectHandlerOnyxTest.tsx — 3/3
NavigateTests.tsx — 14/14 (main touched this file, so worth confirming)
eslint, npm run typecheck, react-compiler-compliance-check, cspell

The PR still shows BLOCKED, same as before the merge — that's the review/checklist gating, not the merge state.

view run · no recording available

blimpich

Looks good but way too many verbose comments. Please keep those to a minimum

aimane-chnaif · 2026-09-14T20:04:30Z

@MelvinBot address #99811 (review)

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot · 2026-09-14T20:15:55Z

Trimmed the comments in 0c33312 — 46 lines removed, 13 added, all of them comment lines (no logic touched).

What I kept is the handful of "why" notes that aren't recoverable from the code: the transaction-count gate existing because a paginating report looks single-expense to the action-derived check, the undefined argument keeping the hook inert, the sibling-set and reportActionID bailouts, and the self-referencing backTo. Everything else — restatements of the code, the long rationale paragraphs, and the test comments that just repeated their it(...) titles — is gone.

Checks

npm test on both handler suites: 23 passed
npm run lint-changed: clean
npm run spell-changed: 0 issues

No typecheck run — every changed line is a comment, verified with git diff.

github-actions · 2026-09-14T21:36:27Z

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

OSBotify · 2026-09-14T21:39:50Z

✋ 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-09-14T21:40:10Z

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

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-cdcb30d-067d20d-ad4294a655742939f5d568b7bb94823c9a70b6a8/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-cdcb30d-067d20d-4dccfa43a839a15fee09635070195ccef1a2c453/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-15T01:27:31Z

🚀 Deployed to staging by https://github.com/blimpich in version: 9.4.78-0 🚀

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

MelvinBot · 2026-09-15T01:34:19Z

No help site changes are required, so I did not create a docs PR.

This PR changes which route the app lands on — the transaction thread of a single-expense report is replaced with the report itself. Nothing about it changes a feature name, a tab, a setting, a button label, or the steps a member follows. No article under docs/articles documents the standalone expense view as a distinct destination, so there is nothing to correct.

What I checked

Searched docs/articles for anything this could invalidate. Nothing in new-expensify/ describes the standalone expense view, the transaction thread, or a deep link to an expense. The expense view / report view phrases appear only in unrelated contexts (Accounting-Search-Shortcuts.md, keyword front matter in Using-Reports-in-New-Expensify.md).

The one flow worth verifying was Edit-Expenses.md:22-27 — "go to Spend > Expenses, select the expense to open it, select the field you want to edit." When that expense is the only one on its report, this PR now lands the member on the report instead. The same concern applies to Understanding-Report-Statuses-and-Actions.md:86 — "Open the expense, then update the Report field."

I ran this on the PR branch on web rather than assume it. Both documented flows still work unchanged: clicking the expense row opens the report, and Merchant, Category, Amount, and Report are all present and editable there.

Report view opened after clicking the expense row

Clicking Merchant opens its edit page as documented:

Merchant edit page opened from the report view

No console errors during the run.

Since no docs PR was needed, there is no linked help site PR to review. If you'd rather I add a note to one of those two articles anyway, say the word and I'll open the draft PR.

MelvinBot and others added 3 commits August 29, 2026 02:46

Redirect the transaction thread of a single-expense report to the rep…

80e8add

…ort view Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

Add tests for the single-expense report redirect

1b34ffc

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

Gate the redirect on the report transaction count

4343587

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot assigned aimane-chnaif Aug 29, 2026

MelvinBot requested a review from a team August 29, 2026 03:01

melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Aug 29, 2026

MelvinBot mentioned this pull request Aug 29, 2026

Navigating to the expense of a single-expense report should redirect to the single-expense report view #99356

Open

MelvinBot and others added 2 commits September 7, 2026 02:00

Merge remote-tracking branch 'origin/main' into claude-redirectSingle…

f828c40

…ExpenseReportThread Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

Run npm run fmt to fix Oxfmt check

4101fdf

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

chatgpt-codex-connector Bot reviewed Sep 7, 2026

View reviewed changes

Carry the notification referrer through the redirect and gate the par…

c51a9f7

…ent action read Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

melvin-bot Bot removed request for a team September 13, 2026 09:44

aimane-chnaif approved these changes Sep 13, 2026

View reviewed changes

melvin-bot Bot requested a review from blimpich September 13, 2026 09:46

chatgpt-codex-connector Bot reviewed Sep 13, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into claude-redirectSingle…

3ffdfd2

…ExpenseReportThread Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

MelvinBot requested a review from a team as a code owner September 13, 2026 15:09

melvin-bot Bot requested review from aimane-chnaif and removed request for a team September 13, 2026 15:09

aimane-chnaif approved these changes Sep 13, 2026

View reviewed changes

blimpich requested changes Sep 14, 2026

View reviewed changes

Trim verbose comments to the non-obvious whys

0c33312

Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>

blimpich approved these changes Sep 14, 2026

View reviewed changes

blimpich merged commit cdcb30d into main Sep 14, 2026
42 of 43 checks passed

blimpich deleted the claude-redirectSingleExpenseReportThread branch September 14, 2026 21:35

OSBotify mentioned this pull request Sep 15, 2026

Deploy Checklist: New Expensify 2026-09-15 #101132

Open

60 tasks

Uh oh!

Conversation

MelvinBot commented Aug 29, 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

Uh oh!

MelvinBot commented Aug 29, 2026

Clicking the single expense from chat opens the expense report view

Transaction thread entry point redirects to the full Expense Report view

Expense report showing expense details plus a newly added comment

With 2 expenses, clicking one opens its own transaction thread (no redirect)

Message deep link highlights the message without redirecting

Uh oh!

MelvinBot commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Single-expense report view after submission

Tapping expense from chat lands on Expense Report view

Transaction thread root URL redirects to Expense Report view

Report shows expense details plus a working added comment

First expense in a 2-expense report opens its own transaction thread

Second expense in a 2-expense report opens its own transaction thread

Message-specific thread link opens thread and highlights message, no redirect

Recordings

Uh oh!

aimane-chnaif commented Sep 7, 2026

Uh oh!

MelvinBot commented Sep 7, 2026

Uh oh!

MelvinBot commented Sep 7, 2026

Uh oh!

aimane-chnaif commented Sep 7, 2026

Uh oh!

aimane-chnaif commented Sep 7, 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 Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Sep 7, 2026

Review

1. This is the third definition of "one-transaction thread" — reuse the existing one

2. The transactionCount gate and the view use different predicates

3. The redirect fires after the thread has already mounted and fetched

4. The queued navigation isn't cancelled

5. The tests mock away the logic that decides the behavior

6. Not ready to mark as ready for review

Uh oh!

aimane-chnaif commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MelvinBot commented Sep 7, 2026

Codex: duplicate parent route

1. Third definition of "one-transaction thread"

5. Tests mocked away the logic

Skipped

Uh oh!

aimane-chnaif commented Sep 7, 2026

Uh oh!

aimane-chnaif commented Sep 7, 2026

Uh oh!

aimane-chnaif commented Sep 7, 2026

1. The redirect drops referrer, which breaks mark-as-read for notification-opened reports

2. Gate the heavy Onyx read behind the cheap counter

3. isBackToParentReport's premise is narrower than the comment claims

4. Navigation.isNavigationReady().then(...) is redundant, and slightly worse than calling directly

5. Two defensive guards (neither is a live bug today)

6. Test fixture nit

Uh oh!

chatgpt-codex-connector Bot left a comment

MelvinBot commented Aug 29, 2026 •

edited

Loading

MelvinBot commented Aug 29, 2026 •

edited

Loading

2. The `transactionCount` gate and the view use different predicates

aimane-chnaif commented Sep 7, 2026 •

edited

Loading

1. The redirect drops `referrer`, which breaks mark-as-read for notification-opened reports

3. `isBackToParentReport`'s premise is narrower than the comment claims

4. `Navigation.isNavigationReady().then(...)` is redundant, and slightly worse than calling directly

1. `AGENT_REPORT` also mounts `ReportScreen`, and it falls through to the inbox route

2. `isBackToParentReport` misses two other shapes of "the parent report"

5. `ReportFetchHandler` still fetches the thread you're about to leave

7. Doc comment in `useIsOneTransactionThread` is misleading

aimane-chnaif Sep 13, 2026 •

edited

Loading

MelvinBot commented Sep 13, 2026 •

edited

Loading

MelvinBot commented Sep 13, 2026 •

edited

Loading