fix: Clear convertedAmount when amount or currency is manually modified#83624
fix: Clear convertedAmount when amount or currency is manually modified#83624DebuggingMax wants to merge 1 commit intoExpensify:mainfrom
Conversation
When a user manually edits an expense's amount or currency, the convertedAmount field was not being cleared. This caused incorrect currency conversion to be applied during report export - the system would re-convert already manually entered amounts. For example, if a user manually entered C$62.85 (matching their credit card charge), the export would incorrectly convert this again, resulting in C$85.63. This fix clears convertedAmount when modifiedAmount or modifiedCurrency is set, ensuring the server recalculates conversion if needed rather than using stale data. Fixes Expensify#83451
|
@mananjadhav 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77aa2bb68b
ℹ️ 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".
| shouldStopSmartscan = true; | ||
| // Clear convertedAmount when amount is manually modified to prevent stale conversion data | ||
| // from being used during export (the server will recalculate if needed) | ||
| updatedTransaction.convertedAmount = undefined; |
There was a problem hiding this comment.
Use null so convertedAmount is actually removed
getUpdatedTransaction() sets convertedAmount to undefined, but this object is later written with Onyx.METHOD.MERGE (src/libs/actions/IOU/index.ts, around the optimistic transaction merge), and this codebase explicitly treats undefined as a no-op for Onyx.merge (see comments in src/pages/workspace/companyCards/addNew/StatementCloseDateStep.tsx:33 and src/libs/actions/TravelInvoicing.ts:74). In the amount/currency edit flow, that means the stale convertedAmount value is preserved instead of cleared, so the export double-conversion issue this commit targets can still occur.
Useful? React with 👍 / 👎.
Details
When a user manually edits an expense's amount or currency, the
convertedAmountfield was not being cleared. This caused incorrect currency conversion to be applied during report export.Root Cause: The
getUpdatedTransactionfunction inTransactionUtilssetsmodifiedAmountandmodifiedCurrencywhen users edit expenses, but it did not clearconvertedAmount. During export, the staleconvertedAmount(calculated from the original currency/amount) would be used, leading to double conversion.Example from the issue:
Fixed Issues
$ #83451
$ #83451
Tests
Added unit tests in
TransactionUtilsTest.tsto verify:convertedAmountis cleared when currency is changedconvertedAmountis cleared when amount is manually modifiedOffline tests
N/A - This change affects local state management that is then sent to the server.
QA Steps
PR Author Checklist
Fixed Issuessection aboveScreenshots/Videos
Web
N/A - Logic change only, no UI changes
Mobile Web - Chrome
N/A - Logic change only, no UI changes
Mobile Web - Safari
N/A - Logic change only, no UI changes
Desktop
N/A - Logic change only, no UI changes
iOS
N/A - Logic change only, no UI changes
Android
N/A - Logic change only, no UI changes