Repository navigation
Fix travel reload/back Not Found by preserving policyID in base path - #95900
arosiclair merged 1 commit into
Conversation
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: dd73ffb16b
ℹ️ 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".
|
@neerajbachani Could you resolve AI's comment? |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc9bce28ef
ℹ️ 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".
fc9bce2 to
90b556d
Compare
Keep policyID on the entry base path for TRAVEL_PUBLIC_DOMAIN_ERROR and TRAVEL_DOMAIN_SELECTOR so reload and in-app back do not rebuild bare /travel.
90b556d to
9039a83
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
We are good to proceed from here |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-17.at.15.48.26.movAndroid: mWeb ChromeScreen.Recording.2026-07-17.at.15.50.49.moviOS: HybridAppScreen.Recording.2026-07-17.at.15.45.42.moviOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-07-17.at.15.29.46.mov |
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
|
Congrats, that's your 5th PR merged! 🎉 Do you know about the ContributorPlus role? It's an opportunity to earn more in the Expensify Open Source community. Keep up the great work - thanks! |
|
🚧 arosiclair has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.4.41-0 🚀
|
|
🤖 Help site review: no changes required I reviewed the changes in this PR and confirmed no updates are needed to the help site files under Why: This is a purely internal routing bug fix. It removes The help site travel articles (e.g. No draft PR was created because there is nothing to change on the help site. |
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.4.41-4 🚀
Bundle Size Analysis (Sentry): |
|
@MelvinBot Help me identify which PR introduced the bug that this PR fixes. |
|
The bug was introduced by #89727 — "migrate travel part 1" ( That PR migrated the travel routes to the dynamic-route system and declared EvidenceThe diff of #89727 added exactly what #95900 reverts: Timeline lines up: #89727 merged 2026-05-14, the regression was reported 2026-06-26. The route bodies at the PR base were unchanged from #89727 until this fix removed the Note the underlying dynamic-suffix / |
Explanation of Change
When navigating to travel dynamic routes (
public-domain-error,domain-selector),policyIDwas declared as a dynamic-suffix query param. On reload and in-app back,getPathWithoutDynamicSuffixstrippedpolicyIDbecause it collided with the My Trips entry-screen param, rebuilding bare/traveland causingAccessOrNotFoundWrapperto show Not Found.This change removes
policyIDfrom those dynamic route suffix configs and updates callers to pass an explicit base path (e.g.travel?policyID=...) viacreateDynamicRoute, sopolicyIDis preserved on the base path through reload and back navigation.Fixed Issues
$ #94652
PROPOSAL: #94652 (comment)
Tests
policyIDin the URLpolicyIDOffline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Screen.Recording.2026-06-30.at.2.07.45.AM.mov