[adhoc-filters] Adding adhoc-filters to all viz types #5206

john-bodley · 2018-06-15T00:53:50Z

So this PR came about because I was working on another feature which required a database migration to update SQL filters when I realized that I needed to update multiple form-data fields, specifically the following fields exist around filters:

adhoc_filters
filters
having
having_filters
where

Hence the purpose of this PR is to condense the number of form-data fields and simplify filter logic by rolling out ad-hoc filters for all visualization types.

Note I sense it's worthwhile to first outline how the current situation works. On the front-end the ad-hoc filters take the legacy filters, having, having_filters, and where fields which are then merged with adhoc_filters. The adhoc_filters persist in the URL form-data (and when the slice is saved) though are transformed (and removed) back into the legacy filters in the backend. This means that all five filters types can actually persist in the form-data.

This PR is the first of a two-phase change (to help minimize code change). This change is mostly a front-end change, i.e., it applies ad-hoc filters to all visualization types and performs a database migration to upgrade the filters (as opposed to doing this in the frontend). Note the backend still transforms the ad-hoc filters to legacy filters (though the adhhoc_filters field persists). Although this represents a duplication of data the ad-hoc filters are the source of truth throughout, i.e., the legacy filters are always derived from the ad-hoc filters. Note this change ensures that old URLs with legacy filters will still work.

For phase II the plan is to clean up the backend and deprecate the legacy filters. It will ensure URLs with legacy filters will continue to work.

Finally the ad-hoc filters include a unique ID which is required for the frontend which gets persisted to the database. There may be merit in removing this in the future.

Closes #5172.

to: @betodealmeida @GabeLoins @michellethomas @mistercrunch @williaster

john-bodley · 2018-06-15T03:21:20Z

superset/utils.py

@@ -881,4 +881,3 @@ def split_adhoc_filters_into_base_filters(fd):
        fd['having'] = ' AND '.join(['({})'.format(sql) for sql in sql_having_filters])
        fd['having_filters'] = simple_having_filters
        fd['filters'] = simple_where_filters
-        del fd['adhoc_filters']


Note we can no longer remove the adhoc_filters field as we need to ensure that ad-hoc filters persist. Currently (for right or wrong) by deleting this field the ad-hoc filters actually get rebuilt on the frontend by merging the legacy and ad-hoc filters.

gabe-lyons

Looks good from my end! Thank you john for making this improvement!

gabe-lyons · 2018-06-18T17:35:42Z

superset/assets/backendSync.json

@@ -1683,18 +1683,6 @@
      "renderTrigger": true,
      "default": ""
    },
-    "where": {


I haven't touched this file before- what is it doing exactly?

That represents the old TextControl where people would write their WHERE clause. Did we even run a db migration to convert the old filters (where & having) to new ones?

Oh I see it's in this PR !

@mistercrunch I'm hoping to clean up the backend later this week so we can remove all references to the old where, having clauses entirely.

gabe-lyons · 2018-06-18T17:37:03Z

superset/assets/spec/javascripts/explore/components/AdhocFilterControl_spec.jsx

@@ -68,49 +59,6 @@ describe('AdhocFilterControl', () => {
    expect(wrapper.find(OnPasteSelect)).to.have.lengthOf(1);
  });

-  it('will translate legacy filters into adhoc filters if no adhoc filters are present', () => {


gabe-lyons · 2018-06-18T17:42:56Z

superset/assets/src/components/AlteredSliceTag.jsx

@@ -56,13 +60,15 @@ export default class AlteredSliceTag extends React.Component {
      return 'N/A';
    } else if (value === null) {
      return 'null';
-    } else if (controls[key] && controls[key].type === 'FilterControl') {
+    } else if (controls[key] && controls[key].type === 'AdhocFilterControl') {


was this line necessary before? Should I have already made this change or was it only necessary once we removed the legacy filters?

@GabeLoins I don't believe ad-hoc filters show up in the altered view. It merely worked if you did Run Query then these filters would be converted to the historical filters, clauses etc. and thus show in the the Altered slice view.

With this change the Altered slice view correctly represents the current UI state.

This reverts commit d483ed1.

#5253) * Revert "[sqllab] Fix sql lab resolution link (#5216)" This reverts commit 93cdf60. * Revert "Pin botocore version (#5184)" This reverts commit 70679d4. * Revert "Describe the use of custom OAuth2 authorization servers (#5220)" This reverts commit a84f430. * Revert "[bubble-chart] Fixing issue w/ metric names (#5237)" This reverts commit 5c106b9. * Revert "[adhoc-filters] Adding adhoc-filters to all viz types (#5206)" This reverts commit d483ed1. * Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (#5240)" This reverts commit 1fc4ee0.

apache#5253) * Revert "[sqllab] Fix sql lab resolution link (apache#5216)" This reverts commit 93cdf60. * Revert "Pin botocore version (apache#5184)" This reverts commit 70679d4. * Revert "Describe the use of custom OAuth2 authorization servers (apache#5220)" This reverts commit a84f430. * Revert "[bubble-chart] Fixing issue w/ metric names (apache#5237)" This reverts commit 5c106b9. * Revert "[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)" This reverts commit d483ed1. * Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (apache#5240)" This reverts commit 1fc4ee0. (cherry picked from commit 62427c8)

apache#5253) * Revert "[sqllab] Fix sql lab resolution link (apache#5216)" This reverts commit 93cdf60. * Revert "Pin botocore version (apache#5184)" This reverts commit 70679d4. * Revert "Describe the use of custom OAuth2 authorization servers (apache#5220)" This reverts commit a84f430. * Revert "[bubble-chart] Fixing issue w/ metric names (apache#5237)" This reverts commit 5c106b9. * Revert "[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)" This reverts commit d483ed1. * Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (apache#5240)" This reverts commit 1fc4ee0.

(cherry picked from commit d483ed1)

apache#5253) * Revert "[sqllab] Fix sql lab resolution link (apache#5216)" This reverts commit 93cdf60. * Revert "Pin botocore version (apache#5184)" This reverts commit 70679d4. * Revert "Describe the use of custom OAuth2 authorization servers (apache#5220)" This reverts commit a84f430. * Revert "[bubble-chart] Fixing issue w/ metric names (apache#5237)" This reverts commit 5c106b9. * Revert "[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)" This reverts commit d483ed1. * Revert "[perf] add webpack 4 + SplitChunks + lazy load visualizations (apache#5240)" This reverts commit 1fc4ee0.

john-bodley force-pushed the john-bodley-adhoc-filters-cleanup branch 10 times, most recently from a086903 to 1561cfb Compare June 15, 2018 03:20

john-bodley commented Jun 15, 2018

View reviewed changes

john-bodley force-pushed the john-bodley-adhoc-filters-cleanup branch 19 times, most recently from 181805b to 99e2832 Compare June 15, 2018 18:43

john-bodley force-pushed the john-bodley-adhoc-filters-cleanup branch 2 times, most recently from 5879d10 to 1a54eb4 Compare June 16, 2018 00:25

[adhoc-filters] Adding adhoc-filters to all viz types

c288ed0

john-bodley force-pushed the john-bodley-adhoc-filters-cleanup branch from 1a54eb4 to c288ed0 Compare June 16, 2018 16:27

gabe-lyons approved these changes Jun 18, 2018

View reviewed changes

john-bodley mentioned this pull request Jun 18, 2018

[sql] Correct SQL parameter formatting #5178

Merged

john-bodley merged commit d483ed1 into apache:master Jun 18, 2018

john-bodley added a commit that referenced this pull request Jun 20, 2018

Revert "[adhoc-filters] Adding adhoc-filters to all viz types (#5206)"

e197b3c

This reverts commit d483ed1.

graceguo-supercat mentioned this pull request Jul 24, 2018

[cherry-pick] migrate all dashboards airbnb/superset-fork#66

Merged

timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018

[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)

c43e1b6

graceguo-supercat pushed a commit to graceguo-supercat/superset that referenced this pull request Jul 26, 2018

[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)

abae53b

(cherry picked from commit d483ed1)

john-bodley mentioned this pull request Jul 31, 2018

[ad-hoc filters] Fixing issue with legacy filters #5525

Merged

wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018

[adhoc-filters] Adding adhoc-filters to all viz types (apache#5206)

2d96fbe

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.26.0 labels Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[adhoc-filters] Adding adhoc-filters to all viz types #5206

[adhoc-filters] Adding adhoc-filters to all viz types #5206

john-bodley commented Jun 15, 2018 •

edited

Loading

john-bodley Jun 15, 2018 •

edited

Loading

gabe-lyons left a comment

gabe-lyons Jun 18, 2018

mistercrunch Jun 18, 2018

mistercrunch Jun 18, 2018

john-bodley Jun 18, 2018

gabe-lyons Jun 18, 2018

gabe-lyons Jun 18, 2018

john-bodley Jun 18, 2018 •

edited

Loading

[adhoc-filters] Adding adhoc-filters to all viz types #5206

[adhoc-filters] Adding adhoc-filters to all viz types #5206

Conversation

john-bodley commented Jun 15, 2018 • edited Loading

john-bodley Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

gabe-lyons left a comment

Choose a reason for hiding this comment

gabe-lyons Jun 18, 2018

Choose a reason for hiding this comment

mistercrunch Jun 18, 2018

Choose a reason for hiding this comment

mistercrunch Jun 18, 2018

Choose a reason for hiding this comment

john-bodley Jun 18, 2018

Choose a reason for hiding this comment

gabe-lyons Jun 18, 2018

Choose a reason for hiding this comment

gabe-lyons Jun 18, 2018

Choose a reason for hiding this comment

john-bodley Jun 18, 2018 • edited Loading

Choose a reason for hiding this comment

john-bodley commented Jun 15, 2018 •

edited

Loading

john-bodley Jun 15, 2018 •

edited

Loading

john-bodley Jun 18, 2018 •

edited

Loading