feat: Add Tax, Billable, and Reimbursable selectors in the Split details page#94434
feat: Add Tax, Billable, and Reimbursable selectors in the Split details page#94434truph01 wants to merge 1 commit into
Conversation
|
@codex review |
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.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bccd150adf
ℹ️ 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".
| reimbursable: transactionDetails?.reimbursable, | ||
| billable: transactionDetails?.billable, | ||
| taxCode: transactionDetails?.taxCode, | ||
| taxAmount: transactionDetails?.taxAmount, |
There was a problem hiding this comment.
Do not copy the full tax amount to every split
When a taxable expense is split into multiple new split expenses and the user saves without opening each tax row, every SplitExpense initialized here carries the original transaction's full taxAmount; the new updateSplitTransactions payload then sends that value for each split. For example, splitting an expense with $10 tax into two lines sends $10 tax on both child transactions, corrupting tax totals unless the user manually edits each one. This should be prorated/recomputed for the split amount or omitted until explicitly set.
Useful? React with 👍 / 👎.
| taxCode: split?.taxCode, | ||
| taxAmount: split?.taxAmount, | ||
| taxValue: split?.taxValue, |
There was a problem hiding this comment.
Keep optimistic split taxes in sync with the payload
These per-split tax fields are now sent to the API, but the optimistic transaction built later in this same function still uses originalTransactionDetails.taxCode/taxAmount/taxValue instead of the current splitExpense values. When a user changes a split's tax rate or tax amount and saves, offline mode and the immediate post-save UI/search state continue showing the original tax values until the server response arrives, which makes the new editor appear not to have saved the tax change locally.
Useful? React with 👍 / 👎.
| splitExpenseDraftTransaction?.comment?.originalTransactionID, | ||
| ]); | ||
|
|
||
| console.log('>>>>>>>>>>>>>>>>>>', splitExpenseDraftTransaction); |
There was a problem hiding this comment.
Remove transaction debug logging
This console.log runs on every render of the split details page and prints the full draft transaction, including merchant, amount, tax, report, and participant-related data, into production browser/native logs. Since this page is opened for real expenses, it leaks sensitive expense details and adds noisy render-time logging; remove the debug statement before shipping.
Useful? React with 👍 / 👎.
Explanation of Change
src/types/onyx/IOU.ts— AddtaxCode?: string,taxAmount?: number,taxValue?: numberto theSplitExpenseinterface.SplitExpenseEditPage.tsx— Add UI rows for Tax Rate, Reimbursable, and Billable (gated by the appropriate policy flags), navigating to the existingMONEY_REQUEST_STEP_TAX_RATE,MONEY_REQUEST_STEP_BILLABLE, andMONEY_REQUEST_STEP_REIMBURSABLEroutes.SplitExpenseItems.ts(initSplitExpenseItemData) — SeedtaxCode/taxAmount/taxValuefrom the parent transaction when initializing split items.SplitExpenseItems.ts(updateSplitExpenseField) — CarrytaxCode/taxAmount/taxValuethrough from the draft transaction when saving a split edit.SplitTransactionUpdate.ts— IncludetaxCode/taxAmount/taxValuein thesplitsAPI payload and in the optimistic Onyx merge.SplitTransactionSplitsParamtype — AddtaxCode,taxAmount,taxValuefields.Fixed Issues
$ #94038
PROPOSAL: #94038 (comment)
Tests
Offline tests
QA Steps
Same as tests
PR 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari