-
Notifications
You must be signed in to change notification settings - Fork 14.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: FacePile is requesting avatars when SLACK_ENABLE_AVATARS is false #30156
fix: FacePile is requesting avatars when SLACK_ENABLE_AVATARS is false #30156
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #30156 +/- ##
===========================================
+ Coverage 60.48% 70.56% +10.07%
===========================================
Files 1931 1976 +45
Lines 76236 79485 +3249
Branches 8568 9054 +486
===========================================
+ Hits 46114 56085 +9971
+ Misses 28017 21171 -6846
- Partials 2105 2229 +124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM, thanks for the 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.
Thanks for the improvement, this is better than the solution I had -> the backend would disregard the call and return a 204 when the flag was disabled https://github.com/apache/superset/blob/master/superset/views/users/api.py#L147-L164
Better to not call at all in the first place though! I didn't know an easy way to get access to the FF from the component...
SUMMARY
#27849 introduced the Slack Avatar integration which augments the
FacePile
component with avatar images. This PR fixes an issue where the avatar images were being requested even whenSLACK_ENABLE_AVATARS
isFalse
, generating unnecessary requests.TESTING INSTRUCTIONS
Check that avatar images are not requested if
SLACK_ENABLE_AVATARS
isFalse
.ADDITIONAL INFORMATION