[Payment due @ikevin127] Render the currency conversion fee payer changelog entry - #97897
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot review |
|
@codex review |
ReviewLooks good — clean, minimal, and it faithfully mirrors the existing Completeness ✅ — the new action is wired into all five render surfaces plus the CONST entry, the shared type, and unit tests:
A repo-wide grep for the reference action returned exactly these five call sites, and all five are handled. Notes (non-blocking)1. Locale typography — 2. Manual translations vs. the generator. Heads up that you've hand-written all 11 locales. Since this is a fork PR, translations aren't auto-generated, so if an Expensify employee runs the Generate static translations workflow it may regenerate the non- 3. Missing-payload fallback. Testing noteI did not run this through the live web app: the message is produced only by the backend action from the Auth PR, which isn't deployed, so the flow (set FX preference → changelog entry) can't be exercised against the test API yet. The behavior here is fully covered by the unit tests in |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…tion ReportActionsUtils has a capped allowance of unsafe type assertions and the new one put it over, failing the lint check.
|
Hey @ishpaul777, how's this one coming? |
…reference-changelog # Conflicts: # src/languages/de.ts # src/languages/es.ts # src/languages/fr.ts # src/languages/it.ts # src/languages/ja.ts # src/languages/nl.ts # src/languages/pl.ts # src/languages/pt-BR.ts # src/languages/zh-hans.ts
|
@ikevin127 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] |
|
this is ready for review now! |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Core design doc issue
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-08-17.at.15.44.01.mov |
The message re-translated "Company pays" / "Employee pays" per locale, so the room disagreed with the picker in most locales. Interpolate the existing workflowsCurrencyConversionFeesPage labels and derive the preference type from CONST.
Cover the LHN preview, search options and thread name surfaces, mirroring the existing UPDATE_REQUIRE_COMPANY_CARDS_ENABLED cases.
…reference-changelog # Conflicts: # src/libs/OptionsListUtils/index.ts # tests/unit/OptionsListUtilsTest.tsx # tests/unit/ReportNameUtilsTest.ts # tests/unit/SidebarUtilsTest.ts
|
@ikevin127 all inline comments are addressed and pushed, and I added the |
|
@ishpaul777 Thanks for addressing the comments. I attempted to manually test this PR but was not able to get the actions in Screen.Recording.2026-08-12.at.18.03.06.movThe latest code is sound. Two small things, neither blocking: 🟢 🟢 expect(lastMessage).toBe('updated the currency conversion fee setting to "Company pays"');Also worth adding the employee case here for symmetry with |
main added UPDATE_REQUIRES_CATEGORY and UPDATE_REQUIRES_TAG, which append to the same CONST block, else-if chains, resolver map and test suites this branch touches, so every file it changes conflicted. Both actions' entries are kept side by side in each list. Also addresses review feedback that overlapped the conflicts: the OptionsListUtils case now asserts the rendered string rather than the helper it calls, and covers both preferences.
SignInModal dismisses itself once IS_LOADING_APP settles to false. The back-handling test never sets that flag, but Onyx carries it in from whatever ran earlier in the worker, so the dismiss effect fired in CI and threw on the unmocked dismissModal before any assertion ran.
|
Auth PR was deployed but another dependency https://github.com/Expensify/Web-Expensify/pull/55150 was merged last week but looks like its not deployed yet i'll update once its deployed, for now please verify only by mocking the data in Onyx |
|
https://github.com/Expensify/Web-Expensify/pull/55150 was deployed to staging |
…elog # Conflicts: # tests/unit/SignInBackButtonTest.tsx
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
🎯 @ikevin127, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
|
@madmax330 can you please final review and merge? |
|
🚧 madmax330 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/madmax330 in version: 9.4.57-0 🚀
|
|
Yes — a help site update is needed. This PR surfaces a new global-reimbursement setting where a Workspace Admin chooses whether the company or the employee pays the currency conversion fees on cross-border reimbursements. The existing article Draft help site PR: #99022 What it changes:
Labels/setting text (Currency conversion fees, Company pays, Employee pays) were verified against the App source strings ( @ishpaul777, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.4.57-3 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Payment issue created: #99122 |
Explanation of Change
Auth now writes a changelog entry into the #admins room when a workspace admin changes who pays the currency conversion fees on cross-border reimbursements (Auth PR). The action is
POLICYCHANGELOG_UPDATE_GLOBAL_REIMBURSEMENTS_FX_PREFERENCE, carrying{"preference": "company"}or{"preference": "employee"}.This renders it in NewDot:
Beyond the room message, the same text is wired into the LHN preview, the search options list, the thread name when the message is grouped into a thread, and copy-to-clipboard, matching how
UPDATE_REQUIRE_COMPANY_CARDS_ENABLEDis handled. Without those, the LHN would fall back to the English text Auth stores rather than the reader's locale.The Auth PR should merge first, since nothing produces this action until it deploys. OldDot renders the same wording in https://github.com/Expensify/Web-Expensify/pull/55150.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/655935
PROPOSAL: N/A
Tests
Requires the globalReimbursement and globalReimbursementFX beta .
updated the currency conversion fee setting to "Company pays"."Employee pays".Offline tests
The message comes from a report action pushed by the backend, so there is no optimistic path. Offline, the room shows whatever actions were already synced and the setting cannot be changed.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari