Skip to content

Commit

Permalink
Introduce EditorConfigProvider to customize vis editor (#20519)
Browse files Browse the repository at this point in the history
* Add possibility to specify base for histogram interval

* Move AggParamsWriter to ES6

* Add param for time_zone in date_histogram

* Prevent writing intervalBase to DSL

* Add basic EditorConfig providers

* Merge multiple configs together

* Allow hiding parameters

* Improve config merging and add tests

* Remove TODO

* Implement review feedback

* Add warning to parameter

* Remove unneeded self
  • Loading branch information
timroes authored Aug 10, 2018
1 parent 9646850 commit 133d25a
Show file tree
Hide file tree
Showing 13 changed files with 661 additions and 78 deletions.
80 changes: 41 additions & 39 deletions src/ui/public/agg_types/__tests__/agg_param_writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,55 +46,57 @@ export default function AggParamWriterHelper(Private) {
* @param {object} opts - describe the properties of this paramWriter
* @param {string} opts.aggType - the name of the aggType we want to test. ('histogram', 'filter', etc.)
*/
function AggParamWriter(opts) {
const self = this;
class AggParamWriter {

self.aggType = opts.aggType;
if (_.isString(self.aggType)) {
self.aggType = aggTypes.byName[self.aggType];
}
constructor(opts) {
this.aggType = opts.aggType;
if (_.isString(this.aggType)) {
this.aggType = aggTypes.byName[this.aggType];
}

// not configurable right now, but totally required
self.indexPattern = stubbedLogstashIndexPattern;
// not configurable right now, but totally required
this.indexPattern = stubbedLogstashIndexPattern;

// the schema that the aggType satisfies
self.visAggSchema = null;
// the schema that the aggType satisfies
this.visAggSchema = null;

self.vis = new Vis(self.indexPattern, {
type: 'histogram',
aggs: [{
id: 1,
type: self.aggType.name,
params: {}
}]
});
}
this.vis = new Vis(this.indexPattern, {
type: 'histogram',
aggs: [{
id: 1,
type: this.aggType.name,
params: {}
}]
});
}

AggParamWriter.prototype.write = function (paramValues) {
const self = this;
paramValues = _.clone(paramValues);
write(paramValues, modifyAggConfig = null) {
paramValues = _.clone(paramValues);

if (self.aggType.params.byName.field && !paramValues.field) {
// pick a field rather than force a field to be specified everywhere
if (self.aggType.type === 'metrics') {
paramValues.field = _.sample(self.indexPattern.fields.byType.number);
} else {
const type = self.aggType.params.byName.field.filterFieldTypes || 'string';
let field;
do {
field = _.sample(self.indexPattern.fields.byType[type]);
} while (!field.aggregatable);
paramValues.field = field.name;
if (this.aggType.params.byName.field && !paramValues.field) {
// pick a field rather than force a field to be specified everywhere
if (this.aggType.type === 'metrics') {
paramValues.field = _.sample(this.indexPattern.fields.byType.number);
} else {
const type = this.aggType.params.byName.field.filterFieldTypes || 'string';
let field;
do {
field = _.sample(this.indexPattern.fields.byType[type]);
} while (!field.aggregatable);
paramValues.field = field.name;
}
}
}

const aggConfig = this.vis.aggs[0];
aggConfig.setParams(paramValues);

const aggConfig = self.vis.aggs[0];
aggConfig.setParams(paramValues);
if (modifyAggConfig) {
modifyAggConfig(aggConfig);
}

return aggConfig.write(self.vis.aggs);
};
return aggConfig.write(this.vis.aggs);
}
}

return AggParamWriter;

}
61 changes: 61 additions & 0 deletions src/ui/public/agg_types/__tests__/buckets/_histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
*/

import expect from 'expect.js';
import sinon from 'sinon';
import ngMock from 'ng_mock';
import { aggTypes } from '../..';
import chrome from '../../../chrome';
import AggParamWriterProvider from '../agg_param_writer';

const config = chrome.getUiSettingsClient();
const histogram = aggTypes.byName.histogram;
describe('Histogram Agg', function () {

Expand All @@ -46,6 +49,13 @@ describe('Histogram Agg', function () {
paramWriter = new AggParamWriter({ aggType: 'histogram' });
}));

describe('intervalBase', () => {
it('should not be written to the DSL', () => {
const output = paramWriter.write({ intervalBase: 100 });
expect(output.params).not.to.have.property('intervalBase');
});
});

describe('interval', function () {
// reads aggConfig.params.interval, writes to dsl.interval

Expand Down Expand Up @@ -74,6 +84,57 @@ describe('Histogram Agg', function () {
const output = paramWriter.write({ interval: [] });
expect(isNaN(output.params.interval)).to.be.ok();
});

describe('interval scaling', () => {

beforeEach(() => {
sinon.stub(config, 'get');
});

it('will respect the histogram:maxBars setting', () => {
config.get.withArgs('histogram:maxBars').returns(5);
const output = paramWriter.write({ interval: 5 },
aggConfig => aggConfig.setAutoBounds({ min: 0, max: 10000 }));
expect(output.params).to.have.property('interval', 2000);
});

it('will return specified interval, if bars are below histogram:maxBars config', () => {
config.get.withArgs('histogram:maxBars').returns(10000);
const output = paramWriter.write({ interval: 5 },
aggConfig => aggConfig.setAutoBounds({ min: 0, max: 10000 }));
expect(output.params).to.have.property('interval', 5);
});

it('will set to intervalBase if interval is below base', () => {
const output = paramWriter.write({ interval: 3, intervalBase: 8 });
expect(output.params).to.have.property('interval', 8);
});

it('will round to nearest intervalBase multiple if interval is above base', () => {
const roundUp = paramWriter.write({ interval: 46, intervalBase: 10 });
expect(roundUp.params).to.have.property('interval', 50);
const roundDown = paramWriter.write({ interval: 43, intervalBase: 10 });
expect(roundDown.params).to.have.property('interval', 40);
});

it('will not change interval if it is a multiple of base', () => {
const output = paramWriter.write({ interval: 35, intervalBase: 5 });
expect(output.params).to.have.property('interval', 35);
});

it('will round to intervalBase after scaling histogram:maxBars', () => {
config.get.withArgs('histogram:maxBars').returns(100);
const output = paramWriter.write({ interval: 5, intervalBase: 6 },
aggConfig => aggConfig.setAutoBounds({ min: 0, max: 1000 }));
// 100 buckets in 0 to 1000 would result in an interval of 10, so we should
// round to the next multiple of 6 -> 12
expect(output.params).to.have.property('interval', 12);
});

afterEach(() => {
config.get.restore();
});
});
});

describe('min_doc_count', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@
import _ from 'lodash';
import moment from 'moment';
import expect from 'expect.js';
import sinon from 'sinon';
import ngMock from 'ng_mock';
import AggParamWriterProvider from '../../agg_param_writer';
import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
import chrome from '../../../../chrome';
import { aggTypes } from '../../..';
import { AggConfig } from '../../../../vis/agg_config';
import { timefilter } from 'ui/timefilter';

const config = chrome.getUiSettingsClient();

describe('params', function () {

let paramWriter;
Expand Down Expand Up @@ -132,6 +136,30 @@ describe('params', function () {
});
});

describe('time_zone', () => {
beforeEach(() => {
sinon.stub(config, 'get');
sinon.stub(config, 'isDefault');
});

it('should use the specified time_zone', () => {
const output = paramWriter.write({ time_zone: 'Europe/Kiev' });
expect(output.params).to.have.property('time_zone', 'Europe/Kiev');
});

it('should use the Kibana time_zone if no parameter specified', () => {
config.isDefault.withArgs('dateFormat:tz').returns(false);
config.get.withArgs('dateFormat:tz').returns('Europe/Riga');
const output = paramWriter.write({});
expect(output.params).to.have.property('time_zone', 'Europe/Riga');
});

afterEach(() => {
config.get.restore();
config.isDefault.restore();
});
});

describe('extended_bounds', function () {
it('should write a long value if a moment passed in', function () {
const then = moment(0);
Expand Down
16 changes: 7 additions & 9 deletions src/ui/public/agg_types/buckets/date_histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,6 @@ export const dateHistogramBucketAgg = new BucketAggType({
output.bucketInterval = interval;
output.params.interval = interval.expression;

const isDefaultTimezone = config.isDefault('dateFormat:tz');
if (isDefaultTimezone) {
output.params.time_zone = detectedTimezone || tzOffset;
} else {
output.params.time_zone = config.get('dateFormat:tz');
}

const scaleMetrics = interval.scaled && interval.scale < 1;
if (scaleMetrics && aggs) {
const all = _.every(aggs.bySchemaGroup.metrics, function (agg) {
Expand All @@ -147,13 +140,18 @@ export const dateHistogramBucketAgg = new BucketAggType({
}
}
},

{
name: 'time_zone',
default: () => {
const isDefaultTimezone = config.isDefault('dateFormat:tz');
return isDefaultTimezone ? detectedTimezone || tzOffset : config.get('dateFormat:tz');
},
},
{
name: 'customInterval',
default: '2h',
write: _.noop
},

{
name: 'format'
},
Expand Down
21 changes: 20 additions & 1 deletion src/ui/public/agg_types/buckets/histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,15 @@ export const histogramBucketAgg = new BucketAggType({
name: 'field',
filterFieldTypes: 'number'
},

{
/*
* This parameter can be set if you want the auto scaled interval to always
* be a multiple of a specific base.
*/
name: 'intervalBase',
default: null,
write: () => {},
},
{
name: 'interval',
editor: intervalTemplate,
Expand Down Expand Up @@ -111,6 +119,17 @@ export const histogramBucketAgg = new BucketAggType({
}
}

const base = aggConfig.params.intervalBase;
if (base) {
if (interval < base) {
// In case the specified interval is below the base, just increase it to it's base
interval = base;
} else if (interval % base !== 0) {
// In case the interval is not a multiple of the base round it to the next base
interval = Math.round(interval / base) * base;
}
}

output.params.interval = interval;
}
},
Expand Down
12 changes: 11 additions & 1 deletion src/ui/public/agg_types/controls/number_interval.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
position="'right'"
content="'Interval will be automatically scaled in the event that the provided value creates more buckets than specified by Advanced Setting\'s histogram:maxBars'"
></icon-tip>

<icon-tip
ng-if="editorConfig.interval.warning"
position="'right'"
content="editorConfig.interval.warning"
type="'alert'"
color="'warning'"
style="float: right"
></icon-tip>
</label>
<input
id="visEditorInterval{{agg.id}}"
Expand All @@ -14,7 +23,8 @@
type="number"
class="form-control"
name="interval"
min="0"
min="{{editorConfig.interval.base || 0}}"
step="{{editorConfig.interval.base}}"
input-number
>
</div>
2 changes: 1 addition & 1 deletion src/ui/public/react_components.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ app.directive('icon', reactDirective => reactDirective(EuiIcon));

app.directive('colorPicker', reactDirective => reactDirective(EuiColorPicker));

app.directive('iconTip', reactDirective => reactDirective(EuiIconTip, ['content', 'type', 'position', 'title']));
app.directive('iconTip', reactDirective => reactDirective(EuiIconTip, ['content', 'type', 'position', 'title', 'color']));

app.directive('callOut', reactDirective => reactDirective(EuiCallOut, ['title', 'color', 'size', 'iconType', 'children']));
Loading

0 comments on commit 133d25a

Please sign in to comment.