-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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 "undefined" error after editing a filter #14984
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14984 +/- ##
==========================================
+ Coverage 77.53% 77.62% +0.08%
==========================================
Files 965 965
Lines 49504 49509 +5
Branches 6259 6260 +1
==========================================
+ Hits 38384 38430 +46
+ Misses 10920 10879 -41
Partials 200 200
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Makes sense; LGTM + tested to work, thanks for the quick fix!
* upstream/master: (68 commits) fix typos (apache#14950) docs: fix custom oauth config (apache#14997) fix: apply template_params on external_metadata (apache#14996) fix: toggle fullscreen on the dashboard (apache#14979) feat(native-filters): add markers and number formatter + simple tests (apache#14981) fix(native-filters): Fix "undefined" error after editing a filter (apache#14984) fix(native-filters): remove implied fetch predicate (apache#14982) fix(native-filters): update cascaded filter state on change (apache#14980) fix(filter box): replace freeform where clause with ilike (apache#14900) refactor: Convert TableElement.jsx component from class to functional with hooks (apache#14830) fix: renamed sqllab filters to _filters (apache#14971) feat(native-filters): apply cascading without instant filtering (apache#14966) chore: bump superset-ui to 0.17.53 (apache#14968) fix(native-filters): cascading filters not rendering in tab (apache#14964) feat: add type_generic and is_dttm to table metadata (apache#14863) additional safeguard (apache#14953) feat: Adding FORCE_SSL as feature flag in config.py (apache#14934) feat(dashboard/native-filters): Hide filters out of scope of current tab (apache#14933) fix: time parser truncate to first day of year/month (apache#14945) fix: is_temporal should overwrite is_dttm (apache#14894) ...
…ache#14984) * fix(native-filters): Fix "undefined" error after editing a filter * Remove console log
…ache#14984) * fix(native-filters): Fix "undefined" error after editing a filter * Remove console log
…ache#14984) * fix(native-filters): Fix "undefined" error after editing a filter * Remove console log
SUMMARY
We calculate
chartsInScope
andtabsInScope
of native filters on first render and recalculate them only when filter's scope or dashboard's layout changes. When user edited a filter without changing it's scope,chartsInScope
andtabsInScope
were being set to undefined, which was causing an caused an error described in #14975.This PR fixes it by merging edited filter into the old filter rather than overriding it. Since the edited filter object has exactly the same fields as the old filter except
chartsInScope
andtabsInScope
, what really happens is that we override the old filter and addchartsInScope
andtabsInScope
fields.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: See linked issue
After:
https://user-images.githubusercontent.com/15073128/120797636-97c65800-c53c-11eb-96ac-8c34f35bb801.mov
TESTING INSTRUCTIONS
DASHBOARD_NATIVE_FILTERS
feature flagADDITIONAL INFORMATION
CC: @junlincc @villebro