[CP Staging] Fix two staging deploy blockers from report-layout None feature#93154
Conversation
Web-E's Request::getArray returns [] for a JSON-string-encoded param, so the prior JSON.stringify caused the server to receive zero NVPs and persist nothing. Each NVP now rides as its own nameValuePairs[<name>] form field so PHP's $_REQUEST parses it as an associative array. Fixes issue Expensify#93133.
The pill and the grouped/flat render are derived from two independent useOnyx hooks, so the optimistic write of (layoutOption=null, groupByOption=tag) lands in two separate render passes and the in-between (null, null) state falls through to the CATEGORY default. Latch the user's last clicked selection until Onyx catches up so the popover never flashes Category between None and Tag. Fixes issue Expensify#93130.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01e4d332e1
ℹ️ 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".
| // (layoutOption=null, groupByOption=null) → CATEGORY default while the two NVPs settle in separate render passes. | ||
| const [pendingLayoutSelection, setPendingLayoutSelection] = useState<OnyxTypes.ReportLayoutSelection | null>(null); | ||
| const onyxLayoutSelection = getReportLayoutSelection(reportLayoutOption, reportLayoutGroupBy); | ||
| const currentSelection: OnyxTypes.ReportLayoutSelection = pendingLayoutSelection && pendingLayoutSelection !== onyxLayoutSelection ? pendingLayoutSelection : onyxLayoutSelection; |
There was a problem hiding this comment.
When a clicked layout reaches Onyx, pendingLayoutSelection is left populated; any later Onyx change while this component stays mounted (for example the failureData rollback after a rejected SetNameValuePairs request, or a preference update from another client) will have pendingLayoutSelection !== onyxLayoutSelection and this line will keep rendering/grouping by the stale local choice indefinitely. The latch should be reset once the pending value matches Onyx (or when the write fails) so subsequent authoritative Onyx updates can be reflected.
Useful? React with 👍 / 👎.
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.
|
Without the clear, a later authoritative Onyx update (failureData rollback on a rejected SetNameValuePairs request, or a preference change from another client) would keep the stale local choice masking the real state. Addresses Codex review on PR Expensify#93154.
|
Seems resolved: Screen.Recording.2026-06-10.at.1.21.46.PM.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp93154-android-hybrid-001.mp4Android: mWeb Chrome93154-mweb-chrome-001.mp4iOS: HybridApp93154-ios-hybrid-001.mp4iOS: mWeb Safari93154-mweb-safari-001.mp4MacOS: Chrome / Safari93154-web-chrome-001.mp4 |
rojiphil
left a comment
rojiphil
left a comment
There was a problem hiding this comment.
Thanks @allgandalf for the fixes.
@MariaHCD Changes LGTM.
Approving for your review. Thanks.
MariaHCD
changed the title
…lockers [CP Staging] Fix two staging deploy blockers from report-layout None feature (cherry picked from commit d4fc0fa) (cherry-picked to staging by mountiny)
|
🚧 @MariaHCD has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.2-2 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review: no doc changes required. This PR is a bug fix that restores the intended behavior of the existing Report Layout (Group‑by) feature — it doesn't add a feature, rename anything, or change any user‑facing label or workflow:
The relevant help article — One pre-existing doc observation (not caused by this PR)The article describes accessing the layout via the More icon (three dots) and lists the options as Group by category / Group by tag / Don't group, whereas the current UI exposes a dedicated Group-by pill/dropdown. If the access path or option labels have drifted from the live UI, that's a separate, pre-existing accuracy cleanup unrelated to these two blockers — happy to open a doc PR for it if you'd like, but it's out of scope here. Since no documentation update is required for the changes in this PR, I did not create a draft help‑site PR. @allgandalf, please confirm you agree no help‑site changes are needed for this PR. If you'd like me to refresh the article's access path / option labels against the live UI as a separate cleanup, reply |
|
Requested QA to retest: https://expensify.slack.com/archives/C09V78U42D8/p1781104064006629 |
|
Keeping an eye on QA testing |
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.4-1 🚀
Bundle Size Analysis (Sentry): |
Help site review — no docs changes requiredI reviewed the four changed files in this PR against the help articles under This PR is two bug fixes to the existing Group by (Report Layout) feature, not a new feature or a user-facing change:
The user-facing behavior (group expenses by Category, Tag, or None) is unchanged — these fixes just make the already-documented behavior work correctly. The feature is covered today in Pre-existing doc observation (out of scope for this PR)While reviewing, I noticed the existing "How to group expenses by category or tag using Report Layout" section describes the access path as the More (three-dots) menu with options Group by category / Group by tag / Don't group, whereas this PR's testing steps reference a Group by pill with options None / Tag / Category. If the entry point and labels have since changed to the pill, that article may be stale — but that is a separate pre-existing accuracy gap, not something introduced by this PR, so I have not created a docs PR for it. Worth a follow-up if you can confirm the current UI. |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.4-3 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.5-0 🚀
Bundle Size Analysis (Sentry): |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.5-6 🚀
|
6 participants
Explanation of Change
Two deploy blockers from App#91952:
93133:
SetNameValuePairswas sent asnameValuePairs: JSON.stringify(...). Web-E'sRequest::getArrayreturns[]for a non-array param, so the server persisted nothing andexpensify_layoutOptionnever made it to the DB, which is why None did not survive a cache reset. Each NVP now goes out as its ownnameValuePairs[<name>]form field so PHP parses it as an associative array.93130: the popover pill is derived from two
useOnyxreads. When toggling None → Tag the optimistic write of(layoutOption=null, groupByOption=tag)lands in two separate render passes, and the in-between(null, null)state falls through to the CATEGORY default ingetReportLayoutSelection. Latched the last-clicked selection in local state until Onyx settles to match.Fixed Issues
$ #93133
$ #93130
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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