You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2018/12/23 02:06:41 UTC
[incubator-superset] branch master updated: [refactor] moving some
datasource-related code to the frontend (#5769)
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 <ma...@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: