-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Prevent unique constraint violation when duplicating split expenses removed from reports #81389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ported transaction is duplicated
NikkiWines
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
trjExpensify
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open the unreported split expense.
Click More > Duplicate.
Verify that duplicate is created successfully in the workspace chat and the no errors are thrown.
I thought it was a bit strange that duplicating an unreported expense auto-reports the duplicates expense, but I see what's what we do on Classic. 👍
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-02-06.at.16.58.17.mov |
@c3024 shouldn't the duplicating expense be created in the selfDM in this case? |
|
I don’t think so. For example, if we track an expense in SelfDM (without going through the split flow, as in this case) and then duplicate it, it gets added to the workspace chat. I added that specificity to the steps so QA and anyone testing knows where to find the duplicated expense. We’re fixing only the specific split → SelfDM → duplicate case here to match the behavior of duplicating a directly tracked expense in SelfDM, so the duplicated expense being added to the workspace chat should be the correct behavior. |
|
@c3024 It(duplicate) is added to the default workspace chat even though it is splitted form other workspace chat. Is it expected? |
I think this is the expected behavior. Once an expense is unreported (removed from the report and moved to Self DM), it becomes an independent expense, and duplicating it should go to the default workspace chat. Could you please confirm? @trjExpensify @NikkiWines |
|
Bug: Duplicating in the non-default workspace expense also moved to the default workspace chat. But I see it is the same on the prod too so not related to our PR. Screen.Recording.2026-02-06.at.17.48.14.mov |
We can update the test step here to clarify where to check
|
Pujan92
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Auth CreateMoneyRequest error is fixed!
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #78450 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
I think that's correct but will rely on @trjExpensify's product expertise here |
|
Yeah, duplicating an unreported expense auto-reports the duplicates expense to the default workspace on Classic. So I think that's correct of parity sake, though I do think it's a bit blunt, but not worth the hassle to change it here and now. 👍 |
| expect(duplicatedTransaction?.comment?.customUnit?.quantity).toEqual(DISTANCE_MI); | ||
| }); | ||
|
|
||
| it('should not carry over linkedTrackedExpenseReportAction from the original transaction', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@c3024 this test does not fail if I revert the changes from Duplicate.ts. Can you take a look please?
youssef-lr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment
Explanation of Change
When a split expense is "removed from report", the transaction's linkedTrackedExpenseReportAction field contains a report action with childReportID pointing to the existing transaction thread report.
When duplicating such a transaction:
We fix this by explicitly excluding linkedTrackedExpenseReportAction to ensure a fresh transaction thread report is created for the duplicate.
Fixed Issues
$ #78450
PROPOSAL: NA
Tests
Offline tests
NA
QA Steps
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.duplicateSplitMovedToSelfDM.mov
Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari