Fix split distance expense showing wrong currency after workspace currency change#83618
Fix split distance expense showing wrong currency after workspace currency change#83618
Conversation
…ncy change When splitting a distance expense, the currency for the merchant string was sourced from the current policy's mileage rate instead of the original transaction's currency. If the workspace currency changed after the expense was created, the split would display the new currency instead of the original. Swap the fallback priority in updateSplitExpenseDistanceFromAmount and updateSplitExpenseField so the original transaction currency takes precedence over the mileage rate currency. Co-authored-by: Puneet Lath <puneetlath@users.noreply.github.com>
|
@MelvinBot |
|
No, The function signature declares function getDistanceMerchantFromDistance(
distanceInUnits: number, unit: Unit | undefined,
rate: number | undefined, currency: string,
transactionCurrency?: string
): stringInside the function body (line 1873), the fallback chain is: const currencyForMerchant = currency ?? transactionCurrency ?? CONST.CURRENCY.USD;Since Additionally, both call sites already resolve the 4th argument with
So the |
|
@MelvinBot |
…istance Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
|
Done — removed the unused
|
|
@MelvinBot |
…ceCurrency Co-authored-by: Puneet Lath <puneetlath@users.noreply.github.com>
|
Fixed the failing The ESLint check was configured with |
melvin-bot
bot
requested review from
ahmedGaber93 and
joekaufmanexpensify
and removed request for
a team
|
@ahmedGaber93 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari2026-02-26.21.50.39.mov |
|
Changes are not related to TS issues |
|
Done — PR has been marked as Ready for review and the branch has been updated with the latest |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff5b5514a0
ℹ️ 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".
| }; | ||
|
|
||
| const merchant = getDistanceMerchantFromDistance(distanceInUnits, unit, rate, mileageRate?.currency ?? transactionCurrency ?? CONST.CURRENCY.USD, transactionCurrency); | ||
| const merchant = getDistanceMerchantFromDistance(distanceInUnits, unit, rate, transactionCurrency ?? mileageRate?.currency ?? CONST.CURRENCY.USD); |
There was a problem hiding this comment.
This change makes updateSplitExpenseDistanceFromAmount() prefer transactionCurrency over mileageRate.currency, which causes wrong merchant currency when a split expense rate is changed to one with a different currency. In split flows, amount recalculations (e.g., updateSplitExpenseAmountField) pass originalTransaction.currency as transactionCurrency, so the amount is recomputed from the new rate but the merchant string is formatted with the old currency, creating a currency/amount mismatch for edited split distances.
Useful? React with 👍 / 👎.
| // Update merchant for distance transactions | ||
| const currency = mileageRate?.currency ?? originalTransaction.currency ?? CONST.CURRENCY.USD; | ||
| updatedItem.merchant = getDistanceMerchantFromDistance(distanceInUnits, unit, rate, currency, originalTransaction.currency); | ||
| const currency = originalTransaction.currency ?? mileageRate?.currency ?? CONST.CURRENCY.USD; |
There was a problem hiding this comment.
When saving split distance edits, this now prioritizes originalTransaction.currency instead of mileageRate.currency for merchant formatting. In the split rate picker flow, changing customUnitRateID updates the draft transaction to the selected rate context, so if that rate uses a different currency, save will still stamp merchant text with the original transaction currency and show an incorrect currency symbol.
Useful? React with 👍 / 👎.
…ceCurrency Co-authored-by: Puneet Lath <puneetlath@users.noreply.github.com>
The areAllConnectionsSet property was lost during the merge of PR #83456 to main. The test file expects this property on DerivedValueContext but the type definition and index.ts were missing it, causing typecheck to fail. Co-authored-by: Puneet Lath <puneetlath@users.noreply.github.com>
…Expensify/App into claude-fixSplitDistanceCurrency Co-authored-by: Puneet Lath <puneetlath@users.noreply.github.com>
|
Fixed the failing Root cause: PR #83456 ( Fix: Restored |
|
@MelvinBot |
Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
|
Reverted the unrelated TS changes. Removed the
Only the Split.ts currency fix remains in the PR. |
|
Fixed the failing typecheck. The previous commit (dd82e84) reverted The ESLint check failure from the earlier run was in files not changed by this PR ( |
|
Check failure analysis:
|
|
I ran out of time before I could finish helping you. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in the #melvinbot-ai. Run details: https://github.com/Expensify/Expensify/actions/runs/22462306224 |
4 participants
Explanation of Change
When splitting a distance expense, the currency for the merchant string was being sourced from the current policy's mileage rate (
mileageRate.currency) instead of the original transaction's currency. If the workspace currency was changed after the expense was created (e.g., INR to YER), the split would incorrectly display the new workspace currency.This PR swaps the fallback priority in two places in
Split.tsso the original transaction's currency takes precedence over the mileage rate currency:updateSplitExpenseDistanceFromAmount()(line 1914) - core function used by all split distance operationsupdateSplitExpenseField()(line 2342) - used when editing individual split fieldsFixed Issues
$ #83606
Tests
SplitTest.ts— all 53 tests passOffline tests
N/A — This change only affects the currency display string in the split flow. The currency is derived from already-available local transaction data. No network calls are involved in this code path.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
N/A — no UI component changes, only currency fallback priority logic
Android: mWeb Chrome
N/A — no UI component changes, only currency fallback priority logic
iOS: Native
N/A — no UI component changes, only currency fallback priority logic
iOS: mWeb Safari
N/A — no UI component changes, only currency fallback priority logic
MacOS: Chrome / Safari
N/A — no UI component changes, only currency fallback priority logic