Guard recently-used Onyx lists against non-array values - #101025
Guard recently-used Onyx lists against non-array values#101025MelvinBot wants to merge 1 commit into
Conversation
Co-authored-by: huult <huult@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
🤖 Android test steps verification results: ✅ pass Verified on standalone Android NewDot: created a manual expense, edited its amount, and changed its currency mid-edit — all succeeded with no crash, error page, or JS runtime error in the app logs. Note on scope: the documented Tests section only asks to verify no JS console errors, because the original crash needs a malformed One unrelated quirk observed: when typing the initial amount, moving focus straight from Amount to Merchant reset the Amount field to empty. This looks like an input-focus/commit behavior, not a crash, and is unrelated to this PR. view run · no recording available |
|
🤖 Web test steps verification results: ✅ pass Verified on dev NewDot web: manual expense creation, amount edit, and currency edit through the edit-amount submit path all completed successfully with no crash or visible error, consistent with the Two caveats worth reading before you rely on this:
view run · no recording available |







Explanation of Change
mergePolicyRecentlyUsedCurrenciesguarded its input with??, which only replacesnullandundefined. The value comes from Onyx (nvp_recentlyUsedCurrencies), and the server can write it as an object rather than an array. A non-array passes the??guard, so the spread on the next line threwTypeError: n is not iterableand crashed the amount-edit page on submit. Theelsebranch had the same problem one line further down, where.sliceis not a function on a plain object.This replaces the nullish guard with an
Array.isArraycheck, matching the siblingmergePolicyRecentlyUsedCategoriestwelve lines above. Because both branches derive from the same local, the single change covers the spread and the.slice. The parameter type is widened toOnyxEntry<string[]>so the guard is not dead code to the compiler, which also makes the misleading?? []at theUpdateMoneyRequestcall site unnecessary.Four other "recently used" helpers read server-written Onyx lists through the same
??pattern and would throw the same error class, so they get the same shape guard: per diem destinations, policy recently-used tags, report field values, and recent attendees. For tags and report fields the outer?.only guarded the record — the inner array being spread was typed non-optional and unchecked.This stops the crash. It does not stop the bad write, which appears to originate server-side and needs a separate internal investigation.
Fixed Issues
$ #100842
PROPOSAL: #100842 (comment)
Tests
// TODO: The human co-author must fill out the tests they ran before marking this PR as "ready for review".
The crash needs a malformed
nvp_recentlyUsedCurrenciesvalue in Onyx, so it does not reproduce from a clean account. The automated coverage added here istests/unit/MergePolicyRecentlyUsedCurrenciesTest.ts, which passes{0: 'USD', 2: 'EUR'}through both the with-currency and without-currency branches.Offline tests
// TODO: The human co-author must fill out the offline test steps before marking this PR as "ready for review".
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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