[CP Staging] Revert "Refactor: migrate convertToDisplayString to useCurrencyListActions hook (part 16)" - #98357
[CP Staging] Revert "Refactor: migrate convertToDisplayString to useCurrencyListActions hook (part 16)"#98357mountiny wants to merge 2 commits into
Conversation
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
Asked here for QA to retest the linked blocker on this revert pr https://expensify.slack.com/archives/C09V78U42D8/p1786478282519219 |
|
@ZhenjaHorbach 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] |
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: 51af935c88
ℹ️ 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 amount = | ||
| type === CONST.IOU.REPORT_ACTION_TYPE.PAY && !isEmptyObject(report) | ||
| ? convertToDisplayStringEnLocale(getMoneyRequestSpendBreakdown(report).totalDisplaySpend, currency, getCurrencyDecimals) | ||
| : convertToDisplayStringEnLocale(total, currency, getCurrencyDecimals); | ||
| ? convertToDisplayStringUtil(getMoneyRequestSpendBreakdown(report).totalDisplaySpend, currency) | ||
| : convertToDisplayStringUtil(total, currency); |
There was a problem hiding this comment.
Keep stored action amounts in English locale
When the app locale is non-English, convertToDisplayStringUtil formats with IntlStore.getCurrentLocale() in CurrencyUtils, but these strings are persisted as report action text alongside hard-coded English phrases like approved, submitted, and paid. This means an offline/optimistic approve, submit, pay, or request action can store mixed-language formatting such as Spanish currency separators inside English action text until the server response replaces it. Please keep this path on the English-locale formatter while avoiding the hook dependency.
Useful? React with 👍 / 👎.
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
PR doesn’t need product input as a revert PR. Unassigning and unsubscribing myself. |
|
@MelvinBot can you sync with latest main |
…oDisplayString-16 Co-authored-by: Vit Horacek <mountiny@users.noreply.github.com>
|
🤖 Synced with latest |
Explanation of Change
Reverts Refactor: migrate convertToDisplayString to useCurrencyListActions hook (part 16) (merge commit
9ba579bbbd303c74f9a7ffef1ae5c88d476c820c) as a candidate fix for the deploy blocker.The revert restores
convertToDisplayStringingetReportPreviewReportActionMessageandgetIOUReportActionMessage, and removes thegetCurrencyDecimalsthreading that PR 97892 added.git revert -m 1conflicted in 16 files because later PRs moved this code. The conflicts were resolved as follows:getCurrencyDecimalswherever code merged after 97892 still needs it —duplicateReport/bulkDuplicateReports(they pass it intoRequestMoneyInformation),AddExistingExpenseFooter(it passes it intoconvertBulkTrackedExpensesToIOU), anduseExpenseActions.getCurrencyDecimalsparameter frombuildOptimisticApprovedReportAction,buildOptimisticUnapprovedReportAction,buildOptimisticSubmittedReportAction,unapproveExpenseReport, and thesubmitReport/buildOptimisticIOUReportActiondestructures, because the revert leaves it unused there. The matching call sites inReportWorkflow.ts,useLifecycleActions.tsx, and the tests were updated.BuildOptimisticIOUReportActionParams.getCurrencyDecimalsandSubmitReportFunctionParams.getCurrencyDecimalswere left on the types so the 61 call sites stay untouched.Verification:
tsc --noEmitis clean (the single remaining error intests/unit/hooks/useDiscardChangesConfirmationNative.test.tsis pre-existing onmain), ESLint is clean on every changed file, and the 9 affected Jest suites pass (2340 tests).Fixed Issues
$ #98327
Tests
Offline tests
QA Steps
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
Not yet tested — needs manual QA.
Android: mWeb Chrome
Not yet tested — needs manual QA.
iOS: Native
Not yet tested — needs manual QA.
iOS: mWeb Safari
Not yet tested — needs manual QA.
MacOS: Chrome / Safari
Not yet tested — needs manual QA.