fix: "Waiting for you to add expenses" is not translated#80715
Conversation
This change adds support for the new translated next step format (ReportNextStep) while preserving backward compatibility with the deprecated format and ensuring local optimistic overrides (error states, offline submission) take precedence.
|
@aimane-chnaif 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✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9502e77c2a
ℹ️ 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".
| // Use new format with translation support when available | ||
| if (nextStepNew?.messageKey && currentUserAccountID !== undefined) { | ||
| return buildNextStepMessage(nextStepNew, translate, currentUserAccountID); |
There was a problem hiding this comment.
Escape user display names before HTML render
When nextStepNew is present this path renders the HTML returned by buildNextStepMessage(). That helper interpolates actor directly into translation strings that contain HTML tags (e.g., nextStep.message.* uses <strong>${actor}</strong> in src/languages/en.ts), and there is no escaping of the display name. Because RenderHTML renders the string as HTML, any participant name containing </> will be interpreted as markup (e.g., injecting links or styling) instead of plain text. This can occur whenever a user sets a display name with HTML characters. Consider escaping actor (and other user-provided params) before translation or adding Str.safeEscape inside buildNextStepMessage to match the safety of parseMessage.
Useful? React with 👍 / 👎.
…ction The actor display name is now escaped using Str.safeEscape before being interpolated into translation strings that contain HTML tags, preventing potential HTML injection when a user's display name contains special characters.
|
Adding some tests to fix code coverage |
2d9c590 to
66153cc
Compare
Adds comprehensive test coverage for the buildNextStepMessage function: - Tests for different actor types (current user, other user, unspecified admin) - Tests for ETA handling with etaKey and dateTime - Tests HTML escaping of actor display names to prevent XSS
66153cc to
6c10d27
Compare
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
@MobileMage please fix conflict |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
…-expenses-translation # Conflicts: # src/components/MoneyReportHeaderStatusBar.tsx # tests/unit/NextStepUtilsTest.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7b50b2d96
ℹ️ 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".
src/components/MoneyReportHeader.tsx
Outdated
| <View style={[styles.flexRow, styles.gap2, styles.justifyContentStart, styles.flexNoWrap, styles.ph5, styles.pb3]}> | ||
| <View style={[styles.flexShrink1, styles.flexGrow1, styles.mnw0, styles.flexWrap, styles.justifyContentCenter]}> | ||
| {showNextStepBar && <MoneyReportHeaderStatusBar nextStep={optimisticNextStep} />} | ||
| {showNextStepBar && <MoneyReportHeaderStatusBar nextStep={hasOptimisticOverride ? optimisticNextStep : (moneyRequestReport?.nextStep ?? optimisticNextStep)} />} |
There was a problem hiding this comment.
Gate the status bar on the same nextStep you render
Because MoneyReportHeaderStatusBar now prefers moneyRequestReport?.nextStep, the component can be hidden even when a new-format next step exists: showNextStepBar is still computed from optimisticNextStep, which is populated from the deprecated NEXT_STEP collection. If the backend stops sending the deprecated collection (or clears it) while still sending report.nextStep, showNextStepBar becomes false and the status bar never renders despite having data. Consider computing the visibility from the same effective nextStep you pass to the component.
Useful? React with 👍 / 👎.
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.3.16-0 🚀
|
Revert the effectiveNextStep logic from Expensify#80715 that incorrectly preferred the server's nextStep over the optimistic one after approval, causing the status bar to show "Waiting for an admin to pay" instead of "Waiting for you to pay". Fixes Expensify#81967
Revert the effectiveNextStep logic from Expensify#80715 that incorrectly preferred the server's nextStep over the optimistic one after approval, causing the status bar to show "Waiting for an admin to pay" instead of "Waiting for you to pay". Fixes Expensify#81967
Revert the effectiveNextStep logic from Expensify#80715 that incorrectly preferred the server's nextStep over the optimistic one after approval, causing the status bar to show "Waiting for an admin to pay" instead of "Waiting for you to pay". Fixes Expensify#81967
|
Hi @MobileMage, we ended up needing to revert #81223 to address a deployblocker, which meant I had to revert this PR as well as it built on top of the messageKey changes. Stay tuned for next steps after @ishpaul777 opens a new PR to re-add messageKey |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.16-9 🚀
|
|
🚀 Deployed to staging by https://github.com/MarioExpensify in version: 9.3.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.17-9 🚀
|
|
we most probably not need this PR once we fully migrate to new format that uses |
Explanation of Change
This PR adds translation support for the next step status bar messages in the MoneyReportHeader component.
Changes:
MoneyReportHeaderStatusBar.tsx:ReportNextStepformat (withmessageKey) that enables translationbuildNextStepMessage()from NextStepUtils when the new format is availableparseMessage()for backward compatibilityMoneyReportHeader.tsx:hasOptimisticOverridecheck to detect when local error states (DEW failures, offline submission, policy violations) override server dataThis approach ensures:
Fixed Issues
$ #79619
PROPOSAL: #79619 (comment)
Tests
Offline tests
QA Steps
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/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari