Conversation
…/Travel/Debug Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 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". |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@hoangzinh 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const travelerPersonalDetailsSelector = (personalDetails: OnyxEntry<PersonalDetailsList>) => pickTravelerPersonalDetails(personalDetails, reservation); | ||
|
|
||
| const [travelerPersonalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: travelerPersonalDetailsSelector}, [travelerPersonalDetailsSelector]); | ||
| const reservationDetails = useMemo(() => getReservationDetailsFromSequence(icons, tripReservations, Number(sequenceIndex)), [icons, tripReservations, sequenceIndex]); |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-0 (docs)
This PR removes manual memoization everywhere else in the batch and relies on React Compiler, but here it introduces a new manual useMemo. The file previously had no manual memoization at all and was fully compiler-managed, so this wrapper is redundant — React Compiler already caches derived values based on their captured dependencies (icons, tripReservations, sequenceIndex). The manual useMemo adds noise (a hand-maintained dependency array) and can interfere with the compiler's own caching.
Remove the useMemo and compute the value inline; the compiler will memoize it automatically:
const {reservation, prevReservation, reservationType, reservationIcon, isCancelled} = getReservationDetailsFromSequence(icons, tripReservations, Number(sequenceIndex));
const travelerEmail = reservation?.travelerPersonalInfo?.email;
const [travelerPersonalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {
selector: (personalDetails: OnyxEntry<PersonalDetailsList>) => pickTravelerPersonalDetails(personalDetails, travelerEmail),
});Reviewed at: c903b8e | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-07-25.at.06.53.10.mov |
| lastReportRouteReportActionID, | ||
| ]); | ||
| const [doesLastReportActionExist] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${lastReportRouteReportID}`, { | ||
| selector: (reportActions: OnyxEntry<ReportActions>) => makeDoesLastReportActionExistSelector(lastReportRouteReportActionID)(reportActions), |
There was a problem hiding this comment.
| selector: (reportActions: OnyxEntry<ReportActions>) => makeDoesLastReportActionExistSelector(lastReportRouteReportActionID)(reportActions), | |
| selector: makeDoesLastReportActionExistSelector(lastReportRouteReportActionID), |
| const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); | ||
| const hasViolations = hasViolationsReportUtils(undefined, transactionViolations, session?.accountID ?? CONST.DEFAULT_NUMBER_ID, session?.email ?? ''); | ||
| const chatEnabledPaidGroupPolicies = (policies: Parameters<typeof chatEnabledPaidGroupPoliciesSelector>[0]) => chatEnabledPaidGroupPoliciesSelector(policies, session?.email); | ||
| const chatEnabledPaidGroupPolicies = useCallback( |
There was a problem hiding this comment.
Do we need to wrap this selector in a useCallback?
| const travelerPersonalDetailsSelector = (personalDetails: OnyxEntry<PersonalDetailsList>) => pickTravelerPersonalDetails(personalDetails, reservation); | ||
|
|
||
| const [travelerPersonalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: travelerPersonalDetailsSelector}, [travelerPersonalDetailsSelector]); | ||
| const reservationDetails = useMemo(() => getReservationDetailsFromSequence(icons, tripReservations, Number(sequenceIndex)), [icons, tripReservations, sequenceIndex]); |
|
@hoangzinh can you complete the checklist please |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.45-0 🚀
|
|
🤖 I reviewed the changes in this PR against the help site content under No help site changes are required, so no draft PR was created. This PR is Batch H of the Since there's nothing user-facing here, there's no behavior for a HelpDot article to reflect, so I did not open a docs PR. |
|
Hi @sosek108. Coming from the "Debug report actions" test, step 5: After adding a comment, the new action doesn’t appear in the Debug page > Actions. Is this expected? Debug.report.actions.-.step.5.mp4 |
|
@IuliiaHerets, it won't work for a single expense report. Can you try on multi expense report or chat report? Screen.Recording.2026-07-28.at.15.09.57.mov |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.4.45-14 🚀
Bundle Size Analysis (Sentry): |
8 participants
Explanation of Change
Part of the useOnyx dependencies-argument removal (#94595), Batch H of the split: Navigation, Wallet, Travel, Debug. Drops the deprecated 3rd useOnyx argument; selectors kept stable (inline/compiler-memoized, useCallback in the React-Compiler-bailout file CreateReportMenuItem). No intended behavior change. The main wrapper still accepts the arg, so this batch is independent.
Fixed Issues
$ #95581
PROPOSAL:
Tests
Inbox tab button
Create report flows
2. Create 2 workspaces, mark first as active
3. Go to inbox, and using FAB create empy report. Verify it’s created for active WS
4. Change active workspace
5. Go back to inbox, create empy report. Verify it’s created for active WS
6. Repeat the same for both FAB buttons
Debug report page
Debug report actions
Trip details page
I do not have access to Trip functionality so I wasn’t able to test this myself
Expensify card page
I do not have access to cards functionality so I wasn’t able to test this myself
Offline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
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
Nagranie.z.ekranu.2026-07-23.o.11.08.41.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Nagranie.z.ekranu.2026-07-23.o.11.04.50.mov