Skip to content
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

feat: Adds support to multiple dependencies to the native filters #18793

Conversation

michael-s-molina
Copy link
Member

@michael-s-molina michael-s-molina commented Feb 17, 2022

SUMMARY

Adds support to multiple dependencies to the native filters. This will allow the user to create hierarchical and graph relationships among filters. The changes are the result of a series of interviews conducted by the design team with Superset users. Here's a summary of the changes:

  • Creates the "Values are dependent on other filters" section
  • Removes the "Filters is hierarchical" section
  • Adds indentation to the collapsable containers
  • Removes the hierarchical display of filters in the left panel of the modal
  • Adjusts the algorithm to validate circular dependencies on the fly
  • Remove cascading popovers from filter bar @kgabryje
  • Effectively uses the filter cards to display filter context
  • Adjusts RTL and Cypress the tests

It's important to note that major refactors are not in the scope at this stage. Our first objective is to deprecate FilterBox. We'll tackle code improvements in the next stage of the native filters revamp.

@kasiazjc @jess-dillard

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-02-24.at.1.43.48.PM.mov

TESTING INSTRUCTIONS

We need to test the whole module. Apart from the basic tests, some important behaviors that should be checked:

  • When selecting "Values are dependent on other filters" a value is automatically suggested and this value does not generate a circular dependency warning if possible.
  • The circular dependency checking should occur on the fly while the user is selecting the filter dependencies.
  • If a filter is removed or has its type changed to one different than "select", all references to this filter should display "(deleted or invalid filter)". After saving and reopening the modal, that reference should disappear.
  • Only filters of type "select" should be available to be referenced as a dependency.
  • All filters in scope should be displayed on the filter bar independently of their relationships.
  • The values of a filter should be affected by its dependencies when applicable.
  • The filter card context is correctly displayed and text is truncated as expected. Please refer to feat(native-filters): Implement filter cards #18874 to see the requirements.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • 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

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #18793 (0c988b0) into master (299b5dc) will decrease coverage by 0.04%.
The diff coverage is 51.89%.

❗ Current head 0c988b0 differs from pull request most recent head 493ea96. Consider uploading reports for the commit 493ea96 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18793      +/-   ##
==========================================
- Coverage   66.56%   66.52%   -0.05%     
==========================================
  Files        1641     1641              
  Lines       63495    63468      -27     
  Branches     6425     6441      +16     
==========================================
- Hits        42265    42221      -44     
- Misses      19550    19584      +34     
+ Partials     1680     1663      -17     
Flag Coverage Δ
javascript 51.28% <51.89%> (-0.10%) ⬇️

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

Impacted Files Coverage Δ
...perset-ui-core/src/chart/components/SuperChart.tsx 100.00% <ø> (ø)
...ts/nativeFilters/FilterBar/FilterControls/utils.ts 25.00% <ø> (-45.00%) ⬇️
...hboard/components/nativeFilters/FilterBar/utils.ts 76.92% <ø> (+10.25%) ⬆️
...tiveFilters/FiltersConfigModal/DraggableFilter.tsx 71.87% <ø> (ø)
...tiveFilters/FiltersConfigModal/FilterTitlePane.tsx 81.25% <ø> (ø)
...mponents/nativeFilters/FiltersConfigModal/state.ts 66.66% <ø> (ø)
superset-frontend/src/dashboard/styles.ts 100.00% <ø> (ø)
...filters/components/GroupBy/GroupByFilterPlugin.tsx 0.00% <0.00%> (ø)
...d/src/filters/components/GroupBy/transformProps.ts 0.00% <0.00%> (ø)
...t-frontend/src/filters/components/GroupBy/types.ts 100.00% <ø> (ø)
... and 50 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 299b5dc...493ea96. Read the comment docs.

@michael-s-molina michael-s-molina force-pushed the change-native-filter-relationships branch from c00753e to c6294b0 Compare February 18, 2022 13:01
@michael-s-molina michael-s-molina force-pushed the change-native-filter-relationships branch from c6294b0 to 7640f27 Compare February 18, 2022 13:29
@michael-s-molina michael-s-molina force-pushed the change-native-filter-relationships branch from 0f526c0 to cf7774d Compare February 22, 2022 16:51
@michael-s-molina michael-s-molina force-pushed the change-native-filter-relationships branch 2 times, most recently from 545ca32 to 5f4da63 Compare February 23, 2022 11:50
@michael-s-molina michael-s-molina force-pushed the change-native-filter-relationships branch 3 times, most recently from e63f4a2 to f80618b Compare February 23, 2022 11:58
@michael-s-molina michael-s-molina removed the request for review from craig-rueda February 23, 2022 11:59
@michael-s-molina michael-s-molina marked this pull request as draft February 23, 2022 12:00
@kgabryje kgabryje force-pushed the change-native-filter-relationships branch from 796d7d3 to 493ea96 Compare March 4, 2022 10:53
@michael-s-molina michael-s-molina merged commit 06e1e42 into apache:master Mar 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

Ephemeral environment shutdown and build artifacts deleted.

@villebro villebro added lts-v1 and removed hold:testing! On hold for testing labels Mar 29, 2022
villebro pushed a commit that referenced this pull request Apr 3, 2022
…8793)

* chore(native-filters): Remove cascading popovers from filter bar

Co-authored-by: Kamil Gabryjelski <[email protected]>
@mistercrunch mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels lts-v1 need:qa-review Requires QA review size/XXL 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants