Skip to content

Commit

Permalink
Merge pull request #3035 from spalger/fix/3028
Browse files Browse the repository at this point in the history
Disable endzones and auto interval for non-timefield date histograms
  • Loading branch information
simianhacker committed Feb 17, 2015
2 parents a56c2a7 + 543bb11 commit f8f6617
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ define(function (require) {
var moment = require('moment');

return function orderedDateAxis(vis, chart) {
var aspects = chart.aspects;
var buckets = aspects.x.agg.buckets;
var xAgg = chart.aspects.x.agg;
var buckets = xAgg.buckets;
var format = buckets.getScaledDateFormat();

chart.xAxisFormatter = function (val) {
Expand All @@ -16,10 +16,13 @@ define(function (require) {
interval: buckets.getInterval(),
};

var axisOnTimeField = xAgg.fieldIsTimeField();
var bounds = buckets.getBounds();
if (bounds) {
if (bounds && axisOnTimeField) {
chart.ordered.min = bounds.min;
chart.ordered.max = bounds.max;
} else {
chart.ordered.endzones = false;
}
};
};
Expand Down
6 changes: 4 additions & 2 deletions src/kibana/components/agg_types/buckets/_interval_options.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ define(function (require) {
{
display: 'Auto',
val: 'auto',
enabled: function (aggConfig) {
return !!aggConfig.vis.indexPattern.timeFieldName;
enabled: function (agg) {
// not only do we need a time field, but the selected field needs
// to be the time field. (see #3028)
return agg.fieldIsTimeField();
}
},
{
Expand Down
21 changes: 13 additions & 8 deletions src/kibana/components/agg_types/buckets/date_histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ define(function (require) {

var tzOffset = moment().format('Z');

function setBounds(agg, force) {
if (agg.buckets._alreadySet && !force) return;
agg.buckets._alreadySet = true;
agg.buckets.setBounds(agg.fieldIsTimeField() && timefilter.getActiveBounds());
}

require('filters/field_type');

return new BucketAggType({
Expand All @@ -32,7 +38,8 @@ define(function (require) {

buckets = new TimeBuckets();
buckets.setInterval(_.get(this, ['params', 'interval']));
buckets.setBounds(timefilter.getActiveBounds());
setBounds(this);

return buckets;
}
}
Expand All @@ -44,6 +51,9 @@ define(function (require) {
filterFieldTypes: 'date',
default: function (agg) {
return agg.vis.indexPattern.timeFieldName;
},
onChange: function (agg) {
setBounds(agg, true);
}
},

Expand All @@ -54,15 +64,10 @@ define(function (require) {
options: Private(require('components/agg_types/buckets/_interval_options')),
editor: require('text!components/agg_types/controls/interval.html'),
onRequest: function (agg) {
// flag that prevents us from clobbering on subsequest calls to write()
agg.buckets._sentToEs = true;
agg.buckets.setBounds(timefilter.getActiveBounds());
setBounds(agg, true);
},
write: function (agg, output) {
if (!agg.buckets._sentToEs) {
agg.buckets.setBounds(timefilter.getActiveBounds());
}

setBounds(agg);
agg.buckets.setInterval(agg.params.interval);

var interval = agg.buckets.getInterval();
Expand Down
3 changes: 2 additions & 1 deletion src/kibana/components/agg_types/controls/field.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
name="field"
required
ng-model="agg.params.field"
ng-options="field as field.displayName group by field.type for field in indexedFields">
ng-options="field as field.displayName group by field.type for field in indexedFields"
ng-change="aggParam.onChange(agg)">
</select>

</div>
22 changes: 13 additions & 9 deletions src/kibana/components/time_buckets/time_buckets.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,10 @@ define(function (require) {
}

function cacheBreaker(prop) {
var breaker = breakers[prop];
var setup = breaker.setup;
var changes = breaker.changes;
var deps = breaker.deps;
var resource = resources[breakers[prop]];
var setup = resource.setup;
var changes = resource.changes;
var deps = resource.deps;
var fn = self[prop];

return {
Expand All @@ -304,7 +304,7 @@ define(function (require) {
var ret = fn.apply(self, arguments);

if (changes.call(self, prev)) {
cache = _.omit(cache, deps);
cache = {};
}

return ret;
Expand All @@ -330,17 +330,21 @@ define(function (require) {
};

var breakers = {
setBounds: {
deps: ['getBounds', 'hasBounds', 'getDuration', 'getInterval', 'getScaledDateFormat'],
setBounds: 'bounds',
clearBounds: 'bounds',
setInterval: 'interval'
};

var resources = {
bounds: {
setup: function () {
return [self._lb, self._ub];
},
changes: function (prev) {
return !sameMoment(prev[0], self._lb) || !sameMoment(prev[1], self._ub);
}
},
setInterval: {
deps: ['getInterval', 'getScaledDateFormat'],
interval: {
setup: function () {
return self._i;
},
Expand Down
5 changes: 5 additions & 0 deletions src/kibana/components/vis/_agg_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,11 @@ define(function (require) {
return field ? (field.displayName || this.fieldName()) : '';
};

AggConfig.prototype.fieldIsTimeField = function () {
var timeFieldName = this.vis.indexPattern.timeFieldName;
return timeFieldName && this.fieldName() === timeFieldName;
};

return AggConfig;
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ define(function (require) {
var yScale = xAxis.yScale;
var ordered = xAxis.ordered;

if (!ordered || ordered.min == null || ordered.max == null) return;
if (!ordered || ordered.endzones === false) return;

var attr = this.handler._attr;
var height = attr.height;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ define(function (require) {
aspects: {
x: {
agg: {
fieldIsTimeField: _.constant(true),
buckets: {
getScaledDateFormat: _.constant('hh:mm:ss'),
getInterval: _.constant(moment.duration(15, 'm')),
Expand Down
20 changes: 14 additions & 6 deletions test/unit/specs/components/agg_types/buckets/_date_histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,27 @@ define(function (require) {

describe('params', function () {
var paramWriter;
var writeInterval;

var aggTypes;
var AggConfig;
var setTimeBounds;
var timeField;

beforeEach(module('kibana'));
beforeEach(inject(function (Private, $injector) {
var AggParamWriter = Private(require('test_utils/agg_param_writer'));
var indexPattern = Private(require('fixtures/stubbed_logstash_index_pattern'));
var timefilter = $injector.get('timefilter');

timeField = indexPattern.timeFieldName;
aggTypes = Private(require('components/agg_types/index'));
AggConfig = Private(require('components/vis/_agg_config'));

paramWriter = new AggParamWriter({ aggType: 'date_histogram' });
writeInterval = function (interval) {
return paramWriter.write({ interval: interval, field: timeField });
};

var now = moment();
setTimeBounds = function (n, units) {
Expand All @@ -31,32 +39,32 @@ define(function (require) {

describe('interval', function () {
it('accepts a valid interval', function () {
var output = paramWriter.write({ interval: 'day' });
var output = writeInterval('day');
expect(output.params).to.have.property('interval', '1d');
});

it('ignores invalid intervals', function () {
var output = paramWriter.write({ interval: 'foo' });
var output = writeInterval('foo');
expect(output.params).to.have.property('interval', '0ms');
});

it('automatically picks an interval', function () {
setTimeBounds(15, 'minutes');
var output = paramWriter.write({ interval: 'auto' });
var output = writeInterval('auto');
expect(output.params.interval).to.be('30s');
});

it('scales up the interval if it will make too many buckets', function () {
setTimeBounds(30, 'minutes');
var output = paramWriter.write({ interval: 'second' });
var output = writeInterval('second');
expect(output.params.interval).to.be('10s');
expect(output.metricScaleText).to.be('second');
expect(output.metricScale).to.be(0.1);
});

it('does not scale down the interval', function () {
setTimeBounds(1, 'minutes');
var output = paramWriter.write({ interval: 'hour' });
var output = writeInterval('hour');
expect(output.params.interval).to.be('1h');
expect(output.metricScaleText).to.be(undefined);
expect(output.metricScale).to.be(undefined);
Expand All @@ -82,7 +90,7 @@ define(function (require) {
var histoConfig = new AggConfig(vis, {
type: aggTypes.byName.date_histogram,
schema: 'segment',
params: { interval: 'second' }
params: { interval: 'second', field: timeField }
});

vis.aggs.push(histoConfig);
Expand Down

0 comments on commit f8f6617

Please sign in to comment.