83867 migrate travel part 2 v2#97028
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db170b1a01
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // eslint-disable-next-line rulesdir/no-beta-handler | ||
| function canUseAllBetas(betas: OnyxEntry<Beta[]>): boolean { | ||
| return !!betas?.includes(CONST.BETAS.ALL); | ||
| return true; |
There was a problem hiding this comment.
Restore the all-betas membership check
When a user's ONYXKEYS.BETAS does not actually include CONST.BETAS.ALL (or before beta configuration is available), this now makes isBetaEnabled() return true for nearly every beta because hasAllBetasEnabled is always true. That bypasses the intended feature gates across the app, so non-beta users can be put into experimental or blocked flows that should only apply to explicitly enrolled accounts.
Useful? React with 👍 / 👎.
| const [isTripSupportLoading, setIsTripSupportLoading] = useState(false); | ||
|
|
||
| const {transactionID, sequenceIndex, pnr, reportID} = route.params; | ||
| const {transactionID, sequenceIndex, pnr, detailsReportID: reportID} = route.params; |
There was a problem hiding this comment.
Fall back to the base reportID for trip detail links
Existing /r/:reportID/trip/:transactionID/:pnr/:sequenceIndex links are still parsed as dynamic routes, but the dynamic-route adapter carries the base report id as route.params.reportID, not as the new detailsReportID query param. With only detailsReportID read here, reportID is undefined for those migrated/deep-linked URLs, so parentReport cannot be loaded from the URL on a cold/offline open and the details page can show Not Found until transaction data happens to be cached.
Useful? React with 👍 / 👎.
Explanation of Change
Fixed Issues
$ #83867
PROPOSAL:
Tests
Same QA step
Offline tests
QA Steps
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