superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maximebeauche...@apache.org
Subject [incubator-superset] branch master updated: [refactor] moving some datasource-related code to the frontend (#5769)
Date Sun, 23 Dec 2018 02:06:41 GMT
This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin 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 fe77b57  [refactor] moving some datasource-related code to the frontend (#5769)
fe77b57 is described below

commit fe77b57581f00144b0c7301e6b6c4ca82aae9993
Author: Maxime Beauchemin <maximebeauchemin@gmail.com>
AuthorDate: Sat Dec 22 18:06:36 2018 -0800

    [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    | 24 ----------
 .../components/FixedOrMetricControl_spec.jsx       |  6 +--
 .../components/controls/FixedOrMetricControl.jsx   | 12 ++---
 superset/assets/src/explore/controls.jsx           | 53 ++++++++++++++--------
 superset/connectors/base/models.py                 | 22 ++-------
 tests/core_tests.py                                |  4 +-
 6 files changed, 48 insertions(+), 73 deletions(-)

diff --git a/superset/assets/spec/fixtures/mockDatasource.js b/superset/assets/spec/fixtures/mockDatasource.js
index 1de7915..1aa311c 100644
--- a/superset/assets/spec/fixtures/mockDatasource.js
+++ b/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',
   },
diff --git a/superset/assets/spec/javascripts/explore/components/FixedOrMetricControl_spec.jsx
b/superset/assets/spec/javascripts/explore/components/FixedOrMetricControl_spec.jsx
index e191c41..baff284 100644
--- a/superset/assets/spec/javascripts/explore/components/FixedOrMetricControl_spec.jsx
+++ b/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);
   });
 });
diff --git a/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx b/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx
index d330b2f..ef51788 100644
--- a/superset/assets/src/explore/components/controls/FixedOrMetricControl.jsx
+++ b/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}
diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx
index d34fc36..6dc2431 100644
--- a/superset/assets/src/explore/controls.jsx
+++ b/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],
   },
diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
index 6876fa0..6de8004 100644
--- a/superset/connectors/base/models.py
+++ b/superset/connectors/base/models.py
@@ -96,10 +96,6 @@ class BaseDatasource(AuditMixinNullable, ImportMixin):
         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])
 
@@ -134,14 +130,6 @@ class BaseDatasource(AuditMixinNullable, ImportMixin):
                 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"""
         return {
@@ -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,
diff --git a/tests/core_tests.py b/tests/core_tests.py
index 5b45cb8..886e92f 100644
--- a/tests/core_tests.py
+++ b/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:


Mime
View raw message