-
Notifications
You must be signed in to change notification settings - Fork 14k
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 filter): clean deleted parent filter ids #24749
fix(native filter): clean deleted parent filter ids #24749
Conversation
Thanks @justinpark for the fix. I pushed a commit which includes a migration to remedy any corrupted dashboards. It would be good to verify said change in your environment. |
Codecov Report
@@ Coverage Diff @@
## master #24749 +/- ##
==========================================
+ Coverage 68.89% 68.93% +0.04%
==========================================
Files 1901 1901
Lines 73925 73925
Branches 8183 8182 -1
==========================================
+ Hits 50931 50963 +32
+ Misses 20873 20848 -25
+ Partials 2121 2114 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
bb96a73
to
08c9eca
Compare
08c9eca
to
b290341
Compare
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. Thank you for the fix and added tests!
Confirmed the migration script worked well and the corrupted cascadingParentIds has been cleaned. |
FYI @eschutho I wonder if you want to cherry-pick this into 2.1.1. Note there's a DB migration and thus you'll have to handle the version. |
Co-authored-by: John Bodley <john.bodley@gmail.com> (cherry picked from commit 4086514)
Co-authored-by: John Bodley <john.bodley@gmail.com> (cherry picked from commit 4086514)
Co-authored-by: John Bodley <john.bodley@gmail.com>
SUMMARY
When a parent filter has been removed, the cascadingParentIds has not updated accordingly.
This regression caused by the redux state mutation migration which remains
filterConfigMap
in obsolete dependency metadata because the redux action will updatestate.nativeFilters
whereasfilterConfigMap
references fromstate.dashboardInfo.metadata.native_filter_configuration
This commit updates the
cleanDeletedParents
logic to retrieve the updatedfilterConfigMap
data and then pass to the onSave payload in order to sync the latest updates.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After:
after--deleted-parent-id.mov
Before:
before--deleted-parent-id.mov
TESTING INSTRUCTIONS
Create a native filter and links a dependent filter
Hit save and then edit the native filter again
Remove the dependent filter only and then save
mouse over the filter has the dependent filter and see an error
ADDITIONAL INFORMATION