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: Native filters fast-follow #12138

Merged
merged 243 commits into from
Dec 19, 2020

Conversation

rusackas
Copy link
Member

@rusackas rusackas commented Dec 19, 2020

SUMMARY

Adds onto #11814

This branch isn't my own work so much as a party! This is a feature branch containing the excellent work of:
@suddjian
@villebro 
@pkdotson 
@agatapst
@simchanielsen
@
amitmiran137

NEW STUFF! 🎉
• adjust styling to match design spec
• better filter delete UX
• cascading filters
• error boundaries
• non-instant filtering
• default values

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2020-12-18 at 11 54 18 PM

Screen Shot 2020-12-18 at 11 53 36 PM

Screen Shot 2020-12-18 at 11 53 22 PM

Screen Shot 2020-12-18 at 11 53 09 PM

TEST PLAN

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-io
Copy link

codecov-io commented Dec 19, 2020

Codecov Report

Merging #12138 (e450af0) into master (865beae) will decrease coverage by 2.55%.
The diff coverage is 46.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12138      +/-   ##
==========================================
- Coverage   65.82%   63.26%   -2.56%     
==========================================
  Files         988      990       +2     
  Lines       48702    48902     +200     
  Branches     4907     4963      +56     
==========================================
- Hits        32056    30937    -1119     
- Misses      16501    17768    +1267     
- Partials      145      197      +52     
Flag Coverage Δ
cypress ?
javascript 61.80% <46.72%> (-0.17%) ⬇️
python 64.20% <ø> (ø)

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

Impacted Files Coverage Δ
...nd/src/dashboard/components/nativeFilters/types.ts 100.00% <ø> (ø)
...hboard/components/nativeFilters/CascadePopover.tsx 22.64% <22.64%> (ø)
...c/dashboard/components/nativeFilters/FilterBar.tsx 53.60% <43.75%> (-6.87%) ⬇️
...ard/components/nativeFilters/FilterConfigModal.tsx 60.24% <46.59%> (-9.76%) ⬇️
...nd/src/dashboard/components/nativeFilters/utils.ts 57.89% <47.05%> (-4.18%) ⬇️
...ashboard/components/nativeFilters/ColumnSelect.tsx 51.51% <60.00%> (+1.51%) ⬆️
...oard/components/nativeFilters/FilterConfigForm.tsx 80.35% <81.25%> (+3.08%) ⬆️
...frontend/src/components/SupersetResourceSelect.tsx 68.96% <83.33%> (+3.58%) ⬆️
superset-frontend/src/utils/cacheWrapper.ts 100.00% <100.00%> (ø)
superset-frontend/src/explore/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
... and 145 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 865beae...e450af0. Read the comment docs.

*/
import { cacheWrapper } from 'src/utils/cacheWrapper';

describe('cacheWrapper', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice tests

const currentValue = filter.currentValue || [];

const activeChildren = children.flatMap(
childFilter => getActiveChildren(childFilter) || [],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool use of flatMap

</>
);

const activeFilters = getActiveChildren(filter) || [filter];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we calling these "active" filters? What exactly does that mean here?

@@ -149,9 +276,68 @@ export function FilterConfigModal({
);
}

function getParentFilters(id: string) {
return filterIds
.filter(filterId => filterId !== id && !removedFilters[filterId])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch keeping removed filters out

Copy link
Member

@suddjian suddjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parts I didn't write LGTM!

@junlincc
Copy link
Member

junlincc commented Dec 19, 2020

if the code looks good, let's merge first? we should test it we all other recent merged PRs

@rusackas
Copy link
Member Author

Reviewed the code and it LGTM!

@suddjian suddjian merged commit 4a471b8 into apache:master Dec 19, 2020
villebro added a commit to preset-io/superset that referenced this pull request Jan 7, 2021
* wip: filter create modal

* add a feature flag

* automatic changes to package lock

* wip

* filter sidebar and basic state management

* move create button to the sidebar

* first step for edit filterconfig

* partially fix tests...

* edits to types and comments

* respect feature flag on the filter sidebar

* add filterconfig form

* get input state working

* feat: tree filter scopes UI

* fix: turn on flag

* sticky filter bar

* stop preferring default export

* feat: finish filter scoping

* fix: under toggle

* fix: title

* fix: add licence

* refactor: update TS

* fix: fix on reopen modal + validation

* new filter bar menu

* adding, but commenting out, bulk scoping action

* adding some placeholder buttons and styles therefor

* feat: add filter chart

* add relative path to package.json

* update modal

* a little input styling... just getting warmed up

* Revert "feat: add filter chart"

This reverts commit b1302d3.

* Revert "add relative path to package.json"

This reverts commit 26a7b40.

* https package lock idk

* feat: add filter chart

* add relative path to package.json

* flexboxes all the way down

* dynamically generate groupby and datasource in select control

* big wip

* fix target column name

* no importing nonexistent things

* styles and name editing

* Add hook for retrieval of all filter states

* start with a new filter when clicking add filter

* handle removed filters gracefully

* fix incorrect default filter configuration

* add fields to useAllFilterState

* add redux for filterconfigs

* add support for native_filters

* remove consoles

* improve filter removal

* unbreak infinite loop

* basic sidebar toggling working!

* collapsing and menu working more smoothly

* linting

* make dataset and column inputs work

* save filter values properly

* add dashboard event for filter updates

* guarded

* apply filters properly

* fix schema

* making New Filter button a link

* gridunits ftw

* centering modal

* tis not a button anymore! nixing type.

* plus and collapse buttons instead of "more" menu

* updating full size filter icons

* adding icons to filter collapsing/expanding

* turning off animation, but leaving class-based animation css

* fix linting error

* fix native filters for legacy charts

* updates test

* no individual apply buttons

* fix bugs with filter config modal

* remove redundant code

* switch to the filter with validation errors on submit

* separate form validation

* switch config button from add to edit

* switch to the filter with validation errors on submit

* separate form validation

* switch config button from add to edit

* update tests

* oops forgot to add the fancy new useChangeEffect hook

* comments and code reorganization

* rename native_filters to extr_form_data and move hook

* disable native filters in viz selector

* add cascading

* implement new extra form data api

* cleanup

* updates tests

* bump npm packages

* fix bad merge on package.json + lock

* lint

* replace in and not in with uppercase

* lint

* lint

* lint

* lint

* bulk test fix

* Sort select input alphabetically

* Change type for sorting elements

* sleeker filter removal UX

* fix rest of unit tests

* make filter operators all uppercase

* Hide Filter bar when there are no filters

* Show edit button for dashboard owners only

* Add visible argument to filters toggle function to avoid future regression

* Improve Toggle filters bar function

* lint

* fix js lint + set createNewOnOpen

* Handle setting extra form data in Filter Bar instead of Filter Control

* Add Handle apply filter function to Apply button

* Allow applying changes instantly

* Fix types

* remove console logs

* fix package

* Add Error Boundary component to Filter bar and Filter Config Modal

* fix jest tests

* update native filters tests to pass

* reset cypress baseUrl

* remove unnecessary field

* Add Parent Filter input field to Config Modal

* Create Cascade Filter & display children filters

* Add Cascade Popover

* Display Filter value both in Filter Bar and in Cascade Popover

* Display the youngest filter value label in the Filter bar

* Add styles to Cascade Popover and filters

* Force to apply changes instantly for parent filters and refactor styles

* Show error for no cyclical hierarchy and refactor

* Add validation for parent filter to be applied instantly

* Add Error Boundary to Filter Config Modal

* cleanup: remove unused state fields

* move unrelated types to an appropriate location

* remove misplaced resource fetch error logic

* fix cascadeParentIds error

* fix cypress password

* initial attempt at fixing scope issue

* fix bad merge

* fix lint

* trying out makeApi for saving filters

* remove unused import

* fix test

* silence bad test

* Improve styling of Filter Config Modal

* Improve styles for whole native filters feature

* Add styles for active filter tab

* Fix text for scoping

* Clean up Filter Bar and Config Modal styles

* Remove fractional gridUnits. Change name for CheckboxFormItem. Add placeholder to Parent Filter select.

* Remove unnecessary button size for Config Modal

* add native-filter feat flag config

* oops fix here

* remove space

* Update superset-frontend/src/common/components/index.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts

Co-authored-by: Evan Rusackas <evan@preset.io>

* Add Cache Wrapper helper to avoid datasets requests deduplication

* Add license to new Cache Wrapper helper

* Add Cache Wrapper tests

* Fix expanding Filter Bar

* use styledMount in tests

* comment

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterConfigurationLink.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* address PR feedback

* fix package lock

* null guards

* Fix charts resizing

* removing emotion/react and going old school on css animation

* fxing css glitch on scoping disclaimer

* src paths.

* using gridUnits

* nixing unnecessary diamonds

* linting

* fix type errors

* Inverting collapsed icons... closer to data src selector design

* restoring feature flag to proper default setting

* missing condition

* fix tests

* patching test

* just a button

* flaky tests

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
Co-authored-by: Phillip Kelley-Dotson <pkelleydotson@yahoo.com>
Co-authored-by: Simcha Shats <simcha.shats@nielsen.com>
Co-authored-by: amitNielsen <amit.miran@nielsen.com>
Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>
Co-authored-by: Agata Stawarz-Pastewska <agata.stawarz-pastewska@polidea.com>
Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
villebro added a commit to preset-io/superset that referenced this pull request Jan 7, 2021
* wip: filter create modal

* add a feature flag

* automatic changes to package lock

* wip

* filter sidebar and basic state management

* move create button to the sidebar

* first step for edit filterconfig

* partially fix tests...

* edits to types and comments

* respect feature flag on the filter sidebar

* add filterconfig form

* get input state working

* feat: tree filter scopes UI

* fix: turn on flag

* sticky filter bar

* stop preferring default export

* feat: finish filter scoping

* fix: under toggle

* fix: title

* fix: add licence

* refactor: update TS

* fix: fix on reopen modal + validation

* new filter bar menu

* adding, but commenting out, bulk scoping action

* adding some placeholder buttons and styles therefor

* feat: add filter chart

* add relative path to package.json

* update modal

* a little input styling... just getting warmed up

* Revert "feat: add filter chart"

This reverts commit b1302d3.

* Revert "add relative path to package.json"

This reverts commit 26a7b40.

* https package lock idk

* feat: add filter chart

* add relative path to package.json

* flexboxes all the way down

* dynamically generate groupby and datasource in select control

* big wip

* fix target column name

* no importing nonexistent things

* styles and name editing

* Add hook for retrieval of all filter states

* start with a new filter when clicking add filter

* handle removed filters gracefully

* fix incorrect default filter configuration

* add fields to useAllFilterState

* add redux for filterconfigs

* add support for native_filters

* remove consoles

* improve filter removal

* unbreak infinite loop

* basic sidebar toggling working!

* collapsing and menu working more smoothly

* linting

* make dataset and column inputs work

* save filter values properly

* add dashboard event for filter updates

* guarded

* apply filters properly

* fix schema

* making New Filter button a link

* gridunits ftw

* centering modal

* tis not a button anymore! nixing type.

* plus and collapse buttons instead of "more" menu

* updating full size filter icons

* adding icons to filter collapsing/expanding

* turning off animation, but leaving class-based animation css

* fix linting error

* fix native filters for legacy charts

* updates test

* no individual apply buttons

* fix bugs with filter config modal

* remove redundant code

* switch to the filter with validation errors on submit

* separate form validation

* switch config button from add to edit

* switch to the filter with validation errors on submit

* separate form validation

* switch config button from add to edit

* update tests

* oops forgot to add the fancy new useChangeEffect hook

* comments and code reorganization

* rename native_filters to extr_form_data and move hook

* disable native filters in viz selector

* add cascading

* implement new extra form data api

* cleanup

* updates tests

* bump npm packages

* fix bad merge on package.json + lock

* lint

* replace in and not in with uppercase

* lint

* lint

* lint

* lint

* bulk test fix

* Sort select input alphabetically

* Change type for sorting elements

* sleeker filter removal UX

* fix rest of unit tests

* make filter operators all uppercase

* Hide Filter bar when there are no filters

* Show edit button for dashboard owners only

* Add visible argument to filters toggle function to avoid future regression

* Improve Toggle filters bar function

* lint

* fix js lint + set createNewOnOpen

* Handle setting extra form data in Filter Bar instead of Filter Control

* Add Handle apply filter function to Apply button

* Allow applying changes instantly

* Fix types

* remove console logs

* fix package

* Add Error Boundary component to Filter bar and Filter Config Modal

* fix jest tests

* update native filters tests to pass

* reset cypress baseUrl

* remove unnecessary field

* Add Parent Filter input field to Config Modal

* Create Cascade Filter & display children filters

* Add Cascade Popover

* Display Filter value both in Filter Bar and in Cascade Popover

* Display the youngest filter value label in the Filter bar

* Add styles to Cascade Popover and filters

* Force to apply changes instantly for parent filters and refactor styles

* Show error for no cyclical hierarchy and refactor

* Add validation for parent filter to be applied instantly

* Add Error Boundary to Filter Config Modal

* cleanup: remove unused state fields

* move unrelated types to an appropriate location

* remove misplaced resource fetch error logic

* fix cascadeParentIds error

* fix cypress password

* initial attempt at fixing scope issue

* fix bad merge

* fix lint

* trying out makeApi for saving filters

* remove unused import

* fix test

* silence bad test

* Improve styling of Filter Config Modal

* Improve styles for whole native filters feature

* Add styles for active filter tab

* Fix text for scoping

* Clean up Filter Bar and Config Modal styles

* Remove fractional gridUnits. Change name for CheckboxFormItem. Add placeholder to Parent Filter select.

* Remove unnecessary button size for Config Modal

* add native-filter feat flag config

* oops fix here

* remove space

* Update superset-frontend/src/common/components/index.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts

Co-authored-by: Evan Rusackas <evan@preset.io>

* Add Cache Wrapper helper to avoid datasets requests deduplication

* Add license to new Cache Wrapper helper

* Add Cache Wrapper tests

* Fix expanding Filter Bar

* use styledMount in tests

* comment

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* Update superset-frontend/src/dashboard/components/nativeFilters/FilterConfigurationLink.tsx

Co-authored-by: Evan Rusackas <evan@preset.io>

* address PR feedback

* fix package lock

* null guards

* Fix charts resizing

* removing emotion/react and going old school on css animation

* fxing css glitch on scoping disclaimer

* src paths.

* using gridUnits

* nixing unnecessary diamonds

* linting

* fix type errors

* Inverting collapsed icons... closer to data src selector design

* restoring feature flag to proper default setting

* missing condition

* fix tests

* patching test

* just a button

* flaky tests

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
Co-authored-by: Phillip Kelley-Dotson <pkelleydotson@yahoo.com>
Co-authored-by: Simcha Shats <simcha.shats@nielsen.com>
Co-authored-by: amitNielsen <amit.miran@nielsen.com>
Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>
Co-authored-by: Agata Stawarz-Pastewska <agata.stawarz-pastewska@polidea.com>
Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 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 size/XXL 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants