App crashes when feed search is invalid and card list is opened - #74366
Conversation
| const workspaceCardFeed = workspaceCardFeeds[getWorkspaceCardFeedKey(cardFeedKey)]; | ||
| if (!workspaceCardFeed) { | ||
| if (!cards || Object.keys(domainFeedCards).length === 0) { | ||
| if (!cards || Object.keys(domainFeedCards).length === 0 || !domainFeedCards[cardFeedKey]) { |
There was a problem hiding this comment.
❌ PERF-2 (docs)
The simple property check !domainFeedCards[cardFeedKey] should be performed before the expensive Object.keys(domainFeedCards).length === 0 operation. Object.keys() iterates over all properties to create an array, which is unnecessary if the specific key doesn't exist.
Suggested fix:
if (!cards || !domainFeedCards[cardFeedKey] || Object.keys(domainFeedCards).length === 0) {
return [];
}There was a problem hiding this comment.
@mkzie2 I think we can make this improvement. What do you think?
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
@mkzie2 could you please make sure to fill out the explanation of change section when raising PRs?
|
@joekaufmanexpensify I added the explanation of change. |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 10 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp74366-android-hybrid-001.mp4Android: mWeb Chrome74366-web-chrome-001.mp4iOS: HybridApp74366-ios-hybrid-001.mp4iOS: mWeb Safari74366-mweb-safari-001.mp4MacOS: Chrome / Safari74366-web-chrome-001.mp4MacOS: Desktop74366-desktop-001.mp4 |
rojiphil
left a comment
There was a problem hiding this comment.
Thanks @mkzie2 for the update
@stitesExpensify Code changes LGTM and works well too.
Over to you. Thanks.
|
@mkzie2 unit tests are failing but it looks unrelated to ours. Can you please merge with the latest main? |
|
@rojiphil All test passed. |
|
✋ 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/stitesExpensify in version: 9.2.47-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.2.47-1 🚀
|
Explanation of Change
Fix a bug where app crashes when open card filter in search page with invalid feed name.
Fixed Issues
$ #73529
PROPOSAL: #73529 (comment)
Tests
Precondition: Set up Expensify card feed and assign a card to yourself.
Offline tests
QA Steps
Precondition: Set up Expensify card feed and assign a card to yourself.
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
Screen.Recording.2025-11-06.at.01.12.35.mov
Android: mWeb Chrome
Screen.Recording.2025-11-06.at.01.13.34.mov
iOS: Native
Screen.Recording.2025-11-06.at.01.29.57.mov
iOS: mWeb Safari
Screen.Recording.2025-11-06.at.01.30.55.mov
MacOS: Chrome / Safari
Screen.Recording.2025-11-06.at.01.31.20.mov
MacOS: Desktop
Screen.Recording.2025-11-06.at.01.32.05.mov