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): Support default to first value in select filter #14869

Merged
merged 28 commits into from
Jun 7, 2021

Conversation

simcha90
Copy link
Contributor

@simcha90 simcha90 commented May 27, 2021

SUMMARY

This PR fixes support for default first value in Select native filter. It includes next logic:

  • if one of control panel items contains requiredFirst flag we cancel delay of native filters loading, delay charts loading until this filter will be initialized - we do assumption if filterState.value === undefined, it means that filter not initialized
  • if it has defaultToFirstValue option enabled - we save on filterState.value undefined value and on filter loading we on the fly put first value

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-05-27.at.16.39.38.mov

TESTING INSTRUCTIONS

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
Copy link
Member

@simcha90 this needs a rebase. Let's also break out the Select filter tests into a separate PR to make review easier.

@villebro villebro added the hold! On hold label May 28, 2021
@junlincc junlincc added dashboard:native-filters Related to the native filters of the Dashboard and removed hold! On hold labels May 28, 2021
� Conflicts:
�	superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
�	superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
�	superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@codecov
Copy link

codecov bot commented May 30, 2021

Codecov Report

Merging #14869 (9fbca09) into master (e2d6015) will increase coverage by 0.12%.
The diff coverage is 73.63%.

❗ Current head 9fbca09 differs from pull request most recent head 182ab90. Consider uploading reports for the commit 182ab90 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14869      +/-   ##
==========================================
+ Coverage   77.55%   77.68%   +0.12%     
==========================================
  Files         965      966       +1     
  Lines       49535    49566      +31     
  Branches     6268     6293      +25     
==========================================
+ Hits        38416    38503      +87     
+ Misses      10918    10861      -57     
- Partials      201      202       +1     
Flag Coverage Δ
javascript 72.62% <73.63%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx 70.03% <0.00%> (-0.55%) ⬇️
superset-frontend/src/dataMask/reducer.ts 70.83% <ø> (ø)
...tend/src/filters/components/Select/controlPanel.ts 58.33% <ø> (ø)
...et-frontend/src/filters/components/Select/types.ts 100.00% <ø> (ø)
...hboard/components/nativeFilters/FilterBar/state.ts 85.41% <50.00%> (-3.22%) ⬇️
...mponents/nativeFilters/FiltersConfigModal/utils.ts 67.50% <50.00%> (-0.45%) ⬇️
...c/filters/components/Select/SelectFilterPlugin.tsx 78.43% <71.42%> (-3.29%) ⬇️
...src/dashboard/components/DashboardBuilder/state.ts 73.17% <73.17%> (ø)
...d/components/DashboardBuilder/DashboardBuilder.tsx 90.90% <83.33%> (+3.81%) ⬆️
...nfigModal/FiltersConfigForm/getControlItemsMap.tsx 91.89% <83.33%> (-5.17%) ⬇️
... and 13 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 e2d6015...182ab90. Read the comment docs.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Some comments

� Conflicts:
�	superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
�	superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
� Conflicts:
�	superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
�	superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
�	superset-frontend/src/dashboard/components/nativeFilters/types.ts
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Two minor comments

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM

@simcha90 simcha90 merged commit 1fc0852 into apache:master Jun 7, 2021
@villebro villebro mentioned this pull request Jun 7, 2021
8 tasks
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jun 8, 2021
* upstream/master:
  fix(explore): Datepicker glitch on hover outside the modal (apache#15033)
  Add ming-height to empty tab (apache#14878)
  Remove nowrap (apache#14954)
  display all metric results in editor (apache#15031)
  feat: Add "is_select_query" method to base engine spec to make it possible to override it (apache#15013)
  fix(dashboard): custom css should be removed on unmount (apache#15025)
  feat(filter-box): hide druid options if druid not enabled (apache#14921)
  fix: adding additional configs and colors for queryHistory (apache#14995)
  chore: rename 'Source' to 'Database' for consistency (apache#15021)
  chore(ci): fix ci conflict (apache#15016)
  fix(native-filters): avoid double load on initialization (apache#15012)
  feat(native-filters): Support default to first value in select filter (apache#14869)
  docs: required information for OAuth2 configuration (apache#15010)
  Update index.mdx (apache#14990)
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
…apache#14869)

* fix:fix get permission function

* feat: add async filters support

* revert: revert ff

* feat: add async filters support

* fix: merge with master

* fix: remove tests

* lint: fix lint

* fix: fix CR notes

* fix: fix with master

* test: fix tests

* refactor: update logic for default first value

* fix: get requiredFirst

* fix: support instant

* docs: update text

* docs: fix comments

* docs: update texts
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
…apache#14869)

* fix:fix get permission function

* feat: add async filters support

* revert: revert ff

* feat: add async filters support

* fix: merge with master

* fix: remove tests

* lint: fix lint

* fix: fix CR notes

* fix: fix with master

* test: fix tests

* refactor: update logic for default first value

* fix: get requiredFirst

* fix: support instant

* docs: update text

* docs: fix comments

* docs: update texts
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
…apache#14869)

* fix:fix get permission function

* feat: add async filters support

* revert: revert ff

* feat: add async filters support

* fix: merge with master

* fix: remove tests

* lint: fix lint

* fix: fix CR notes

* fix: fix with master

* test: fix tests

* refactor: update logic for default first value

* fix: get requiredFirst

* fix: support instant

* docs: update text

* docs: fix comments

* docs: update texts
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.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 dashboard:native-filters Related to the native filters of the Dashboard size/L 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants