[dashboard] clean up usage for old filter immune metadata #9146
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9146 +/- ##
=========================================
+ Coverage 59.06% 59.1% +0.03%
=========================================
Files 372 372
Lines 11922 11915 -7
Branches 2919 2917 -2
=========================================
Hits 7042 7042
+ Misses 4698 4693 -5
+ Partials 182 180 -2
Continue to review full report at Codecov.
|
superset/views/core.py
Outdated
| if "filter_scopes" in data: | ||
| md.pop("filter_immune_slices", None) | ||
| md.pop("filter_immune_slice_fields", None) | ||
| md["filter_scopes"] = json.loads(data.get("filter_scopes", "{}")) |
There was a problem hiding this comment.
| md["filter_scopes"] = json.loads(data.get("filter_scopes", "{}")) | |
| md["filter_scopes"] = json.loads(data["filter_scopes"]) |
not yours, but might as well clean this up
There was a problem hiding this comment.
there might be many dashboards holding NULL value in data.get('filter_scopes').
There was a problem hiding this comment.
Since, this is inside the if statement if "filter_scopes" in data:, we don't need to use .get because we know the key is there. For the case you mentioned, I think this should be:
md["filter_scopes"] = json.loads(data["filter_scopes"] or "{}")For explanation,
data = {"filter_scopes": None}
print(data.get("filter_scopes", "{}")) # prints NoneThere was a problem hiding this comment.
you are right! added fix. thank you!
graceguo-supercat
force-pushed
the
gg-SimplifyDashboardInit
branch
from
d61f942
to
f365f4f
Compare
mistercrunch
added
🏷️ bot
CATEGORY
Choose one
SUMMARY
After migration/convert old filter immune attribute in #9109 and #9145, dashboard front-end doesn't have to handle backward compatibility issue with old metadata. This PR is to clean up the usage for
filter_immune_slicesandfilter_immune_slice_fieldseverywhere.TEST PLAN
CI and manual tests.
ADDITIONAL INFORMATION
REVIEWERS
@john-bodley @serenajiang @etr2460 @mistercrunch