updated expense-moving logic to use the default workspace, except when an admin moves a member's report#87962
Conversation
…n an admin moves a member's report
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.
|
|
@bernhardoj 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] |
|
@bernhardoj Sorry for the ping here, but I’m not sure a C+ review is needed here, as this PR simply follows up on the missed case. |
|
@deetergp Tagging you to keep you in the loop on this PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dc07dbb78
ℹ️ 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".
| // When moving an expense that belongs to another user, using the policy of their report | ||
| // (or the transaction's policy as fallback) so the workspace context is preserved. | ||
| // For the current user's own expenses, fall back to undefined to let the default workspace apply. | ||
| const targetExpensePolicyID = ownerAccountID !== session?.accountID ? (selectedReport?.policyID ?? transactionPolicyID) : undefined; |
There was a problem hiding this comment.
Preserve per-diem policy when resolving own expense workspace
This change sets targetExpensePolicyID to undefined for any expense owned by the current user, which makes usePolicyForMovingExpenses fall back to the default workspace even for per-diem expenses. In this flow, createReportForPolicy() still uses perDiemOriginalPolicy, but IOURequestEditReportCommon validates per-diem move compatibility against policyForMovingExpenses; if the default workspace has different or missing per-diem custom units, users get a blocking "cannot move" modal even though the actual destination policy is valid. This regression occurs when moving your own per-diem expense and your default workspace differs from the expense workspace.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@deetergp , I don’t have a strong opinion on this. Should we exclude the logic for per diem expenses? In the current production version, Create report behaves differently for per diem:
- From 'Move to report' → 'Create report', the expense can’t be moved as it defaults to the default workspace.
- From Expense Details → Report → 'Create report', the expense can be moved to the workspace that already has a per diem report.
Sharing the video below for both cases:
current-per-diem.mov
|
@MelvinBot, why the React compiler check is failing? |
|
The React Compiler is failing on
The issue is that the React Compiler doesn't support optional chaining ( const targetExpensePolicyID = selectedReport?.ownerAccountID !== session?.accountID || hasPerDiemTransactions ? selectedReport?.policyID : undefined;has const isOtherOwnerOrPerDiem = selectedReport?.ownerAccountID !== session?.accountID || hasPerDiemTransactions;
const targetExpensePolicyID = isOtherOwnerOrPerDiem ? selectedReport?.policyID : undefined;This avoids optional chaining in the ternary test, which satisfies the React Compiler. |
|
@deetergp To address this comment #87962 (comment), I've added Issue was that, per diem transactions were falling through to the default workspace With this change 602d0c5, if the selection includes any per diem transactions, we now |

Explanation of Change
The "Create report" button was showing the current expenses's workspace instead of the default workspace when moving an individual expense via the Report field. This PR make sure that the default workspace is always used consistently, matching the behavior of the bulk-move flow.
Note: For the admin case (#80414), when an admin views a member's expense, the "Create report" button shows the expense's workspace (not the admin's default) - this is the expected behavior and remains unchanged.
Fixed Issues
$ #87441
PROPOSAL: #87441 (comment)
Tests
Test 1
Preconditions
Steps
Test 2
Preconditions
Steps
Test 3
Create report, then select it and confirm that a new report is created in that workspace.Create report, then select it and confirm that a new report is created in the selected non-default workspace.Offline tests
Same as Tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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.mov
Android: mWeb Chrome
mweb-chrome.mov
iOS: Native
ios.mov
iOS: mWeb Safari
mweb-safari.mov
MacOS: Chrome / Safari
Individual expense flow
user-ownself.mov
Admin viewing a member's expense
admin-change-report.mov