-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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(native-filters): fix removing native filter #13688
Conversation
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, small non-blocking comment
export const getInitialMask = (id: string): MaskWithId => ({ | ||
id, | ||
extraFormData: {}, | ||
currentState: {}, | ||
}); |
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.
Is there a reason we're changing this to an arrow function? Personally I prefer to keep exported functions as vanilla functions vs arrows.
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 just preferred to use arrow functions to save order of functions declarations, but it's not critical, I can revert it
Codecov Report
@@ Coverage Diff @@
## master #13688 +/- ##
==========================================
- Coverage 77.40% 72.96% -4.44%
==========================================
Files 924 614 -310
Lines 46863 22039 -24824
Branches 5657 5867 +210
==========================================
- Hits 36272 16081 -20191
+ Misses 10455 5822 -4633
Partials 136 136
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
* fix: fix removeing native filter * fix: fix native-cross filters * fix: fix native-cross filters * fix: fix native-cross filters * fix: fix function declaration
SUMMARY
Fix next flow:
Actual: Charts not updated after second filter was removed
Expected: Charts should be updated after second filter was removed
Also it fix case that when native filters changed it was remove cross filters metadata
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION