BT Migrate Report Core 1#90113
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: 02ea918c8f
ℹ️ 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".
Screen.Recording.2026-05-11.at.10.09.46.movBUG: Cannot go back from the report detail page. Becasue the url is |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-05-27.at.10.04.02.mov |
This comment was marked as outdated.
This comment was marked as outdated.
|
@huult Although I've fixed the issue about details not opening. I've raised a comment for @collectioneur's help. Can you review the other parts meanwhile? |
Screen.Recording.2026-05-22.at.15.14.51.movBug: ‘Not found’ appears when navigating back from invite page |
The invite and members pages are the same issue. I fixed it with a fallback value because we're having backTo route. If we have the backTo it'll take priority if not then it'll use the dynamic path. |
@huult I don't think we have any UI wiring this route. Hence I've migrated the route and added the test steps to accordingly reflect that. |
Yes, I tested it and it works, thanks. Maybe it’s coming from the email flow |
…p into mj-83365-dr-report-core-1
|
@huult All the checks passed. |
|
@mananjadhav please resolve the conflict and we're good to merge |
Explanation of Change
Fixed Issues
$ #83365
PROPOSAL:
Tests
This PR migrates report-scoped navigation off legacy
REPORT_WITH_ID_*helpers toDYNAMIC_ROUTESfor report details, share code, export, and change workspace. Other call sites that were updated to usecreateDynamicRoute/ new screen keys should be covered by targeted smoke checks rather than a file-by-file checklist. For instance:1. Report details (dynamic root)
r/<reportID>/details(no duplicatedetails, no unexpectedbackTo).2. Change workspace
r/<reportID>/change-workspace.3. Share code (report details + referral)
r/<reportID>/details/shareCode.backTo/ prior screen).4. Export (report details export modal)
details/export/<connectionName>on web when applicable).r/<reportID>/details/export/<connectionName>(use a validconnectionNamefor the policy, e.g.quickbooksOnline,xero,netsuite,intacctper policy setup).Offline tests
Same as test steps
QA Steps
Same as test steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
web-bt-report-core-1.mov
web-bt-report-profile-regression-1.mov