You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/10/18 10:12:06 UTC

[GitHub] [superset] villebro opened a new pull request #17140: fix(filter-indicator): show filters handled by jinja as applied

villebro opened a new pull request #17140:
URL: https://github.com/apache/superset/pull/17140

### SUMMARY
Currently only legacy charts support identifying filters applied via jinja templates. However, this only supports the `filter_values` function, and is done using regex on the virtual table query. Therefore, filters applied via the `get_filters` function doesn't work on either v1 or legacy charts. In addition, this approach has the unlikely downside of false positives, if the regex would happen to match a parameter in the regular query, as it didn't check for opening/closing curly braces.

This PR replaces the regex-based identification method with a similar approach to how removed filters are currently identified. The applied filters are added to a list, which is made available to the code that is using the template processor. This way the collection of applied filters is automated and happens during template evaluation.

The changes proposed here are backwards compatible, and will display filters correctly when the cache is updated (for reviewers remember to force refresh the chart if caching is enabled).

### BEFORE
On a dashboard with a virtual dataset referencing the `filter_values` jinja function, we can see that the ECharts plugin which uses the v1 chart data endpoint is unable to identify the applied filter value correctly due to the column referenced not being available in the dataset. Notice, however, the NVD3 line chart on the right which identifies the filter correctly due to the regex in `viz.py`:
![image](https://user-images.githubusercontent.com/33317356/137710483-8689c36f-767e-40f6-84ed-b84b4b2b1fc0.png)

### AFTER
Now applied filters are identified correctly both on legacy and v1 charts:
![image](https://user-images.githubusercontent.com/33317356/137710388-892d9ccb-5a55-4929-94d0-40861710122a.png)

### TESTING INSTRUCTIONS
1. Enable the `ENABLE_TEMPLATE_PROCESSING` feature flag.
2. Create a virtual dataset with the following query: `select *, '{{ filter_values('gender', 'unset')[0] }}' as series from random_time_series`
3. Create a legacy and v1 chart referencing the new virtual dataset
4. Create a dashboard with both charts and add a native filter referencing the "gender" column on the "birth_names" dataset
5. Apply a filter, e.g. "boy"
6. See the indicators work on both the new and old chart

### ADDITIONAL INFORMATION
- [x] Has associated issue: closes #15209
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org