You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kr...@apache.org on 2019/04/01 20:37:49 UTC

[incubator-superset] branch master updated: fix: do not crash when a vis type is disabled. (#7180)

This is an automated email from the ASF dual-hosted git repository.

kristw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 2af9a84  fix: do not crash when a vis type is disabled. (#7180)
2af9a84 is described below

commit 2af9a84d09c81eea89a61d402abe259c77850e89
Author: Krist Wongsuphasawat <kr...@gmail.com>
AuthorDate: Mon Apr 1 13:37:40 2019 -0700

    fix: do not crash when a vis type is disabled. (#7180)
    
    * fix: make vis type control handle unregistered type
    
    * fix: hide when not needed
    
    * fix: do not try to read controlpanelconfig for invalid type
    
    * feat: add t
---
 .../explore/components/ControlPanelsContainer.jsx  |   4 +-
 .../explore/components/controls/VizTypeControl.jsx | 113 ++++++++++-----------
 superset/assets/src/explore/controlPanels/index.js |  36 +++----
 superset/assets/src/explore/store.js               |   4 +-
 4 files changed, 73 insertions(+), 84 deletions(-)

diff --git a/superset/assets/src/explore/components/ControlPanelsContainer.jsx b/superset/assets/src/explore/components/ControlPanelsContainer.jsx
index 9cc74f2..db2e212 100644
--- a/superset/assets/src/explore/components/ControlPanelsContainer.jsx
+++ b/superset/assets/src/explore/components/ControlPanelsContainer.jsx
@@ -58,8 +58,8 @@ class ControlPanelsContainer extends React.Component {
     let mapF = controls[controlName].mapStateToProps;
 
     // Looking to find mapStateToProps override for this viz type
-    const config = controlPanelConfigs[this.props.controls.viz_type.value].controlOverrides;
-    const controlOverrides = config || {};
+    const config = controlPanelConfigs[this.props.controls.viz_type.value] || {};
+    const controlOverrides = config.controlOverrides || {};
     if (controlOverrides[controlName] && controlOverrides[controlName].mapStateToProps) {
       mapF = controlOverrides[controlName].mapStateToProps;
     }
diff --git a/superset/assets/src/explore/components/controls/VizTypeControl.jsx b/superset/assets/src/explore/components/controls/VizTypeControl.jsx
index b97ffb5..cf3c8c9 100644
--- a/superset/assets/src/explore/components/controls/VizTypeControl.jsx
+++ b/superset/assets/src/explore/components/controls/VizTypeControl.jsx
@@ -20,7 +20,8 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import {
   Label, Row, Col, FormControl, Modal, OverlayTrigger,
-  Tooltip } from 'react-bootstrap';
+  Tooltip,
+} from 'react-bootstrap';
 import { t } from '@superset-ui/translation';
 import { getChartMetadataRegistry } from '@superset-ui/chart';
 
@@ -39,6 +40,24 @@ const defaultProps = {
   onChange: () => {},
 };
 
+const registry = getChartMetadataRegistry();
+
+const IMAGE_PER_ROW = 6;
+const LABEL_STYLE = { cursor: 'pointer' };
+const DEFAULT_ORDER = [
+  'line', 'big_number', 'table', 'filter_box', 'dist_bar', 'area', 'bar',
+  'deck_polygon', 'pie', 'time_table', 'pivot_table', 'histogram',
+  'big_number_total', 'deck_scatter', 'deck_hex', 'time_pivot', 'deck_arc',
+  'heatmap', 'deck_grid', 'dual_line', 'deck_screengrid', 'line_multi',
+  'treemap', 'box_plot', 'separator', 'sunburst', 'sankey', 'word_cloud',
+  'mapbox', 'kepler', 'cal_heatmap', 'rose', 'bubble', 'deck_geojson',
+  'horizon', 'markup', 'deck_multi', 'compare', 'partition', 'event_flow',
+  'deck_path', 'directed_force', 'world_map', 'paired_ttest', 'para',
+  'iframe', 'country_map',
+];
+
+const typesWithDefaultOrder = new Set(DEFAULT_ORDER);
+
 export default class VizTypeControl extends React.PureComponent {
   constructor(props) {
     super(props);
@@ -51,71 +70,35 @@ export default class VizTypeControl extends React.PureComponent {
     this.setSearchRef = this.setSearchRef.bind(this);
     this.focusSearch = this.focusSearch.bind(this);
   }
+
   onChange(vizType) {
     this.props.onChange(vizType);
     this.setState({ showModal: false });
   }
+
   setSearchRef(searchRef) {
     this.searchRef = searchRef;
   }
+
   toggleModal() {
     this.setState({ showModal: !this.state.showModal });
   }
+
   changeSearch(event) {
     this.setState({ filter: event.target.value });
   }
+
   focusSearch() {
     if (this.searchRef) {
       this.searchRef.focus();
     }
   }
-  buildVizTypeLookup(types) {
-    const lookup = new Map();
-    types.forEach((type) => {
-      lookup.set(type.key, type);
-    });
-    return lookup;
-  }
-  sortVizTypes(types) {
-    const defaultOrder = [
-      'line', 'big_number', 'table', 'filter_box', 'dist_bar', 'area', 'bar',
-      'deck_polygon', 'pie', 'time_table', 'pivot_table', 'histogram',
-      'big_number_total', 'deck_scatter', 'deck_hex', 'time_pivot', 'deck_arc',
-      'heatmap', 'deck_grid', 'dual_line', 'deck_screengrid', 'line_multi',
-      'treemap', 'box_plot', 'separator', 'sunburst', 'sankey', 'word_cloud',
-      'mapbox', 'kepler', 'cal_heatmap', 'rose', 'bubble', 'deck_geojson',
-      'horizon', 'markup', 'deck_multi', 'compare', 'partition', 'event_flow',
-      'deck_path', 'directed_force', 'world_map', 'paired_ttest', 'para',
-      'iframe', 'country_map',
-    ];
-
-    const sorted = [];
-    const loadedKeys = new Set();
-    const vizTypeLookup = this.buildVizTypeLookup(types);
-
-    // Sort based on the `defaultOrder`
-    defaultOrder.forEach((key) => {
-      const vizType = vizTypeLookup.get(key);
-      if (typeof vizType !== 'undefined') {
-        sorted.push(vizType);
-        loadedKeys.add(key);
-      }
-    });
-
-    // Load the rest of Viz Types not mandated by the `defualtOrder`
-    types.forEach((vizType) => {
-      if (!loadedKeys.has(vizType.key)) {
-        sorted.push(vizType);
-        loadedKeys.add(vizType.key);
-      }
-    });
-
-    return sorted;
-  }
+
   renderItem(entry) {
     const { value } = this.props;
     const { key, value: type } = entry;
     const isSelected = key === value;
+
     return (
       <div
         className={`viztype-selector-container ${isSelected ? 'selected' : ''}`}
@@ -132,44 +115,50 @@ export default class VizTypeControl extends React.PureComponent {
         </div>
       </div>);
   }
+
   render() {
     const { filter, showModal } = this.state;
     const { value } = this.props;
 
-    const registry = getChartMetadataRegistry();
-    const types = this.sortVizTypes(registry.entries());
-    const filteredTypes = filter.length > 0
-      ? types.filter(type => type.value.name.toLowerCase().includes(filter.toLowerCase()))
-      : types;
-
-    const selectedType = registry.get(value);
+    const filterString = filter.toLowerCase();
+    const filteredTypes = DEFAULT_ORDER
+      .filter(type => registry.has(type))
+      .map(type => ({
+        key: type,
+        value: registry.get(type),
+      }))
+      .concat(registry.entries().filter(({ key }) => !typesWithDefaultOrder.has(key)))
+      .filter(entry => entry.value.name.toLowerCase().includes(filterString));
 
-    const imgPerRow = 6;
     const rows = [];
-    for (let i = 0; i <= filteredTypes.length; i += imgPerRow) {
+    for (let i = 0; i <= filteredTypes.length; i += IMAGE_PER_ROW) {
       rows.push(
         <Row key={`row-${i}`}>
-          {filteredTypes.slice(i, i + imgPerRow).map(entry => (
-            <Col md={12 / imgPerRow} key={`grid-col-${entry.key}`}>
+          {filteredTypes.slice(i, i + IMAGE_PER_ROW).map(entry => (
+            <Col md={12 / IMAGE_PER_ROW} key={`grid-col-${entry.key}`}>
               {this.renderItem(entry)}
             </Col>
           ))}
         </Row>);
     }
+
     return (
       <div>
-        <ControlHeader
-          {...this.props}
-        />
+        <ControlHeader {...this.props} />
         <OverlayTrigger
           placement="right"
           overlay={
-            <Tooltip id={'error-tooltip'}>{t('Click to change visualization type')}</Tooltip>
+            <Tooltip id="error-tooltip">{t('Click to change visualization type')}</Tooltip>
           }
         >
-          <Label onClick={this.toggleModal} style={{ cursor: 'pointer' }}>
-            {selectedType.name}
-          </Label>
+          <React.Fragment>
+            <Label onClick={this.toggleModal} style={LABEL_STYLE}>
+              {registry.has(value) ? registry.get(value).name : `${value}`}
+            </Label>
+            {(!registry.has(value) && <div className="text-danger">
+              <i className="fa fa-exclamation-circle text-danger" /> <small>{t('This visualization type is not supported.')}</small>
+            </div>)}
+          </React.Fragment>
         </OverlayTrigger>
         <Modal
           show={showModal}
diff --git a/superset/assets/src/explore/controlPanels/index.js b/superset/assets/src/explore/controlPanels/index.js
index 221c5cf..e663f54 100644
--- a/superset/assets/src/explore/controlPanels/index.js
+++ b/superset/assets/src/explore/controlPanels/index.js
@@ -122,32 +122,32 @@ export const controlPanelConfigs = extraOverrides({
   deck_polygon: DeckPolygon,
   deck_scatter: DeckScatter,
   deck_screengrid: DeckScreengrid,
-
 });
 
 export default controlPanelConfigs;
 
 export function sectionsToRender(vizType, datasourceType) {
-  const config = controlPanelConfigs[vizType];
+  const { sectionOverrides = {}, controlPanelSections = [] } = controlPanelConfigs[vizType] || {};
 
   const sectionsCopy = { ...sections };
-  if (config.sectionOverrides) {
-    Object.entries(config.sectionOverrides).forEach(([section, overrides]) => {
-      if (typeof overrides === 'object' && overrides.constructor === Object) {
-        sectionsCopy[section] = {
-          ...sectionsCopy[section],
-          ...overrides,
-        };
-      } else {
-        sectionsCopy[section] = overrides;
-      }
-    });
-  }
+
+  Object.entries(sectionOverrides).forEach(([section, overrides]) => {
+    if (typeof overrides === 'object' && overrides.constructor === Object) {
+      sectionsCopy[section] = {
+        ...sectionsCopy[section],
+        ...overrides,
+      };
+    } else {
+      sectionsCopy[section] = overrides;
+    }
+  });
+
+  const { datasourceAndVizType, sqlaTimeSeries, druidTimeSeries, filters } = sectionsCopy;
 
   return [].concat(
-    sectionsCopy.datasourceAndVizType,
-    datasourceType === 'table' ? sectionsCopy.sqlaTimeSeries : sectionsCopy.druidTimeSeries,
-    isFeatureEnabled(FeatureFlag.SCOPED_FILTER) ? sectionsCopy.filters : undefined,
-    config.controlPanelSections,
+    datasourceAndVizType,
+    datasourceType === 'table' ? sqlaTimeSeries : druidTimeSeries,
+    isFeatureEnabled(FeatureFlag.SCOPED_FILTER) ? filters : undefined,
+    controlPanelSections,
   ).filter(section => section);
 }
diff --git a/superset/assets/src/explore/store.js b/superset/assets/src/explore/store.js
index b62975b..be2be7b 100644
--- a/superset/assets/src/explore/store.js
+++ b/superset/assets/src/explore/store.js
@@ -65,7 +65,7 @@ export function getControlsState(state, form_data) {
 
   const controlNames = getControlNames(vizType, state.datasource.type);
 
-  const viz = controlPanelConfigs[vizType];
+  const viz = controlPanelConfigs[vizType] || {};
   const controlOverrides = viz.controlOverrides || {};
   const controlsState = {};
   controlNames.forEach((k) => {
@@ -120,7 +120,7 @@ export function getControlsState(state, form_data) {
 export function applyDefaultFormData(form_data) {
   const datasourceType = form_data.datasource.split('__')[1];
   const vizType = form_data.viz_type || 'table';
-  const viz = controlPanelConfigs[vizType];
+  const viz = controlPanelConfigs[vizType] || {};
   const controlNames = getControlNames(vizType, datasourceType);
   const controlOverrides = viz.controlOverrides || {};
   const formData = {};