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(filter-box): hide druid options if druid not enabled #14921

Merged
merged 3 commits into from
Jun 7, 2021

Conversation

villebro
Copy link
Member

@villebro villebro commented May 31, 2021

SUMMARY

Currently the filter box control panel shows legacy Druid specific controls even when legacy Druid isn't enabled. This PR removes the Druid specific sections if legacy Druid is not enabled, and also moves the controls to their own rows. In addition, a new property is added to bootstrapData to list installed optional Python dependencies to be able to customize control panels that depend on this functionality (in this case this will be used to show/hide the forecast section in the ECharts timeseries plugin).

BEFORE

Currently the SQL and Druid sections labels overlap each other:
image

AFTER

With DRUID_IS_ACTIVE set to true:
image

With DRUID_IS_ACTIVE set to false:
image

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

@codecov
Copy link

codecov bot commented May 31, 2021

Codecov Report

Merging #14921 (8e0e088) into master (7f4e036) will increase coverage by 0.08%.
The diff coverage is 0.00%.

❗ Current head 8e0e088 differs from pull request most recent head 38b6de8. Consider uploading reports for the commit 38b6de8 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14921      +/-   ##
==========================================
+ Coverage   77.43%   77.52%   +0.08%     
==========================================
  Files         966      963       -3     
  Lines       49590    49331     -259     
  Branches     6293     6240      -53     
==========================================
- Hits        38402    38243     -159     
+ Misses      10986    10889      -97     
+ Partials      202      199       -3     
Flag Coverage Δ
javascript 72.42% <0.00%> (-0.21%) ⬇️

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

Impacted Files Coverage Δ
...tend/src/visualizations/FilterBox/controlPanel.jsx 0.00% <0.00%> (ø)
...src/filters/components/Range/RangeFilterPlugin.tsx 0.00% <0.00%> (-88.68%) ⬇️
...end/src/filters/components/Range/transformProps.ts 12.50% <0.00%> (-87.50%) ⬇️
...d/src/dashboard/components/gridComponents/Tabs.jsx 76.31% <0.00%> (-11.07%) ⬇️
...nd/src/dashboard/containers/DashboardComponent.jsx 84.84% <0.00%> (-7.46%) ⬇️
...et-frontend/src/dashboard/actions/nativeFilters.ts 76.59% <0.00%> (-6.10%) ⬇️
...nd/src/dashboard/components/nativeFilters/utils.ts 81.63% <0.00%> (-4.62%) ⬇️
...d/components/DashboardBuilder/DashboardBuilder.tsx 87.09% <0.00%> (-3.82%) ⬇️
...et-frontend/src/SqlLab/components/TableElement.jsx 89.02% <0.00%> (-2.98%) ⬇️
...ts/nativeFilters/FiltersConfigModal/FilterTabs.tsx 84.61% <0.00%> (-2.06%) ⬇️
... and 48 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 7f4e036...38b6de8. Read the comment docs.

@villebro villebro force-pushed the villebro/filterbox-controls branch from fccaa38 to 8e0e088 Compare May 31, 2021 10:51
@pull-request-size pull-request-size bot added size/M and removed size/L labels May 31, 2021
@@ -20,6 +20,37 @@ import React from 'react';
import { t } from '@superset-ui/core';
import { sections } from '@superset-ui/chart-controls';

const appContainer = document.getElementById('app');
const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
Copy link
Member

Choose a reason for hiding this comment

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

I think that we usually parse bootstrapped data in the main index file of a given view. Do you think we could move those 2 lines higher in the files hierarchy as well?

Copy link
Member

@michael-s-molina michael-s-molina Jun 1, 2021

Choose a reason for hiding this comment

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

I found this code in src/explore/index.jsx but I don't know if we are storingbootstrapData.common.conf somewhere...

const exploreViewContainer = document.getElementById('app');
const bootstrapData = JSON.parse(
  exploreViewContainer.getAttribute('data-bootstrap'),
);
initFeatureFlags(bootstrapData.common.feature_flags);

@junlincc junlincc added rush! Requires immediate attention #bug:blocking! Blocking issues with high priority and removed #bug:blocking! Blocking issues with high priority labels Jun 2, 2021
@yousoph
Copy link
Member

yousoph commented Jun 7, 2021

🏷️ 2021.21

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

LGTM, and manual tests/QA looks good after the last commit.

@rusackas rusackas merged commit 422c32c into apache:master Jun 7, 2021
@rusackas rusackas deleted the villebro/filterbox-controls branch June 7, 2021 19:50
henryyeh pushed a commit to preset-io/superset that referenced this pull request Jun 8, 2021
* feat(filter-box): hide druid options if druid not enabled

* add bootstrap export

(cherry picked from commit 422c32c)
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
* feat(filter-box): hide druid options if druid not enabled

* add bootstrap export
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
* feat(filter-box): hide druid options if druid not enabled

* add bootstrap export
@geido geido added explore:control Related to the controls panel of Explore and removed viz:explore:dynamic-control labels Feb 9, 2022
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* feat(filter-box): hide druid options if druid not enabled

* add bootstrap export
@mistercrunch mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label 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 explore:control Related to the controls panel of Explore preset:2021.21 preset-io rush! Requires immediate attention size/M 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants