From 960afc91ea71313da4906be177d13a271472a210 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Fri, 19 May 2023 16:37:05 -0700 Subject: [PATCH 01/15] reuse format-duration helper --- .../core/addon/components/info-table-row.hbs | 2 +- .../core/addon/components/info-table-row.js | 38 ++++++++----------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/ui/lib/core/addon/components/info-table-row.hbs b/ui/lib/core/addon/components/info-table-row.hbs index 1ceb489ce671..a1baf6929802 100644 --- a/ui/lib/core/addon/components/info-table-row.hbs +++ b/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")}} * ``` * - * @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) { From 154d098ab693a5dc781e21b283122c5e8b8beb46 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Mon, 22 May 2023 12:40:01 -0700 Subject: [PATCH 02/15] add changelog --- changelog/20697.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/20697.txt diff --git a/changelog/20697.txt b/changelog/20697.txt new file mode 100644 index 000000000000..be80443714da --- /dev/null +++ b/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') +``` From 5bd2d38c8e809668a56214cfeebb2fe30a1990c0 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Mon, 22 May 2023 14:36:36 -0700 Subject: [PATCH 03/15] update duration --- ui/lib/core/addon/components/info-table-row.hbs | 2 +- ui/lib/core/addon/components/info-table-row.js | 9 +++++++++ ui/lib/core/addon/helpers/format-duration.js | 1 - ui/tests/integration/components/info-table-row-test.js | 4 +++- .../components/pki/page/pki-role-details-test.js | 2 +- 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ui/lib/core/addon/components/info-table-row.hbs b/ui/lib/core/addon/components/info-table-row.hbs index a1baf6929802..1ceb489ce671 100644 --- a/ui/lib/core/addon/components/info-table-row.hbs +++ b/ui/lib/core/addon/components/info-table-row.hbs @@ -62,7 +62,7 @@ {{else if @formatDate}} {{date-format @value @formatDate}} {{else if @formatTtl}} - {{format-duration @value}} + {{this.formattedTtl}} {{else}} {{#if (eq @type "array")}} `); - 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'); this.set('value', '45m'); await settled(); assert.dom('[data-test-value-div]').hasText('45m', 'Renders non-number values as-is'); diff --git a/ui/tests/integration/components/pki/page/pki-role-details-test.js b/ui/tests/integration/components/pki/page/pki-role-details-test.js index 9f3eedc1ab18..12c9cfd0beb6 100644 --- a/ui/tests/integration/components/pki/page/pki-role-details-test.js +++ b/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) { From c77772bda06c481e7b3634b5a1a1f533f2f854a7 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Mon, 22 May 2023 18:15:17 -0700 Subject: [PATCH 04/15] fix 0 assuming 0s --- ui/lib/core/addon/components/info-table-row.hbs | 2 +- ui/lib/core/addon/components/info-table-row.js | 9 --------- ui/lib/core/addon/helpers/format-duration.js | 5 ++++- .../acceptance/secrets/backend/kv/secret-test.js | 2 +- .../settings/mount-secret-backend-test.js | 13 ++++++++----- .../integration/helpers/format-duration-test.js | 7 +++++++ 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ui/lib/core/addon/components/info-table-row.hbs b/ui/lib/core/addon/components/info-table-row.hbs index 1ceb489ce671..a1baf6929802 100644 --- a/ui/lib/core/addon/components/info-table-row.hbs +++ b/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")}} Date: {{format-duration this.number nullable=true}}

`); assert.dom('[data-test-format-duration]').hasText('Date:'); }); + + test('it renders 0 if nullable false', async function (assert) { + this.set('number', 0); + + await render(hbs`

Date: {{format-duration this.number}}

`); + assert.dom('[data-test-format-duration]').hasText('Date: 0'); + }); }); From 0e0eb2027a3465e2c28b292b388d54cb33402c93 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 11:18:50 -0700 Subject: [PATCH 05/15] VAULT-16593/ update tests, remove formatting assumptions --- ui/lib/core/addon/helpers/format-duration.js | 26 ++++----- .../helpers/format-duration-test.js | 53 ++++--------------- 2 files changed, 20 insertions(+), 59 deletions(-) diff --git a/ui/lib/core/addon/helpers/format-duration.js b/ui/lib/core/addon/helpers/format-duration.js index 78481c4f5f80..2f4ed378acad 100644 --- a/ui/lib/core/addon/helpers/format-duration.js +++ b/ui/lib/core/addon/helpers/format-duration.js @@ -6,25 +6,17 @@ import { helper } from '@ember/component/helper'; 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 - if (nullable && (time === '0' || time === 0)) { - return null; - } - +export function duration([time]) { // time must be in seconds - const duration = Number.parseInt(time, 10); - - // Sometimes 0 does not mean 0 seconds, but can mean to use system defaults - // to avoid making any assumptions, just return 0 - if (isNaN(duration) || duration === 0) { - return time; + // 0 does not always mean 0 seconds, i.e. it can representing using system defaults + if (Number.isInteger(time) && time !== 0) { + const milliseconds = time * 1000; + // pass milliseconds to intervalToDuration returns a durationObject: { years: 0, months: 0, days: 0, hours: 1, minutes: 0, seconds: 6 } + // formatDuration converts to human-readable format: '1 hour 6 seconds' + return formatDuration(intervalToDuration({ start: 0, end: milliseconds })); } - - return formatDuration(intervalToDuration({ start: 0, end: duration * 1000 })); + // to avoid making any assumptions return strings and 0 as-is + return time; } export default helper(duration); diff --git a/ui/tests/integration/helpers/format-duration-test.js b/ui/tests/integration/helpers/format-duration-test.js index 72623dbd703b..81296a095040 100644 --- a/ui/tests/integration/helpers/format-duration-test.js +++ b/ui/tests/integration/helpers/format-duration-test.js @@ -5,58 +5,27 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; -import hbs from 'htmlbars-inline-precompile'; +import { duration } from 'core/helpers/format-duration'; module('Integration | Helper | format-duration', function (hooks) { setupRenderingTest(hooks); - test('it supports strings and formats seconds', async function (assert) { - await render(hbs`

Date: {{format-duration '3606'}}

`); - - assert - .dom('[data-test-format-duration]') - .includesText('1 hour 6 seconds', 'it renders the duration in hours and seconds'); + test('it formats seconds', async function (assert) { + assert.strictEqual(duration([3606]), '1 hour 6 seconds'); }); - test('it is able to format seconds and days', async function (assert) { - await render(hbs`

Date: {{format-duration '93606000'}}

`); - - 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 format seconds and days', async function (assert) { + assert.strictEqual(duration([93606000]), '2 years 11 months 18 days 9 hours 40 minutes'); }); - test('it is able to format numbers', async function (assert) { - this.set('number', 60); - await render(hbs`

Date: {{format-duration this.number}}

`); - - assert - .dom('[data-test-format-duration]') - .includesText('1 minute', 'it renders duration when a number is passed in.'); + test('it returns the integer 0', async function (assert) { + assert.strictEqual(duration([0]), 0); }); - test('it renders the input if time not found', async function (assert) { + test('it returns string inputs', async function (assert) { this.set('number', 'arg'); - - await render(hbs`

Date: {{format-duration this.number}}

`); - assert.dom('[data-test-format-duration]').hasText('Date: arg'); - }); - - test('it renders no value if nullable true', async function (assert) { - this.set('number', 0); - - await render(hbs`

Date: {{format-duration this.number nullable=true}}

`); - assert.dom('[data-test-format-duration]').hasText('Date:'); - }); - - test('it renders 0 if nullable false', async function (assert) { - this.set('number', 0); - - await render(hbs`

Date: {{format-duration this.number}}

`); - assert.dom('[data-test-format-duration]').hasText('Date: 0'); + assert.strictEqual(duration(['0']), '0'); + assert.strictEqual(duration(['arg']), 'arg'); + assert.strictEqual(duration(['1245']), '1245'); }); }); From 6f89133d4ac016cbcf93f79277fddfef57b55d13 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 11:41:00 -0700 Subject: [PATCH 06/15] more tests --- .../integration/components/info-table-row-test.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ui/tests/integration/components/info-table-row-test.js b/ui/tests/integration/components/info-table-row-test.js index 456674e69abb..a9aabaf62988 100644 --- a/ui/tests/integration/components/info-table-row-test.js +++ b/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'; @@ -280,8 +280,16 @@ module('Integration | Component | InfoTableRow', function (hooks) { 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 as-is when formatTtl present', async function (assert) { this.set('value', '45m'); - await settled(); + await render(hbs``); + assert.dom('[data-test-value-div]').hasText('45m', 'Renders non-number values as-is'); }); }); From aec4e63b15a072eced7a2e5a951f1a57e4f7235f Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 12:46:16 -0700 Subject: [PATCH 07/15] add calc function --- .../acceptance/settings/mount-secret-backend-test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ui/tests/acceptance/settings/mount-secret-backend-test.js b/ui/tests/acceptance/settings/mount-secret-backend-test.js index b079d70fd1f6..ada396cf4288 100644 --- a/ui/tests/acceptance/settings/mount-secret-backend-test.js +++ b/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 remainingHours = hours / 24; + return `${days} days ${remainingHours} 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, '4 days 4 hours', 'shows the proper TTL'); - assert.strictEqual(configPage.maxTTL, '12 days 12 hours', '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) { @@ -77,7 +81,7 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) { '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, '12 days 12 hours', 'shows the proper max TTL'); + 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) { From 60ed98cee0aa3b4e1698e15f78edb8ab5717e5cf Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 12:47:44 -0700 Subject: [PATCH 08/15] woops, typo use % --- ui/tests/acceptance/settings/mount-secret-backend-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/tests/acceptance/settings/mount-secret-backend-test.js b/ui/tests/acceptance/settings/mount-secret-backend-test.js index ada396cf4288..b26ff8166180 100644 --- a/ui/tests/acceptance/settings/mount-secret-backend-test.js +++ b/ui/tests/acceptance/settings/mount-secret-backend-test.js @@ -26,7 +26,7 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) { this.uid = uuidv4(); this.calcDays = (hours) => { const days = Math.floor(hours / 24); - const remainingHours = hours / 24; + const remainingHours = hours % 24; return `${days} days ${remainingHours} hours`; }; return authPage.login(); From 9d46204406b22872c998af13379ed9c052793514 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 12:48:41 -0700 Subject: [PATCH 09/15] update variable name --- ui/tests/acceptance/settings/mount-secret-backend-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/tests/acceptance/settings/mount-secret-backend-test.js b/ui/tests/acceptance/settings/mount-secret-backend-test.js index b26ff8166180..a853d31722a0 100644 --- a/ui/tests/acceptance/settings/mount-secret-backend-test.js +++ b/ui/tests/acceptance/settings/mount-secret-backend-test.js @@ -26,8 +26,8 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) { this.uid = uuidv4(); this.calcDays = (hours) => { const days = Math.floor(hours / 24); - const remainingHours = hours % 24; - return `${days} days ${remainingHours} hours`; + const remainder = hours % 24; + return `${days} days ${remainder} hours`; }; return authPage.login(); }); From 8caa011000414654409fd74c327feb8fe687ff90 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 12:54:41 -0700 Subject: [PATCH 10/15] add back one template test --- ui/tests/integration/helpers/format-duration-test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ui/tests/integration/helpers/format-duration-test.js b/ui/tests/integration/helpers/format-duration-test.js index 81296a095040..afc9a56b5a4f 100644 --- a/ui/tests/integration/helpers/format-duration-test.js +++ b/ui/tests/integration/helpers/format-duration-test.js @@ -6,10 +6,20 @@ 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 'ember-cli-htmlbars'; module('Integration | Helper | format-duration', function (hooks) { setupRenderingTest(hooks); + test('it formats-duration in template view', async function (assert) { + await render(hbs`

Date: {{format-duration 3606 }}

`); + + assert + .dom('[data-test-format-duration]') + .includesText('1 hour 6 seconds', 'it renders the duration in hours and seconds'); + }); + test('it formats seconds', async function (assert) { assert.strictEqual(duration([3606]), '1 hour 6 seconds'); }); From d691f67e71d61bf2713513218053383ca0382bcc Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Thu, 25 May 2023 16:45:14 -0700 Subject: [PATCH 11/15] refactor to handle all duration string cases, including 3m --- ui/lib/core/addon/components/ttl-picker.js | 16 +++---------- ui/lib/core/addon/helpers/format-duration.js | 24 ++++++++++++------- ui/lib/core/addon/utils/duration-utils.ts | 18 ++++++++++++++ .../kubernetes/page/role/details-test.js | 2 +- .../helpers/format-duration-test.js | 15 +++++++++--- ui/types/global.d.ts | 5 ++++ 6 files changed, 55 insertions(+), 25 deletions(-) diff --git a/ui/lib/core/addon/components/ttl-picker.js b/ui/lib/core/addon/components/ttl-picker.js index 4ee5c649f9a1..8e688923fc48 100644 --- a/ui/lib/core/addon/components/ttl-picker.js +++ b/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,8 @@ 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 seconds = durationToSeconds({ duration: initialValue, fallback: 0 }); + const unit = largestUnitFromSeconds(seconds); this.time = convertFromSeconds(seconds, unit); this.unit = unit; diff --git a/ui/lib/core/addon/helpers/format-duration.js b/ui/lib/core/addon/helpers/format-duration.js index 2f4ed378acad..6b0269121070 100644 --- a/ui/lib/core/addon/helpers/format-duration.js +++ b/ui/lib/core/addon/helpers/format-duration.js @@ -4,18 +4,26 @@ */ import { helper } from '@ember/component/helper'; +import { durationToSeconds } from 'core/utils/duration-utils'; import { formatDuration, intervalToDuration } from 'date-fns'; export function duration([time]) { - // time must be in seconds - // 0 does not always mean 0 seconds, i.e. it can representing using system defaults - if (Number.isInteger(time) && time !== 0) { - const milliseconds = time * 1000; - // pass milliseconds to intervalToDuration returns a durationObject: { years: 0, months: 0, days: 0, hours: 1, minutes: 0, seconds: 6 } - // formatDuration converts to human-readable format: '1 hour 6 seconds' - return formatDuration(intervalToDuration({ start: 0, end: milliseconds })); + // 0 does not necessarily mean 0 seconds, i.e. it can represent using system ttl defaults + if (time === 0) return time; + + const seconds = durationToSeconds({ duration: time, fallback: time }); + + if (Number.isInteger(seconds)) { + // intervalToDuration returns a 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'; + } + // converts to human-readable format: '1 hour 6 seconds' + return formatDuration(durationObject); } - // to avoid making any assumptions return strings and 0 as-is return time; } diff --git a/ui/lib/core/addon/utils/duration-utils.ts b/ui/lib/core/addon/utils/duration-utils.ts index 75cf790a5459..2a06074866ea 100644 --- a/ui/lib/core/addon/utils/duration-utils.ts +++ b/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,19 @@ export const largestUnitFromSeconds = (seconds: number) => { } return unit; }; + +interface Args { + duration: string | number; + fallback: unknown; +} +// parses number or duration string ('3m') and returns seconds +export const durationToSeconds = ({ duration, fallback }: Args) => { + // any number we assume is in seconds + if (typeof duration === 'number') return duration; + try { + return Duration.parse(duration).seconds(); + } catch (e) { + // value to return if parsing fails + return fallback; + } +}; diff --git a/ui/tests/integration/components/kubernetes/page/role/details-test.js b/ui/tests/integration/components/kubernetes/page/role/details-test.js index 0c13d9e6850e..6644dba1e961 100644 --- a/ui/tests/integration/components/kubernetes/page/role/details-test.js +++ b/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`); }); }; diff --git a/ui/tests/integration/helpers/format-duration-test.js b/ui/tests/integration/helpers/format-duration-test.js index afc9a56b5a4f..d0adf4179740 100644 --- a/ui/tests/integration/helpers/format-duration-test.js +++ b/ui/tests/integration/helpers/format-duration-test.js @@ -32,10 +32,19 @@ module('Integration | Helper | format-duration', function (hooks) { assert.strictEqual(duration([0]), 0); }); - test('it returns string inputs', async function (assert) { - this.set('number', 'arg'); - assert.strictEqual(duration(['0']), '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'); + }); + + 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'); }); }); diff --git a/ui/types/global.d.ts b/ui/types/global.d.ts index ca2678eae375..d82c0ff558e0 100644 --- a/ui/types/global.d.ts +++ b/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; +} From 556de917c0b765124c639b23fba1c573a2b49a2e Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Fri, 26 May 2023 10:06:27 -0700 Subject: [PATCH 12/15] ok lets do that differently --- ui/lib/core/addon/components/ttl-picker.js | 12 +++++++++++- ui/lib/core/addon/helpers/format-duration.js | 2 +- ui/lib/core/addon/utils/duration-utils.ts | 12 ++++-------- .../integration/components/info-table-row-test.js | 4 ++-- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/ui/lib/core/addon/components/ttl-picker.js b/ui/lib/core/addon/components/ttl-picker.js index 8e688923fc48..bbf9ddc22aec 100644 --- a/ui/lib/core/addon/components/ttl-picker.js +++ b/ui/lib/core/addon/components/ttl-picker.js @@ -80,7 +80,17 @@ export default class TtlPickerComponent extends Component { initializeTtl() { const initialValue = this.args.initialValue; - const seconds = durationToSeconds({ duration: initialValue, fallback: 0 }); + + let seconds = 0; + + if (typeof initialValue === 'number') { + // if the passed value is a number, assume unit is seconds + seconds = initialValue; + } else { + const parseDuration = durationToSeconds(initialValue); + if (!parseDuration) return; + seconds = parseDuration; + } const unit = largestUnitFromSeconds(seconds); this.time = convertFromSeconds(seconds, unit); diff --git a/ui/lib/core/addon/helpers/format-duration.js b/ui/lib/core/addon/helpers/format-duration.js index 6b0269121070..590cc5f73507 100644 --- a/ui/lib/core/addon/helpers/format-duration.js +++ b/ui/lib/core/addon/helpers/format-duration.js @@ -11,7 +11,7 @@ export function duration([time]) { // 0 does not necessarily mean 0 seconds, i.e. it can represent using system ttl defaults if (time === 0) return time; - const seconds = durationToSeconds({ duration: time, fallback: time }); + const seconds = durationToSeconds(time); if (Number.isInteger(seconds)) { // intervalToDuration returns a durationObject: { years: 0, months: 0, days: 0, hours: 1, minutes: 0, seconds: 6 } diff --git a/ui/lib/core/addon/utils/duration-utils.ts b/ui/lib/core/addon/utils/duration-utils.ts index 2a06074866ea..a2d7eda5760a 100644 --- a/ui/lib/core/addon/utils/duration-utils.ts +++ b/ui/lib/core/addon/utils/duration-utils.ts @@ -47,18 +47,14 @@ export const largestUnitFromSeconds = (seconds: number) => { return unit; }; -interface Args { - duration: string | number; - fallback: unknown; -} -// parses number or duration string ('3m') and returns seconds -export const durationToSeconds = ({ duration, fallback }: Args) => { +// parses duration string ('3m') and returns seconds +export const durationToSeconds = (duration: string) => { // any number we assume is in seconds if (typeof duration === 'number') return duration; try { return Duration.parse(duration).seconds(); } catch (e) { - // value to return if parsing fails - return fallback; + // return false so parent can decide how to handle parsing error + return false; } }; diff --git a/ui/tests/integration/components/info-table-row-test.js b/ui/tests/integration/components/info-table-row-test.js index a9aabaf62988..685a6ab9aa02 100644 --- a/ui/tests/integration/components/info-table-row-test.js +++ b/ui/tests/integration/components/info-table-row-test.js @@ -282,7 +282,7 @@ module('Integration | Component | InfoTableRow', function (hooks) { .hasText('1 hour 40 minutes', 'Translates number value to largest unit with carryover of minutes'); }); - test('Formats string value as-is when formatTtl present', async function (assert) { + test('Formats string value when formatTtl present', async function (assert) { this.set('value', '45m'); await render(hbs``); - assert.dom('[data-test-value-div]').hasText('45m', 'Renders non-number values as-is'); + assert.dom('[data-test-value-div]').hasText('45 minutes', 'it formats string duration'); }); }); From e38215b3b52ef0e55dee41305d940e77616a4a94 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Fri, 26 May 2023 10:47:59 -0700 Subject: [PATCH 13/15] comment cleanup --- ui/lib/core/addon/components/ttl-picker.js | 1 + ui/lib/core/addon/helpers/format-duration.js | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ui/lib/core/addon/components/ttl-picker.js b/ui/lib/core/addon/components/ttl-picker.js index bbf9ddc22aec..32352b8059ab 100644 --- a/ui/lib/core/addon/components/ttl-picker.js +++ b/ui/lib/core/addon/components/ttl-picker.js @@ -88,6 +88,7 @@ export default class TtlPickerComponent extends Component { seconds = initialValue; } else { const parseDuration = durationToSeconds(initialValue); + // if parsing fails leave it empty if (!parseDuration) return; seconds = parseDuration; } diff --git a/ui/lib/core/addon/helpers/format-duration.js b/ui/lib/core/addon/helpers/format-duration.js index 590cc5f73507..18f5128e0c22 100644 --- a/ui/lib/core/addon/helpers/format-duration.js +++ b/ui/lib/core/addon/helpers/format-duration.js @@ -8,20 +8,21 @@ import { durationToSeconds } from 'core/utils/duration-utils'; import { formatDuration, intervalToDuration } from 'date-fns'; export function duration([time]) { - // 0 does not necessarily mean 0 seconds, i.e. it can represent using system ttl defaults + // 0 (integer) does not necessarily mean 0 seconds, i.e. it can represent using system ttl defaults if (time === 0) return time; + // assume numbers are seconds or parses duration strings into seconds const seconds = durationToSeconds(time); if (Number.isInteger(seconds)) { - // intervalToDuration returns a durationObject: { years: 0, months: 0, days: 0, hours: 1, minutes: 0, seconds: 6 } + // 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'; } - // converts to human-readable format: '1 hour 6 seconds' + // convert to human-readable format: '1 hour 6 seconds' return formatDuration(durationObject); } return time; From 3eea3eec3334bd56c28f165126492f23777e09bc Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Fri, 26 May 2023 12:23:02 -0700 Subject: [PATCH 14/15] address comments --- ui/lib/core/addon/components/ttl-picker.js | 2 +- ui/lib/core/addon/utils/duration-utils.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/lib/core/addon/components/ttl-picker.js b/ui/lib/core/addon/components/ttl-picker.js index 32352b8059ab..c05fff67644f 100644 --- a/ui/lib/core/addon/components/ttl-picker.js +++ b/ui/lib/core/addon/components/ttl-picker.js @@ -89,7 +89,7 @@ export default class TtlPickerComponent extends Component { } else { const parseDuration = durationToSeconds(initialValue); // if parsing fails leave it empty - if (!parseDuration) return; + if (parseDuration === null) return; seconds = parseDuration; } diff --git a/ui/lib/core/addon/utils/duration-utils.ts b/ui/lib/core/addon/utils/duration-utils.ts index a2d7eda5760a..3e891c0e2922 100644 --- a/ui/lib/core/addon/utils/duration-utils.ts +++ b/ui/lib/core/addon/utils/duration-utils.ts @@ -54,7 +54,7 @@ export const durationToSeconds = (duration: string) => { try { return Duration.parse(duration).seconds(); } catch (e) { - // return false so parent can decide how to handle parsing error - return false; + // since 0 is falsy, parent should explicitly check for null and decide how to handle parsing error + return null; } }; From 026af0569b7018e5337b22a6bb9b976c96e5f39e Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 30 May 2023 09:37:13 -0700 Subject: [PATCH 15/15] push to rerun checks --- ui/lib/core/addon/utils/duration-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/lib/core/addon/utils/duration-utils.ts b/ui/lib/core/addon/utils/duration-utils.ts index 3e891c0e2922..89e984025705 100644 --- a/ui/lib/core/addon/utils/duration-utils.ts +++ b/ui/lib/core/addon/utils/duration-utils.ts @@ -49,7 +49,7 @@ export const largestUnitFromSeconds = (seconds: number) => { // parses duration string ('3m') and returns seconds export const durationToSeconds = (duration: string) => { - // any number we assume is in seconds + // we assume numbers are seconds if (typeof duration === 'number') return duration; try { return Duration.parse(duration).seconds();