Repository: moment/luxon
Pull request #1299 (2022-10-10 06:13:05)

feat: add rescale option to Duration.normalize method
## The Problem

There's a use case right now where you have a duration from a set of units but want them to be displayed to the user in a more "human readable" format.

For example, you might want to display the duration between various 2 datetimes, which can defer in hours or just a few seconds:

```js
const dur = Interval.fromDateTimes(start, end).toDuration(['hours', 'minutes', 'seconds', 'milliseconds'])
console.log(dur.toHuman())

// example outputs:
// 4 hours, 0 minutes, 12 seconds and 0 milliseconds
// 0 hours, 0 minutes, 5 seconds and 0 milliseconds
// 0 hours, 54 minutes, 0 seconds and 0 milliseconds
```

Because the duration can vary greatly between just a few seconds and many hours, the current output is completely useless.
One would expect the `normalize()` method to solve this problem but unfortunately, **the current implementation only adjust the  units currently in use**.

```js
const dur = Duration.fromObject({ milliseconds: 90000 })
console.log(dur.normalize().toHuman())

// output: 90000 milliseconds
```

In this example, normalizing 90000 milliseconds will still get you... 90000 milliseconds. Not useful at all. Again, because the duration can vary greatly, using `shiftTo()` is not a viable option.

## The Solution

This PR adds a `rescale` option to the `normalize()` method which also shifts units to their largest representation, on top of the current normalization. This basically remove any units with values of 0, giving you a clean output.

```js
const dur = Interval.fromDateTimes(start, end).toDuration(['hours', 'minutes', 'seconds', 'milliseconds'])
console.log(dur.normalize({ rescale: true }).toHuman())

// example outputs:
// 4 hours and 12 seconds
// 5 seconds
// 54 minutes
```

```js
const dur = Duration.fromObject({ milliseconds: 90000 })
console.log(dur.normalize({ rescale: true }).toHuman())

// output: 1 minute and 30 seconds
```

## Notes

This could possibly be its own method (e.g. `rescale()`), but in my opinion, this is what you would expect the `normalize()` method to do. Therefore, I think it makes more sense to simply add an optional flag to the current `normalize()` method.

Thanks!

linux-foundation-easycla[bot]:  <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19772872/47118129/1299/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328"></a><br/><br /><ul><li><a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19772872/47118129/1299/#/?version=2' target='_blank'>:x:</a> - login: @NGPixel / name: Nicolas Giard . The commit (724b57f4910b5b0d0ed678d301cb808b0221fd33, 8aa2e11b5a9bfe215a19c4c2adaaaecd807b03fa, 2ff70c1bdd075b5194678624cd88415c383d0a3d, c6607991daaeec7c2bb6d746f580ee3e6ac23ae4, 350aa7124ab4fc6d35b92c412b0744c45f10126b) is not authorized under a signed CLA. <a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19772872/47118129/1299/#/?version=2' target='_blank'>Please click here to be authorized</a>. For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul>
icambron: Ok, so this makes sense, but here are some changes I'd like. Essentially, I'd like to decompose this.

1. Let's add `removeZeros()` as a top-level method. It just removes all the zeroed -.
2. Let's add a `shiftToAll()` method defined as `shiftTo("years", "months", "weeks", "days", "hours", "minutes", "seconds", "milliseconds")`
3. Let's not add other features to `normalize` but instead just add a new method called `rescale()`. Rescale is implemented as `this.normalize().shiftToAll().removeZeros()`

Does that make sense?
NGPixel: @icambron I refactored the code as requested.

Patch:
diff --git a/src/duration.js b/src/duration.js
index 21dcf5fb..fc512b6e 100644
--- a/src/duration.js
+++ b/src/duration.js
@@ -155,6 +155,17 @@ function normalizeValues(matrix, vals) {
   }, null);
 }
 
+// Remove all properties with a value of 0 from an object
+function removeZeroes(vals) {
+  const newVals = {};
+  for (const [key, value] of Object.entries(vals)) {
+    if (value !== 0) {
+      newVals[key] = value;
+    }
+  }
+  return newVals;
+}
+
 /**
  * A Duration object represents a period of time, like "2 months" or "1 day, 1 hour". Conceptually, it's just a map of units to their quantities, accompanied by some additional configuration and methods for creating, parsing, interrogating, transforming, and formatting them. They can be used on their own or in conjunction with other Luxon types; for example, you can use {@link DateTime#plus} to add a Duration object to a DateTime, producing another DateTime.
  *
@@ -701,6 +712,17 @@ export default class Duration {
     return clone(this, { values: vals }, true);
   }
 
+  /**
+   * Rescale units to its largest representation
+   * @example Duration.fromObject({ milliseconds: 90000 }).rescale().toObject() //=> { minutes: 1, seconds: 30 }
+   * @return {Duration}
+   */
+  rescale() {
+    if (!this.isValid) return this;
+    const vals = removeZeroes(this.normalize().shiftToAll().toObject());
+    return clone(this, { values: vals }, true);
+  }
+
   /**
    * Convert this Duration into its representation in a different set of units.
    * @example Duration.fromObject({ hours: 1, seconds: 30 }).shiftTo('minutes', 'milliseconds').toObject() //=> { minutes: 60, milliseconds: 30000 }
@@ -765,6 +787,25 @@ export default class Duration {
     return clone(this, { values: built }, true).normalize();
   }
 
+  /**
+   * Shift this Duration to all available units.
+   * Same as shiftTo("years", "months", "weeks", "days", "hours", "minutes", "seconds", "milliseconds")
+   * @return {Duration}
+   */
+  shiftToAll() {
+    if (!this.isValid) return this;
+    return this.shiftTo(
+      "years",
+      "months",
+      "weeks",
+      "days",
+      "hours",
+      "minutes",
+      "seconds",
+      "milliseconds"
+    );
+  }
+
   /**
    * Return the negative of this Duration.
    * @example Duration.fromObject({ hours: 1, seconds: 30 }).negate().toObject() //=> { hours: -1, seconds: -30 }


Test patch:
diff --git a/test/duration/units.test.js b/test/duration/units.test.js
index e96c29ad..676f4d6f 100644
--- a/test/duration/units.test.js
+++ b/test/duration/units.test.js
@@ -96,6 +96,29 @@ test("Duration#shiftTo boils hours down to hours and minutes", () => {
   });
 });
 
+//------
+// #shiftToAll()
+//-------
+test("Duration#shiftToAll shifts to all available units", () => {
+  const dur = Duration.fromMillis(5760000).shiftToAll();
+  expect(dur.toObject()).toEqual({
+    years: 0,
+    months: 0,
+    weeks: 0,
+    days: 0,
+    hours: 1,
+    minutes: 36,
+    seconds: 0,
+    milliseconds: 0,
+  });
+});
+
+test("Duration#shiftToAll maintains invalidity", () => {
+  const dur = Duration.invalid("because").shiftToAll();
+  expect(dur.isValid).toBe(false);
+  expect(dur.invalidReason).toBe("because");
+});
+
 //------
 // #normalize()
 //-------
@@ -207,6 +230,30 @@ test("Duration#normalize can convert all unit pairs", () => {
   }
 });
 
+//------
+// #rescale()
+//-------
+test("Duration#rescale normalizes, shifts to all units and remove units with a value of 0", () => {
+  const sets = [
+    [{ milliseconds: 90000 }, { minutes: 1, seconds: 30 }],
+    [
+      { minutes: 70, milliseconds: 12100 },
+      { hours: 1, minutes: 10, seconds: 12, milliseconds: 100 },
+    ],
+    [{ months: 2, days: -30 }, { months: 1 }],
+  ];
+
+  sets.forEach(([from, to]) => {
+    expect(Duration.fromObject(from).rescale().toObject()).toEqual(to);
+  });
+});
+
+test("Duration#rescale maintains invalidity", () => {
+  const dur = Duration.invalid("because").rescale();
+  expect(dur.isValid).toBe(false);
+  expect(dur.invalidReason).toBe("because");
+});
+
 //------
 // #as()
 //-------