fix: bulk edit, recompute taxAmount on amount-only bulk edit offline #88528
fix: bulk edit, recompute taxAmount on amount-only bulk edit offline #88528marcaaron merged 3 commits intoExpensify:mainfrom
Conversation
|
I'll start reviewing shortly. |
|
@codex review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid_hybrid.mp4Android: mWeb Chromeandroid_mWeb.mp4iOS: HybridAppios_hybrid.mp4iOS: mWeb Safariios_mWeb.mp4MacOS: Chrome / Safariweb_chrome.mp4 |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ 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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a843b598ed
ℹ️ 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".
…de is unknown to the policy
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b688172207
ℹ️ 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".
| if (changes.amount !== undefined && !changes.taxCode && transaction.taxCode && supportsExpenseFields && canEditField(CONST.EDIT_REQUEST_FIELD.TAX_RATE)) { | ||
| const taxValue = getTaxValue(transactionPolicy, transaction, transaction.taxCode); | ||
| if (taxValue) { | ||
| const decimals = getCurrencyDecimals(getCurrency(transaction)); | ||
| const taxAmount = calculateTaxAmount(taxValue, Math.abs(changes.amount), decimals); | ||
| transactionChanges.taxAmount = convertToBackendAmount(taxAmount); |
There was a problem hiding this comment.
Skip tax recompute when transaction policy is unresolved
When allPolicies does not contain the expense’s workspace, transactionPolicy falls back to the bulk-edit policy, and this new block recomputes taxAmount from that fallback policy’s rate. If another workspace uses the same tax code string (for example common IDs like id_TAX_RATE_1) with a different percentage, amount-only bulk edit will queue and optimistically apply an incorrect taxAmount for that transaction. This is a data-integrity regression introduced by the new recomputation path for cross-policy/offline cases.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
For this to fire, the user would have to bulk-select a transaction whose workspace policy isn't in their Onyx cache. In practice if you can see a transaction to select it, you're a member of that workspace and its policy is loaded, fallback to the shared bulk-edit policy is a safety net for missing data, not a normal code path. The if (taxValue) guard already handles the more reachable "taxCode not in any policy we can see" case. I would hold with this one until it will be confirmed issue
|
@Krishna2323 put you on as an official reviewer. |
|
🚧 @marcaaron has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/marcaaron in version: 9.3.65-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes required. This PR is an internal bug fix that recomputes |
Explanation of Change
Fixed Issues
$ #88262
PROPOSAL:
Tests
Precondition:
Offline tests
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.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-04-22.at.13.56.42.mov