Expense - Expense is created in another report with error when Submissions are disabled - #94386
Conversation
…ctly routed. Update tests to verify that expenses with the current user as the sole recipient are processed as self-DM track expenses, preventing erroneous money request submissions
|
@codex review |
| const selectedParticipantsForRequest = iouType === CONST.IOU.TYPE.SPLIT ? splitParticipants : selectedParticipants; | ||
|
|
||
| const soleSelectedParticipant = selectedParticipants.length === 1 ? selectedParticipants.at(0) : undefined; | ||
| const isSelfDMDestination = |
There was a problem hiding this comment.
Old flow: the amount step (AmountSubmission) resolves the destination before navigating, and when the target is the self-DM it navigates to the confirmation with iouType: TRACK baked into the route. So the confirmation reads TRACK reliably → trackExpense.
New flow: the confirmation is embedded, and the conversion to TRACK happens after mount via an async navigation.setParams({iouType: TRACK}). createTransaction decides trackExpense vs requestMoney purely from that route iouType. When the conversion hasn't landed at submit time, it falls through to requestMoney with the current user as recipient → backend rejects with "you cannot request money from yourself."
There was a problem hiding this comment.
We should derive the track decision from the resolved participant (the same signal the old flow uses: destination is the self-DM), not the async route iouType
|
Codex Review: Didn't find any major issues. Swish! 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". |
|
Checked the draft — the approach looks correct, and you're right that my earlier proposal was aimed at the wrong layer. 👍 Why my proposal missed it: I theorized the expense was being routed to a policy expense chat because of a stale Your fix addresses exactly that gap. Deciding CI is green (the two A couple of optional, non-blocking thoughts:
Thanks for digging into this and correcting the direction. |
|
I reviewed this — the approach is sound and at the right layer, and you correctly identified that my earlier proposal was aimed at the wrong place. Quick reconciliation for the record:
Your fix encodes the correct invariant directly at the submission boundary: an expense whose only recipient is the current user can never be a money request, so it must be a track expense. Catching it in A few things worth tightening before it goes up for review:
Net: I agree with this direction over my earlier |
![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: 03a31d01ac
ℹ️ 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".
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
| iouType !== CONST.IOU.TYPE.SPLIT && | ||
| iouType !== CONST.IOU.TYPE.INVOICE && | ||
| iouType !== CONST.IOU.TYPE.PAY && | ||
| !!soleSelectedParticipant && | ||
| !soleSelectedParticipant.isSender && | ||
| !soleSelectedParticipant.isPolicyExpenseChat && | ||
| (soleSelectedParticipant.isSelfDM === true || (!!currentUserPersonalDetails.accountID && soleSelectedParticipant.accountID === currentUserPersonalDetails.accountID)); |
There was a problem hiding this comment.
Do we need all of these conditions?
Some existing self-DM detection logic:
App/src/hooks/useParticipantSubmission.ts
Lines 215 to 220 in d41e19c
App/src/hooks/useParticipantSubmission.ts
Lines 215 to 220 in d41e19c
Both rely on the single participant.isSelfDM flag.
And I assume parallel accountID === currentUserPersonalDetails.accountID fallback is needed in case where the new manual flow leaves isSelfDM unset and doesn't run the iouType→TRACK conversion — so the expense reaches useExpenseSubmission still as CREATE.
Anyway, your conditions are safe. Can we move to IOUUtils?
Something like this:
/**
* A participant points at the current user's self-DM when it carries the self-DM flag,
* or — for flows that seed the raw account before the flag is set — when it's the sole
* recipient and resolves to the current user. Excludes workspace/invoice participants.
*/
function isSelfDMParticipant(participant: Participant | undefined, currentUserAccountID: number | undefined): boolean {
if (!participant || participant.isSender || participant.isPolicyExpenseChat) {
return false;
}
return participant.isSelfDM === true || (!!currentUserAccountID && participant.accountID === currentUserAccountID);
}
|
Codex Review: Didn't find any major issues. Hooray! 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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6e21f5b67
ℹ️ 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".
|
@situchan Please help review again |
Regression reviewI traced the three highest‑risk areas of this diff (the self‑DM heuristic, the distance‑request early‑return, and the 1.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 148ac6fdcc
ℹ️ 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".
| // Telemetry spans (SPAN_SUBMIT_EXPENSE, SPAN_SUBMIT_TO_DESTINATION_VISIBLE) | ||
| // are started by SubmitExpenseOrchestrator before calling createTransaction. | ||
| if (!isTrackExpense && isDistanceRequest && !isMovingTransactionFromTrackExpense && !isUnreported) { | ||
| if (!isTrackExpense && !isSelfDMDestination && isDistanceRequest && !isMovingTransactionFromTrackExpense && !isUnreported) { |
There was a problem hiding this comment.
When a self-DM distance expense reaches this path, it now skips createDistanceRequest and falls through to trackExpense, but the draft transaction can still carry the generated P2P/source reportID from participant selection. trackExpense preserves distance draft fields by merging the existing transaction over the optimistic one, so that stale reportID overrides the self-DM/UNREPORTED report and the optimistic expense is still attached to the wrong report. This affects distance expenses whose sole recipient is the current user in the new manual flow; clear or override the draft reportID before calling trackExpense.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the same path the pre-existing iouType === TRACK distance flow already uses, so no fix is needed.
| } | ||
|
|
||
| if (!isPerDiemRequest && (isTrackExpense || isCategorizingTrackExpense || isSharingTrackExpense)) { | ||
| if (!isPerDiemRequest && (isTrackExpense || isCategorizingTrackExpense || isSharingTrackExpense || isSelfDMDestination)) { |
There was a problem hiding this comment.
This branch routes self-DM CREATE/SUBMIT expenses through trackExpense, but the earlier tax calculation still treats only iouType === TRACK as a tracked expense. In the raw-current-user case this change is meant to handle, transaction.reportID may not be UNREPORTED and isPolicyExpenseChat is false, so isTaxTrackingEnabled(...) returns false and the taxCode/taxValue passed to trackExpense are blank even when the user selected tax on a tax-enabled workspace. Include isSelfDMDestination in the track-like tax calculation before submitting.
Useful? React with 👍 / 👎.
|
Please check ^ |
|
@situchan All good now |
|
@thelullabyy you have a conflict. :) |
|
@thelullabyy feel free to ping me on slack once you've fixed conflicts so that I can merge. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 puneetlath 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/puneetlath in version: 9.4.34-0 🚀
|
|
🤖 I reviewed the changes in this PR against Expensify's help site files under Why: This PR is an internal bug fix, not a behavior or feature change. It corrects routing in the new manual expense flow so that an expense whose sole recipient is the current user is created in the self-DM as a tracked expense (via The changes are confined to internal logic — Since no changes are needed, I did not create a draft PR. @thelullabyy, if you believe this change does introduce user-facing behavior that the help site should document, let me know and I'll draft the corresponding docs PR. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
Bundle Size Analysis (Sentry): |
7 participants
Explanation of Change
Fixed Issues
$ #94282
PROPOSAL: N/A
Tests
Precondition:
newManualExpenseFlowbetaOffline tests
QA Steps
Precondition:
newManualExpenseFlowbetaPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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
Screen.Recording.2026-06-24.at.01.29.33.mov
Android: mWeb Chrome
Screen.Recording.2026-06-24.at.01.30.45.mov
iOS: Native
Screen.Recording.2026-06-24.at.01.34.15.mov
iOS: mWeb Safari
Screen.Recording.2026-06-24.at.01.35.11.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-23.at.23.03.13.mov