Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/ui/public/chrome/config/filter.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@
to="timefilter.time.to"
mode="timefilter.time.mode"
active-tab="'filter'"
interval="timefilter.refreshInterval">
interval="timefilter.refreshInterval"
on-filter-select="updateFilter(from, to)"
on-interval-select="updateInterval(interval)">
</kbn-timepicker>
4 changes: 3 additions & 1 deletion src/ui/public/chrome/config/interval.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@
to="timefilter.time.to"
mode="timefilter.time.mode"
active-tab="'interval'"
interval="timefilter.refreshInterval">
interval="timefilter.refreshInterval"
on-filter-select="updateFilter(from, to)"
on-interval-select="updateInterval(interval)">
</kbn-timepicker>
85 changes: 25 additions & 60 deletions src/ui/public/directives/__tests__/timepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ const init = function () {
}
};
$parentScope.timefilter = timefilter;
$parentScope.updateInterval = sinon.spy();
$parentScope.updateFilter = sinon.spy();

// Create the element
$elem = angular.element(
Expand All @@ -54,7 +56,9 @@ const init = function () {
' to="timefilter.time.to"' +
' mode="timefilter.time.mode"' +
' active-tab="timefilter.timepickerActiveTab"' +
' interval="timefilter.refreshInterval">' +
' interval="timefilter.refreshInterval"' +
' on-interval-select="updateInterval(interval)"' +
' on-filter-select="updateFilter(from, to)">' +
'</kbn-timepicker>'
);

Expand Down Expand Up @@ -99,64 +103,34 @@ describe('timepicker directive', function () {
done();
});

it('should have a $scope.setRefreshInterval() that sets interval variable', function (done) {
it('should have a $scope.setRefreshInterval() that calls handler', function (done) {
$scope.setRefreshInterval({ value : 10000 });
expect($scope.interval).to.have.property('value', 10000);
done();
});

it('should set the interval on the courier', function (done) {
// Change refresh interval and digest
$scope.setRefreshInterval({ value : 1000 });
$elem.scope().$digest();
expect($courier.searchLooper.loopInterval()).to.be(1000);
done();
});

it('should disable the looper when paused', function (done) {
$scope.setRefreshInterval({ value : 1000, pause: true });
$elem.scope().$digest();
expect($courier.searchLooper.loopInterval()).to.be(0);
expect($scope.interval.value).to.be(1000);
done();
});

it('but keep interval.value set', function (done) {
$scope.setRefreshInterval({ value : 1000, pause: true });
$elem.scope().$digest();
expect($scope.interval.value).to.be(1000);
sinon.assert.calledOnce($parentScope.updateInterval);
expect($parentScope.updateInterval.firstCall.args[0]).to.have.property('value', 10000);
done();
});

it('should unpause when setRefreshInterval is called without pause:true', function (done) {
$scope.setRefreshInterval({ value : 1000, pause: true });
expect($scope.interval.pause).to.be(true);
expect($parentScope.updateInterval.getCall(0).args[0]).to.have.property('pause', true);

$scope.setRefreshInterval({ value : 1000, pause: false });
expect($scope.interval.pause).to.be(false);
expect($parentScope.updateInterval.getCall(1).args[0]).to.have.property('pause', false);

$scope.setRefreshInterval({ value : 1000 });
expect($scope.interval.pause).to.be(false);
expect($parentScope.updateInterval.getCall(2).args[0]).to.have.property('pause', false);

done();
});


it('should highlight the current active interval', function (done) {
$scope.setRefreshInterval({ value: 300000 });
$scope.interval = { value: 300000 };
$elem.scope().$digest();
expect($elem.find('.refresh-interval-active').length).to.be(1);
expect($elem.find('.refresh-interval-active').text().trim()).to.be('5 minutes');
done();
});

it('should default the interval on the courier with incorrect values', function (done) {
// Change refresh interval and digest
$scope.setRefreshInterval();
$elem.scope().$digest();
expect($courier.searchLooper.loopInterval()).to.be(0);
done();
});
});

describe('mode setting', function () {
Expand Down Expand Up @@ -198,10 +172,11 @@ describe('timepicker directive', function () {
done();
});

it('should have a $scope.setQuick() that sets the to and from variables to strings', function (done) {
it('should have a $scope.setQuick() that calls handler', function (done) {
$scope.setQuick('now', 'now');
expect($scope.from).to.be('now');
expect($scope.to).to.be('now');
sinon.assert.calledOnce($parentScope.updateFilter);
expect($parentScope.updateFilter.firstCall.args[0]).to.be('now');
expect($parentScope.updateFilter.firstCall.args[1]).to.be('now');
done();
});
});
Expand Down Expand Up @@ -312,24 +287,25 @@ describe('timepicker directive', function () {
$scope.relative.count = 1;
$scope.relative.unit = 's';
$scope.applyRelative();
expect($scope.from).to.be('now-1s');
sinon.assert.calledOnce($parentScope.updateFilter);
expect($parentScope.updateFilter.getCall(0).args[0]).to.be('now-1s');

$scope.relative.count = 2;
$scope.relative.unit = 'm';
$scope.applyRelative();
expect($scope.from).to.be('now-2m');
expect($parentScope.updateFilter.getCall(1).args[0]).to.be('now-2m');

$scope.relative.count = 3;
$scope.relative.unit = 'h';
$scope.applyRelative();
expect($scope.from).to.be('now-3h');
expect($parentScope.updateFilter.getCall(2).args[0]).to.be('now-3h');

// Enable rounding
$scope.relative.round = true;
$scope.relative.count = 7;
$scope.relative.unit = 'd';
$scope.applyRelative();
expect($scope.from).to.be('now-7d/d');
expect($parentScope.updateFilter.getCall(3).args[0]).to.be('now-7d/d');

done();
});
Expand Down Expand Up @@ -398,16 +374,6 @@ describe('timepicker directive', function () {
done();
});

it('should parse the time of scope.from and scope.to to set its own variables', function (done) {
$scope.setQuick('now-30m', 'now');
$scope.setMode('absolute');
$scope.$digest();

expect($scope.absolute.from.valueOf()).to.be(moment().subtract(30, 'minutes').valueOf());
expect($scope.absolute.to.valueOf()).to.be(moment().valueOf());
done();
});

it('should update its own variables if timefilter time is updated', function (done) {
$scope.setMode('absolute');
$scope.$digest();
Expand Down Expand Up @@ -438,18 +404,17 @@ describe('timepicker directive', function () {
});

it('should only copy its input to scope.from and scope.to when scope.applyAbsolute() is called', function (done) {
$scope.setQuick('now-30m', 'now');
expect($scope.from).to.be('now-30m');
expect($scope.to).to.be('now');
$scope.from = 'now-30m';
$scope.to = 'now';

$scope.absolute.from = moment('2012-02-01');
$scope.absolute.to = moment('2012-02-11');
expect($scope.from).to.be('now-30m');
expect($scope.to).to.be('now');

$scope.applyAbsolute();
expect($scope.from.valueOf()).to.be(moment('2012-02-01').valueOf());
expect($scope.to.valueOf()).to.be(moment('2012-02-11').valueOf());
expect($parentScope.updateFilter.firstCall.args[0]).to.eql(moment('2012-02-01'));
expect($parentScope.updateFilter.firstCall.args[1]).to.eql(moment('2012-02-11'));

$scope.$digest();

Expand Down
2 changes: 2 additions & 0 deletions src/ui/public/timepicker/__tests__/toggle.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import $ from 'jquery';

describe('kbnGlobalTimepicker', function () {
let compile;

beforeEach(() => {
ngMock.module('kibana');
ngMock.inject(($compile, $rootScope) => {
compile = () => {
const $el = $('<kbn-global-timepicker></kbn-global-timepicker>');
$el.data('$kbnTopNavController', {}); // Mock the kbnTopNav
$rootScope.$apply();
$compile($el)($rootScope);
return $el;
Expand Down
14 changes: 13 additions & 1 deletion src/ui/public/timepicker/kbn_global_timepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ UiModules
return {
template: toggleHtml,
replace: true,
link: ($scope) => {
require: '^kbnTopNav',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to require this? Can we add onFilterSelect and onIntervalSelect callbacks, and follow the same pattern as timerpicker.js? This would be a little more explicit, and less brittle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you took at the corresponding HTML template, you'll see kbnTopNav written all over it. I agree that the way these all tie in together is brittle, but I think it's outside of the scope of this PR to disentangle this. At very least, this require call makes it more explicit that the directive relies on being nested inside the kbnTopNav directive.

link: ($scope, element, attributes, kbnTopNav) => {
listenForUpdates($rootScope);

$rootScope.timefilter = timefilter;
Expand All @@ -34,6 +35,17 @@ UiModules
$scope.back = function () {
assign(timefilter.time, timeNavigation.stepBackward(timefilter.getBounds()));
};

$scope.updateFilter = function (from, to) {
timefilter.time.from = from;
timefilter.time.to = to;
kbnTopNav.close('filter');
};

$scope.updateInterval = function (interval) {
timefilter.refreshInterval = interval;
kbnTopNav.close('interval');
};
},
};
});
21 changes: 13 additions & 8 deletions src/ui/public/timepicker/timepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
to: '=',
mode: '=',
interval: '=',
activeTab: '='
activeTab: '=',
onFilterSelect: '&',
onIntervalSelect: '&'
},
template: html,
controller: function ($scope) {
Expand Down Expand Up @@ -124,8 +126,7 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
};

$scope.setQuick = function (from, to) {
$scope.from = from;
$scope.to = to;
$scope.onFilterSelect({ from, to });
};

$scope.setToNow = function () {
Expand All @@ -139,17 +140,21 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
};

$scope.applyRelative = function () {
$scope.from = getRelativeString();
$scope.to = 'now';
$scope.onFilterSelect({
from: getRelativeString(),
to: 'now'
});
};

function getRelativeString() {
return 'now-' + $scope.relative.count + $scope.relative.unit + ($scope.relative.round ? '/' + $scope.relative.unit : '');
}

$scope.applyAbsolute = function () {
$scope.from = moment($scope.absolute.from);
$scope.to = moment($scope.absolute.to);
$scope.onFilterSelect({
from: moment($scope.absolute.from),
to: moment($scope.absolute.to)
});
};

$scope.setRefreshInterval = function (interval) {
Expand All @@ -159,7 +164,7 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter

notify.log('after: ' + interval.pause);

$scope.interval = interval;
$scope.onIntervalSelect({ interval });
};

$scope.setMode($scope.mode);
Expand Down
3 changes: 0 additions & 3 deletions test/support/page_objects/header_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ export default class HeaderPage {
})
.then(() => {
return this.isGlobalLoadingIndicatorHidden();
})
.then(() => {
return this.clickTimepicker();
});
}

Expand Down