Split expense flow (Phase 2 - Editing existing split expenses) #63738
Conversation
|
@dominictb 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] |
|
@dominictb |
|
@Expensify/design Could you please help me with one thing? We need to change the background color of the element we are trying to edit 
|
|
@eh2077 @ZhenjaHorbach told me you could test the whole flow with optimistic data already. Could you do that please, so we can identify bugs before backend changes are done? |
This seems incorrect to me. Anything in the product that is selected uses the same global selectedBG color. Can we please update to that?
Similar to above, we have a global "row-hover" color we should use here: |
|
Oh 
|
|
Ah, that seems like a mistake to me. Would you agree @Expensify/design ? |
|
Yeah, that's definitely a mistake. Agree with what Shawn is saying that it should be the global selectedBG color |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.2.27-0 🚀
|
| * Retrieves all “child” transactions associated with a given original transaction | ||
| */ | ||
| function getChildTransactions(originalTransactionID: string | undefined) { | ||
| return Object.values(allTransactions ?? {}).filter((currentTransaction) => { |
There was a problem hiding this comment.
@ZhenjaHorbach Please don't use allTransactions (let's pass it from UI)
There was a problem hiding this comment.
Yeah
Sorry
It's just the old PR
And perhaps at the time of writing this code, the Onyx connect refactoring project hadn't happened yet
But I think I'll update this in a future PR!
There was a problem hiding this comment.
@ZhenjaHorbach Could you please raise a follow-up PR to address this?
cc @eh2077
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.2.27-6 🚀
|
| optimisticData: deleteExpenseOptimisticData, | ||
| failureData: deleteExpenseFailureData, | ||
| successData: deleteExpenseSuccessData, | ||
| } = getDeleteTrackExpenseInformation(splitTransaction?.reportID ?? String(CONST.DEFAULT_NUMBER_ID), undeletedTransaction?.transactionID, currentReportAction); |
There was a problem hiding this comment.
@ZhenjaHorbach We will remove getReportNameValuePairs in the method canUserPerformWriteAction in #72585. This requires the methods that use canUserPerformWriteAction to use isReportArchived which is passed from the View. Please help to add it in getDeleteTrackExpenseInformation
| return { | ||
| transactionID: transactionID ?? transactionDetails?.transactionID ?? String(CONST.DEFAULT_NUMBER_ID), | ||
| amount: amount ?? transactionDetails?.amount ?? 0, | ||
| description: transactionDetails?.comment, | ||
| category: transactionDetails?.category, | ||
| tags: transaction?.tag ? [transaction?.tag] : [], | ||
| created: transactionDetails?.created ?? DateUtils.formatWithUTCTimeZone(DateUtils.getDBTime(), CONST.DATE.FNS_FORMAT_STRING), | ||
| merchant: transaction?.modifiedMerchant ? transaction.modifiedMerchant : (transaction?.merchant ?? ''), | ||
| statusNum: currentReport?.statusNum ?? 0, | ||
| reportID: reportID ?? transaction?.reportID ?? String(CONST.DEFAULT_NUMBER_ID), | ||
| }; |
There was a problem hiding this comment.
We should have included the reimbursable field here as well. Not doing so caused #72396
| delete transactionChanges[key as keyof typeof transactionChanges]; | ||
| // Ensure we pass the currency to getUpdateMoneyRequestParams as well, so the amount message is created correctly | ||
| } else if (key === 'amount') { | ||
| transactionChanges.currency = originalTransactionDetails?.currency; |
There was a problem hiding this comment.
Coming #74265, we need to remove transactionID in transactionChanges when reversing the split. We do that to prevent showing changed the expense system message
12 participants
Split expense flow (Phase 2 - Editing existing split expenses)
Explanation of Change
Fixed Issues
$ #61028
$ #70481
$ #69371
PROPOSAL: NA
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
2025-09-29.16.52.42.mov
Android: mWeb Chrome
2025-09-29.16.52.42.mov
iOS: Native
2025-09-29.16.52.42.mov
iOS: mWeb Safari
2025-09-29.16.52.42.mov
MacOS: Chrome / Safari
2025-09-29.16.52.42.mov
MacOS: Desktop
2025-09-29.16.52.42.mov