fix: Red dot is displayed on report preview after dismissed the error messages#77115
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@abzokhattab 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] |
|
Thanks @nkdengineer for the quick PR. i just have one point. is it possible to add test cases for those util files to test the new functionality. |
trjExpensify
left a comment
There was a problem hiding this comment.
Pesky dots!
P.S Can you try to make your PR titles more reflective of what code changes the PR is making / fill out the explanation of changes section? Thanks!
|
Because there's no visible bug in those files, what do we need to test for them? |
|
That the new function is working as expected meaning its treating objects with null keys as empty ... we can just test the |
|
|
||
| export {isEmptyObject, getEmptyObject}; | ||
| function isEmptyValueObject<T>(obj: T | EmptyValue) { | ||
| return Object.values(obj ?? {}).filter(Boolean).length === 0; |
There was a problem hiding this comment.
Let’s make this check stricter than Boolean. Using filter(Boolean) treats 0 as falsy, so an object containing this values could be incorrectly considered “empty”. We should only treat values as empty when they’re null
What do u think?
There was a problem hiding this comment.
Agree, I updated.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-12-14.at.16.21.37.movAndroid: mWeb ChromeScreen.Recording.2025-12-14.at.16.24.37.moviOS: HybridAppScreen.Recording.2025-12-14.at.16.11.26.moviOS: mWeb SafariScreen.Recording.2025-12-14.at.16.13.32.movMacOS: Chrome / SafariScreen.Recording.2025-12-14.at.16.05.53.mov |
There was a problem hiding this comment.
@nkdengineer lets me know what u think about the comments above .. but other than this the changes are fixing the bug and look good to me
|
✋ 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/rlinoz in version: 9.2.79-0 🚀
|
|
@nkdengineer QA team faced this PR with an original issue on Native apps 1765946853140.77115A.mp4 |
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.2.81-0 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.81-5 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.81-5 🚀
|
|
@nkdengineer did you get a chance to check this one? #77115 (comment) |
Explanation of Change
Some
errorsobjects can have the key withnullvalue. This PR introducesisEmptyValueObjectutil to check whether anerrorsobject has a valid errorFixed Issues
$ #76522
PROPOSAL: #76522 (comment)
Tests
Offline tests
None
QA Steps
Same as test
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-12-09.at.14.49.43.mov
Android: mWeb Chrome
Screen.Recording.2025-12-09.at.14.48.12.mov
iOS: Native
Screen.Recording.2025-12-09.at.14.44.20.mov
iOS: mWeb Safari
Screen.Recording.2025-12-09.at.14.45.15.mov
MacOS: Chrome / Safari
Screen.Recording.2025-12-09.at.14.54.29.mov