feat(native-filters): select group by support

[amitmiran137]

SUMMARY

This new native filter allows the user to choose columns that he wants to group by all charts across the dashboard.
applying a groupBy is a very intrusive action , therefore it is up to the receiving plugin to implement the reaction

here is the table-plugin PR

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-04-18.at.11.47.59.mov

TEST PLAN

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • 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

[villebro]

Great stuff, looking forward to this! One quick comment

[codecov]

Codecov Report

Merging #14217 (3c405b5) into master (1448f78) will decrease coverage by 0.09%.
The diff coverage is 1.69%.

@@            Coverage Diff             @@
##           master   #14217      +/-   ##
==========================================
- Coverage   76.13%   76.04%   -0.10%     
==========================================
  Files         945      951       +6     
  Lines       47927    47985      +58     
  Branches     5950     5966      +16     
==========================================
  Hits        36491    36491              
- Misses      11230    11287      +57     
- Partials      206      207       +1     

Flag	Coverage Δ
javascript	70.06% <1.69%> (-0.19%)	⬇️

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

Impacted Files	Coverage Δ
...filters/components/GroupBy/GroupByFilterPlugin.tsx	0.00% <0.00%> (ø)
...ntend/src/filters/components/GroupBy/buildQuery.ts	0.00% <0.00%> (ø)
...end/src/filters/components/GroupBy/controlPanel.ts	0.00% <0.00%> (ø)
...t-frontend/src/filters/components/GroupBy/index.ts	0.00% <0.00%> (ø)
...d/src/filters/components/GroupBy/transformProps.ts	0.00% <0.00%> (ø)
...t-frontend/src/filters/components/GroupBy/types.ts	0.00% <0.00%> (ø)
superset-frontend/src/filters/components/index.ts	0.00% <0.00%> (ø)
...-frontend/src/visualizations/presets/MainPreset.js	0.00% <ø> (ø)
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx	83.72% <100.00%> (ø)
... and 5 more

---

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 1448f78...3c405b5. Read the comment docs.

[junlincc]

super valuable feature, functionalities LGTM! This could be considered as 'drilldown' by some definitions.
It could get fairly complicated when multiple filters are applied to different chart types on the same dashboard.
should this action be globally applied to the dashboard? what about charts without groupby columns?

• always sort by metrics 🟢
• query group-by always take precedence 🟢
• dashboard group-by value select order sets the column order 🟢
• when a native group-by column is the same as a query group-by and is added to the table, it doesn't affect original group-by order. 🟢

A couple of comments

1. Saved native groupby filters should be carried back to Explore view, with the warning sign, for consistency. Unless we want to change the existing behavior for all. 🌕 @villebro @ktmud @zhaoyongjie thoughts?
2. Add drag and drop in native filter for reordering select values. Since order actually matters now. 🌕

Screen.Recording.2021-04-19.at.10.31.28.PM.mov

[amitmiran137]

super valuable feature, functionalities LGTM! This could be considered as 'drilldown' by some definitions.
It could get fairly complicated when multiple filters are applied to different chart types on the same dashboard.
should this action be globally applied to the dashboard? what about charts without groupby columns?

• always sort by metrics 🟢
• query group-by always take precedence 🟢
• dashboard group-by value select order sets the column order 🟢
• when a native group-by column is the same as a query group-by and is added to the table, it doesn't affect original group-by order. 🟢

A couple of comments

1. Saved native groupby filters should be carried back to Explore view, with the warning sign, for consistency. Unless we want to change the existing behavior for all. 🌕 @villebro @ktmud @zhaoyongjie thoughts?
2. Add drag and drop in native filter for reordering select values. Since order actually matters now. 🌕

Screen.Recording.2021-04-19.at.10.31.28.PM.mov

cross filter scoping is already enforcing the scope of groupby , we should generify the naming here :
filters-->actions(or something)
cross (?) or interactive

[amitmiran137]

about comment number 1 , lets leave explore view pass params to a follow up discussion/PR

[amitmiran137]

2. @junlincc is there any drag and drop magician you know of they can easily kill this task on your side?

order of filter should be determine by a number order property in each native filter metadata and corresponding to the drag and drop actions

[villebro]

1. Saved native groupby filters should be carried back to Explore view, with the warning sign, for consistency. Unless we want to change the existing behavior for all. 🌕

We'd need to add some new fields to the QueryFormData schema to be able to relay information about which columns originated from a cross drilldown/groupby. That feature is beyond the scope of this PR.

2. Add drag and drop in native filter for reordering select values. Since order actually matters now. 🌕

As long as this component is based on AntD, it's going to be difficult to add DnD, as the AntD Select component doesn't natively support reordering.

cross filter scoping is already enforcing the scope of groupby , we should generify the naming here :
filters-->actions(or something)
cross (?) or interactive

Agreed, we need to finalize the naming here, it's difficult to talk about these features when we have many names for the same functionality.

[villebro]

Code LGTM and works well in testing, super cool feature! 🎉

[villebro]

Yes, we really need to address this at some point. I suggest once we do dynamic controls we make sure we can use the same control panel for the filter config modal and Explore.

[ktmud]

Hi, very cool feature! But I also had some doubts regarding priority. Why are we keep adding new features to the native filters? Shouldn't we be focusing on making it ready for prime time (unless we believe current state is already good enough)?

[junlincc]

Hi, very cool feature! But I also had some doubts regarding priority. Why are we keep adding new features to the native filters? Shouldn't we be focusing on making it ready for prime time (unless we believe current state is already good enough)?

Different organizations have different priorities and we are an open source project, so let's keep it open:).
Agreed that we should work on stability and optimization, especially we have 20+ issues identified and filed in the feature announcement issue thread. Native filter starts growing organically and the UI is falling out of shape.

since we are adding new dashboard interaction to charts one at a time, and the timeline of this project & Echarts project are highly intertwined, meaning we may not get to feature complete in any time soon. let's spend a cycle to clear some debt and polish before moving forward. Some known performance issues are also blocking another project - filterbox migration, led by Airbnb team.

Hope we can all reach an agreement to 'slow down' in development for a now or push experiment to a feature branch and shift focus to optimization.

(FYI, we are working on dashboard Drilldown in a separate branch.)

cc
@amitmiran137 @simcha90 @ktmud @graceguo-supercat @villebro @mistercrunch @mihir174

[graceguo-supercat]

Thank you @ktmud and @junlincc. Airbnb just started the project to migrate from old filter_box to new native dashboard filter. The first priority for us is to make sure that native filter supports all functionalities that filter_box did. Right now we found following features are not available in native filter:

• filter_box has Sort Metric
• filter_box can add adhoc filter which is to narrow down the options for a given column.

These missing features blocked us from moving towards native filter. Performance, which is another big concern, we can't even start to evaluate since our filter_box can't be tested in native filter yet.

If @amitmiran137 has some bandwidth, I really hope you can help us on feature parity, bug fixing, and perf improvement for the primary use. After the majority of users migrated to new filter component, we can enrich it with a solid foundation. Thank you!!

[rumbin]

It seems that this PR should be included since Superset 1.2, however, in 1.4.0 the Group By option is not selectable in the Filter Type dropdown. Am I missing a feature flag here?