add validation rule for VisualizeModal

- add check for requiresDimension
- add check for requiresAggregationFn
- use chart types defined from visTypes.js

superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx

@@ -9,14 +9,12 @@ import Select from 'react-select';
 import { Table } from 'reactable';
 import shortid from 'shortid';
 import { getExploreUrl } from '../../explore/exploreUtils';
+import { visTypes } from '../../explore/stores/visTypes';
 import * as actions from '../actions';
 
-const CHART_TYPES = [
-  { value: 'dist_bar', label: 'Distribution - Bar Chart', requiresTime: false, requiresMatrix: true },
-  { value: 'pie', label: 'Pie Chart', requiresTime: false, requiresMatrix: true },
-  { value: 'line', label: 'Time Series - Line Chart', requiresTime: true, requiresMatrix: false },
-  { value: 'bar', label: 'Time Series - Bar Chart', requiresTime: true, requiresMatrix: true },
-];
+const CHART_TYPES = Object.keys(visTypes)
+  .filter(key => ['bar', 'line', 'pie', 'dist_bar'].indexOf(key) > -1)
+  .map(key => Object.assign({}, visTypes[key], { value: key }));
 
 const propTypes = {
   actions: PropTypes.object.isRequired,
@@ -88,24 +86,34 @@ class VisualizeModal extends React.PureComponent {
     });
     if (this.state.chartType === null) {
       hints.push('Pick a chart type!');
-    } else if (this.state.chartType.requiresTime) {
-      let hasTime = false;
-      for (const colName in cols) {
-        const col = cols[colName];
-        if (col.hasOwnProperty('is_date') && col.is_date) {
-          hasTime = true;
+    } else {
+      if (this.state.chartType.requiresTime) {
+        let hasTime = false;
+        for (const colName in cols) {
+          const col = cols[colName];
+          if (col.hasOwnProperty('is_date') && col.is_date) {
+            hasTime = true;
+          }
+        }
+        if (!hasTime) {
+          hints.push(
+            'To use this chart type you need at least one column ' +
+            'flagged as a date');
         }
       }
-      if (!hasTime) {
-        hints.push(
-          'To use this chart type you need at least one column ' +
-          'flagged as a date');
+      if (this.state.chartType.requiresDimension) {
+        const hasMatrix = Object.keys(cols).filter(key => (cols[key].is_dim)).length > 0;
+        if (!hasMatrix) {
+          hints.push(
+            'To use this chart type you need at least one dimension');
+        }
       }
-    } else if (this.state.chartType.requiresMatrix) {
-      const hasMatrix = Object.keys(cols).filter(key => (cols[key].is_dim)).length > 0;
-      if (!hasMatrix) {
-        hints.push(
-          'To use this chart type you need at least one matrix');
+      if (this.state.chartType.requiresAggregationFn) {
+        const hasMatrix = Object.keys(cols).filter(key => (cols[key].agg)).length > 0;
+        if (!hasMatrix) {
+          hints.push(
+            'To use this chart type you need at least one aggregation function');
+        }
       }
     }
     this.setState({ hints });

superset/assets/javascripts/explore/stores/visTypes.js

@@ -72,9 +72,10 @@ export const sections = {
   ],
 };
 
-const visTypes = {
+export const visTypes = {
   dist_bar: {
     label: 'Distribution - Bar Chart',
+    requiresDimension: true,
     controlPanelSections: [
       {
         label: 'Chart Options',
@@ -105,6 +106,7 @@ const visTypes = {
 
   pie: {
     label: 'Pie Chart',
+    requiresDimension: true,
     controlPanelSections: [
       {
         label: null,
@@ -179,6 +181,7 @@ const visTypes = {
   bar: {
     label: 'Time Series - Bar Chart',
     requiresTime: true,
+    requiresDimension: true,
     controlPanelSections: [
       sections.NVD3TimeSeries[0],
       {

superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx

@@ -3,6 +3,7 @@ import configureStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
 
 import { Modal } from 'react-bootstrap';
+import Select from 'react-select';
 import { shallow } from 'enzyme';
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
@@ -15,6 +16,7 @@ import { sqlLabReducer } from '../../../javascripts/SqlLab/reducers';
 import * as actions from '../../../javascripts/SqlLab/actions';
 import VisualizeModal from '../../../javascripts/SqlLab/components/VisualizeModal';
 import * as exploreUtils from '../../../javascripts/explore/exploreUtils';
+import { visTypes } from '../../../javascripts/explore/stores/visTypes';
 
 global.notify = {
   info: () => {},
@@ -44,18 +46,8 @@ describe('VisualizeModal', () => {
       type: 'STRING',
     },
   };
-  const mockChartTypeBarChart = {
-    label: 'Distribution - Bar Chart',
-    requiresTime: false,
-    requiresMatrix: true,
-    value: 'dist_bar',
-  };
-  const mockChartTypeTB = {
-    label: 'Time Series - Bar Chart',
-    requiresTime: true,
-    requiresMatrix: false,
-    value: 'bar',
-  };
+  const mockChartTypeBarChart = visTypes.bar;
+  const mockChartTypeTB = visTypes.line;
   const mockEvent = {
     target: {
       value: 'mock event value',
@@ -193,7 +185,7 @@ describe('VisualizeModal', () => {
       wrapper.instance().validate();
       expect(wrapper.state().hints).to.have.length(1);
     });
-    it('should check matrix', () => {
+    it('should check dimension', () => {
       // no is_dim
       columnsStub.returns({
         ds: {
@@ -325,4 +317,19 @@ describe('VisualizeModal', () => {
       datasourceSpy.restore();
     });
   });
+
+  describe('render', () => {
+    const wrapper = getVisualizeModalWrapper();
+    expect(wrapper.find(Modal)).to.have.length(1);
+
+    const selectorOptions = wrapper.find(Modal).dive()
+      .find(Modal.Body).dive()
+      .find(Select)
+      .props().options;
+    expect(selectorOptions).to.have.length(4);
+
+    const selectorOptionsValues =
+      Object.keys(selectorOptions).map(key => selectorOptions[key].value);
+    expect(selectorOptionsValues).to.have.same.members(['bar', 'line', 'pie', 'dist_bar']);
+  });
 });