-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix crash from AvatarWithIndicator error checking methods #10652
Conversation
|
src/libs/PolicyUtils.js
Outdated
* @returns {Boolean} | ||
*/ | ||
function hasPolicyMemberError(policyMemberList) { | ||
if (!policyMemberList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added #10650 to the list of issues this fixes. |
Thanks. Updated @vitHoracek and @tgolen. |
Updated again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for a quick fix 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that! LGTM
const cleanPolicies = _.filter(props.policies, policy => policy); | ||
const cleanPolicyMembers = _.filter(props.policiesMemberList, member => member); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering if using a compact which is a dedicated lodash function for this would not be better?? https://lodash.com/docs/4.17.15#compact
Or is there any of the falsey values which we need to account for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tim answered here https://expensify.slack.com/archives/C07J32337/p1661786311194409?thread_ts=1661782875.058629&cid=C07J32337 no problem to use filter then
Fix crash from AvatarWithIndicator error checking methods (cherry picked from commit 6989ea8)
…-10652 🍒 Cherry pick PR #10652 to staging 🍒
🚀 Cherry-picked to staging by @tgolen in version: 1.1.93-4 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
cc @vitHoracek
Details
It's possible that some of the policies in the the policies object are null, so return early in util methods if that's the case.
Fixed Issues
$ #10649
$ #10650
Tests
Create a workspace (if needed)
Delete a workspace
Make sure the app doesn't crash.
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
Same as tests
Screenshots
Web
Mobile Web
Desktop
iOS
Android