From 06948c7588f135c2aa2c5ae921de90d24d27384c Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Fri, 3 Oct 2025 15:33:36 -0400 Subject: [PATCH] [UI][Bugfix]: General Settings Bugfix Pt.1 (#9820) (#9837) * VAULT-39898 page scroll locked after discarding * VAULT-39904 update lease duration card text * Ensure ttl fields dont get reset if there are errors * Fix failing tests * VAULT-39900 close the unsaved changes modal if it errors on save * Switch max and lease duration * Refactor modal save/close * Remove promise! * Code cleanup! Co-authored-by: Kianna <30884335+kiannaquach@users.noreply.github.com> --- ...ease-duration.hbs => secrets-duration.hbs} | 13 ++++---- .../secret-engine/page/general-settings.hbs | 13 +++++--- .../secret-engine/page/general-settings.ts | 30 ++++++++++++++----- .../components/secret-engine/ttl-picker-v2.ts | 16 +++++++--- .../secrets/backend/keymgmt/workflow-test.js | 2 +- ...ation-test.js => secrets-duration-test.js} | 14 +++++---- .../page/general-setting-test.js | 2 +- .../secret-engine/ttl-picker-v2-test.js | 10 ++++--- 8 files changed, 66 insertions(+), 34 deletions(-) rename ui/app/components/secret-engine/card/{lease-duration.hbs => secrets-duration.hbs} (61%) rename ui/tests/integration/components/secret-engine/card/{lease-duration-test.js => secrets-duration-test.js} (71%) diff --git a/ui/app/components/secret-engine/card/lease-duration.hbs b/ui/app/components/secret-engine/card/secrets-duration.hbs similarity index 61% rename from ui/app/components/secret-engine/card/lease-duration.hbs rename to ui/app/components/secret-engine/card/secrets-duration.hbs index 2f6b28b17b..98a0373c1f 100644 --- a/ui/app/components/secret-engine/card/lease-duration.hbs +++ b/ui/app/components/secret-engine/card/secrets-duration.hbs @@ -6,16 +6,15 @@ @level="mid" @hasBorder={{true}} class="has-padding-m has-top-bottom-margin" - data-test-card-container="lease-duration" + data-test-card-container="secrets duration" ...attributes > - Lease Duration + Secrets duration - Lease, measured by the “time-to-live” value, defines how long any secret issued - by this engine remains valid. - {{! TODO: Verify what link this is supposed to be }} + Time-to-live defines how long secrets issued by this engine remain valid before + expiring. - - + + \ No newline at end of file diff --git a/ui/app/components/secret-engine/page/general-settings.hbs b/ui/app/components/secret-engine/page/general-settings.hbs index 27fb127cb4..2d9dcaef23 100644 --- a/ui/app/components/secret-engine/page/general-settings.hbs +++ b/ui/app/components/secret-engine/page/general-settings.hbs @@ -31,7 +31,12 @@ - + @@ -61,20 +66,20 @@
Would you like to apply them? - + diff --git a/ui/app/components/secret-engine/page/general-settings.ts b/ui/app/components/secret-engine/page/general-settings.ts index e915b10e07..7c86d8998c 100644 --- a/ui/app/components/secret-engine/page/general-settings.ts +++ b/ui/app/components/secret-engine/page/general-settings.ts @@ -49,6 +49,9 @@ export default class GeneralSettingsComponent extends Component { @tracked showUnsavedChangesModal = false; @tracked changedFields: string[] = []; + @tracked defaultLeaseUnit = ''; + @tracked maxLeaseUnit = ''; + originalModel = JSON.parse(JSON.stringify(this.args.model)); getUnsavedChanges(newModel: SecretsEngineResource, originalModel: SecretsEngineResource) { @@ -183,20 +186,31 @@ export default class GeneralSettingsComponent extends Component { @action closeUnsavedChangesModal() { - this.showUnsavedChangesModal = !this.showUnsavedChangesModal; + this.showUnsavedChangesModal = false; this.changedFields = []; } @action - discardChanges() { - this.closeUnsavedChangesModal(); - this.router.transitionTo(this.args?.model?.secretsEngine?.backendConfigurationLink); + closeAndHandle(close: () => void, action: 'save' | 'discard') { + close(); + + if (action === 'save') { + this.saveGeneralSettings.perform(); + } + + if (action === 'discard') { + this.router.transitionTo(this.args?.model?.secretsEngine?.backendConfigurationLink); + } } - saveGeneralSettings = task(async (event) => { - event.preventDefault(); + saveGeneralSettings = task(async (event?) => { + // event is an optional arg because saveGeneralSettings can be called in the closeAndHandle function. + // There are instances where we will save in the modal where that doesn't require an event. + if (event) event.preventDefault(); - const { defaultLeaseTime, maxLeaseTime } = this.getFormData(); + const { defaultLeaseTime, defaultLeaseUnit, maxLeaseTime, maxLeaseUnit } = this.getFormData(); + this.defaultLeaseUnit = defaultLeaseUnit; + this.maxLeaseUnit = maxLeaseUnit; if (!this.validateTtl(defaultLeaseTime) || !this.validateTtl(maxLeaseTime)) { this.errorMessage = 'Only use numbers for this setting.'; @@ -214,11 +228,11 @@ export default class GeneralSettingsComponent extends Component { }); this.flashMessages.success('Engine settings successfully updated.'); + this.router.transitionTo(this.args?.model?.secretsEngine?.backendConfigurationLink); } catch (e) { const { message } = await this.api.parseError(e); this.errorMessage = message; - this.flashMessages.danger(`Try again or check your network connection. ${message}`); } }); } diff --git a/ui/app/components/secret-engine/ttl-picker-v2.ts b/ui/app/components/secret-engine/ttl-picker-v2.ts index 7505077a96..e7c73d1151 100644 --- a/ui/app/components/secret-engine/ttl-picker-v2.ts +++ b/ui/app/components/secret-engine/ttl-picker-v2.ts @@ -33,6 +33,7 @@ interface Args { model: { secretsEngine: SecretsEngineResource; }; + initialUnit: string; ttlKey: 'default_lease_ttl' | 'max_lease_ttl'; } @@ -66,7 +67,11 @@ export default class TtlPickerV2 extends Component { } else { const parseDuration = durationToSeconds(ttlValue || ''); // if parsing fails leave it empty - if (parseDuration === null) return; + if (parseDuration === null) { + this.time = ttlValue || ''; + this.selectedUnit = this.args.initialUnit; + return; + } seconds = parseDuration; } @@ -87,11 +92,14 @@ export default class TtlPickerV2 extends Component { get formField() { return { - label: this.args?.ttlKey === 'default_lease_ttl' ? 'Time-to-live (TTL)' : 'Maximum Time-to-live (TTL)', + label: + this.args?.ttlKey === 'default_lease_ttl' + ? 'Default time-to-live (TTL)' + : 'Maximum time-to-live (TTL)', helperText: this.args?.ttlKey === 'default_lease_ttl' - ? 'Standard expiry deadline.' - : 'Maximum possible extension for expiry.', + ? 'How long secrets in this engine stay valid.' + : 'Maximum extension for the secrets life beyond default.', }; } diff --git a/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js b/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js index cc11704470..8be956de8a 100644 --- a/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js +++ b/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js @@ -43,7 +43,7 @@ module('Acceptance | Enterprise | keymgmt-configuration-workflow', function (hoo assert.dom(SELECTORS.versionCard.engineType).hasText(keymgmtType, 'shows keymgmt engine type'); assert.dom(GENERAL.cardContainer('metadata')).exists('metadata card exists'); assert.dom(GENERAL.inputByAttr('path')).hasValue(`${keymgmtType}/`, 'show path value'); - assert.dom(GENERAL.cardContainer('lease-duration')).exists('lease-duration card exists'); + assert.dom(GENERAL.cardContainer('secrets duration')).exists('secrets duration card exists'); assert.dom(GENERAL.cardContainer('security')).exists('security card exists'); // fill in values to tune diff --git a/ui/tests/integration/components/secret-engine/card/lease-duration-test.js b/ui/tests/integration/components/secret-engine/card/secrets-duration-test.js similarity index 71% rename from ui/tests/integration/components/secret-engine/card/lease-duration-test.js rename to ui/tests/integration/components/secret-engine/card/secrets-duration-test.js index 020ff545fa..63bb1b27ac 100644 --- a/ui/tests/integration/components/secret-engine/card/lease-duration-test.js +++ b/ui/tests/integration/components/secret-engine/card/secrets-duration-test.js @@ -21,12 +21,16 @@ module('Integration | Component | SecretEngine::Card::LeaseDuration', function ( test('it shows default and max ttl pickers', async function (assert) { assert.expect(5); await render(hbs` - + `); assert.dom(SELECTORS.ttlPickerV2).exists({ count: 2 }); - assert.dom(GENERAL.fieldLabelbyAttr('default_lease_ttl')).hasText('Time-to-live (TTL)'); - assert.dom(GENERAL.fieldLabelbyAttr('max_lease_ttl')).hasText('Maximum Time-to-live (TTL)'); - assert.dom(GENERAL.helpTextByAttr('default_lease_ttl')).hasText('Standard expiry deadline.'); - assert.dom(GENERAL.helpTextByAttr('max_lease_ttl')).hasText('Maximum possible extension for expiry.'); + assert.dom(GENERAL.fieldLabelbyAttr('default_lease_ttl')).hasText('Default time-to-live (TTL)'); + assert.dom(GENERAL.fieldLabelbyAttr('max_lease_ttl')).hasText('Maximum time-to-live (TTL)'); + assert + .dom(GENERAL.helpTextByAttr('default_lease_ttl')) + .hasText('How long secrets in this engine stay valid.'); + assert + .dom(GENERAL.helpTextByAttr('max_lease_ttl')) + .hasText('Maximum extension for the secrets life beyond default.'); }); }); diff --git a/ui/tests/integration/components/secret-engine/page/general-setting-test.js b/ui/tests/integration/components/secret-engine/page/general-setting-test.js index c8c0891e46..a82649a5f1 100644 --- a/ui/tests/integration/components/secret-engine/page/general-setting-test.js +++ b/ui/tests/integration/components/secret-engine/page/general-setting-test.js @@ -48,7 +48,7 @@ module('Integration | Component | SecretEngine::Page::GeneralSettings', function await render(hbs` `); - assert.dom(GENERAL.cardContainer('lease-duration')).exists(`Lease duration card exists`); + assert.dom(GENERAL.cardContainer('secrets duration')).exists(`Lease duration card exists`); assert.dom(GENERAL.cardContainer('security')).exists(`Security card exists`); assert.dom(GENERAL.cardContainer('version')).exists(`Version card exists`); assert.dom(GENERAL.cardContainer('metadata')).exists(`Metadata card exists`); diff --git a/ui/tests/integration/components/secret-engine/ttl-picker-v2-test.js b/ui/tests/integration/components/secret-engine/ttl-picker-v2-test.js index c86aee5ad2..bfc725bb69 100644 --- a/ui/tests/integration/components/secret-engine/ttl-picker-v2-test.js +++ b/ui/tests/integration/components/secret-engine/ttl-picker-v2-test.js @@ -23,8 +23,8 @@ module('Integration | Component | SecretEngine::TtlPickerV2', function (hooks) { await render(hbs` `); - assert.dom(GENERAL.fieldLabelbyAttr(this.ttlKey)).hasText('Time-to-live (TTL)'); - assert.dom(GENERAL.helpTextByAttr(this.ttlKey)).hasText('Standard expiry deadline.'); + assert.dom(GENERAL.fieldLabelbyAttr(this.ttlKey)).hasText('Default time-to-live (TTL)'); + assert.dom(GENERAL.helpTextByAttr(this.ttlKey)).hasText('How long secrets in this engine stay valid.'); await fillIn(GENERAL.inputByAttr(this.ttlKey), 5); await fillIn(GENERAL.selectByAttr(this.ttlKey), 'm'); assert.dom(GENERAL.inputByAttr(this.ttlKey)).hasValue('5'); @@ -37,8 +37,10 @@ module('Integration | Component | SecretEngine::TtlPickerV2', function (hooks) { await render(hbs` `); - assert.dom(GENERAL.fieldLabelbyAttr(this.ttlKey)).hasText('Maximum Time-to-live (TTL)'); - assert.dom(GENERAL.helpTextByAttr(this.ttlKey)).hasText('Maximum possible extension for expiry.'); + assert.dom(GENERAL.fieldLabelbyAttr(this.ttlKey)).hasText('Maximum time-to-live (TTL)'); + assert + .dom(GENERAL.helpTextByAttr(this.ttlKey)) + .hasText('Maximum extension for the secrets life beyond default.'); await fillIn(GENERAL.inputByAttr(this.ttlKey), 10); await fillIn(GENERAL.selectByAttr(this.ttlKey), 'm'); assert.dom(GENERAL.inputByAttr(this.ttlKey)).hasValue('10');