Conversation
|
|
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: 2bbafd0757
ℹ️ 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".
|
@fabioh8010 should I review this? |
Not sure how it works but @DylanDylann was assigned in the previous PR that was reverted. cc @Julesssss for review too. |
|
PR doesn’t have any new product considerations. Unassigning and unsubscribing myself. |
Hey @DylanDylann could you review this one please as a follow up to the original reverted PR. Thanks |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@DylanDylann bump |
|
🚧 @mountiny 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! 🧪🧪
|
|
Can we revert #87394? It causes bad regression Screen.Recording.2026-04-10.at.11.26.19.pm.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-04-10.at.11.40.14.pm.moviOS: mWeb SafariMacOS: Chrome / Safari87179.mov87197.mov87199.mov87200.mov87201.mov87209.mov |
Regression tests87179.mov87197.mov87199.mov87200.mov87201.mov87209.mov |
PR ReviewOverall: This looks correct and well-structured. The mutation fixes follow a consistent clone-on-write pattern across all three derived config files. What the PR does
Review of the mutation fixesAll three files follow the same pattern: shallow-clone the top-level object from
No issues found. |
| @@ -347,8 +350,13 @@ export default createOnyxDerivedValueConfig({ | |||
| continue; | |||
| } | |||
|
|
|||
| reportAttributes[chatReportID].brickRoadStatus = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR; | |||
| reportAttributes[chatReportID].actionBadge = CONST.REPORT.ACTION_BADGE.FIX; | |||
| // Clone the entry before mutating — it may be a reference carried over from | |||
| // currentValue.reports that wasn't recomputed in this incremental run. | |||
| reportAttributes[chatReportID] = { | |||
| ...reportAttributes[chatReportID], | |||
| brickRoadStatus: CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR, | |||
| actionBadge: CONST.REPORT.ACTION_BADGE.FIX, | |||
There was a problem hiding this comment.
For easy code review, these are only the changes in this file.
Others are prettier diff.
|
Codex Review: Didn't find any major issues. Swish! ℹ️ 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". |
mountiny
left a comment
There was a problem hiding this comment.
Thanks for the review, I will move this ahead as we have bunch of onyx changes to try to push through so dont want to waste any time
|
🚧 @mountiny 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.59-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required. This PR is purely technical — it bumps the Onyx dependency to 3.0.58 and fixes internal data mutation bugs in three Onyx Derived config files. There are no user-facing feature changes, UI changes, renamed settings, or workflow changes that would affect help site documentation. |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.3.59-4 🚀
|
Explanation of Change
This PR bumps Onyx to 3.0.58, which includes the following changes:
Additionally we are fixing several cases where we were mutating Onyx data in Onyx Derived files. This should never be allowed and the bugs only surfaced now because of the Onyx PR above. These were the deploy blockers associated:
Fixed Issues
$ #86181
$ #87209
$ #87201
$ #87179
$ #87200
$ #87197
$ #87199
PROPOSAL:
Tests
#87209 - Expense report loads infinitely after creating expense
Steps:
Expected: Expense report shows expense details after creation
#87201 - Second expense doesn't auto-appear in preview
Steps:
Expected: Second expense auto-appears in expense preview immediately
#87179 - Report preview doesn't update after adding expense
Steps:
Expected: Report preview updates in real-time (updated total, newly added expense)
#87200 - Deleted expense appears as skeleton loader
Steps:
Expected: Deleted expense does not appear in the expense preview
#87197 - Expense preview blank after splitting expense
Steps:
Expected: Expense preview shows two expense previews after splitting
#87199 - "Not here" page after bulk deleting expenses
Steps:
Expected: "Not here" page does not open after deleting expenses
Offline tests
N/A
QA Steps
Same as Tests.
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
Screen.Recording.2026-04-09.at.11.23.12-compressed.mov
Android: mWeb Chrome
N/A, emulator always crashing for me
iOS: Native
Screen.Recording.2026-04-09.at.11.40.22-compressed.mov
iOS: mWeb Safari
Screen.Recording.2026-04-09.at.11.44.42-compressed.mov
MacOS: Chrome / Safari
Screen.Recording.2026-04-09.at.11.50.27-compressed.mov
Screen.Recording.2026-04-09.at.11.52.48-compressed.mov