Merge branch '37298-the-prettytime-utils-module-could-use-some-light-maintenance' into 'master'

Resolve some tech debt in the prettyTime module

Closes #37298

See merge request gitlab-org/gitlab-ce!14058

app/assets/javascripts/lib/utils/pretty_time.js

 import _ from 'underscore';
 
-(() => {
-  /*
+/*
  * TODO: Make these methods more configurable (e.g. stringifyTime condensed or
  * non-condensed, abbreviateTimelengths)
  * */
 
-  const utils = window.gl.utils = gl.utils || {};
-  const prettyTime = utils.prettyTime = {
-    /*
+/*
  * Accepts seconds and returns a timeObject { weeks: #, days: #, hours: #, minutes: # }
  * Seconds can be negative or positive, zero or non-zero. Can be configured for any day
  * or week length.
-    */
-    parseSeconds(seconds, { daysPerWeek = 5, hoursPerDay = 8 } = {}) {
+*/
+
+export function parseSeconds(seconds, { daysPerWeek = 5, hoursPerDay = 8 } = {}) {
   const DAYS_PER_WEEK = daysPerWeek;
   const HOURS_PER_DAY = hoursPerDay;
   const MINUTES_PER_HOUR = 60;
@@ -27,7 +25,7 @@ import _ from 'underscore';
     minutes: 1,
   };
 
-      let unorderedMinutes = prettyTime.secondsToMinutes(seconds);
+  let unorderedMinutes = Math.abs(seconds / MINUTES_PER_HOUR);
 
   return _.mapObject(timePeriodConstraints, (minutesPerPeriod) => {
     const periodCount = Math.floor(unorderedMinutes / minutesPerPeriod);
@@ -36,33 +34,28 @@ import _ from 'underscore';
 
     return periodCount;
   });
-    },
+}
 
-    /*
-    * Accepts a timeObject and returns a condensed string representation of it
-    * (e.g. '1w 2d 3h 1m' or '1h 30m'). Zero value units are not included.
-    */
+/*
+* Accepts a timeObject (see parseSeconds) and returns a condensed string representation of it
+* (e.g. '1w 2d 3h 1m' or '1h 30m'). Zero value units are not included.
+*/
 
-    stringifyTime(timeObject) {
+export function stringifyTime(timeObject) {
   const reducedTime = _.reduce(timeObject, (memo, unitValue, unitName) => {
     const isNonZero = !!unitValue;
     return isNonZero ? `${memo} ${unitValue}${unitName.charAt(0)}` : memo;
   }, '').trim();
   return reducedTime.length ? reducedTime : '0m';
-    },
+}
 
-    /*
-    * Accepts a time string of any size (e.g. '1w 2d 3h 5m' or '1w 2d') and returns
-    *  the first non-zero unit/value pair.
-    */
+/*
+* Accepts a time string of any size (e.g. '1w 2d 3h 5m' or '1w 2d') and returns
+*  the first non-zero unit/value pair.
+*/
 
-    abbreviateTime(timeStr) {
+export function abbreviateTime(timeStr) {
   return timeStr.split(' ')
     .filter(unitStr => unitStr.charAt(0) !== '0')[0];
-    },
+}
 
-    secondsToMinutes(seconds) {
-      return Math.abs(seconds / 60);
-    },
-  };
-})(window.gl || (window.gl = {}));

app/assets/javascripts/sidebar/components/time_tracking/collapsed_state.js

 import stopwatchSvg from 'icons/_icon_stopwatch.svg';
-
-import '../../../lib/utils/pretty_time';
+import { abbreviateTime } from '../../../lib/utils/pretty_time';
 
 export default {
   name: 'time-tracking-collapsed-state',
@@ -79,7 +78,7 @@ export default {
   },
   methods: {
     abbreviateTime(timeStr) {
-      return gl.utils.prettyTime.abbreviateTime(timeStr);
+      return abbreviateTime(timeStr);
     },
   },
   template: `

app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.js

-import '../../../lib/utils/pretty_time';
-
-const prettyTime = gl.utils.prettyTime;
+import { parseSeconds, stringifyTime } from '../../../lib/utils/pretty_time';
 
 export default {
   name: 'time-tracking-comparison-pane',
@@ -23,12 +21,12 @@ export default {
     },
   },
   computed: {
-    parsedRemaining() {
+    parsedTimeRemaining() {
       const diffSeconds = this.timeEstimate - this.timeSpent;
-      return prettyTime.parseSeconds(diffSeconds);
+      return parseSeconds(diffSeconds);
     },
     timeRemainingHumanReadable() {
-      return prettyTime.stringifyTime(this.parsedRemaining);
+      return stringifyTime(this.parsedTimeRemaining);
     },
     timeRemainingTooltip() {
       const prefix = this.timeRemainingMinutes < 0 ? 'Over by' : 'Time remaining:';
@@ -44,13 +42,6 @@ export default {
     timeRemainingStatusClass() {
       return this.timeEstimate >= this.timeSpent ? 'within_estimate' : 'over_estimate';
     },
-    /* Parsed time values */
-    parsedEstimate() {
-      return prettyTime.parseSeconds(this.timeEstimate);
-    },
-    parsedSpent() {
-      return prettyTime.parseSeconds(this.timeSpent);
-    },
   },
   template: `
     <div class="time-tracking-comparison-pane">

spec/javascripts/pretty_time_spec.js

-import '~/lib/utils/pretty_time';
+import { parseSeconds, abbreviateTime, stringifyTime } from '~/lib/utils/pretty_time';
 
-(() => {
-  const prettyTime = gl.utils.prettyTime;
+function assertTimeUnits(obj, minutes, hours, days, weeks) {
+  expect(obj.minutes).toBe(minutes);
+  expect(obj.hours).toBe(hours);
+  expect(obj.days).toBe(days);
+  expect(obj.weeks).toBe(weeks);
+}
 
-  describe('prettyTime methods', function () {
-    describe('parseSeconds', function () {
-      it('should correctly parse a negative value', function () {
-        const parser = prettyTime.parseSeconds;
+describe('prettyTime methods', () => {
+  describe('parseSeconds', () => {
+    it('should correctly parse a negative value', () => {
+      const zeroSeconds = parseSeconds(-1000);
 
-        const zeroSeconds = parser(-1000);
-
-        expect(zeroSeconds.minutes).toBe(16);
-        expect(zeroSeconds.hours).toBe(0);
-        expect(zeroSeconds.days).toBe(0);
-        expect(zeroSeconds.weeks).toBe(0);
+      assertTimeUnits(zeroSeconds, 16, 0, 0, 0);
     });
 
-      it('should correctly parse a zero value', function () {
-        const parser = prettyTime.parseSeconds;
-
-        const zeroSeconds = parser(0);
+    it('should correctly parse a zero value', () => {
+      const zeroSeconds = parseSeconds(0);
 
-        expect(zeroSeconds.minutes).toBe(0);
-        expect(zeroSeconds.hours).toBe(0);
-        expect(zeroSeconds.days).toBe(0);
-        expect(zeroSeconds.weeks).toBe(0);
+      assertTimeUnits(zeroSeconds, 0, 0, 0, 0);
     });
 
-      it('should correctly parse a small non-zero second values', function () {
-        const parser = prettyTime.parseSeconds;
-
-        const subOneMinute = parser(10);
-
-        expect(subOneMinute.minutes).toBe(0);
-        expect(subOneMinute.hours).toBe(0);
-        expect(subOneMinute.days).toBe(0);
-        expect(subOneMinute.weeks).toBe(0);
-
-        const aboveOneMinute = parser(100);
-
-        expect(aboveOneMinute.minutes).toBe(1);
-        expect(aboveOneMinute.hours).toBe(0);
-        expect(aboveOneMinute.days).toBe(0);
-        expect(aboveOneMinute.weeks).toBe(0);
-
-        const manyMinutes = parser(1000);
+    it('should correctly parse a small non-zero second values', () => {
+      const subOneMinute = parseSeconds(10);
+      const aboveOneMinute = parseSeconds(100);
+      const manyMinutes = parseSeconds(1000);
 
-        expect(manyMinutes.minutes).toBe(16);
-        expect(manyMinutes.hours).toBe(0);
-        expect(manyMinutes.days).toBe(0);
-        expect(manyMinutes.weeks).toBe(0);
+      assertTimeUnits(subOneMinute, 0, 0, 0, 0);
+      assertTimeUnits(aboveOneMinute, 1, 0, 0, 0);
+      assertTimeUnits(manyMinutes, 16, 0, 0, 0);
     });
 
-      it('should correctly parse large second values', function () {
-        const parser = prettyTime.parseSeconds;
+    it('should correctly parse large second values', () => {
+      const aboveOneHour = parseSeconds(4800);
+      const aboveOneDay = parseSeconds(110000);
+      const aboveOneWeek = parseSeconds(25000000);
 
-        const aboveOneHour = parser(4800);
-
-        expect(aboveOneHour.minutes).toBe(20);
-        expect(aboveOneHour.hours).toBe(1);
-        expect(aboveOneHour.days).toBe(0);
-        expect(aboveOneHour.weeks).toBe(0);
-
-        const aboveOneDay = parser(110000);
-
-        expect(aboveOneDay.minutes).toBe(33);
-        expect(aboveOneDay.hours).toBe(6);
-        expect(aboveOneDay.days).toBe(3);
-        expect(aboveOneDay.weeks).toBe(0);
-
-        const aboveOneWeek = parser(25000000);
-
-        expect(aboveOneWeek.minutes).toBe(26);
-        expect(aboveOneWeek.hours).toBe(0);
-        expect(aboveOneWeek.days).toBe(3);
-        expect(aboveOneWeek.weeks).toBe(173);
+      assertTimeUnits(aboveOneHour, 20, 1, 0, 0);
+      assertTimeUnits(aboveOneDay, 33, 6, 3, 0);
+      assertTimeUnits(aboveOneWeek, 26, 0, 3, 173);
     });
 
-      it('should correctly accept a custom param for hoursPerDay', function () {
-        const parser = prettyTime.parseSeconds;
+    it('should correctly accept a custom param for hoursPerDay', () => {
       const config = { hoursPerDay: 24 };
 
-        const aboveOneHour = parser(4800, config);
-
-        expect(aboveOneHour.minutes).toBe(20);
-        expect(aboveOneHour.hours).toBe(1);
-        expect(aboveOneHour.days).toBe(0);
-        expect(aboveOneHour.weeks).toBe(0);
-
-        const aboveOneDay = parser(110000, config);
+      const aboveOneHour = parseSeconds(4800, config);
+      const aboveOneDay = parseSeconds(110000, config);
+      const aboveOneWeek = parseSeconds(25000000, config);
 
-        expect(aboveOneDay.minutes).toBe(33);
-        expect(aboveOneDay.hours).toBe(6);
-        expect(aboveOneDay.days).toBe(1);
-        expect(aboveOneDay.weeks).toBe(0);
-
-        const aboveOneWeek = parser(25000000, config);
-
-        expect(aboveOneWeek.minutes).toBe(26);
-        expect(aboveOneWeek.hours).toBe(8);
-        expect(aboveOneWeek.days).toBe(4);
-
-        expect(aboveOneWeek.weeks).toBe(57);
+      assertTimeUnits(aboveOneHour, 20, 1, 0, 0);
+      assertTimeUnits(aboveOneDay, 33, 6, 1, 0);
+      assertTimeUnits(aboveOneWeek, 26, 8, 4, 57);
     });
 
-      it('should correctly accept a custom param for daysPerWeek', function () {
-        const parser = prettyTime.parseSeconds;
+    it('should correctly accept a custom param for daysPerWeek', () => {
       const config = { daysPerWeek: 7 };
 
-        const aboveOneHour = parser(4800, config);
-
-        expect(aboveOneHour.minutes).toBe(20);
-        expect(aboveOneHour.hours).toBe(1);
-        expect(aboveOneHour.days).toBe(0);
-        expect(aboveOneHour.weeks).toBe(0);
-
-        const aboveOneDay = parser(110000, config);
-
-        expect(aboveOneDay.minutes).toBe(33);
-        expect(aboveOneDay.hours).toBe(6);
-        expect(aboveOneDay.days).toBe(3);
-        expect(aboveOneDay.weeks).toBe(0);
-
-        const aboveOneWeek = parser(25000000, config);
+      const aboveOneHour = parseSeconds(4800, config);
+      const aboveOneDay = parseSeconds(110000, config);
+      const aboveOneWeek = parseSeconds(25000000, config);
 
-        expect(aboveOneWeek.minutes).toBe(26);
-        expect(aboveOneWeek.hours).toBe(0);
-        expect(aboveOneWeek.days).toBe(0);
-
-        expect(aboveOneWeek.weeks).toBe(124);
+      assertTimeUnits(aboveOneHour, 20, 1, 0, 0);
+      assertTimeUnits(aboveOneDay, 33, 6, 3, 0);
+      assertTimeUnits(aboveOneWeek, 26, 0, 0, 124);
     });
 
-      it('should correctly accept custom params for daysPerWeek and hoursPerDay', function () {
-        const parser = prettyTime.parseSeconds;
+    it('should correctly accept custom params for daysPerWeek and hoursPerDay', () => {
       const config = { daysPerWeek: 55, hoursPerDay: 14 };
 
-        const aboveOneHour = parser(4800, config);
-
-        expect(aboveOneHour.minutes).toBe(20);
-        expect(aboveOneHour.hours).toBe(1);
-        expect(aboveOneHour.days).toBe(0);
-        expect(aboveOneHour.weeks).toBe(0);
-
-        const aboveOneDay = parser(110000, config);
-
-        expect(aboveOneDay.minutes).toBe(33);
-        expect(aboveOneDay.hours).toBe(2);
-        expect(aboveOneDay.days).toBe(2);
-        expect(aboveOneDay.weeks).toBe(0);
+      const aboveOneHour = parseSeconds(4800, config);
+      const aboveOneDay = parseSeconds(110000, config);
+      const aboveOneWeek = parseSeconds(25000000, config);
 
-        const aboveOneWeek = parser(25000000, config);
-
-        expect(aboveOneWeek.minutes).toBe(26);
-        expect(aboveOneWeek.hours).toBe(0);
-        expect(aboveOneWeek.days).toBe(1);
-
-        expect(aboveOneWeek.weeks).toBe(9);
+      assertTimeUnits(aboveOneHour, 20, 1, 0, 0);
+      assertTimeUnits(aboveOneDay, 33, 2, 2, 0);
+      assertTimeUnits(aboveOneWeek, 26, 0, 1, 9);
     });
   });
 
-    describe('stringifyTime', function () {
-      it('should stringify values with all non-zero units', function () {
+  describe('stringifyTime', () => {
+    it('should stringify values with all non-zero units', () => {
       const timeObject = {
         weeks: 1,
         days: 4,
@@ -168,12 +87,12 @@ import '~/lib/utils/pretty_time';
         minutes: 20,
       };
 
-        const timeString = prettyTime.stringifyTime(timeObject);
+      const timeString = stringifyTime(timeObject);
 
       expect(timeString).toBe('1w 4d 7h 20m');
     });
 
-      it('should stringify values with some non-zero units', function () {
+    it('should stringify values with some non-zero units', () => {
       const timeObject = {
         weeks: 0,
         days: 4,
@@ -181,12 +100,12 @@ import '~/lib/utils/pretty_time';
         minutes: 20,
       };
 
-        const timeString = prettyTime.stringifyTime(timeObject);
+      const timeString = stringifyTime(timeObject);
 
       expect(timeString).toBe('4d 20m');
     });
 
-      it('should stringify values with no non-zero units', function () {
+    it('should stringify values with no non-zero units', () => {
       const timeObject = {
         weeks: 0,
         days: 0,
@@ -194,22 +113,21 @@ import '~/lib/utils/pretty_time';
         minutes: 0,
       };
 
-        const timeString = prettyTime.stringifyTime(timeObject);
+      const timeString = stringifyTime(timeObject);
 
       expect(timeString).toBe('0m');
     });
   });
 
-    describe('abbreviateTime', function () {
-      it('should abbreviate stringified times for weeks', function () {
+  describe('abbreviateTime', () => {
+    it('should abbreviate stringified times for weeks', () => {
       const fullTimeString = '1w 3d 4h 5m';
-        expect(prettyTime.abbreviateTime(fullTimeString)).toBe('1w');
+      expect(abbreviateTime(fullTimeString)).toBe('1w');
     });
 
-      it('should abbreviate stringified times for non-weeks', function () {
+    it('should abbreviate stringified times for non-weeks', () => {
       const fullTimeString = '0w 3d 4h 5m';
-        expect(prettyTime.abbreviateTime(fullTimeString)).toBe('3d');
-      });
+      expect(abbreviateTime(fullTimeString)).toBe('3d');
     });
   });
-})(window.gl || (window.gl = {}));
+});