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 46579b1 Refactor out controlUtils.js module + unit tests (#7350)
46579b1 is described below
commit 46579b1bd67b798f19e0242f997cd8df60843e40
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Tue Apr 30 11:45:06 2019 -0700
Refactor out controlUtils.js module + unit tests (#7350)
* [WiP]refactor out a controlUtils.js file
* unit tests
* add missing license
* Addressing comments
---
.../components/ControlPanelsContainer_spec.jsx | 3 +-
.../spec/javascripts/explore/controlUtils_spec.jsx | 164 +++++++++++++++++++++
.../assets/spec/javascripts/explore/store_spec.jsx | 66 +++++++++
.../explore/components/ExploreViewContainer.jsx | 2 +-
superset/assets/src/explore/controlUtils.js | 124 ++++++++++++++++
.../assets/src/explore/reducers/exploreReducer.js | 13 +-
.../assets/src/explore/reducers/getInitialState.js | 3 +-
superset/assets/src/explore/store.js | 127 +++-------------
superset/assets/src/visualizations/deckgl/utils.js | 3 +
9 files changed, 392 insertions(+), 113 deletions(-)
diff --git
a/superset/assets/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx
b/superset/assets/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx
index 842c5e2..cda5e0a 100644
---
a/superset/assets/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx
+++
b/superset/assets/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx
@@ -18,7 +18,8 @@
*/
import React from 'react';
import { shallow } from 'enzyme';
-import { getFormDataFromControls, defaultControls } from 'src/explore/store';
+import { defaultControls } from 'src/explore/store';
+import { getFormDataFromControls } from 'src/explore/controlUtils';
import { ControlPanelsContainer } from
'src/explore/components/ControlPanelsContainer';
import ControlPanelSection from 'src/explore/components/ControlPanelSection';
import * as featureFlags from 'src/featureFlags';
diff --git a/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx
b/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx
new file mode 100644
index 0000000..50f766a
--- /dev/null
+++ b/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx
@@ -0,0 +1,164 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getControlConfig,
+ getControlState,
+ getControlKeys,
+ applyMapStateToPropsToControl,
+} from '../../../src/explore/controlUtils';
+
+describe('controlUtils', () => {
+ const state = {
+ datasource: {
+ columns: [
+ 'a', 'b', 'c',
+ ],
+ metrics: [
+ { metric_name: 'first' },
+ { metric_name: 'second' },
+ ],
+ },
+ };
+
+ describe('getControlConfig', () => {
+ it('returns a valid spatial controlConfig', () => {
+ const spatialControl = getControlConfig('spatial', 'deck_grid');
+ expect(spatialControl.type).toEqual('SpatialControl');
+ expect(spatialControl.validators).toHaveLength(1);
+ });
+ it('overrides according to vizType', () => {
+ let control = getControlConfig('metric', 'line');
+ expect(control.type).toEqual('MetricsControl');
+ expect(control.validators).toHaveLength(1);
+
+ // deck_polygon overrides and removes validators
+ control = getControlConfig('metric', 'deck_polygon');
+ expect(control.type).toEqual('MetricsControl');
+ expect(control.validators).toHaveLength(0);
+ });
+ });
+
+ describe('getControlKeys', () => {
+
+ window.featureFlags = {
+ SCOPED_FILTER: false,
+ };
+
+ it('gets only strings, even when React components are in conf', () => {
+ const keys = getControlKeys('filter_box');
+ expect(keys.every(k => typeof k === 'string')).toEqual(true);
+ expect(keys).toHaveLength(16);
+ });
+ it('gets the right set of controlKeys for filter_box', () => {
+ const keys = getControlKeys('filter_box');
+ expect(keys.sort()).toEqual([
+ 'adhoc_filters',
+ 'cache_timeout',
+ 'datasource',
+ 'date_filter',
+ 'druid_time_origin',
+ 'filter_configs',
+ 'granularity',
+ 'instant_filtering',
+ 'show_druid_time_granularity',
+ 'show_druid_time_origin',
+ 'show_sqla_time_column',
+ 'show_sqla_time_granularity',
+ 'slice_id',
+ 'time_range',
+ 'url_params',
+ 'viz_type',
+ ]);
+ });
+ });
+
+ describe('applyMapStateToPropsToControl,', () => {
+ it('applies state to props as expected', () => {
+ let control = getControlConfig('all_columns', 'table');
+ control = applyMapStateToPropsToControl(control, state);
+ expect(control.options).toEqual(['a', 'b', 'c']);
+ });
+
+ it('removes the mapStateToProps key from the object', () => {
+ let control = getControlConfig('all_columns', 'table');
+ control = applyMapStateToPropsToControl(control, state);
+ expect(control.mapStateToProps).toBe(undefined);
+ });
+
+ });
+
+ describe('getControlState', () => {
+
+ it('to be function free', () => {
+ const control = getControlState('all_columns', 'table', state, ['a']);
+ expect(control.mapStateToProps).toBe(undefined);
+ expect(control.validators).toBe(undefined);
+ });
+
+ it('to fix multi with non-array values', () => {
+ const control = getControlState('all_columns', 'table', state, 'a');
+ expect(control.value).toEqual(['a']);
+ });
+
+ it('removes missing/invalid choice', () => {
+ let control = getControlState('stacked_style', 'area', state, 'stack');
+ expect(control.value).toBe('stack');
+
+ control = getControlState('stacked_style', 'area', state, 'FOO');
+ expect(control.value).toBe(null);
+ });
+
+ it('applies the default function for metrics', () => {
+ const control = getControlState('metrics', 'table', state);
+ expect(control.default).toEqual(['first']);
+ });
+
+ it('applies the default function for metric', () => {
+ const control = getControlState('metric', 'table', state);
+ expect(control.default).toEqual('first');
+ });
+
+ it('applies the default function, prefers count if it exists', () => {
+ const stateWithCount = {
+ ...state,
+ datasource: {
+ ...state.datasource,
+ metrics: [
+ { metric_name: 'first' },
+ { metric_name: 'second' },
+ { metric_name: 'count' },
+ ],
+ },
+ };
+ const control = getControlState('metrics', 'table', stateWithCount);
+ expect(control.default).toEqual(['count']);
+ });
+
+ });
+
+ describe('validateControl', () => {
+
+ it('validates the control, returns an error if empty', () => {
+ const control = getControlState('metric', 'table', state, null);
+ expect(control.validationErrors).toEqual(['cannot be empty']);
+ });
+
+ });
+
+});
diff --git a/superset/assets/spec/javascripts/explore/store_spec.jsx
b/superset/assets/spec/javascripts/explore/store_spec.jsx
new file mode 100644
index 0000000..4884ed1
--- /dev/null
+++ b/superset/assets/spec/javascripts/explore/store_spec.jsx
@@ -0,0 +1,66 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { applyDefaultFormData } from '../../../src/explore/store';
+
+describe('store', () => {
+
+ describe('applyDefaultFormData', () => {
+
+ window.featureFlags = {
+ SCOPED_FILTER: false,
+ };
+
+ it('applies default to formData if the key is missing', () => {
+ const inputFormData = {
+ datasource: '11_table',
+ viz_type: 'table',
+ };
+ let outputFormData = applyDefaultFormData(inputFormData);
+ expect(outputFormData.row_limit).toEqual(10000);
+
+ const inputWithRowLimit = {
+ ...inputFormData,
+ row_limit: 888,
+ };
+ outputFormData = applyDefaultFormData(inputWithRowLimit);
+ expect(outputFormData.row_limit).toEqual(888);
+ });
+
+ it('keeps null if key is defined with null', () => {
+ const inputFormData = {
+ datasource: '11_table',
+ viz_type: 'table',
+ row_limit: null,
+ };
+ const outputFormData = applyDefaultFormData(inputFormData);
+ expect(outputFormData.row_limit).toBe(null);
+ });
+
+ it('removes out of scope, or deprecated keys', () => {
+ const inputFormData = {
+ datasource: '11_table',
+ viz_type: 'table',
+ this_should_no_be_here: true,
+ };
+ const outputFormData = applyDefaultFormData(inputFormData);
+ expect(outputFormData.this_should_no_be_here).toBe(undefined);
+ });
+
+ });
+});
diff --git a/superset/assets/src/explore/components/ExploreViewContainer.jsx
b/superset/assets/src/explore/components/ExploreViewContainer.jsx
index 431f252..9e6647f 100644
--- a/superset/assets/src/explore/components/ExploreViewContainer.jsx
+++ b/superset/assets/src/explore/components/ExploreViewContainer.jsx
@@ -29,7 +29,7 @@ import SaveModal from './SaveModal';
import QueryAndSaveBtns from './QueryAndSaveBtns';
import { getExploreUrlAndPayload, getExploreLongUrl } from '../exploreUtils';
import { areObjectsEqual } from '../../reduxUtils';
-import { getFormDataFromControls } from '../store';
+import { getFormDataFromControls } from '../controlUtils';
import { chartPropShape } from '../../dashboard/util/propShapes';
import * as exploreActions from '../actions/exploreActions';
import * as saveModalActions from '../actions/saveModalActions';
diff --git a/superset/assets/src/explore/controlUtils.js
b/superset/assets/src/explore/controlUtils.js
new file mode 100644
index 0000000..65e8c47
--- /dev/null
+++ b/superset/assets/src/explore/controlUtils.js
@@ -0,0 +1,124 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import controlPanelConfigs, { sectionsToRender } from './controlPanels';
+import controls from './controls';
+
+export function getFormDataFromControls(controlsState) {
+ const formData = {};
+ Object.keys(controlsState).forEach((controlName) => {
+ formData[controlName] = controlsState[controlName].value;
+ });
+ return formData;
+}
+
+export function validateControl(control) {
+ const validators = control.validators;
+ if (validators && validators.length > 0) {
+ const validatedControl = { ...control };
+ const validationErrors = [];
+ validators.forEach((f) => {
+ const v = f(control.value);
+ if (v) {
+ validationErrors.push(v);
+ }
+ });
+ delete validatedControl.validators;
+ return { ...validatedControl, validationErrors };
+ }
+ return control;
+}
+
+export function getControlKeys(vizType, datasourceType) {
+ const controlKeys = [];
+ sectionsToRender(vizType, datasourceType).forEach(
+ section => section.controlSetRows.forEach(
+ fieldsetRow => fieldsetRow.forEach(
+ (field) => {
+ if (typeof field === 'string') {
+ controlKeys.push(field);
+ }
+ })));
+ return controlKeys;
+}
+
+export function getControlConfig(controlKey, vizType) {
+ // Gets the control definition, applies overrides, and executes
+ // the mapStatetoProps
+ const vizConf = controlPanelConfigs[vizType] || {};
+ const controlOverrides = vizConf.controlOverrides || {};
+ const control = {
+ ...controls[controlKey],
+ ...controlOverrides[controlKey],
+ };
+ return control;
+}
+
+export function applyMapStateToPropsToControl(control, state) {
+ if (control.mapStateToProps) {
+ const appliedControl = { ...control };
+ if (state) {
+ Object.assign(appliedControl, control.mapStateToProps(state, control));
+ }
+ delete appliedControl.mapStateToProps;
+ return appliedControl;
+ }
+ return control;
+}
+
+function handleMissingChoice(controlKey, control) {
+ // If the value is not valid anymore based on choices, clear it
+ const value = control.value;
+ if (
+ control.type === 'SelectControl' &&
+ !control.freeForm &&
+ control.choices &&
+ value
+ ) {
+ const alteredControl = { ...control };
+ const choiceValues = control.choices.map(c => c[0]);
+ if (control.multi && value.length > 0) {
+ alteredControl.value = value.filter(el => choiceValues.indexOf(el) > -1);
+ return alteredControl;
+ } else if (!control.multi && choiceValues.indexOf(value) < 0) {
+ alteredControl.value = null;
+ return alteredControl;
+ }
+ }
+ return control;
+}
+
+export function getControlState(controlKey, vizType, state, value) {
+ let controlValue = value;
+ const controlConfig = getControlConfig(controlKey, vizType);
+ let controlState = { ...controlConfig };
+ controlState = applyMapStateToPropsToControl(controlState, state);
+
+ // If default is a function, evaluate it
+ if (typeof controlState.default === 'function') {
+ controlState.default = controlState.default(controlState);
+ }
+
+ // If a choice control went from multi=false to true, wrap value in array
+ if (controlConfig.multi && value && !Array.isArray(value)) {
+ controlValue = [value];
+ }
+ controlState.value = controlValue === undefined ? controlState.default :
controlValue;
+ controlState = handleMissingChoice(controlKey, controlState);
+ return validateControl(controlState);
+}
diff --git a/superset/assets/src/explore/reducers/exploreReducer.js
b/superset/assets/src/explore/reducers/exploreReducer.js
index 6f6a1a9..44d5a72 100644
--- a/superset/assets/src/explore/reducers/exploreReducer.js
+++ b/superset/assets/src/explore/reducers/exploreReducer.js
@@ -17,8 +17,10 @@
* under the License.
*/
/* eslint camelcase: 0 */
-import { validateControl, getControlsState, getFormDataFromControls } from
'../store';
-import controls from '../controls';
+import {
+ getControlsState,
+} from '../store';
+import { getControlState, getFormDataFromControls } from '../controlUtils';
import * as actions from '../actions/exploreActions';
export default function exploreReducer(state = {}, action) {
@@ -78,11 +80,10 @@ export default function exploreReducer(state = {}, action) {
[actions.SET_FIELD_VALUE]() {
// These errors are reported from the Control components
let errors = action.validationErrors || [];
- let control = {
- ...controls[action.controlName],
- value: action.value,
+ const vizType = state.form_data.viz_type;
+ const control = {
+ ...getControlState(action.controlName, vizType, state, action.value),
};
- control = validateControl(control);
// These errors are based on control config `validators`
errors = errors.concat(control.validationErrors || []);
diff --git a/superset/assets/src/explore/reducers/getInitialState.js
b/superset/assets/src/explore/reducers/getInitialState.js
index 98b9799..892224d 100644
--- a/superset/assets/src/explore/reducers/getInitialState.js
+++ b/superset/assets/src/explore/reducers/getInitialState.js
@@ -20,7 +20,8 @@ import shortid from 'shortid';
import getToastsFromPyFlashMessages from
'../../messageToasts/utils/getToastsFromPyFlashMessages';
import { getChartKey } from '../exploreUtils';
-import { getControlsState, getFormDataFromControls } from '../store';
+import { getControlsState } from '../store';
+import { getFormDataFromControls } from '../controlUtils';
export default function getInitialState(bootstrapData) {
const controls = getControlsState(bootstrapData, bootstrapData.form_data);
diff --git a/superset/assets/src/explore/store.js
b/superset/assets/src/explore/store.js
index df456c2..6d58b8d 100644
--- a/superset/assets/src/explore/store.js
+++ b/superset/assets/src/explore/store.js
@@ -17,44 +17,13 @@
* under the License.
*/
/* eslint camelcase: 0 */
-import React from 'react';
+import {
+ getControlState,
+ getControlKeys,
+ getFormDataFromControls,
+} from './controlUtils';
import controls from './controls';
-import controlPanelConfigs, { sectionsToRender } from './controlPanels';
-
-export function getFormDataFromControls(controlsState) {
- const formData = {};
- Object.keys(controlsState).forEach((controlName) => {
- formData[controlName] = controlsState[controlName].value;
- });
- return formData;
-}
-
-export function validateControl(control) {
- const validators = control.validators;
- const validationErrors = [];
- if (validators && validators.length > 0) {
- validators.forEach((f) => {
- const v = f(control.value);
- if (v) {
- validationErrors.push(v);
- }
- });
- }
- if (validationErrors.length > 0) {
- return { ...control, validationErrors };
- }
- return control;
-}
-
-
-export function getControlNames(vizType, datasourceType) {
- const controlNames = [];
- sectionsToRender(vizType, datasourceType).forEach(
- section => section.controlSetRows.forEach(
- fsr => fsr.forEach(
- f => controlNames.push(f))));
- return controlNames;
-}
+import controlPanelConfigs from './controlPanels';
function handleDeprecatedControls(formData) {
// Reacffectation / handling of deprecated controls
@@ -66,104 +35,54 @@ function handleDeprecatedControls(formData) {
}
}
-export function getControlsState(state, form_data) {
+export function getControlsState(state, inputFormData) {
/*
* Gets a new controls object to put in the state. The controls object
* is similar to the configuration control with only the controls
* related to the current viz_type, materializes mapStateToProps functions,
- * adds value keys coming from form_data passed here. This can't be an action
creator
+ * adds value keys coming from inputFormData passed here. This can't be an
action creator
* just yet because it's used in both the explore and dashboard views.
* */
// Getting a list of active control names for the current viz
- const formData = Object.assign({}, form_data);
+ const formData = Object.assign({}, inputFormData);
const vizType = formData.viz_type || 'table';
handleDeprecatedControls(formData);
- const controlNames = getControlNames(vizType, state.datasource.type);
+ const controlNames = getControlKeys(vizType, state.datasource.type);
const viz = controlPanelConfigs[vizType] || {};
- const controlOverrides = viz.controlOverrides || {};
const controlsState = {};
- controlNames.forEach((k) => {
- if (React.isValidElement(k)) {
- // no state
- return;
- }
- const control = Object.assign({}, controls[k], controlOverrides[k]);
- if (control.mapStateToProps) {
- Object.assign(control, control.mapStateToProps(state, control));
- delete control.mapStateToProps;
- }
-
- formData[k] = (control.multi && formData[k] &&
!Array.isArray(formData[k])) ? [formData[k]]
- : formData[k];
-
- // If the value is not valid anymore based on choices, clear it
- if (
- control.type === 'SelectControl' &&
- !control.freeForm &&
- control.choices &&
- k !== 'datasource' &&
- formData[k]
- ) {
- const choiceValues = control.choices.map(c => c[0]);
- if (control.multi && formData[k].length > 0) {
- formData[k] = formData[k].filter(el => choiceValues.indexOf(el) > -1);
- } else if (!control.multi && choiceValues.indexOf(formData[k]) < 0) {
- delete formData[k];
- }
- }
- if (typeof control.default === 'function') {
- control.default = control.default(control);
- }
- control.validationErrors = [];
- control.value = control.default;
- // formData[k]'s type should match control value type
- if (formData[k] !== undefined &&
- (Array.isArray(formData[k]) && control.multi || !control.multi)
- ) {
- control.value = formData[k];
- }
- controlsState[k] = validateControl(control);
+ controlNames.forEach((k) => {
+ const control = getControlState(k, vizType, state, formData[k]);
+ controlsState[k] = control;
+ formData[k] = control.value;
});
+
if (viz.onInit) {
return viz.onInit(controlsState);
}
return controlsState;
}
-export function applyDefaultFormData(form_data) {
- const datasourceType = form_data.datasource.split('__')[1];
- const vizType = form_data.viz_type || 'table';
- const viz = controlPanelConfigs[vizType] || {};
- const controlNames = getControlNames(vizType, datasourceType);
- const controlOverrides = viz.controlOverrides || {};
+export function applyDefaultFormData(inputFormData) {
+ const datasourceType = inputFormData.datasource.split('__')[1];
+ const vizType = inputFormData.viz_type;
+ const controlNames = getControlKeys(vizType, datasourceType);
const formData = {};
controlNames.forEach((k) => {
- const control = Object.assign({}, controls[k]);
- if (controlOverrides[k]) {
- Object.assign(control, controlOverrides[k]);
- }
- if (form_data[k] === undefined) {
- if (typeof control.default === 'function') {
- formData[k] = control.default(controls[k]);
- } else {
- formData[k] = control.default;
- }
+ const controlState = getControlState(k, vizType, null, inputFormData[k]);
+ if (inputFormData[k] === undefined) {
+ formData[k] = controlState.value;
} else {
- formData[k] = form_data[k];
+ formData[k] = inputFormData[k];
}
});
return formData;
}
-export const autoQueryControls = [
- 'datasource',
- 'viz_type',
-];
const defaultControls = Object.assign({}, controls);
Object.keys(controls).forEach((f) => {
diff --git a/superset/assets/src/visualizations/deckgl/utils.js
b/superset/assets/src/visualizations/deckgl/utils.js
index b2b130a..62024ef 100644
--- a/superset/assets/src/visualizations/deckgl/utils.js
+++ b/superset/assets/src/visualizations/deckgl/utils.js
@@ -34,6 +34,9 @@ export function getBreakPoints({
// compute evenly distributed break points based on number of buckets
const numBuckets = formDataNumBuckets ? parseInt(formDataNumBuckets, 10) :
DEFAULT_NUM_BUCKETS;
const [minValue, maxValue] = extent(features, accessor);
+ if (minValue === undefined) {
+ return [];
+ }
const delta = (maxValue - minValue) / numBuckets;
const precision = delta === 0
? 0
