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_r279581917
##########
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:
I added some tests to validate assumptions around `applyDefaultFormData`.
Part of the goal here is to be more consistent and dry across the board.
Before this PR, the dashboard app used `applyDefaultFormData` to try and mimic
a subset of what is going on in `getControlState`, mostly applying overrides
and defaults. This makes it such the logic is closer/DRYer between explore and
dashboard. There's still divergence around edge cases, mostly related to the
fact that `mapStateToProps` doesn't currently work on the dashboard side since
the state is different there. Now the role of `mapStateToProps` seems to be
mostly to configure the controls themselves (think columns' `choices` attribute
based on datasource for instance), and should not affect the `value` property.
Ideally we should pass the same context on both sides, but this PR fails at
fully bringing this back together. Another PR could push further in that
direction.
----------------------------------------------------------------
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]
