Fix: use server-rendered message for moved-report action when destination policy is unavailable locally - #98238
Conversation
…tion policy is unavailable locally Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
…s live Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
|
Fixed the failing check. Pushed What was failing: Root cause (directly from this PR's change): the test's MOVED action uses Fix: seed Other non-passing checks (not addressed — not code failures)
|
|
The real failure is already fixed by my earlier push (
The only checks still showing as failed are |
|
The code-related failures are resolved by my earlier push ( The only checks still red are |
Updated PR descriptionTests
Offline tests
QA StepsSame as Tests and Offline tests.
PR Author Checklist
Screenshots/VideosAndroid: NativeAndroid-pr98238.mp4Android: mWeb ChromeAndroid-mWeb-pr98238.mp4iOS: Nativeios-pr98238.mp4iOS: mWeb Safariios-mWeb-pr98238.mp4MacOS: Chrome / SafariChrome-pr98238.mp4@MelvinBot please replace the PR description from |
|
✅ Done — I replaced the PR description from |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid-pr98238.mp4Android: mWeb ChromeAndroid-mWeb-pr98238.mp4iOS: HybridAppios-pr98238.mp4iOS: mWeb Safariios-mWeb-pr98238.mp4MacOS: Chrome / SafariChrome-pr98238.mp4 |
|
@MelvinBot please mark this PR as ready for review. |
|
@nyomanjyotisa 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] |
|
✅ Marked this PR as ready for review. All CI checks are passing except |
|
🤖 I looked into this — there are no failing code checks to fix. Every test/lint/typecheck job is green ( The only red mark in the status rollup is a stale Nothing to push here. If GitHub is still showing that old run as failing in the UI, a no-op re-run of the "PR Author Checklist" workflow will clear the stale status. |
|
I'll just merge this since we are just fixing a bug (missing policy name) and not changing the product |
|
🚧 aldo-expensify 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/aldo-expensify in version: 9.4.53-0 🚀
|
Help site review — no docs changes requiredI reviewed the changes in this PR and no updates to Expensify's help site ( Why: This is an internal rendering bug fix, not a behavior or feature change. The Nothing user-facing changed that the help site documents:
Help articles like Managing Expenses in a Report and Expense and Report Actions describe the moved-report workflow, and both remain accurate as-is. @nyomanjyotisa, since no help site changes were required, no draft docs PR was opened. If you believe an article should still be updated, let me know and I'll create one. |
Explanation of Change
The workspace name in the "moved this report to the … workspace" system message was recomputed on every render from the current user's local Onyx policy collection, with a bare empty-string fallback:
When User A is not a member of the destination workspace and never opened it,
policy_<toPolicyID>isn't in their Onyx, so the lookup returns''and the message renders with a blank name. It only appeared later because creating another expense delivered that policy into User A'sCOLLECTION.POLICY, after which the message re-rendered correctly.This change makes
getMovedActionMessagefall back to the server-rendered action message (getReportActionHtml(action)) when the destination policy isn't available locally (sotoPolicyNamewould be empty). The broadcast action already carries the name the server rendered, so User A sees it immediately. When the policy is present locally, the current live-computed string is kept so nothing else changes. This mirrors the existing pattern where messages fall back to the stored action text when their computed inputs aren't available. Returning the stored HTML covers both consumption paths — the HTML-rendered report view and the plain-text LHN/preview callers that wrap the result inParser.htmlToText(...).A unit test was added covering the fallback path (destination policy absent locally → stored action HTML returned).
Fixed Issues
$ #98108
PROPOSAL: #98108 (comment)
Tests
Offline tests
QA Steps
Same as Tests and Offline 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-pr98238.mp4
Android: mWeb Chrome
Android-mWeb-pr98238.mp4
iOS: Native
ios-pr98238.mp4
iOS: mWeb Safari
ios-mWeb-pr98238.mp4
MacOS: Chrome / Safari
Chrome-pr98238.mp4