pt
/
superset
mirror of https://github.com/apache/superset.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor: separate vis-specific controls from centralized controls (#7569) 

* Separate vis-specific controls from centralized controls

* Remove comment

* Update state's vizType when switching to a new visualization. This fixes the renderTrigger bug.

* Rename functions for better readability

* Fix lint issues

* Fix nits

* Fix vscode rename mistake
This commit is contained in:
felixcodes 2019-08-23 13:41:35 -07:00 committed by Krist Wongsuphasawat
parent 610b35a01b
commit 6ca3e347d2
7 changed files with 174 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
superset/assets/spec/javascripts/explore/controlUtils_spec.jsx
View File

@ -16,10 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
import { t } from '@superset-ui/translation';
import {
getControlConfig,
getControlState,
getControlKeys,
applyMapStateToPropsToControl,
} from '../../../src/explore/controlUtils';
@ -42,6 +42,7 @@ describe('controlUtils', () => {
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');
@ -52,39 +53,20 @@ describe('controlUtils', () => {
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',
]);
it('returns correct control config when control config is defined ' +
'in the control panel definition', () => {
const roseAreaProportionControlConfig = getControlConfig('rose_area_proportion', 'rose');
expect(roseAreaProportionControlConfig).toEqual({
type: 'CheckboxControl',
label: t('Use Area Proportions'),
description: t(
'Check if the Rose Chart should use segment area instead of ' +
'segment radius for proportioning',
),
default: false,
renderTrigger: true,
});
});
});
@ -104,7 +86,6 @@ describe('controlUtils', () => {
});
describe('getControlState', () => {
it('to be function free', () => {
const control = getControlState('all_columns', 'table', state, ['a']);
expect(control.mapStateToProps).toBe(undefined);
@ -149,16 +130,12 @@ describe('controlUtils', () => {
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']);
});
});
});

102
superset/assets/src/explore/components/ControlPanelsContainer.jsx
View File

@ -22,14 +22,15 @@ import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import { Alert, Tab, Tabs } from 'react-bootstrap';
import { isPlainObject } from 'lodash';
import { t } from '@superset-ui/translation';
import controlPanelConfigs, { sectionsToRender } from '../controlPanels';
import ControlPanelSection from './ControlPanelSection';
import ControlRow from './ControlRow';
import Control from './Control';
import controls from '../controls';
import * as actions from '../actions/exploreActions';
import controlConfigs from '../controls';
import * as exploreActions from '../actions/exploreActions';
const propTypes = {
actions: PropTypes.object.isRequired,
@ -44,10 +45,13 @@ const propTypes = {
class ControlPanelsContainer extends React.Component {
constructor(props) {
super(props);
this.removeAlert = this.removeAlert.bind(this);
this.getControlData = this.getControlData.bind(this);
this.removeAlert = this.removeAlert.bind(this);
this.renderControl = this.renderControl.bind(this);
this.renderControlPanelSection = this.renderControlPanelSection.bind(this);
}
getControlData(controlName) {
if (React.isValidElement(controlName)) {
return controlName;
@ -55,7 +59,7 @@ class ControlPanelsContainer extends React.Component {
const control = this.props.controls[controlName];
// Identifying mapStateToProps function to apply (logic can't be in store)
let mapF = controls[controlName].mapStateToProps;
let mapF = controlConfigs[controlName].mapStateToProps;
// Looking to find mapStateToProps override for this viz type
const config = controlPanelConfigs[this.props.controls.viz_type.value] || {};
@ -69,19 +73,62 @@ class ControlPanelsContainer extends React.Component {
}
return control;
}
sectionsToRender() {
return sectionsToRender(this.props.form_data.viz_type, this.props.datasource_type);
}
removeAlert() {
this.props.actions.removeControlPanelAlert();
}
renderControl(name, config, lookupControlData) {
const { actions, controls, exploreState, form_data: formData } = this.props;
// Looking to find mapStateToProps override for this viz type
const controlPanelConfig = controlPanelConfigs[controls.viz_type.value].controlOverrides || {};
const overrides = controlPanelConfig[name];
// Identifying mapStateToProps function to apply (logic can't be in store)
const mapFn = (overrides && overrides.mapStateToProps)
? overrides.mapStateToProps
: config.mapStateToProps;
// If the control item is not an object, we have to look up the control data from
// the centralized controls file.
// When it is an object we read control data straight from `config` instead
const controlData = lookupControlData ?
controls[name] : config;
// Applying mapStateToProps if needed
const additionalProps = mapFn
? { ...controlData, ...mapFn(exploreState, controlData, actions) }
: controlData;
const { validationErrors, provideFormDataToProps } = controlData;
return (
<Control
name={name}
key={`control-${name}`}
value={formData[name]}
validationErrors={validationErrors}
actions={actions}
formData={provideFormDataToProps ? formData : null}
{...additionalProps}
/>
);
}
renderControlPanelSection(section) {
const ctrls = this.props.controls;
const { controls } = this.props;
const hasErrors = section.controlSetRows.some(rows => rows.some(s => (
ctrls[s] &&
ctrls[s].validationErrors &&
(ctrls[s].validationErrors.length > 0)
controls[s] &&
controls[s].validationErrors &&
(controls[s].validationErrors.length > 0)
)));
return (
<ControlPanelSection
key={section.label}
@ -94,21 +141,24 @@ class ControlPanelsContainer extends React.Component {
<ControlRow
key={`controlsetrow-${i}`}
className="control-row"
controls={controlSets.map((controlName) => {
if (!controlName) {
controls={controlSets.map((controlItem) => {
if (!controlItem) {
// When the item is invalid
return null;
} else if (React.isValidElement(controlName)) {
return controlName;
} else if (ctrls[controlName]) {
return (<Control
name={controlName}
key={`control-${controlName}`}
value={this.props.form_data[controlName]}
validationErrors={ctrls[controlName].validationErrors}
actions={this.props.actions}
formData={ctrls[controlName].provideFormDataToProps ? this.props.form_data : null}
{...this.getControlData(controlName)}
/>);
} else if (React.isValidElement(controlItem)) {
// When the item is a React element
return controlItem;
} else if (isPlainObject(controlItem) && controlItem.name && controlItem.config) {
const { name, config } = controlItem;
return this.renderControl(name, config, false);
} else if (controls[controlItem]) {
// When the item is string name, meaning the control config
// is not specified directly. Have to look up the config from
// centralized configs.
const name = controlItem;
return this.renderControl(name, controlConfigs[name], true);
}
return null;
})}
@ -124,10 +174,10 @@ class ControlPanelsContainer extends React.Component {
allSectionsToRender.forEach((section) => {
if (section.controlSetRows.some(rows => rows.some(
control => (
controls[control] &&
controlConfigs[control] &&
(
!controls[control].renderTrigger ||
controls[control].tabOverride === 'data'
!controlConfigs[control].renderTrigger ||
controlConfigs[control].tabOverride === 'data'
)
)))) {
querySectionsToRender.push(section);
@ -178,7 +228,7 @@ function mapStateToProps({ explore }) {
function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators(actions, dispatch),
actions: bindActionCreators(exploreActions, dispatch),
};
}

14
superset/assets/src/explore/controlPanels/Rose.js
View File

@ -29,7 +29,19 @@ export default {
controlSetRows: [
['color_scheme', 'label_colors'],
['number_format', 'date_time_format'],
['rich_tooltip', 'rose_area_proportion'],
['rich_tooltip', {
name: 'rose_area_proportion',
config: {
type: 'CheckboxControl',
label: t('Use Area Proportions'),
description: t(
'Check if the Rose Chart should use segment area instead of ' +
'segment radius for proportioning',
),
default: false,
renderTrigger: true,
},
}],
],
},
NVD3TimeSeries[1],

53
superset/assets/src/explore/controlUtils.js
View File

@ -44,17 +44,8 @@ export function validateControl(control) {
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;
function isGlobalControl(controlKey) {
return controlKey in controls;
}
export function getControlConfig(controlKey, vizType) {
@ -62,11 +53,28 @@ export function getControlConfig(controlKey, vizType) {
// the mapStatetoProps
const vizConf = controlPanelConfigs[vizType] || {};
const controlOverrides = vizConf.controlOverrides || {};
const control = {
if (!isGlobalControl(controlKey)) {
for (const section of vizConf.controlPanelSections) {
for (const controlArr of section.controlSetRows) {
for (const control of controlArr) {
if (control != null && typeof control === 'object') {
if (control.config && control.name === controlKey) {
return {
...control.config,
...controlOverrides[controlKey],
};
}
}
}
}
}
}
return {
...controls[controlKey],
...controlOverrides[controlKey],
};
return control;
}
export function applyMapStateToPropsToControl(control, state) {
@ -122,3 +130,22 @@ export function getControlState(controlKey, vizType, state, value) {
controlState = handleMissingChoice(controlKey, controlState);
return validateControl(controlState);
}
export function getAllControlsState(vizType, datasourceType, state, formData) {
const controlsState = {};
sectionsToRender(vizType, datasourceType).forEach(
section => section.controlSetRows.forEach(
fieldsetRow => fieldsetRow.forEach((field) => {
if (typeof field === 'string') {
controlsState[field] = getControlState(field, vizType, state, formData[field]);
} else if (field != null && typeof field === 'object') {
if (field.config && field.name) {
controlsState[field.name] = { ...field.config };
}
}
}),
),
);
return controlsState;
}

11
superset/assets/src/explore/controls.jsx
View File

@ -2140,17 +2140,6 @@ export const controls = {
controlName: 'TimeSeriesColumnControl',
},
rose_area_proportion: {
type: 'CheckboxControl',
label: t('Use Area Proportions'),
description: t(
'Check if the Rose Chart should use segment area instead of ' +
'segment radius for proportioning',
),
default: false,
renderTrigger: true,
},
time_series_option: {
type: 'SelectControl',
label: t('Options'),

10
superset/assets/src/explore/reducers/exploreReducer.js
View File

@ -78,9 +78,16 @@ export default function exploreReducer(state = {}, action) {
};
},
[actions.SET_FIELD_VALUE]() {
let new_form_data = state.form_data;
if (action.controlName === 'viz_type') {
new_form_data = JSON.parse(JSON.stringify(new_form_data));
// Update state's vizType if we are switching to a new visualization
new_form_data.viz_type = action.value;
}
// These errors are reported from the Control components
let errors = action.validationErrors || [];
const vizType = state.form_data.viz_type;
const vizType = new_form_data.viz_type;
const control = {
...getControlState(action.controlName, vizType, state, action.value),
};
@ -90,6 +97,7 @@ export default function exploreReducer(state = {}, action) {
const hasErrors = errors && errors.length > 0;
return {
...state,
form_data: new_form_data,
triggerRender: control.renderTrigger && !hasErrors,
controls: {
...state.controls,

38
superset/assets/src/explore/store.js
View File

@ -18,8 +18,7 @@
*/
/* eslint camelcase: 0 */
import {
getControlState,
getControlKeys,
getAllControlsState,
getFormDataFromControls,
} from './controlUtils';
import controls from './controls';
@ -50,36 +49,41 @@ export function getControlsState(state, inputFormData) {
handleDeprecatedControls(formData);
const controlNames = getControlKeys(vizType, state.datasource.type);
const controlsState = getAllControlsState(
vizType,
state.datasource.type,
state,
formData,
);
const viz = controlPanelConfigs[vizType] || {};
const controlsState = {};
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(inputFormData) {
const datasourceType = inputFormData.datasource.split('__')[1];
const vizType = inputFormData.viz_type;
const controlNames = getControlKeys(vizType, datasourceType);
const controlsState =
getAllControlsState(
vizType,
datasourceType,
null,
{ ...inputFormData },
);
const formData = {};
controlNames.forEach((k) => {
const controlState = getControlState(k, vizType, null, inputFormData[k]);
if (inputFormData[k] === undefined) {
formData[k] = controlState.value;
Object.keys(controlsState).forEach((controlName) => {
if (inputFormData[controlName] === undefined) {
formData[controlName] = controlsState[controlName].value;
} else {
formData[k] = inputFormData[k];
formData[controlName] = inputFormData[controlName];
}
});
return formData;
}