Remove display from timefilter refreshInterval state (#20348)

* Remove display from refresh state

* fix spelling error, ensure garbage cannot be added to timefilter state

src/core_plugins/kibana/ui_setting_defaults.js

@@ -374,7 +374,6 @@ export function getUiSettingDefaults() {
       name: 'Time picker refresh interval',
       value:
 `{
-  "display": "Off",
   "pause": false,
   "value": 0
 }`,

src/ui/public/timefilter/timefilter.js

@@ -52,8 +52,12 @@ class Timefilter extends SimpleEmitter {
     // Object.assign used for partially composed updates
     const newTime = Object.assign(this.getTime(), time);
     if (areTimePickerValsDifferent(this.getTime(), newTime)) {
-      this._time = newTime;
-      timeHistory.add(newTime);
+      this._time = {
+        from: newTime.from,
+        to: newTime.to,
+        mode: newTime.mode
+      };
+      timeHistory.add(this._time);
       this.emit('timeUpdate');
       this.emit('fetch');
     }
@@ -66,14 +70,20 @@ class Timefilter extends SimpleEmitter {
   /**
    * Set timefilter refresh interval.
    * @param {Object} refreshInterval
-   * @property {number} time.value
+   * @property {number} time.value Refresh interval in milliseconds. Positive integer
    * @property {boolean} time.pause
    */
   setRefreshInterval = (refreshInterval) => {
     // Object.assign used for partially composed updates
     const newRefreshInterval = Object.assign(this.getRefreshInterval(), refreshInterval);
+    if (newRefreshInterval.value < 0) {
+      newRefreshInterval.value = 0;
+    }
     if (areTimePickerValsDifferent(this.getRefreshInterval(), newRefreshInterval)) {
-      this._refreshInterval = newRefreshInterval;
+      this._refreshInterval = {
+        value: newRefreshInterval.value,
+        pause: newRefreshInterval.pause
+      };
       this.emit('refreshIntervalUpdate');
       if (!newRefreshInterval.pause && newRefreshInterval.value !== 0) {
         this.emit('fetch');

src/ui/public/timefilter/timefilter.test.js

@@ -78,6 +78,12 @@ describe('setTime', () => {
     expect(timefilter.getTime()).to.eql({ from: 5, to: 10, mode: 'absolute' });
   });
 
+  test('should not add unexpected object keys to time state', () => {
+    const unexpectedKey = 'unexpectedKey';
+    timefilter.setTime({ from: 5, to: 10, [unexpectedKey]: 'I should not be added to time state' });
+    expect(timefilter.getTime()).not.to.have.property(unexpectedKey);
+  });
+
   test('should allow partial updates to time', () => {
     timefilter.setTime({ from: 5, to: 10 });
     expect(timefilter.getTime()).to.eql({ from: 5, to: 10, mode: 'absolute' });
@@ -94,7 +100,6 @@ describe('setTime', () => {
     expect(update.called).to.be(true);
     expect(fetch.called).to.be(true);
   });
-
 });
 
 describe('setRefreshInterval', () => {
@@ -118,6 +123,12 @@ describe('setRefreshInterval', () => {
     expect(timefilter.getRefreshInterval()).to.eql({ pause: true, value: 10 });
   });
 
+  test('should not add unexpected object keys to refreshInterval state', () => {
+    const unexpectedKey = 'unexpectedKey';
+    timefilter.setRefreshInterval({ pause: true, value: 10, [unexpectedKey]: 'I should not be added to refreshInterval state' });
+    expect(timefilter.getRefreshInterval()).not.to.have.property(unexpectedKey);
+  });
+
   test('should allow partial updates to refresh interval', () => {
     timefilter.setRefreshInterval({ value: 10 });
     expect(timefilter.getRefreshInterval()).to.eql({ pause: false, value: 10 });

src/ui/public/timepicker/kbn_global_timepicker.html

@@ -32,9 +32,9 @@
 
     <span
       ng-show="timefilterValues.refreshInterval.value > 0"
-      aria-label="{{ 'Data will refresh every ' + timefilterValues.refreshInterval.display }}"
+      aria-label="Data will refresh every {{timefilterValues.display.refreshInterval}}"
     >
-      {{ timefilterValues.refreshInterval.display }}
+      {{ timefilterValues.display.refreshInterval }}
     </span>
   </button>
 
@@ -61,11 +61,12 @@
     aria-haspopup="true"
   >
     <span aria-hidden="true" class="kuiIcon fa-clock-o"></span>
-    <pretty-duration
-      from="timefilterValues.time.from"
-      to="timefilterValues.time.to"
+    <span
+      aria-label="Current time range is {{timefilterValues.display.time}}"
       data-test-subj="globalTimepickerRange"
-    ></pretty-duration>
+    >
+      {{timefilterValues.display.time}}
+    </span>
   </button>
 
   <button

src/ui/public/timepicker/kbn_global_timepicker.js

@@ -22,10 +22,14 @@ import { uiModules } from '../modules';
 import toggleHtml from './kbn_global_timepicker.html';
 import { timeNavigation } from './time_navigation';
 import { timefilter } from 'ui/timefilter';
+import { prettyDuration } from './pretty_duration';
+import { prettyInterval } from './pretty_interval';
 
 uiModules
   .get('kibana')
-  .directive('kbnGlobalTimepicker', (globalState) => {
+  .directive('kbnGlobalTimepicker', (globalState, config) => {
+    const getConfig = (...args) => config.get(...args);
+
     const listenForUpdates = ($scope) => {
       $scope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', () => {
         globalState.refreshInterval = timefilter.getRefreshInterval();
@@ -45,9 +49,15 @@ uiModules
     };
 
     function setTimefilterValues($scope) {
+      const time = timefilter.getTime();
+      const refreshInterval = timefilter.getRefreshInterval();
       $scope.timefilterValues = {
-        refreshInterval: timefilter.getRefreshInterval(),
-        time: timefilter.getTime(),
+        refreshInterval: refreshInterval,
+        time: time,
+        display: {
+          time: prettyDuration(time.from, time.to, getConfig),
+          refreshInterval: prettyInterval(refreshInterval.value),
+        },
         isAutoRefreshSelectorEnabled: timefilter.isAutoRefreshSelectorEnabled,
         isTimeRangeSelectorEnabled: timefilter.isTimeRangeSelectorEnabled,
       };

src/ui/public/timepicker/pretty_interval.js

@@ -0,0 +1,45 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+const MS_IN_SECOND = 1000;
+const MS_IN_MINUTE = 60 * MS_IN_SECOND;
+const MS_IN_HOUR = 60 * MS_IN_MINUTE;
+const MS_IN_DAY = 24 * MS_IN_HOUR;
+
+export function prettyInterval(intervalInMs) {
+  if (intervalInMs === 0) {
+    return 'Off';
+  } else if (intervalInMs < MS_IN_MINUTE) {
+    const intervalInSeconds = Math.round(intervalInMs / MS_IN_SECOND);
+    const units = intervalInSeconds > 1 ? 'seconds' : 'second';
+    return `${intervalInSeconds} ${units}`;
+  } else if (intervalInMs < MS_IN_HOUR) {
+    const intervalInMinutes = Math.round(intervalInMs / MS_IN_MINUTE);
+    const units = intervalInMinutes > 1 ? 'minutes' : 'minute';
+    return `${intervalInMinutes} ${units}`;
+  } else if (intervalInMs < MS_IN_DAY) {
+    const intervalInHours = Math.round(intervalInMs / MS_IN_HOUR);
+    const units = intervalInHours > 1 ? 'hours' : 'hour';
+    return `${intervalInHours} ${units}`;
+  }
+
+  const intervalInDays = Math.round(intervalInMs / MS_IN_DAY);
+  const units = intervalInDays > 1 ? 'days' : 'day';
+  return `${intervalInDays} ${units}`;
+}

src/ui/public/timepicker/pretty_interval.test.js

@@ -17,32 +17,29 @@
  * under the License.
  */
 
-import { uiModules } from '../modules';
-import { prettyDuration } from '../timepicker/pretty_duration';
-const module = uiModules.get('kibana');
+import expect from 'expect.js';
+import { prettyInterval } from './pretty_interval';
 
-module.directive('prettyDuration', function (config) {
-  return {
-    restrict: 'E',
-    scope: {
-      from: '=',
-      to: '='
-    },
-    link: function ($scope, $elem) {
-      const getConfig = (...args) => config.get(...args);
-
-      function setText(text) {
-        $elem.text(text);
-        $elem.attr('aria-label', `Current time range is ${text}`);
-      }
-
-      function stringify() {
-        setText(prettyDuration($scope.from, $scope.to, getConfig));
-      }
-
-      $scope.$watch('from', stringify);
-      $scope.$watch('to', stringify);
-
-    }
-  };
+test('Off', () => {
+  expect(prettyInterval(0)).to.be('Off');
+});
+
+test('seconds', () => {
+  expect(prettyInterval(1000)).to.be('1 second');
+  expect(prettyInterval(15000)).to.be('15 seconds');
+});
+
+test('minutes', () => {
+  expect(prettyInterval(60000)).to.be('1 minute');
+  expect(prettyInterval(1800000)).to.be('30 minutes');
+});
+
+test('hours', () => {
+  expect(prettyInterval(3600000)).to.be('1 hour');
+  expect(prettyInterval(43200000)).to.be('12 hours');
+});
+
+test('days', () => {
+  expect(prettyInterval(86400000)).to.be('1 day');
+  expect(prettyInterval(86400000 * 2)).to.be('2 days');
 });

src/ui/public/timepicker/refresh_intervals.js

@@ -21,21 +21,21 @@ import { uiModules } from '../modules';
 const module = uiModules.get('kibana');
 
 module.constant('refreshIntervals', [
-  { value: 0, display: 'Off', section: 0 },
+  { value: 0, section: 0 },
 
-  { value: 5000, display: '5 seconds', section: 1 },
-  { value: 10000, display: '10 seconds', section: 1 },
-  { value: 30000, display: '30 seconds', section: 1 },
-  { value: 45000, display: '45 seconds', section: 1 },
+  { value: 5000, section: 1 },
+  { value: 10000, section: 1 },
+  { value: 30000, section: 1 },
+  { value: 45000, section: 1 },
 
-  { value: 60000, display: '1 minute', section: 2 },
-  { value: 300000, display: '5 minutes', section: 2 },
-  { value: 900000, display: '15 minutes', section: 2 },
-  { value: 1800000, display: '30 minutes', section: 2 },
+  { value: 60000, section: 2 },
+  { value: 300000, section: 2 },
+  { value: 900000, section: 2 },
+  { value: 1800000, section: 2 },
 
-  { value: 3600000, display: '1 hour', section: 3 },
-  { value: 7200000, display: '2 hour', section: 3 },
-  { value: 43200000, display: '12 hour', section: 3 },
-  { value: 86400000, display: '1 day', section: 3 }
+  { value: 3600000, section: 3 },
+  { value: 7200000, section: 3 },
+  { value: 43200000, section: 3 },
+  { value: 86400000, section: 3 }
 ]);
 

src/ui/public/timepicker/timepicker.html

@@ -122,7 +122,7 @@
               ng-click="setRefreshInterval(inter)"
               kbn-accessible-click
             >
-              {{inter.display}}
+              {{prettyInterval(inter)}}
             </a>
           </li>
         </ul>

src/ui/public/timepicker/timepicker.js

@@ -35,6 +35,7 @@ import './kbn_global_timepicker';
 import { uiModules } from '../modules';
 import { TIME_MODES } from './modes';
 import { timeUnits } from './time_units';
+import { prettyInterval } from './pretty_interval';
 const module = uiModules.get('ui/timepicker');
 
 module.directive('kbnTimepicker', function (refreshIntervals) {
@@ -234,10 +235,20 @@ module.directive('kbnTimepicker', function (refreshIntervals) {
         });
       };
 
+      $scope.prettyInterval = function (interval) {
+        return prettyInterval(interval.value);
+      };
+
       $scope.setRefreshInterval = function (interval) {
         interval = _.clone(interval || {});
         interval.pause = (interval.pause == null || interval.pause === false) ? false : true;
-        $scope.onIntervalSelect({ interval });
+
+        $scope.onIntervalSelect({
+          interval: {
+            value: interval.value,
+            pause: interval.pause,
+          }
+        });
       };
 
       $scope.setMode($scope.mode);

test/functional/apps/dashboard/_bwc_shared_urls.js

@@ -29,7 +29,7 @@ export default function ({ getService, getPageObjects }) {
   let kibanaBaseUrl;
 
   const urlQuery = `` +
-    `_g=(refreshInterval:(display:Off,pause:!f,value:0),` +
+    `_g=(refreshInterval:(pause:!f,value:0),` +
         `time:(from:'2012-11-17T00:00:00.000Z',mode:absolute,to:'2015-11-17T18:01:36.621Z'))&` +
     `_a=(description:'',filters:!(),` +
         `fullScreenMode:!f,` +

test/functional/apps/discover/_discover.js

@@ -57,7 +57,7 @@ export default function ({ getService, getPageObjects }) {
       const queryName1 = 'Query # 1';
 
       it('should show correct time range string by timepicker', async function () {
-        const actualTimeString = await PageObjects.discover.getTimespanText();
+        const actualTimeString = await PageObjects.header.getPrettyDuration();
 
         const expectedTimeString = `${fromTimeString} to ${toTimeString}`;
         expect(actualTimeString).to.be(expectedTimeString);

test/functional/apps/discover/_shared_links.js

@@ -76,7 +76,7 @@ export default function ({ getService, getPageObjects }) {
         const expectedUrl =
           baseUrl +
           '/app/kibana?_t=1453775307251#' +
-          '/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time' +
+          '/discover?_g=(refreshInterval:(pause:!f,value:0),time' +
           ':(from:\'2015-09-19T06:31:44.000Z\',mode:absolute,to:\'2015-09' +
           '-23T18:31:44.000Z\'))&_a=(columns:!(_source),index:\'logstash-' +
           '*\',interval:auto,query:(language:lucene,query:\'\')' +

test/functional/page_objects/discover_page.js

@@ -41,10 +41,6 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
         .findByCssSelector('button[aria-label=\'Search\']');
     }
 
-    async getTimespanText() {
-      return await testSubjects.getVisibleText('globalTimepickerRange');
-    }
-
     getChartTimespan() {
       return getRemote()
         .findByCssSelector('center.small > span:nth-child(1)')