Onyx.connect removal for CurrencyUtils#83804
Onyx.connect removal for CurrencyUtils#83804ShridharGoel wants to merge 1 commit intoExpensify:mainfrom
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dae8fe805
ℹ️ 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".
| function getCurrencyDecimals(currency: string = CONST.CURRENCY.USD, currencies?: CurrencyList): number { | ||
| const decimals = currencies?.[currency]?.decimals; | ||
| return decimals ?? 2; |
There was a problem hiding this comment.
Preserve currency decimals when no list is passed
This change makes getCurrencyDecimals depend entirely on the optional currencies parameter, so any unchanged caller that still uses helpers like convertToDisplayString(amount, currency) without a fourth argument now falls back to 2 decimals for every currency. For example, existing call sites such as src/libs/MoneyRequestReportUtils.ts:176 do not provide a currency list, which will misformat currencies like JPY/KRW that should not use two decimals.
Useful? React with 👍 / 👎.
| function isValidCurrencyCode(currencyCode: string, currencies?: CurrencyList): boolean { | ||
| const currency = currencies?.[currencyCode]; | ||
| return !!currency; |
There was a problem hiding this comment.
Restore default currency lookup in code validation
isValidCurrencyCode now only checks the optional currencies argument, so one-argument callers now always get false. In src/libs/Formula.ts this causes formatAmount() to reject valid displayCurrency values (return '') and to skip the symbolized formatting branch for normal currencies, changing formula output for valid inputs.
Useful? React with 👍 / 👎.
Explanation of Change
Fixed Issues
$ #66294
PROPOSAL:
Tests
USD,JPY).Open Auto Pay Reports Under and Per Diem details pages.
Verify default values and displayed amounts use correct decimals/symbols.
Verify that no errors appear in the JS console
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