diff --git a/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx b/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx index 50f766a1b7b..15b94c20a9f 100644 --- a/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx +++ b/superset/assets/spec/javascripts/explore/controlUtils_spec.jsx @@ -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']); }); - }); - }); diff --git a/superset/assets/src/explore/components/ControlPanelsContainer.jsx b/superset/assets/src/explore/components/ControlPanelsContainer.jsx index fdb0a7c53a7..348e6138e45 100644 --- a/superset/assets/src/explore/components/ControlPanelsContainer.jsx +++ b/superset/assets/src/explore/components/ControlPanelsContainer.jsx @@ -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 ( + + ); + } + 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 ( { - 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 (); + } 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), }; } diff --git a/superset/assets/src/explore/controlPanels/Rose.js b/superset/assets/src/explore/controlPanels/Rose.js index 135b0d5adb7..4e86b658c00 100644 --- a/superset/assets/src/explore/controlPanels/Rose.js +++ b/superset/assets/src/explore/controlPanels/Rose.js @@ -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], diff --git a/superset/assets/src/explore/controlUtils.js b/superset/assets/src/explore/controlUtils.js index 65e8c47ea79..446c3aa6750 100644 --- a/superset/assets/src/explore/controlUtils.js +++ b/superset/assets/src/explore/controlUtils.js @@ -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; +} diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index ae673de545e..366ea57cce0 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -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'), diff --git a/superset/assets/src/explore/reducers/exploreReducer.js b/superset/assets/src/explore/reducers/exploreReducer.js index 44d5a72c1eb..58db0db5bc1 100644 --- a/superset/assets/src/explore/reducers/exploreReducer.js +++ b/superset/assets/src/explore/reducers/exploreReducer.js @@ -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, diff --git a/superset/assets/src/explore/store.js b/superset/assets/src/explore/store.js index 6d58b8d0405..d79b1943c1b 100644 --- a/superset/assets/src/explore/store.js +++ b/superset/assets/src/explore/store.js @@ -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; }