mistercrunch commented on a change in pull request #7350: Refactor out
controlUtils.js module + unit tests
URL:
https://github.com/apache/incubator-superset/pull/7350#discussion_r279513362
##########
File path: superset/assets/src/explore/store.js
##########
@@ -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]);
Review comment:
It repeated some of the logic (like applying defaults), but skipped
important part that should be consistent like `mapStateToProps`. I'm guessing
this was mostly fine because in the end the logic here cares only about the
`value` part of the `controlState`.
Let me see if I can throw in some useful unit tests here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
