[visualize] Fix agg param race (#13429)

* [visualize/aggConfig] do not rely on the view to reset params

* [vis/editor/aggParams] check for agg and type before accessing
This commit is contained in:
Spencer 2017-08-25 12:54:36 -07:00 committed by GitHub
parent 5619dbeaa9
commit f7721fe420
2 changed files with 23 additions and 36 deletions

View file

@ -19,12 +19,16 @@ export function VisAggConfigProvider(Private) {
self._opts = opts = (opts || {});
self.enabled = typeof opts.enabled === 'boolean' ? opts.enabled : true;
// start with empty params so that checks in type/schema setters don't freak
// because self.params is undefined
self.params = {};
// setters
self.type = opts.type;
self.schema = opts.schema;
// resolve the params
self.fillDefaults(opts.params);
// set the params to the values from opts, or just to the defaults
self.setParams(opts.params || {});
}
/**
@ -82,6 +86,15 @@ export function VisAggConfigProvider(Private) {
}
this.__type = type;
// clear out the previous params except for a few special ones
this.setParams({
// split row/columns is "outside" of the agg, so don't reset it
row: this.params.row,
// almost every agg has fields, so we try to persist that when type changes
field: _.get(this.getFieldOptions(), ['byName', this.getField()])
});
}
},
schema: {
@ -105,7 +118,7 @@ export function VisAggConfigProvider(Private) {
* used when initializing
* @return {undefined}
*/
AggConfig.prototype.fillDefaults = function (from) {
AggConfig.prototype.setParams = function (from) {
const self = this;
from = from || self.params || {};
const to = self.params = {};
@ -143,22 +156,6 @@ export function VisAggConfigProvider(Private) {
});
};
/**
* Clear the parameters for this aggConfig
*
* @return {object} the new params object
*/
AggConfig.prototype.resetParams = function () {
let field;
const fieldOptions = this.getFieldOptions();
if (fieldOptions) {
field = fieldOptions.byName[this.fieldName()] || null;
}
return this.fillDefaults({ row: this.params.row, field: field });
};
AggConfig.prototype.write = function () {
return this.type.params.write(this);
};

View file

@ -52,7 +52,7 @@ uiModules
let $aggParamEditors; // container for agg type param editors
let $aggParamEditorsScope;
function updateAggParamEditor(newType, oldType) {
function updateAggParamEditor() {
if ($aggParamEditors) {
$aggParamEditors.remove();
$aggParamEditors = null;
@ -64,33 +64,23 @@ uiModules
$aggParamEditorsScope = null;
}
if (!$scope.agg || !$scope.agg.type) {
return;
}
// create child scope, used in the editors
$aggParamEditorsScope = $scope.$new();
$aggParamEditorsScope.indexedFields = $scope.agg.getFieldOptions();
const agg = $scope.agg;
if (!agg) return;
const type = $scope.agg.type;
if (newType !== oldType) {
// don't reset on initial load, the
// saved params should persist
agg.resetParams();
}
if (!type) return;
const aggParamHTML = {
basic: [],
advanced: []
};
// build collection of agg params html
type.params.forEach(function (param, i) {
$scope.agg.type.params.forEach(function (param, i) {
let aggParam;
let fields;
if (agg.schema.hideCustomLabel && param.name === 'customLabel') {
if ($scope.agg.schema.hideCustomLabel && param.name === 'customLabel') {
return;
}
// if field param exists, compute allowed fields