UI: updates info table row jsdoc (#20697)

* reuse format-duration helper

* add changelog

* update duration

* fix 0 assuming 0s

* VAULT-16593/ update tests, remove formatting assumptions

* more tests

* add calc function

* woops, typo use %

* update variable name

* add back one template test

* refactor to handle all duration string cases, including 3m

* ok lets do that differently

* comment cleanup

* address comments

* push to rerun checks

changelog/20697.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+ui: update detail views that render ttl durations to display full unit instead of letter (i.e. 'days' instead of 'd')
+```

ui/lib/core/addon/components/info-table-row.hbs

@@ -62,7 +62,7 @@
       {{else if @formatDate}}
         {{date-format @value @formatDate}}
       {{else if @formatTtl}}
-        {{this.formattedTtl}}
+        {{format-duration @value}}
       {{else}}
         {{#if (eq @type "array")}}
           <InfoTableItemArray

ui/lib/core/addon/components/info-table-row.js

@@ -7,7 +7,6 @@ import { typeOf } from '@ember/utils';
 import Component from '@glimmer/component';
 import { tracked } from '@glimmer/tracking';
 import { action } from '@ember/object';
-import { convertFromSeconds, largestUnitFromSeconds } from 'core/utils/duration-utils';
 
 /**
  * @module InfoTableRow
@@ -19,19 +18,22 @@ import { convertFromSeconds, largestUnitFromSeconds } from 'core/utils/duration-
  * <InfoTableRow @value={{5}} @label="TTL" @helperText="Some description"/>
  * ```
  *
- * @param label=null {string} - The display name for the value.
- * @param helperText=null {string} - Text to describe the value displayed beneath the label.
- * @param value=null {any} - The the data to be displayed - by default the content of the component will only show if there is a value. Also note that special handling is given to boolean values - they will render `Yes` for true and `No` for false. Overridden by block if exists
- * @param [alwaysRender=false] {Boolean} - Indicates if the component content should be always be rendered.  When false, the value of `value` will be used to determine if the component should render.
- * @param [defaultShown] {String} - Text that renders as value if alwaysRender=true. Eg. "Vault default"
- * @param [tooltipText] {String} - Text if a tooltip should display over the value.
- * @param [isTooltipCopyable] {Boolean} - Allows tooltip click to copy
- * @param [type=array] {string} - The type of value being passed in.  This is used for when you want to trim an array.  For example, if you have an array value that can equal length 15+ this will trim to show 5 and count how many more are there
- * @param [isLink=true] {Boolean} - Passed through to InfoTableItemArray. Indicates if the item should contain a link-to component.  Only setup for arrays, but this could be changed if needed.
- * @param [modelType=null] {string} - Passed through to InfoTableItemArray. Tells what model you want data for the allOptions to be returned from.  Used in conjunction with the the isLink.
- * @param [queryParam] {String} - Passed through to InfoTableItemArray. If you want to specific a tab for the View All XX to display to.  Ex= role
- * @param [backend] {String} - Passed through to InfoTableItemArray. To specify secrets backend to point link to  Ex= transformation
- * @param [viewAll] {String} - Passed through to InfoTableItemArray. Specify the word at the end of the link View all.
+ * @param {string} label=null - The display name for the value.
+ * @param {string} helperText=null - Text to describe the value displayed beneath the label.
+ * @param {any} value=null  - The the data to be displayed - by default the content of the component will only show if there is a value. Also note that special handling is given to boolean values - they will render `Yes` for true and `No` for false. Overridden by block if exists
+ * @param {boolean} [alwaysRender=false] - Indicates if the component content should be always be rendered.  When false, the value of `value` will be used to determine if the component should render.
+ * @param {string} [defaultShown] - Text that renders as value if alwaysRender=true. Eg. "Vault default"
+ * @param {string} [tooltipText] - Text if a tooltip should display over the value.
+ * @param {boolean} [isTooltipCopyable]  - Allows tooltip click to copy
+ * @param {string} [formatDate] - A string of the desired date format that's passed to the date-format helper to render timestamps (ex. "MMM d yyyy, h:mm:ss aaa", see: https://date-fns.org/v2.30.0/docs/format)
+ * @param {boolean} [formatTtl=false] - When true, value is passed to the format-duration helper, useful for TTL values
+ * @param {string} [type=array] - The type of value being passed in.  This is used for when you want to trim an array.  For example, if you have an array value that can equal length 15+ this will trim to show 5 and count how many more are there
+ * * InfoTableItemArray *
+ * @param {boolean} [isLink=true] - Passed through to InfoTableItemArray. Indicates if the item should contain a link-to component.  Only setup for arrays, but this could be changed if needed.
+ * @param {string} [modelType=null] - Passed through to InfoTableItemArray. Tells what model you want data for the allOptions to be returned from.  Used in conjunction with the the isLink.
+ * @param {string} [queryParam] - Passed through to InfoTableItemArray. If you want to specific a tab for the View All XX to display to.  Ex= role
+ * @param {string} [backend] - Passed through to InfoTableItemArray. To specify secrets backend to point link to  Ex= transformation
+ * @param {string} [viewAll] - Passed through to InfoTableItemArray. Specify the word at the end of the link View all.
  */
 
 export default class InfoTableRowComponent extends Component {
@@ -62,14 +64,6 @@ export default class InfoTableRowComponent extends Component {
         return false;
     }
   }
-  get formattedTtl() {
-    const { value } = this.args;
-    if (Number.isInteger(value)) {
-      const unit = largestUnitFromSeconds(value);
-      return `${convertFromSeconds(value, unit)}${unit}`;
-    }
-    return value;
-  }
 
   @action
   calculateLabelOverflow(el) {

ui/lib/core/addon/components/ttl-picker.js

@@ -30,13 +30,13 @@ import Component from '@glimmer/component';
 import { typeOf } from '@ember/utils';
 import { tracked } from '@glimmer/tracking';
 import { action } from '@ember/object';
-import Duration from '@icholy/duration';
 import { guidFor } from '@ember/object/internals';
 import Ember from 'ember';
 import { restartableTask, timeout } from 'ember-concurrency';
 import {
   convertFromSeconds,
   convertToSeconds,
+  durationToSeconds,
   goSafeConvertFromSeconds,
   largestUnitFromSeconds,
 } from 'core/utils/duration-utils';
@@ -80,18 +80,19 @@ export default class TtlPickerComponent extends Component {
 
   initializeTtl() {
     const initialValue = this.args.initialValue;
+
     let seconds = 0;
+
     if (typeof initialValue === 'number') {
       // if the passed value is a number, assume unit is seconds
       seconds = initialValue;
     } else {
-      try {
-        seconds = Duration.parse(initialValue).seconds();
-      } catch (e) {
-        // if parsing fails leave it empty
-        return;
-      }
+      const parseDuration = durationToSeconds(initialValue);
+      // if parsing fails leave it empty
+      if (parseDuration === null) return;
+      seconds = parseDuration;
     }
+
     const unit = largestUnitFromSeconds(seconds);
     this.time = convertFromSeconds(seconds, unit);
     this.unit = unit;

ui/lib/core/addon/helpers/format-duration.js

@@ -4,25 +4,28 @@
  */
 
 import { helper } from '@ember/component/helper';
+import { durationToSeconds } from 'core/utils/duration-utils';
 import { formatDuration, intervalToDuration } from 'date-fns';
 
-export function duration([time], { nullable = false }) {
-  // intervalToDuration creates a durationObject that turns the seconds (ex 3600) to respective:
-  // { years: 0, months: 0, days: 0, hours: 1, minutes: 0, seconds: 0 }
-  // then formatDuration returns the filled in keys of the durationObject
-  // nullable if you don't want a value to be returned instead of 0s
+export function duration([time]) {
+  // 0 (integer) does not necessarily mean 0 seconds, i.e. it can represent using system ttl defaults
+  if (time === 0) return time;
 
-  if (nullable && (time === '0' || time === 0)) {
-    return null;
+  // assume numbers are seconds or parses duration strings into seconds
+  const seconds = durationToSeconds(time);
+
+  if (Number.isInteger(seconds)) {
+    // durationObject: { years: 0, months: 0, days: 0, hours: 1, minutes: 0, seconds: 6 }
+    const durationObject = intervalToDuration({ start: 0, end: seconds * 1000 });
+
+    if (Object.values(durationObject).every((v) => v === 0)) {
+      // formatDuration returns an empty string if every value is 0
+      return '0 seconds';
+    }
+    // convert to human-readable format: '1 hour 6 seconds'
+    return formatDuration(durationObject);
   }
-
-  // time must be in seconds
-  const duration = Number.parseInt(time, 10);
-  if (isNaN(duration)) {
-    return time;
-  }
-
-  return formatDuration(intervalToDuration({ start: 0, end: duration * 1000 }));
+  return time;
 }
 
 export default helper(duration);

ui/lib/core/addon/utils/duration-utils.ts

@@ -7,6 +7,8 @@
  * These utils are used for managing Duration type values
  * (eg. '30m', '365d'). Most often used in the context of TTLs
  */
+import Duration from '@icholy/duration';
+
 interface SecondsMap {
   s: 1;
   m: 60;
@@ -44,3 +46,15 @@ export const largestUnitFromSeconds = (seconds: number) => {
   }
   return unit;
 };
+
+// parses duration string ('3m') and returns seconds
+export const durationToSeconds = (duration: string) => {
+  // we assume numbers are seconds
+  if (typeof duration === 'number') return duration;
+  try {
+    return Duration.parse(duration).seconds();
+  } catch (e) {
+    // since 0 is falsy, parent should explicitly check for null and decide how to handle parsing error
+    return null;
+  }
+};

ui/tests/acceptance/secrets/backend/kv/secret-test.js

@@ -225,7 +225,7 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
     assert.strictEqual(cas.trim(), 'Yes', 'displays the cas set when configuring the secret-engine');
     assert.strictEqual(
       deleteVersionAfter.trim(),
-      '1s',
+      '1 second',
       'displays the delete version after set when configuring the secret-engine'
     );
     await deleteEngine(enginePath, assert);

ui/tests/acceptance/settings/mount-secret-backend-test.js

@@ -24,6 +24,11 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
 
   hooks.beforeEach(function () {
     this.uid = uuidv4();
+    this.calcDays = (hours) => {
+      const days = Math.floor(hours / 24);
+      const remainder = hours % 24;
+      return `${days} days ${remainder} hours`;
+    };
     return authPage.login();
   });
 
@@ -32,7 +37,6 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
     const path = `mount-kv-${this.uid}`;
     const defaultTTLHours = 100;
     const maxTTLHours = 300;
-
     await page.visit();
 
     assert.strictEqual(currentRouteName(), 'vault.cluster.settings.mount-secret-backend');
@@ -49,8 +53,8 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
       .maxTTLVal(maxTTLHours)
       .submit();
     await configPage.visit({ backend: path });
-    assert.strictEqual(configPage.defaultTTL, `${defaultTTLHours}h`, 'shows the proper TTL');
-    assert.strictEqual(configPage.maxTTL, `${maxTTLHours}h`, 'shows the proper max TTL');
+    assert.strictEqual(configPage.defaultTTL, `${this.calcDays(defaultTTLHours)}`, 'shows the proper TTL');
+    assert.strictEqual(configPage.maxTTL, `${this.calcDays(maxTTLHours)}`, 'shows the proper max TTL');
   });
 
   test('it sets the ttl when enabled then disabled', async function (assert) {
@@ -67,14 +71,17 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
       .path(path)
       .toggleOptions()
       .enableDefaultTtl()
-      .enableDefaultTtl()
       .enableMaxTtl()
       .maxTTLUnit('h')
       .maxTTLVal(maxTTLHours)
       .submit();
     await configPage.visit({ backend: path });
-    assert.strictEqual(configPage.defaultTTL, '0s', 'shows the proper TTL');
-    assert.strictEqual(configPage.maxTTL, `${maxTTLHours}h`, 'shows the proper max TTL');
+    assert.strictEqual(
+      configPage.defaultTTL,
+      '0',
+      'shows 0 (with no seconds) which means using the system default TTL'
+    ); // https://developer.hashicorp.com/vault/api-docs/system/mounts#default_lease_ttl-1
+    assert.strictEqual(configPage.maxTTL, `${this.calcDays(maxTTLHours)}`, 'shows the proper max TTL');
   });
 
   test('it sets the max ttl after pki chosen, resets after', async function (assert) {

ui/tests/integration/components/info-table-row-test.js

@@ -7,7 +7,7 @@ import { module, test } from 'qunit';
 import { resolve } from 'rsvp';
 import Service from '@ember/service';
 import { setupRenderingTest } from 'ember-qunit';
-import { render, settled, triggerEvent } from '@ember/test-helpers';
+import { render, triggerEvent } from '@ember/test-helpers';
 import hbs from 'htmlbars-inline-precompile';
 
 const VALUE = 'test value';
@@ -277,9 +277,19 @@ module('Integration | Component | InfoTableRow', function (hooks) {
       @formatTtl={{true}}
     />`);
 
-    assert.dom('[data-test-value-div]').hasText('100m', 'Translates number value to largest unit');
+    assert
+      .dom('[data-test-value-div]')
+      .hasText('1 hour 40 minutes', 'Translates number value to largest unit with carryover of minutes');
+  });
+
+  test('Formats string value when formatTtl present', async function (assert) {
     this.set('value', '45m');
-    await settled();
-    assert.dom('[data-test-value-div]').hasText('45m', 'Renders non-number values as-is');
+    await render(hbs`<InfoTableRow
+      @label={{this.label}}
+      @value={{this.value}}
+      @formatTtl={{true}}
+    />`);
+
+    assert.dom('[data-test-value-div]').hasText('45 minutes', 'it formats string duration');
   });
 });

ui/tests/integration/components/kubernetes/page/role/details-test.js

@@ -63,7 +63,7 @@ module('Integration | Component | kubernetes | Page::Role::Details', function (h
           .dom(`[data-test-row-label="${field.label}"]`)
           .hasText(field.label, `${field.label} label renders`);
         const modelValue = this.model[field.key];
-        const value = field.key.includes('Ttl') ? duration([modelValue], {}) : modelValue;
+        const value = field.key.includes('Ttl') ? duration([modelValue]) : modelValue;
         assert.dom(`[data-test-row-value="${field.label}"]`).hasText(value, `${field.label} value renders`);
       });
     };

ui/tests/integration/components/pki/page/pki-role-details-test.js

@@ -42,7 +42,7 @@ module('Integration | Component | pki role details page', function (hooks) {
       .dom(SELECTORS.extKeyUsageValue)
       .hasText('bar,baz', 'Key usage shows comma-joined values when array has items');
     assert.dom(SELECTORS.noStoreValue).containsText('Yes', 'noStore shows opposite of what the value is');
-    assert.dom(SELECTORS.customTtlValue).containsText('10m', 'TTL shown as duration');
+    assert.dom(SELECTORS.customTtlValue).containsText('10 minutes', 'TTL shown as duration');
   });
 
   test('it should render the notAfter date if present', async function (assert) {

ui/tests/integration/helpers/format-duration-test.js

@@ -5,51 +5,46 @@
 
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
+import { duration } from 'core/helpers/format-duration';
 import { render } from '@ember/test-helpers';
-import hbs from 'htmlbars-inline-precompile';
+import { hbs } from 'ember-cli-htmlbars';
 
 module('Integration | Helper | format-duration', function (hooks) {
   setupRenderingTest(hooks);
 
-  test('it supports strings and formats seconds', async function (assert) {
-    await render(hbs`<p data-test-format-duration>Date: {{format-duration '3606'}}</p>`);
+  test('it formats-duration in template view', async function (assert) {
+    await render(hbs`<p data-test-format-duration>Date: {{format-duration 3606 }}</p>`);
 
     assert
       .dom('[data-test-format-duration]')
       .includesText('1 hour 6 seconds', 'it renders the duration in hours and seconds');
   });
 
-  test('it is able to format seconds and days', async function (assert) {
-    await render(hbs`<p data-test-format-duration>Date: {{format-duration '93606000'}}</p>`);
-
-    assert
-      .dom('[data-test-format-duration]')
-      .includesText(
-        '2 years 11 months 18 days 9 hours 40 minutes',
-        'it renders with years months and days and hours and minutes'
-      );
+  test('it formats seconds', async function (assert) {
+    assert.strictEqual(duration([3606]), '1 hour 6 seconds');
   });
 
-  test('it is able to format numbers', async function (assert) {
-    this.set('number', 60);
-    await render(hbs`<p data-test-format-duration>Date: {{format-duration this.number}}</p>`);
-
-    assert
-      .dom('[data-test-format-duration]')
-      .includesText('1 minute', 'it renders duration when a number is passed in.');
+  test('it format seconds and days', async function (assert) {
+    assert.strictEqual(duration([93606000]), '2 years 11 months 18 days 9 hours 40 minutes');
   });
 
-  test('it renders the input if time not found', async function (assert) {
-    this.set('number', 'arg');
-
-    await render(hbs`<p data-test-format-duration>Date: {{format-duration this.number}}</p>`);
-    assert.dom('[data-test-format-duration]').hasText('Date: arg');
+  test('it returns the integer 0', async function (assert) {
+    assert.strictEqual(duration([0]), 0);
   });
 
-  test('it renders no value if nullable true', async function (assert) {
-    this.set('number', 0);
+  test('it returns plain or non-parsable string inputs', async function (assert) {
+    assert.strictEqual(duration(['0']), '0 seconds'); // assume seconds for '0' string values only
+    assert.strictEqual(duration(['arg']), 'arg');
+    assert.strictEqual(duration(['1245']), '1245');
+    assert.strictEqual(duration(['11y']), '11y');
+  });
 
-    await render(hbs`<p data-test-format-duration>Date: {{format-duration this.number nullable=true}}</p>`);
-    assert.dom('[data-test-format-duration]').hasText('Date:');
+  test('it formats duration string inputs', async function (assert) {
+    assert.strictEqual(duration(['0s']), '0 seconds');
+    assert.strictEqual(duration(['5s']), '5 seconds');
+    assert.strictEqual(duration(['545h']), '22 days 17 hours');
+    assert.strictEqual(duration(['8h']), '8 hours');
+    assert.strictEqual(duration(['3m']), '3 minutes');
+    assert.strictEqual(duration(['10d']), '10 days');
   });
 });

ui/types/global.d.ts

@@ -10,3 +10,8 @@ declare module 'vault/templates/*' {
   const tmpl: TemplateFactory;
   export default tmpl;
 }
+
+declare module '@icholy/duration' {
+  import Duration from '@icholy/duration';
+  export default Duration;
+}