Fix bulk delete of unreported expenses silently skipping all deletions#92682
Fix bulk delete of unreported expenses silently skipping all deletions#92682mountiny wants to merge 6 commits into
Conversation
SearchBulkActionsButton is rendered outside SearchScopeProvider, so useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS) reads the real Onyx collection instead of the search snapshot. IOU actions for unreported expenses live in the selfDM report and are only present in the snapshot, not in the real collection unless the selfDM view has been explicitly opened. This causes deleteTransactions to find action=undefined for every selected transaction and silently skip the deletion. Fix mirrors the existing useAllTransactions pattern: supplement flattenedReportActions with report actions from currentSearchResults.data so the snapshot actions are always available regardless of provider context. Fixes #92539 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: debe7e18de
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Fixes bulk deletion of unreported expenses from the Spend search page by ensuring useSearchBulkActions can resolve the necessary IOU ReportActions even when the component is rendered outside SearchScopeProvider (so useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS) does not automatically read from the search snapshot).
Changes:
- Merge
REPORT_ACTIONSfromcurrentSearchResults.data(search snapshot) into the flattened report actions list used byuseDeleteTransactions. - Deduplicate merged actions by
reportActionID, preferring real Onyx values over snapshot values on conflict.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Covers the fixed scenario: IOU action exists only in the search snapshot (not in real Onyx REPORT_ACTIONS) → deleteMoneyRequest is called. Also covers the no-action case which is a graceful no-op. Adds __mocks__/react-native-nitro-fetch.ts so jest/setup.ts can resolve the module that is mocked there. Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix comment: 'missing from fromOnyx' → 'absent from the Onyx collection' (comment was ambiguous when read in isolation) - Replace unsafe `key as keyof typeof searchData` double-cast with a type-predicate filter, mirroring the pattern used in useAllTransactions - Add __mocks__/react-native-nitro-fetch.ts so jest/setup.ts can resolve the module that is mocked there (was omitted from the test commit) Co-authored-by: Cursor <cursoragent@cursor.com>
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
Package is in package.json so it is installed during npm install. The manual mock was only needed locally before running npm install. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Just a heads up - I'm in the process of being onboarded as a BE contributor, so probably best to re-assign this to another C+ 🙏 |
|
Seems to work well on the adhoc! |
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ 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". |
Explanation of Change
SearchBulkActionsButtonis rendered inSearchActionsBarWidewhich sits outsideSearchScopeProviderinSearchPageWide.tsx. Because of this,useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS)insideuseSearchBulkActionsreads the real Onyx collection rather than the search snapshot.For unreported expenses, IOU report actions live in the selfDM report and are only present in the search snapshot — not in the real Onyx
REPORT_ACTIONScollection unless the user has explicitly opened the selfDM view. This causesflattenedReportActionsto be missing those actions, anddeleteTransactionsinuseDeleteTransactionsthen findsaction = undefinedfor every selected transaction and silentlycontinues past each one — noDeleteMoneyRequestAPI call is ever made.The fix mirrors the existing
useAllTransactionspattern: supplementflattenedReportActionswith report actions fromcurrentSearchResults.data(the search snapshot) so the correct actions are always available regardless of whether the component is insideSearchScopeProvider. Real Onyx values win on conflict so live updates always take precedence over snapshot data.Fixed Issues
$ #92539
Tests
DeleteMoneyRequestrequest is sent.Offline tests
N/A — no offline-specific behavior changed. The delete path already handles offline optimistic updates; this change only affects which report actions are available for the action lookup.
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