[CP Staging] Fix issue: Expense - Expense report RHP does not close and opens not here page after rejecting expense#79718
Conversation
00077de to
4ff2e35
Compare
|
@ShridharGoel 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-01-16.12.44.32.movAndroid: mWeb Chrome2026-01-16.12.43.19.moviOS: HybridApp2026-01-16.12.44.32.moviOS: mWeb Safari2026-01-16.12.43.19.movMacOS: Chrome / Safari2026-01-16.12.41.51.mov |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ff2e35c43
ℹ️ 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".
src/libs/actions/Search.ts
Outdated
| if (isUserOnSearchMoneyRequestReport) { | ||
| const lastRouteParams = lastRoute?.params as SearchFullscreenNavigatorParamList[typeof SCREENS.SEARCH.ROOT]; | ||
| urlToNavigateBack = ROUTES.SEARCH_ROOT.getRoute({query: lastRouteParams?.q}); |
There was a problem hiding this comment.
Preserve search params when rebuilding back route
This rebuilds the search route using only q, but ROUTES.SEARCH_ROOT.getRoute accepts rawQuery/name, and SearchPage rebuilds filters from route.params.rawQuery. If the user opened the report from a saved search or a manually typed query (where rawQuery/name are set), this navigation drops those params and returns them to a different search state after rejecting. Consider carrying over the full params (or using the original backTo) to avoid resetting the search context.
Useful? React with 👍 / 👎.
|
LGTM! |
…here page after rejecting expense
4ff2e35 to
6fe192b
Compare
mountiny
left a comment
There was a problem hiding this comment.
NAB, thanks for the fix
| const isUserOnSearchMoneyRequestReport = | ||
| focusedNavigator?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR && | ||
| focusedNavigator?.state?.routes?.some((route) => route.name === SCREENS.RIGHT_MODAL.SEARCH_MONEY_REQUEST_REPORT); | ||
| if (isUserOnSearchPage) { | ||
| urlToNavigateBack = undefined; | ||
| } else if (isUserOnSearchMoneyRequestReport) { | ||
| const lastRouteParams = lastRoute?.params; | ||
| urlToNavigateBack = lastRouteParams && 'backTo' in lastRouteParams ? lastRouteParams?.backTo : undefined; | ||
| if (isUserOnSearchMoneyRequestReport && lastRoute?.params) { | ||
| const searchParams = lastRoute.params as SearchFullscreenNavigatorParamList[typeof SCREENS.SEARCH.ROOT]; | ||
| urlToNavigateBack = ROUTES.SEARCH_ROOT.getRoute({ | ||
| query: searchParams.q, | ||
| ...(searchParams?.rawQuery && {rawQuery: searchParams.rawQuery}), | ||
| ...(searchParams?.name && {name: searchParams.name}), | ||
| }); | ||
| } else { | ||
| urlToNavigateBack = undefined; | ||
| } |
There was a problem hiding this comment.
In a follow up, lets try to look for ways to make this clearer, this feels very complex and we should at least add more comments @WojtekBoman
…pense-reports [CP Staging] Fix issue: Expense - Expense report RHP does not close and opens not here page after rejecting expense (cherry picked from commit b696e21) (cherry-picked to staging by mountiny)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.3-5 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.3-8 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.4-0 🚀
|
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.4-6 🚀
|
Explanation of Change
The app behaviour after being fixed:
Screen.Recording.2026-01-16.at.11.18.28.mov
Fixed Issues
$ #79706
PROPOSAL:
Tests
Test the linked issue
Offline tests
Test the linked issue
QA Steps
Test the linked issue
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari