fix(extra-filters): add logic for identifying applied extra filters #11325

villebro · 2020-10-19T07:34:41Z

SUMMARY

Adds support for applied_filters and rejected_filters to chart data endpoint + adds support for temporal extras to both viz.py and chart data endpoint.

Short summary of changes:

In the legacy data json_explore endpoint, temporal extras (__time_range etc) are applied to form_data in merge_extras (see: https://github.com/apache/incubator-superset/blob/735123d1f5513d366955db1865faf6b064c9bd4a/superset/utils/core.py#L922-L953 ), information of which temporal filters have been overridden is lost. To get around this, we add the applied temporal extras in a dict called applied_time_extras that we can later reference to determine which of them were applied and which were not.
since superset-ui/core merges temporal extras to the query object prior to sending the request, essentially performing the above operation (see: https://github.com/apache-superset/superset-ui/blob/master/packages/superset-ui-core/src/query/extractExtras.ts) , we need to pass along applied_time_extras in the request.

Please note: needs to upgrade to new version of superset-ui/core introduced here to function properly with v1 API plugins: apache-superset/superset-ui#809 .

SCREENSHOTS

In the below screenshot we can see that all temporal SQL filters are showing up as applied, while the druid ones aren't, as they aren't applicable to SQL datasources:

In the below screenshot we can see that all filters are flagged as incompatible, as the datasource doesn't have the gender column, nor any temporal SQL or Druid columns:

In this screenshot, we have both legacy (top row) and v1 (bottom row) viz plugins on the same dashboard. Notice that the filters apply similar to the legacy plugins:

TEST PLAN

CI with updated old tests + new E2E test

ADDITIONAL INFORMATION

* button translations missing * blank space before text

* feat: update time_compare description and choices * Update sections.jsx

villebro · 2020-10-19T07:46:24Z

superset/charts/schemas.py

+    applied_time_extras = fields.Dict(
+        description="A mapping of temporal extras that have been applied to the query",
+        required=False,
+        example={"__time_range": "1 year ago : now"},
+    )


I tried creating a dedicated schema with named properties (__time_range etc) and then adding a nested reference here, but that didn't work, probably due to the double underscores. Leaving these as a fields.Dict for now to get around the double underscore problem.

villebro · 2020-10-19T07:47:37Z

superset/charts/schemas.py

+    druid_time_origin = fields.String(
+        description="Starting point for time grain counting on legacy Druid "
+        "datasources. Used to change e.g. Monday/Sunday first-day-of-week. "
+        "This field is deprecated and should be passed to `extras` "
+        "as `druid_time_origin`.",
+        allow_none=True,
+    )


Turns out this was missing from the chart data schema. Also added to deprecated field mapping in query_object.py.

codecov-io · 2020-10-19T08:19:50Z

Codecov Report

Merging #11325 into feature/filter-p0 will increase coverage by 0.04%.
The diff coverage is 80.70%.

@@                  Coverage Diff                  @@
##           feature/filter-p0   #11325      +/-   ##
=====================================================
+ Coverage              60.78%   60.83%   +0.04%     
=====================================================
  Files                    393      393              
  Lines                  24848    24900      +52     
=====================================================
+ Hits                   15105    15147      +42     
- Misses                  9743     9753      +10

Flag	Coverage Δ
#python	`60.83% <80.70%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/views/dashboard/mixin.py	`95.00% <ø> (ø)`
superset/utils/core.py	`88.67% <73.17%> (-0.97%)`	⬇️
superset/charts/schemas.py	`100.00% <100.00%> (ø)`
superset/common/query_context.py	`87.80% <100.00%> (+0.46%)`	⬆️
superset/common/query_object.py	`90.90% <100.00%> (+0.16%)`	⬆️
superset/models/dashboard.py	`80.63% <100.00%> (ø)`
superset/viz.py	`57.74% <100.00%> (+0.04%)`	⬆️
superset/connectors/base/models.py	`89.78% <0.00%> (+0.35%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a123b24...1d63d1c. Read the comment docs.

villebro · 2020-10-19T09:44:02Z

superset-frontend/spec/javascripts/explore/components/QueryAndSaveBtns_spec.jsx

I'm unsure why GH is showing this line (and other similar ones) as changed, as I haven't touched them (the branch is based on apache:feature/filter-p0 and is up-to-date)

suddjian · 2020-10-19T16:37:36Z

superset/charts/schemas.py

    data = fields.List(fields.Dict(), description="A list with results")
+    applied_filters = fields.List(
+        fields.Dict(), description="A list with applied filters"
+    )


I forgot to add this, thank you!

suddjian · 2020-10-19T16:40:31Z

superset/models/dashboard.py


    def __repr__(self) -> str:
-        return f"Dashboard<{self.slug or self.id}>"
+        return f"Dashboard<{self.id or self.slug}>"


shouldn't this be the other way round? Is this one of those lines you didn't touch?

I guess if it's for the __repr__ function it makes more sense to use the id

I haven't changed these lines, I think this is GH acting up showing some changes that have come in after the branch was cut.

Huh. Maybe you merged master or something

suddjian

Thanks Ville!

* Initial commit of new filters badge. * refactor applied/rejected filters code * finished filter indicators * filter badge tested * unnecessary imports * formatting and types * fixes * license * code quality tweaks * state management for showing focused filter scope * clean up filter key extraction code * remove unnecessary styles * temp css to demonstrate highlighting * fix focused filter logic * no more color badges * new toys for highlighting dash components (#11144) * tweak style for the filter chart when filter is focused * style: Filters p0 css2 (#11151) * nixing background tweak * src paths * another quick theme color * src paths, adjusting pill icon color, changing icons, showing applied/busted counts * linting stuff * fixing and tweaking tests * show filter indicator when filters are not active * chart title bar cleanup * open the right panel when popover opens * unused import * fix EditableTitle tests * margin on dashboard header * show the chart dropdown menu * fix blur filter breaking dropdowns * style tweak - no pointer events when irrelevant charts are blurred * fix box shadow on filter highlight * it's an array * attempt fixing e2e * style: filters p0 icon churn (#11215) * new filters icon * icon styling * bigger icons in list views * better sizing of table actions and favStars * more icon sizing... * fixing more button size jankiness * linting * Filters performance (#11255) * fixing time filter "ok" button * making unset filter menu collapsible * sort alphabetically * fix highlighting when removing items * try a flex layout (for browser render perf) * more specific transitioning * temp: comment out some code as a test * temp: comment out more code * temp: remove possibly expensive computations from ChartHolder * Revert "temp: comment out some code as a test" This reverts commit 309b880. * Revert "temp: comment out more code" This reverts commit 64c88b2. * Revert "temp: remove possibly expensive computations from ChartHolder" This reverts commit 37ce021. * experiment: upgrade react-select to v3 * Revert "experiment: upgrade react-select to v3" This reverts commit c3972ba. * fix the damn problem * remove code used for testing purposes * awful hack to avoid adding a class to a container * approaching infinity... and not beyond! * fix ref forwarding * add theme to tests as necessary * fix(extra-filters): add logic for identifying applied extra filters (#11325) * fix: use dashboard id for stable cache key (#11293) * fix: button translations missing (#11187) * button translations missing * blank space before text * feat: update time_compare description and choices (#11294) * feat: update time_compare description and choices * Update sections.jsx * fix(extra-filters): add logic for identifying applied extra filters * lint Co-authored-by: Jesse Yang <[email protected]> Co-authored-by: rubenSastre <[email protected]> Co-authored-by: Erik Ritter <[email protected]> * address design feedback * slight tweak to panel logic, keep panels open that user has opened * rearrange code to be more graceful * fix: bump superset-ui/core (#11385) * use is_dttm instead of is_temporal * types, names * only show unset filter panel if there are unset filters * fix highlighting the filter control * fix filterbox layout * translations * fix cypress * actually add the test attribute * Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx Co-authored-by: Evan Rusackas <[email protected]> * formatting * add link comment to hack * Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx Co-authored-by: Evan Rusackas <[email protected]> * stop importing lodash * Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * skip broken test * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * adjust colors of titles * linting * no indicators when chart is loading * support all time fields * fix lock file Co-authored-by: Natalie Ruhe <[email protected]> Co-authored-by: Evan Rusackas <[email protected]> Co-authored-by: Ville Brofeldt <[email protected]> Co-authored-by: Jesse Yang <[email protected]> Co-authored-by: rubenSastre <[email protected]> Co-authored-by: Erik Ritter <[email protected]> Co-authored-by: Ville Brofeldt <[email protected]>

* Initial commit of new filters badge. * refactor applied/rejected filters code * finished filter indicators * filter badge tested * unnecessary imports * formatting and types * fixes * license * code quality tweaks * state management for showing focused filter scope * clean up filter key extraction code * remove unnecessary styles * temp css to demonstrate highlighting * fix focused filter logic * no more color badges * new toys for highlighting dash components (apache#11144) * tweak style for the filter chart when filter is focused * style: Filters p0 css2 (apache#11151) * nixing background tweak * src paths * another quick theme color * src paths, adjusting pill icon color, changing icons, showing applied/busted counts * linting stuff * fixing and tweaking tests * show filter indicator when filters are not active * chart title bar cleanup * open the right panel when popover opens * unused import * fix EditableTitle tests * margin on dashboard header * show the chart dropdown menu * fix blur filter breaking dropdowns * style tweak - no pointer events when irrelevant charts are blurred * fix box shadow on filter highlight * it's an array * attempt fixing e2e * style: filters p0 icon churn (apache#11215) * new filters icon * icon styling * bigger icons in list views * better sizing of table actions and favStars * more icon sizing... * fixing more button size jankiness * linting * Filters performance (apache#11255) * fixing time filter "ok" button * making unset filter menu collapsible * sort alphabetically * fix highlighting when removing items * try a flex layout (for browser render perf) * more specific transitioning * temp: comment out some code as a test * temp: comment out more code * temp: remove possibly expensive computations from ChartHolder * Revert "temp: comment out some code as a test" This reverts commit 309b880. * Revert "temp: comment out more code" This reverts commit 64c88b2. * Revert "temp: remove possibly expensive computations from ChartHolder" This reverts commit 37ce021. * experiment: upgrade react-select to v3 * Revert "experiment: upgrade react-select to v3" This reverts commit c3972ba. * fix the damn problem * remove code used for testing purposes * awful hack to avoid adding a class to a container * approaching infinity... and not beyond! * fix ref forwarding * add theme to tests as necessary * fix(extra-filters): add logic for identifying applied extra filters (apache#11325) * fix: use dashboard id for stable cache key (apache#11293) * fix: button translations missing (apache#11187) * button translations missing * blank space before text * feat: update time_compare description and choices (apache#11294) * feat: update time_compare description and choices * Update sections.jsx * fix(extra-filters): add logic for identifying applied extra filters * lint Co-authored-by: Jesse Yang <[email protected]> Co-authored-by: rubenSastre <[email protected]> Co-authored-by: Erik Ritter <[email protected]> * address design feedback * slight tweak to panel logic, keep panels open that user has opened * rearrange code to be more graceful * fix: bump superset-ui/core (apache#11385) * use is_dttm instead of is_temporal * types, names * only show unset filter panel if there are unset filters * fix highlighting the filter control * fix filterbox layout * translations * fix cypress * actually add the test attribute * Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx Co-authored-by: Evan Rusackas <[email protected]> * formatting * add link comment to hack * Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx Co-authored-by: Evan Rusackas <[email protected]> * stop importing lodash * Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * skip broken test * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <[email protected]> * adjust colors of titles * linting * no indicators when chart is loading * support all time fields * fix lock file Co-authored-by: Natalie Ruhe <[email protected]> Co-authored-by: Evan Rusackas <[email protected]> Co-authored-by: Ville Brofeldt <[email protected]> Co-authored-by: Jesse Yang <[email protected]> Co-authored-by: rubenSastre <[email protected]> Co-authored-by: Erik Ritter <[email protected]> Co-authored-by: Ville Brofeldt <[email protected]>

ktmud and others added 4 commits October 19, 2020 09:07

fix: use dashboard id for stable cache key (apache#11293)

5e66735

fix: button translations missing (apache#11187)

4c011cd

* button translations missing * blank space before text

feat: update time_compare description and choices (apache#11294)

78bd8e3

* feat: update time_compare description and choices * Update sections.jsx

fix(extra-filters): add logic for identifying applied extra filters

1a86007

superset-github-bot bot added the preset-io label Oct 19, 2020

pull-request-size bot added the size/L label Oct 19, 2020

villebro changed the title ~~Villebro/p0 extras~~ [WIP] fix(extra-filters): add logic for identifying applied extra filters Oct 19, 2020

villebro commented Oct 19, 2020

View reviewed changes

villebro changed the title ~~[WIP] fix(extra-filters): add logic for identifying applied extra filters~~ fix(extra-filters): add logic for identifying applied extra filters Oct 19, 2020

lint

1d63d1c

villebro force-pushed the villebro/p0-extras branch from 77b41f6 to 1d63d1c Compare October 19, 2020 10:13

villebro mentioned this pull request Oct 19, 2020

feat(core): add applied_time_extras to QueryObject apache-superset/superset-ui#809

Merged

suddjian reviewed Oct 19, 2020

View reviewed changes

suddjian approved these changes Oct 19, 2020

View reviewed changes

suddjian merged commit 53293e3 into apache:feature/filter-p0 Oct 19, 2020

villebro deleted the villebro/p0-extras branch October 19, 2020 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(extra-filters): add logic for identifying applied extra filters #11325

fix(extra-filters): add logic for identifying applied extra filters #11325

Uh oh!

villebro commented Oct 19, 2020 •

edited

Loading

Uh oh!

villebro Oct 19, 2020

Uh oh!

villebro Oct 19, 2020

Uh oh!

codecov-io commented Oct 19, 2020 •

edited

Loading

Uh oh!

villebro Oct 19, 2020 •

edited

Loading

Uh oh!

suddjian Oct 19, 2020

Uh oh!

suddjian Oct 19, 2020

Uh oh!

suddjian Oct 19, 2020

Uh oh!

villebro Oct 19, 2020

Uh oh!

suddjian Oct 19, 2020

Uh oh!

suddjian left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix(extra-filters): add logic for identifying applied extra filters #11325

fix(extra-filters): add logic for identifying applied extra filters #11325

Uh oh!

Conversation

villebro commented Oct 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

SCREENSHOTS

TEST PLAN

ADDITIONAL INFORMATION

Uh oh!

villebro Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

villebro Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Oct 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

villebro Oct 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suddjian Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

suddjian Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

suddjian Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

villebro Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

suddjian Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

suddjian left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

villebro commented Oct 19, 2020 •

edited

Loading

codecov-io commented Oct 19, 2020 •

edited

Loading

villebro Oct 19, 2020 •

edited

Loading