Refactor `time` module for conventions, consistency

Update the `time` utility module to more closely match
our current conventions. Also make time math consistent
(always use `ms` units), organize for concision and
rename a few references for clarity.

The only functional change is the addition of the string
"ago" to some of the relative time templating.

src/sidebar/util/test/time-test.js

@@ -2,10 +2,10 @@
 
 const time = require('../time');
 
-const minute = 60;
+const second = 1000;
+const minute = second * 60;
 const hour = minute * 60;
 const day = hour * 24;
-const msPerSecond = 1000;
 
 describe('sidebar.util.time', function() {
   let sandbox;
@@ -41,7 +41,7 @@ describe('sidebar.util.time', function() {
     // - A user in the UK who views the annotation will see “Jan 1”
     //   on the annotation card (correct)
     // - A user in San Francisco who views the annotation will see
-    //   “Dec 31st 2018" on the annotation card (also correct from
+    //   “Dec 31, 2018" on the annotation card (also correct from
     //   their point of view).
     const date = new Date(isoString);
     date.getFullYear = sinon.stub().returns(date.getUTCFullYear());
@@ -58,11 +58,11 @@ describe('sidebar.util.time', function() {
     [
       { now: '1970-01-01T00:00:10.000Z', text: 'Just now' },
       { now: '1970-01-01T00:00:29.000Z', text: 'Just now' },
-      { now: '1970-01-01T00:00:49.000Z', text: '49 secs' },
-      { now: '1970-01-01T00:01:05.000Z', text: '1 min' },
-      { now: '1970-01-01T00:03:05.000Z', text: '3 mins' },
-      { now: '1970-01-01T01:00:00.000Z', text: '1 hr' },
-      { now: '1970-01-01T04:00:00.000Z', text: '4 hrs' },
+      { now: '1970-01-01T00:00:49.000Z', text: '49 secs ago' },
+      { now: '1970-01-01T00:01:05.000Z', text: '1 min ago' },
+      { now: '1970-01-01T00:03:05.000Z', text: '3 mins ago' },
+      { now: '1970-01-01T01:00:00.000Z', text: '1 hr ago' },
+      { now: '1970-01-01T04:00:00.000Z', text: '4 hrs ago' },
     ].forEach(test => {
       it('creates correct fuzzy string for fixture ' + test.now, () => {
         const timeStamp = fakeDate('1970-01-01T00:00:00.000Z');
@@ -158,15 +158,15 @@ describe('sidebar.util.time', function() {
       const date = new Date().toISOString();
       const callback = sandbox.stub();
       time.decayingInterval(date, callback);
-      sandbox.clock.tick(6 * msPerSecond);
+      sandbox.clock.tick(6 * second);
       assert.calledWith(callback, date);
-      sandbox.clock.tick(6 * msPerSecond);
+      sandbox.clock.tick(6 * second);
       assert.calledTwice(callback);
     });
 
     it('uses a longer delay for older timestamps', function() {
       const date = new Date().toISOString();
-      const ONE_MINUTE = minute * msPerSecond;
+      const ONE_MINUTE = minute;
       sandbox.clock.tick(10 * ONE_MINUTE);
       const callback = sandbox.stub();
       time.decayingInterval(date, callback);
@@ -183,13 +183,13 @@ describe('sidebar.util.time', function() {
       const callback = sandbox.stub();
       const cancel = time.decayingInterval(date, callback);
       cancel();
-      sandbox.clock.tick(minute * msPerSecond);
+      sandbox.clock.tick(minute);
       assert.notCalled(callback);
     });
 
     it('does not set a timeout for dates > 24hrs ago', function() {
       const date = new Date().toISOString();
-      const ONE_DAY = day * msPerSecond;
+      const ONE_DAY = day;
       sandbox.clock.tick(10 * ONE_DAY);
       const callback = sandbox.stub();
 
@@ -208,9 +208,9 @@ describe('sidebar.util.time', function() {
     });
 
     [
-      { now: '1970-01-01T00:00:10.000Z', expectedUpdateTime: 5 }, // we have a minimum of 5 secs
-      { now: '1970-01-01T00:00:20.000Z', expectedUpdateTime: 5 },
-      { now: '1970-01-01T00:00:49.000Z', expectedUpdateTime: 5 },
+      { now: '1970-01-01T00:00:10.000Z', expectedUpdateTime: 5 * second }, // we have a minimum of 5 secs
+      { now: '1970-01-01T00:00:20.000Z', expectedUpdateTime: 5 * second },
+      { now: '1970-01-01T00:00:49.000Z', expectedUpdateTime: 5 * second },
       { now: '1970-01-01T00:01:05.000Z', expectedUpdateTime: minute },
       { now: '1970-01-01T00:03:05.000Z', expectedUpdateTime: minute },
       { now: '1970-01-01T04:00:00.000Z', expectedUpdateTime: hour },

src/sidebar/util/time.js

 'use strict';
 
-const minute = 60;
-const hour = minute * 60;
-
-function lessThanThirtySecondsAgo(date, now) {
-  return now - date < 30 * 1000;
-}
-
-function lessThanOneMinuteAgo(date, now) {
-  return now - date < 60 * 1000;
-}
-
-function lessThanOneHourAgo(date, now) {
-  return now - date < 60 * 60 * 1000;
-}
-
-function lessThanOneDayAgo(date, now) {
-  return now - date < 24 * 60 * 60 * 1000;
-}
-
-function thisYear(date, now) {
-  return date.getFullYear() === now.getFullYear();
-}
-
-function delta(date, now) {
-  return Math.round((now - date) / 1000);
-}
-
-function nSec(date, now) {
-  return '{} secs'.replace('{}', Math.floor(delta(date, now)));
-}
-
-function nMin(date, now) {
-  const n = Math.floor(delta(date, now) / minute);
-  let template = '{} min';
-
-  if (n > 1) {
-    template = template + 's';
-  }
-
-  return template.replace('{}', n);
-}
-
-function nHr(date, now) {
-  const n = Math.floor(delta(date, now) / hour);
-  let template = '{} hr';
-
-  if (n > 1) {
-    template = template + 's';
-  }
+/**
+ * Utility functions for generating formatted "fuzzy" date strings and
+ * computing decaying intervals for updating those dates in a UI.
+ */
 
-  return template.replace('{}', n);
-}
+const SECOND = 1000;
+const MINUTE = 60 * SECOND;
+const HOUR = 60 * MINUTE;
 
 // Cached DateTimeFormat instances,
 // because instantiating a DateTimeFormat is expensive.
@@ -63,18 +19,32 @@ let formatters = {};
 function clearFormatters() {
   formatters = {};
 }
+
 /**
- * Efficiently return `date` formatted with `options`.
+ * Calculate time delta in milliseconds between two `Date` objects
  *
- * This is a wrapper for Intl.DateTimeFormat.format() that caches
- * DateTimeFormat instances because they're expensive to create.
- * Calling Date.toLocaleDateString() lots of times is also expensive in some
+ * @param {Date} date
+ * @param {Date} now
+ */
+function delta(date, now) {
+  return now - date;
+}
+
+/**
+ * Efficiently return date string formatted with `options`.
+ *
+ * This is a wrapper for `Intl.DateTimeFormat.format()` that caches
+ * `DateTimeFormat` instances because they're expensive to create.
+ * Calling `Date.toLocaleDateString()` lots of times is also expensive in some
  * browsers as it appears to create a new formatter for each call.
  *
+ * @param {Date} date
+ * @param {Object} options - Options for `Intl.DateTimeFormat.format()`
+ * @param {Object} Intl - JS internationalization API implementation; this
+ *                      param is present for dependency injection during test.
  * @returns {string}
- *
  */
-function format(date, options, Intl) {
+function formatIntl(date, options, Intl) {
   // If the tests have passed in a mock Intl then use it, otherwise use the
   // real one.
   if (typeof Intl === 'undefined') {
@@ -96,12 +66,36 @@ function format(date, options, Intl) {
   }
 }
 
+/**
+ * Date templating functions.
+ *
+ * @param {Date} date
+ * @param {Date} now
+ * @return {String} formatted date
+ */
+function nSec(date, now) {
+  const n = Math.floor(delta(date, now) / SECOND);
+  return `${n} secs ago`;
+}
+
+function nMin(date, now) {
+  const n = Math.floor(delta(date, now) / MINUTE);
+  const plural = n > 1 ? 's' : '';
+  return `${n} min${plural} ago`;
+}
+
+function nHr(date, now) {
+  const n = Math.floor(delta(date, now) / HOUR);
+  const plural = n > 1 ? 's' : '';
+  return `${n} hr${plural} ago`;
+}
+
 function dayAndMonth(date, now, Intl) {
-  return format(date, { month: 'short', day: 'numeric' }, Intl);
+  return formatIntl(date, { month: 'short', day: 'numeric' }, Intl);
 }
 
 function dayAndMonthAndYear(date, now, Intl) {
-  return format(
+  return formatIntl(
     date,
     { day: 'numeric', month: 'short', year: 'numeric' },
     Intl
@@ -110,37 +104,39 @@ function dayAndMonthAndYear(date, now, Intl) {
 
 const BREAKPOINTS = [
   {
-    test: lessThanThirtySecondsAgo,
-    format: function() {
-      return 'Just now';
-    },
-    nextUpdate: 1,
+    // Less than 30 seconds
+    test: (date, now) => delta(date, now) < 30 * SECOND,
+    formatFn: () => 'Just now',
+    nextUpdate: 1 * SECOND,
   },
   {
-    test: lessThanOneMinuteAgo,
-    format: nSec,
-    nextUpdate: 1,
+    // Less than 1 minute
+    test: (date, now) => delta(date, now) < 1 * MINUTE,
+    formatFn: nSec,
+    nextUpdate: 1 * SECOND,
   },
   {
-    test: lessThanOneHourAgo,
-    format: nMin,
-    nextUpdate: minute,
+    // less than one hour
+    test: (date, now) => delta(date, now) < 1 * HOUR,
+    formatFn: nMin,
+    nextUpdate: 1 * MINUTE,
   },
   {
-    test: lessThanOneDayAgo,
-    format: nHr,
-    nextUpdate: hour,
+    // less than one day
+    test: (date, now) => delta(date, now) < 24 * HOUR,
+    formatFn: nHr,
+    nextUpdate: 1 * HOUR,
   },
   {
-    test: thisYear,
-    format: dayAndMonth,
+    // this year
+    test: (date, now) => date.getFullYear() === now.getFullYear(),
+    formatFn: dayAndMonth,
     nextUpdate: null,
   },
   {
-    test: function() {
-      return true;
-    },
-    format: dayAndMonthAndYear,
+    // everything else (default case)
+    test: () => true,
+    formatFn: dayAndMonthAndYear,
     nextUpdate: null,
   },
 ];
@@ -151,40 +147,47 @@ const BREAKPOINTS = [
  *
  * @param {Date} date - The date to consider as the timestamp to format.
  * @param {Date} now - The date to consider as the current time.
- * @return {breakpoint} A dict that describes how to format the date.
+ * @return {breakpoint|null} An object that describes how to format the date or
+ *                           null if no breakpoint matches.
  */
 function getBreakpoint(date, now) {
-  let breakpoint;
-  for (let i = 0; i < BREAKPOINTS.length; i++) {
-    breakpoint = BREAKPOINTS[i];
+  for (let breakpoint of BREAKPOINTS) {
     if (breakpoint.test(date, now)) {
       return breakpoint;
     }
   }
-
   return null;
 }
 
+/**
+ * Return the number of milliseconds until the next update for a given date
+ * should be handled, based on the delta between `date` and `now`.
+ *
+ * @param {Date} date
+ * @param {Date} now
+ * @return {Number|null} - ms until next update or `null` if no update
+ *                         should occur
+ */
 function nextFuzzyUpdate(date, now) {
   if (!date) {
     return null;
   }
 
-  let secs = getBreakpoint(date, now).nextUpdate;
+  let nextUpdate = getBreakpoint(date, now).nextUpdate;
 
-  if (secs === null) {
+  if (nextUpdate === null) {
     return null;
   }
 
   // We don't want to refresh anything more often than 5 seconds
-  secs = Math.max(secs, 5);
+  nextUpdate = Math.max(nextUpdate, 5 * SECOND);
 
   // setTimeout limit is MAX_INT32=(2^31-1) (in ms),
   // which is about 24.8 days. So we don't set up any timeouts
   // longer than 24 days, that is, 2073600 seconds.
-  secs = Math.min(secs, 2073600);
+  nextUpdate = Math.min(nextUpdate, 2073600 * SECOND);
 
-  return secs;
+  return nextUpdate;
 }
 
 /**
@@ -195,48 +198,55 @@ function nextFuzzyUpdate(date, now) {
  * update frequency depends on the age of a timestamp.
  *
  * @param {String} date - An ISO 8601 date string timestamp to format.
- * @param {Date} callback - A callback function to call when the timestamp changes.
+ * @param {UpdateCallback} callback - A callback function to call when the timestamp changes.
  * @return {Function} A function that cancels the automatic refresh.
  */
 function decayingInterval(date, callback) {
   let timer;
   const timeStamp = date ? new Date(date) : null;
-  const update = function() {
+
+  const update = () => {
     const fuzzyUpdate = nextFuzzyUpdate(timeStamp, new Date());
     if (fuzzyUpdate === null) {
       return;
     }
-    const nextUpdate = 1000 * fuzzyUpdate + 500;
-    timer = setTimeout(function() {
+    const nextUpdate = fuzzyUpdate + 500;
+    timer = setTimeout(() => {
       callback(date);
       update();
     }, nextUpdate);
   };
+
   update();
 
-  return function() {
-    clearTimeout(timer);
-  };
+  return () => clearTimeout(timer);
 }
 
+/**
+ * This callback is a param for the `decayingInterval` function.
+ * @callback UpdateCallback
+ * @param {Date} - The date associated with the current interval/timeout
+ */
+
 /**
  * Formats a date as a string relative to the current date.
  *
  * @param {Date} date - The date to consider as the timestamp to format.
  * @param {Date} now - The date to consider as the current time.
- * @param {Object} Intl - JS internationalization API implementation.
+ * @param {Object} Intl - JS internationalization API implementation; this
+ *                      param is present for dependency injection during test.
  * @return {string} A 'fuzzy' string describing the relative age of the date.
  */
 function toFuzzyString(date, now, Intl) {
   if (!date) {
     return '';
   }
-  return getBreakpoint(date, now).format(date, now, Intl);
+  return getBreakpoint(date, now).formatFn(date, now, Intl);
 }
 
 module.exports = {
-  clearFormatters: clearFormatters,
+  clearFormatters: clearFormatters, // For testing
   decayingInterval: decayingInterval,
-  nextFuzzyUpdate: nextFuzzyUpdate,
+  nextFuzzyUpdate: nextFuzzyUpdate, // For testing
   toFuzzyString: toFuzzyString,
 };