refactor: Remove shouldHandleNavigation from requestMoney and trackExpense#88055
refactor: Remove shouldHandleNavigation from requestMoney and trackExpense#88055TaduJR wants to merge 13 commits intoExpensify:mainfrom
Conversation
…rackExpense to UI layer
melvin-bot
bot
requested review from
joekaufmanexpensify and
linhvovan29546
and removed request for
a team and
joekaufmanexpensify
|
@linhvovan29546 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}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a063caf466
ℹ️ 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".
…de-to-keep-navigation-handling-in-the-UI-components
…de-to-keep-navigation-handling-in-the-UI-components # Conflicts: # src/hooks/useExpenseActions.ts # src/pages/iou/request/step/IOURequestStepAmount.tsx # src/pages/iou/request/step/IOURequestStepConfirmation.tsx # tests/ui/components/IOURequestStepConfirmationPageTest.tsx
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…de-to-keep-navigation-handling-in-the-UI-components # Conflicts: # Mobile-Expensify # android/app/build.gradle # ios/NewExpensify/Info.plist # ios/NotificationServiceExtension/Info.plist # ios/ShareViewController/Info.plist # package-lock.json # package.json # src/pages/iou/request/step/IOURequestStepAmount.tsx # src/pages/iou/request/step/IOURequestStepConfirmation.tsx # src/stories/Form.stories.tsx
…de-to-keep-navigation-handling-in-the-UI-components
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 924d702b30
ℹ️ 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".
1 participant
Explanation of Change
Explanation of Change
PR 3 of #84631 extends the PR 2 (#86619) pattern to the two largest remaining action functions, moving navigation and draft cleanup out of
requestMoneyandtrackExpenseinto their UI callers, and introducing a canonical post-creation helper that PRs 4–5 will reuse.New helper:
src/libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate.ts— wraps deferred draft cleanup, optional linked-track-report popping,activeReportIDresolution, and delegation tonavigateAfterExpenseCreate. Lives alongside its sibling helpers; per the architectural constraint, never invoked fromsrc/libs/actions/.Action layer:
shouldHandleNavigation,backToReport,draftTransactionIDsfromrequestMoneyandtrackExpense(and fromRequestMoneyInformation/CreateTrackExpenseParamstypes). Removed the gatedremoveDraftTransactionsByIDs+Navigation.removeScreenByKeyblocks and thehandleNavigateAfterExpenseCreatecalls from both functions.MoneyRequest.ts::createTransaction,handleMoneyRequestStepScanParticipants, andhandleMoneyRequestStepDistanceNavigationnow accept anonTransactionsCreated?: (lastTransactionID) => voidcallback that UI threads through to invoke cleanup + navigation in the right place.Duplicate.ts(duplicateExpenseTransaction,duplicateReport,bulkDuplicateExpenses) no longer passes the removed params.MoneyReportHeader.tsx,MoneyRequestHeaderSecondaryActions.tsx,useBulkDuplicateAction.ts— drop the now-unuseddraftTransactionIDsfield from their call sites.UI layer (calls
cleanupAndNavigateAfterExpenseCreateafter the action):IOURequestStepConfirmation.tsx(requestMoney + trackExpense batch loops),IOURequestStepAmount.tsx,SubmitDetailsPage.tsx.useReceiptScan.tsprovides theonTransactionsCreatedcallback for the scan flow (handles the multi-receipt batch + GPS-granted / GPS-denied / no-location paths).IOURequestStepDistance.tsx,IOURequestStepDistanceMap.tsx,IOURequestStepDistanceManual.tsx,IOURequestStepDistanceOdometer.tsx,IOURequestStepDistanceGPS/index.native.tsx) provide the callback for the TRACK skip-confirm distance flow.SubmitDetailsPage.tsxandIOURequestStepAmount.tsxpre-resolve and passoptimisticChatReportIDso the action and post-action navigation agree on the destination ifreporthasn't hydrated from Onyx yet (closes codex P2).Tests:
MoneyRequestTest.ts: 11 new contract tests foronTransactionsCreated— fires once per batch (not per file), AFTER all action calls, with the last file's transactionID; covers GPS-granted, GPS-denied, no-location, and early-return paths inhandleMoneyRequestStepScanParticipants; covers the TRACK skip-confirm and early-return paths inhandleMoneyRequestStepDistanceNavigation; undefined-callback safe.shouldHandleNavigation/backToReport/draftTransactionIDsassertions from action call fixtures (IOUTest.ts,DeleteMoneyRequestTest.ts,DuplicateTest.ts,PayMoneyRequestTest.ts,ReportWorkflowTest.ts,SplitTest.ts,TrackExpenseTest.ts,IOURequestStepAmountDraftTest.tsx,TimeExpenseConfirmationTest.tsx,UnreadIndicatorsTest.tsx,GoogleTagManagerTest.tsx).IOURequestStepConfirmationPageTest.tsx,TimeExpenseConfirmationTest.tsx,MoneyRequestTest.ts): extended Navigation mocks (getReportRouteByID,removeScreenByKey,getTopmostReportId, etc.) socleanupAndNavigateAfterExpenseCreateworks in tests.Fixed Issues
$ #84631
PROPOSAL: #84631 (comment)
Tests
Offline tests
Same as tests
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 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