Repository navigation
[Payment due @daledah] Allow admins to merge card + cash expenses across draft reports - #102438
stitesExpensify merged 15 commits into
Conversation
Remove the same-report shortcut for admins in getTransactionsForMerging. Admins now discover cross-report and cross-workspace candidates via the API (online) or the local filter (offline). Only non-admin managers reviewing a processing report keep the same-report restriction. The offline filter (getTransactionsForMergingLocally) now resolves admin access per-candidate against that candidate's own workspace policy. Candidates are excluded when their workspace policy is absent from the local cache rather than assumed accessible. MergeTransactionsListContent subscribes to allPolicies and allReports and forwards them for the offline per-candidate check. Tests: 5 new cases covering admin cross-report (same workspace), non-admin workspace rejection, missing-cache exclusion, manager same-report restriction, and admin-as-submitter cross-report.
…-manual-merge-expenses-drafts
…-manual-merge-expenses-drafts
|
All contributors have signed the CLA ✍️ ✅ |
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.
|
3fd0cf5 to
e9cbf48
Compare
Reviewer Checklist
Screenshots/VideosiOS: HybridAppios.movMacOS: Chrome / SafariScreen.Recording.2026-09-29.at.16.45.20.mov |
|
This is pending an internal discussion. |
…sef-lr-app-678389-admins-merge
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
Ready again @daledah |
|
🎯 @daledah, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
|
🚧 flaviadefaria has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Running some tests now and I'm not quite sure when this happens but I sometimes run into an attendee field when merging two expenses. Not sure what the logic is for displaying the attendee option, but I didn't expect to see it if there are no attendees (the field is disabled in the expense) or only one attendee in both or either expenses. 2026-10-13_14-42-03.mp42026-10-13_15-03-25.mp4 |
|
Other than that which is a small thing, and if needed could be fixed in a separare PR, all looks good! |
|
Thanks for testing @flaviadefaria! Looking into your comments now. |
|
Yeah I can reproduce the issues. I think let’s get this fixed in this PR. I’ll update it shortly. |
Compare attendees as stored instead of applying the report-owner display fallback, which is only resolvable for locally loaded reports and made one side [owner] and the other [] when merging across reports. When either expense has no attendees, keep the other side's attendees instead of flagging a conflict. Remove the now-unused owner-fallback parameters.
|
Fix pushed. Tested 7 different scenarios locally. Screen.Recording.2026-10-13.at.14.11.00.mov |
|
@daledah can you review again please |
daledah
left a comment
There was a problem hiding this comment.
the code changes LGTM and tested well
…-admins-merge # Conflicts: # src/libs/ReportActionsUtils.ts
stitesExpensify
left a comment
There was a problem hiding this comment.
Pretty huge PR, but I didn't see anything that looked obviously incorrect or that could cause problems. Excited to get this across the finish line
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 stitesExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 9.5.13-0 🚀
|
|
Yes, this PR needs a help site update. I opened Docs updates for E/A#102438 Let admins merge expenses across a member's reports. This PR lets workspace admins merge a member's expenses across different reports. That makes this sentence in How to Merge Duplicate Expenses incorrect:
The docs PR splits that sentence into two bullets:
I didn't document the new "merged expenses and moved one to Deleted" system message, because it already explains itself in the app. @youssef-lr, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
Explanation of Change
Admins have always been allowed to merge transactions from different reports in the Spend page. This PR brings this functionality to the expense view, as well as add a system message notifying the submitter of the deleted merged expense.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/678389
PROPOSAL:
Tests
Added unit tests for admin card and cash merges across reports.
Offline tests
QA Steps
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
iOS: mWeb Safari
MacOS: Chrome / Safari
admins.merging.card.cash.mov