[refactor] moving some datasource-related code to the frontend (#5769)

* [refactor] moving some datasource-related code to the frontend

* fix js tests

* fix tests

* fix test

superset/assets/spec/fixtures/mockDatasource.js

@@ -20,7 +20,6 @@ export default {
       avg__num: 'avg__num',
       avg__sum_boys: 'avg__sum_boys',
     },
-    gb_cols: [['gender', 'gender'], ['name', 'name'], ['state', 'state']],
     metrics: [
       {
         expression: 'SUM(birth_names.num)',
@@ -160,20 +159,6 @@ export default {
       ['P1W', 'week'],
       ['P1M', 'month'],
     ],
-    filterable_cols: [
-      ['gender', 'gender'],
-      ['name', 'name'],
-      ['state', 'state'],
-    ],
-    all_cols: [
-      ['ds', 'ds'],
-      ['gender', 'gender'],
-      ['name', 'name'],
-      ['num', 'num'],
-      ['state', 'state'],
-      ['sum_boys', 'sum_boys'],
-      ['sum_girls', 'sum_girls'],
-    ],
     filter_select: true,
     order_by_choices: [
       ['["ds", true]', 'ds [asc]'],
@@ -191,15 +176,6 @@ export default {
       ['["sum_girls", true]', 'sum_girls [asc]'],
       ['["sum_girls", false]', 'sum_girls [desc]'],
     ],
-    metrics_combo: [
-      ['count', 'COUNT(*)'],
-      ['avg__num', 'avg__num'],
-      ['avg__sum_boys', 'avg__sum_boys'],
-      ['avg__sum_girls', 'avg__sum_girls'],
-      ['sum__num', 'sum__num'],
-      ['sum__sum_boys', 'sum__sum_boys'],
-      ['sum__sum_girls', 'sum__sum_girls'],
-    ],
     type: 'table',
     edit_url: '/tablemodelview/edit/7',
   },

superset/assets/spec/javascripts/explore/components/FixedOrMetricControl_spec.jsx

@@ -5,10 +5,10 @@ import { OverlayTrigger } from 'react-bootstrap';
 
 import FixedOrMetricControl from
   '../../../../src/explore/components/controls/FixedOrMetricControl';
-import SelectControl from
-  '../../../../src/explore/components/controls/SelectControl';
 import TextControl from
   '../../../../src/explore/components/controls/TextControl';
+import MetricsControl from
+  '../../../../src/explore/components/controls/MetricsControl';
 import ControlHeader from '../../../../src/explore/components/ControlHeader';
 
 const defaultProps = {
@@ -32,6 +32,6 @@ describe('FixedOrMetricControl', () => {
   it('renders a TextControl and a SelectControl', () => {
     const popOver = shallow(inst.renderPopover());
     expect(popOver.find(TextControl)).toHaveLength(1);
-    expect(popOver.find(SelectControl)).toHaveLength(1);
+    expect(popOver.find(MetricsControl)).toHaveLength(1);
   });
 });

superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx

@@ -2,9 +2,8 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { Label, Popover, OverlayTrigger } from 'react-bootstrap';
 
-import controls from '../../controls';
 import TextControl from './TextControl';
-import SelectControl from './SelectControl';
+import MetricsControl from './MetricsControl';
 import ControlHeader from '../ControlHeader';
 import PopoverSection from '../../../components/PopoverSection';
 
@@ -17,7 +16,7 @@ const propTypes = {
   onChange: PropTypes.func,
   value: PropTypes.object,
   isFloat: PropTypes.bool,
-  datasource: PropTypes.object,
+  datasource: PropTypes.object.isRequired,
   default: PropTypes.shape({
     type: PropTypes.oneOf(['fix', 'metric']),
     value: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
@@ -63,7 +62,6 @@ export default class FixedOrMetricControl extends React.Component {
   renderPopover() {
     const value = this.props.value || this.props.default;
     const type = value.type || controlTypes.fixed;
-    const metricsCombo = this.props.datasource ? this.props.datasource.metrics_combo : null;
     return (
       <Popover id="filter-popover">
         <div style={{ width: '240px' }}>
@@ -84,10 +82,10 @@ export default class FixedOrMetricControl extends React.Component {
             isSelected={type === controlTypes.metric}
             onSelect={() => { this.onChange(controlTypes.metric); }}
           >
-            <SelectControl
-              {...controls.metric}
+            <MetricsControl
               name="metric"
-              choices={metricsCombo}
+              datasource={this.state.datasource}
+              multi={false}
               onFocus={() => { this.setType(controlTypes.metric); }}
               onChange={this.setMetric}
               value={this.state.metricValue}

superset/assets/src/explore/controls.jsx

@@ -163,6 +163,15 @@ const jsFunctionInfo = (
   </div>
 );
 
+function columnChoices(datasource) {
+  if (datasource && datasource.columns) {
+    return datasource.columns
+      .map(col => [col.column_name, col.verbose_name || col.column_name])
+      .sort((opt1, opt2) => opt1[1].toLowerCase() > opt2[1].toLowerCase() ? 1 : -1);
+  }
+  return [];
+}
+
 function jsFunctionControl(label, description, extraDescr = null, height = 100, defaultText = '') {
   return {
     type: 'TextAreaControl',
@@ -609,7 +618,7 @@ export const controls = {
     validators: [v.nonEmpty],
     description: t('Point to your spatial columns'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -619,7 +628,7 @@ export const controls = {
     validators: [v.nonEmpty],
     description: t('Point to your spatial columns'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -629,7 +638,7 @@ export const controls = {
     validators: [v.nonEmpty],
     description: t('Point to your spatial columns'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -640,7 +649,7 @@ export const controls = {
     validators: [v.nonEmpty],
     description: t('Select the longitude column'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -651,7 +660,7 @@ export const controls = {
     validators: [v.nonEmpty],
     description: t('Select the latitude column'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -668,7 +677,17 @@ export const controls = {
     validators: [v.nonEmpty],
     description: t('Select the geojson column'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
+    }),
+  },
+
+  polygon: {
+    type: 'SelectControl',
+    label: t('Polygon Column'),
+    validators: [v.nonEmpty],
+    description: t('Select the polygon column. Each row should contain JSON.array(N) of [longitude, latitude] points'),
+    mapStateToProps: state => ({
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -697,7 +716,7 @@ export const controls = {
     default: null,
     description: t('Columns to display'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -707,7 +726,7 @@ export const controls = {
     default: null,
     description: t('Columns to display'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -1077,26 +1096,22 @@ export const controls = {
   },
 
   series: {
-    type: 'SelectControl',
+    ...groupByControl,
     label: t('Series'),
+    multi: false,
     default: null,
     description: t('Defines the grouping of entities. ' +
     'Each series is shown as a specific color on the chart and ' +
     'has a legend toggle'),
-    mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.gb_cols : [],
-    }),
   },
 
   entity: {
-    type: 'SelectControl',
+    ...groupByControl,
     label: t('Entity'),
     default: null,
+    multi: false,
     validators: [v.nonEmpty],
     description: t('This defines the element to be plotted on the chart'),
-    mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.gb_cols : [],
-    }),
   },
 
   x: {
@@ -1684,7 +1699,7 @@ export const controls = {
     'Non-numerical columns will be used to label points. ' +
     'Leave empty to get a count of points in each cluster.'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -1744,7 +1759,7 @@ export const controls = {
     'Either a numerical column or `Auto`, which scales the point based ' +
     'on the largest cluster'),
     mapStateToProps: state => ({
-      choices: [].concat([['Auto', 'Auto']], state.datasource.all_cols),
+      choices: columnChoices(state.datasource),
     }),
   },
 
@@ -2136,7 +2151,7 @@ export const controls = {
     default: null,
     description: t('The database columns that contains lines information'),
     mapStateToProps: state => ({
-      choices: (state.datasource) ? state.datasource.all_cols : [],
+      choices: columnChoices(state.datasource),
     }),
     validators: [v.nonEmpty],
   },

superset/connectors/base/models.py

@@ -95,10 +95,6 @@ class BaseDatasource(AuditMixinNullable, ImportMixin):
         """String representing the schema of the Datasource (if it applies)"""
         return None
 
-    @property
-    def groupby_column_names(self):
-        return sorted([c.column_name for c in self.columns if c.groupby])
-
     @property
     def filterable_column_names(self):
         return sorted([c.column_name for c in self.columns if c.filterable])
@@ -133,14 +129,6 @@ class BaseDatasource(AuditMixinNullable, ImportMixin):
                 metric.table_id = self.id
                 self.metrics += [metric]
 
-    @property
-    def metrics_combo(self):
-        return sorted(
-            [
-                (m.metric_name, m.verbose_name or m.metric_name or '')
-                for m in self.metrics],
-            key=lambda x: x[1])
-
     @property
     def short_data(self):
         """Data representation of the datasource sent to the frontend"""
@@ -193,18 +181,16 @@ class BaseDatasource(AuditMixinNullable, ImportMixin):
             'cache_timeout': self.cache_timeout,
             'params': self.params,
             'perm': self.perm,
+            'edit_url': self.url,
 
             # sqla-specific
             'sql': self.sql,
 
-            # computed fields
-            'all_cols': utils.choicify(self.column_names),
+            # one to many
             'columns': [o.data for o in self.columns],
-            'edit_url': self.url,
-            'filterable_cols': utils.choicify(self.filterable_column_names),
-            'gb_cols': utils.choicify(self.groupby_column_names),
             'metrics': [o.data for o in self.metrics],
-            'metrics_combo': self.metrics_combo,
+
+            # TODO deprecate, move logic to JS
             'order_by_choices': order_by_choices,
             'owners': [owner.id for owner in self.owners],
             'verbose_map': verbose_map,

tests/core_tests.py

@@ -536,8 +536,8 @@ class CoreTests(SupersetTestCase):
         )
         resp = self.get_json_resp(url)
         keys = [
-            'name', 'filterable_cols', 'gb_cols', 'type', 'all_cols',
-            'order_by_choices', 'metrics_combo', 'granularity_sqla',
+            'name', 'type',
+            'order_by_choices', 'granularity_sqla',
             'time_grain_sqla', 'id',
         ]
         for k in keys: