diff --git a/changelog/_10148.txt b/changelog/_10148.txt new file mode 100644 index 0000000000..87d26e7124 --- /dev/null +++ b/changelog/_10148.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: After clicking Save or Discard in the unsaved changes modal, the user will now navigate to the intended destination link. +``` \ 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 aaae8e154b..e0f275602e 100644 --- a/ui/app/components/secret-engine/page/general-settings.hbs +++ b/ui/app/components/secret-engine/page/general-settings.hbs @@ -45,7 +45,7 @@ aria-label="general settings form" id="general-settings-form" > - + Mount parameters that you can tune to fit required engine behavior. @@ -69,7 +69,12 @@
- +
@@ -77,6 +82,6 @@ \ No newline at end of file diff --git a/ui/app/components/secret-engine/page/general-settings.ts b/ui/app/components/secret-engine/page/general-settings.ts index 80f56e1e41..bb3fb36b3f 100644 --- a/ui/app/components/secret-engine/page/general-settings.ts +++ b/ui/app/components/secret-engine/page/general-settings.ts @@ -8,9 +8,8 @@ import { task } from 'ember-concurrency'; import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { convertToSeconds } from 'core/utils/duration-utils'; -import { action } from '@ember/object'; -import type Router from '@ember/routing/router'; +import type RouterService from '@ember/routing/router-service'; import type FlashMessageService from 'vault/services/flash-messages'; import type ApiService from 'vault/services/api'; import type SecretsEngineResource from 'vault/resources/secrets/engine'; @@ -40,7 +39,7 @@ interface Args { } export default class GeneralSettingsComponent extends Component { - @service declare readonly router: Router; + @service declare readonly router: RouterService; @service declare readonly api: ApiService; @service declare readonly flashMessages: FlashMessageService; @service declare readonly unsavedChanges: UnsavedChangesService; @@ -172,17 +171,10 @@ export default class GeneralSettingsComponent extends Component { this.flashMessages.success('Engine settings successfully updated.', { title: 'Configuration saved' }); - this.unsavedChanges.showModal = false; - this.router.transitionTo(this.router.currentRouteName); + this.unsavedChanges.transition('vault.cluster.secrets.backend.configuration.general-settings'); } catch (e) { const { message } = await this.api.parseError(e); this.errorMessage = message; } }); - - @action - discardChanges() { - const currentRouteName = this.router.currentRouteName; - this.router.transitionTo(currentRouteName); - } } diff --git a/ui/app/routes/vault/cluster/secrets/backend/configuration/general-settings.ts b/ui/app/routes/vault/cluster/secrets/backend/configuration/general-settings.ts index 475a225d21..b3bb404040 100644 --- a/ui/app/routes/vault/cluster/secrets/backend/configuration/general-settings.ts +++ b/ui/app/routes/vault/cluster/secrets/backend/configuration/general-settings.ts @@ -33,8 +33,6 @@ export default class SecretsBackendConfigurationGeneralSettingsRoute extends Rou @service declare readonly pluginCatalog: PluginCatalogService; @service declare readonly unsavedChanges: UnsavedChangesService; - oldModel: Record | undefined; - async model() { const secretsEngine = this.modelFor('vault.cluster.secrets.backend') as SecretsEngineResource; const { data } = await this.pluginCatalog.getRawPluginCatalogData(); @@ -67,22 +65,16 @@ export default class SecretsBackendConfigurationGeneralSettingsRoute extends Rou willTransition(transition: Transition) { // eslint-disable-next-line ember/no-controller-access-in-routes const controller = this.controllerFor(this.routeName) as RouteController; - const { model } = controller; + const state = model ? (model['secretsEngine'] as Record | undefined) : {}; + this.unsavedChanges.setup(state); // Only intercept transition if leaving THIS route and there are changes const targetRoute = transition?.to?.name ?? ''; - // Saving transitions to the index route, so we do not one to intercept the transition then - if (this.routeName !== targetRoute && this.oldModel && model) { - const oldModel = this.oldModel['secretsEngine'] as Record | undefined; - const currentModel = model['secretsEngine'] as Record | undefined; - this.unsavedChanges.setupProperties(oldModel, currentModel); - this.unsavedChanges.getDiff(); - if (this.unsavedChanges.hasChanges) { - transition.abort(); - this.unsavedChanges.showModal = true; - } + if (this.routeName !== targetRoute && this.unsavedChanges.hasChanges) { + transition.abort(); + this.unsavedChanges.show(transition); } return true; } diff --git a/ui/app/services/unsaved-changes.ts b/ui/app/services/unsaved-changes.ts index 58d7c48dca..f7a4ab2a19 100644 --- a/ui/app/services/unsaved-changes.ts +++ b/ui/app/services/unsaved-changes.ts @@ -6,35 +6,67 @@ import Service from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { create } from 'jsondiffpatch'; +import { action } from '@ember/object'; +import { service } from '@ember/service'; + +import type Transition from '@ember/routing/transition'; +import type RouterService from '@ember/routing/router-service'; // this service tracks the unsaved changes modal state. export default class UnsavedChangesService extends Service { - @tracked changedFields: Array = []; + @service declare readonly router: RouterService; + @tracked showModal = false; @tracked initialState: Record | undefined; @tracked currentState: Record | undefined; + @tracked intendedTransition: Transition | undefined; // saved transition from willTransition hook before exiting with unsaved changes - setupProperties( - initialState: Record | undefined, - currentState: Record | undefined - ) { - this.initialState = initialState; - this.currentState = currentState; + setup(state: Record | undefined) { + // ensure unsaved-changes intendedTransition is intiially set to undefined each time the user transition + this.intendedTransition = undefined; + // set up unsaved-changes service state + this.currentState = state; } - getDiff() { + get changedFields() { const diffpatcher = create({}); const delta = diffpatcher.diff(this.initialState, this.currentState); - const changedFields = delta ? Object.keys(delta) : []; - - this.changedFields = changedFields; - - return changedFields; + return delta ? Object.keys(delta) : []; } get hasChanges() { return this.changedFields.length > 0; } + + get transitionInfo() { + return { + routeName: this.intendedTransition?.to?.name, + params: this.intendedTransition?.to?.params, + }; + } + + show(transition: Transition) { + this.intendedTransition = transition; + this.showModal = true; + } + + // This method is to update the initial state so it can be called after a successful + // save or if the user has decided to discard changes + @action + resetUnsavedState() { + this.initialState = this.currentState; + } + + @action + transition(route: string) { + const { routeName: intendedRoute } = this.transitionInfo || {}; + if (intendedRoute) { + this.resetUnsavedState(); + this.router.transitionTo(intendedRoute); + } else { + this.router.transitionTo(route); + } + } } diff --git a/ui/lib/core/addon/components/unsaved-changes-modal.ts b/ui/lib/core/addon/components/unsaved-changes-modal.ts index 8d8a025444..fbbcf17d0a 100644 --- a/ui/lib/core/addon/components/unsaved-changes-modal.ts +++ b/ui/lib/core/addon/components/unsaved-changes-modal.ts @@ -48,7 +48,10 @@ export default class UnsavedChangesModal extends Component { } if (action === 'discard') { - this.unsavedChanges.changedFields = []; + // If a user has clicked "Discard" the models have already been compared + // and they do not want to save the changes. + // Update initialState so the transition does not re-abort. + this.unsavedChanges.resetUnsavedState(); this.args.onDiscard(); } } diff --git a/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js b/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js index 892fa64031..638a23ad9f 100644 --- a/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js +++ b/ui/tests/acceptance/secrets/backend/keymgmt/workflow-test.js @@ -77,10 +77,12 @@ module('Acceptance | Enterprise | keymgmt-configuration-workflow', function (hoo await fillIn(GENERAL.inputByAttr('default_lease_ttl'), 11); await fillIn(GENERAL.selectByAttr('default_lease_ttl'), 'm'); await fillIn(GENERAL.textareaByAttr('description'), 'Updated awesome description.'); - await click(GENERAL.breadcrumbAtIdx(0)); + await click(GENERAL.breadcrumbAtIdx(1)); assert.dom(GENERAL.modal.container('unsaved-changes')).exists('Unsaved changes exists'); await click(GENERAL.button('save')); - assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.configuration.general-settings'); + assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.list-root'); + await visit(`/vault/secrets-engines/${keymgmtType}/configuration`); + assert .dom(GENERAL.textareaByAttr('description')) .hasValue('Updated awesome description.', 'description was tuned from unsaved changes modal'); @@ -93,10 +95,12 @@ module('Acceptance | Enterprise | keymgmt-configuration-workflow', function (hoo await fillIn(GENERAL.inputByAttr('default_lease_ttl'), 12); await fillIn(GENERAL.selectByAttr('default_lease_ttl'), 'm'); await fillIn(GENERAL.textareaByAttr('description'), 'Some awesome description.'); - await click(GENERAL.breadcrumbAtIdx(0)); + await click(GENERAL.breadcrumbAtIdx(1)); assert.dom(GENERAL.modal.container('unsaved-changes')).exists('Unsaved changes exists'); await click(GENERAL.button('discard')); - assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.configuration.general-settings'); + assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.list-root'); + await visit(`/vault/secrets-engines/${keymgmtType}/configuration`); + assert .dom(GENERAL.textareaByAttr('description')) .hasValue( diff --git a/ui/tests/integration/components/unsaved-changes-modal-test.js b/ui/tests/integration/components/unsaved-changes-modal-test.js index a00a88392c..150334a270 100644 --- a/ui/tests/integration/components/unsaved-changes-modal-test.js +++ b/ui/tests/integration/components/unsaved-changes-modal-test.js @@ -15,7 +15,10 @@ module('Integration | Component | UnsavedChangesModal', function (hooks) { hooks.beforeEach(function () { this.unsavedChanges = this.owner.lookup('service:unsavedChanges'); this.unsavedChanges.showModal = true; - this.unsavedChanges.changedFields = ['Description', 'Secrets duration']; + const initialState = { title: 'Title 1', description: 'Old description' }; + const currentState = { title: 'Title 1', description: 'New description' }; + this.unsavedChanges.initialState = initialState; + this.unsavedChanges.currentState = currentState; this.save = () => 'saved!'; this.discard = () => 'discarded!'; }); @@ -25,19 +28,16 @@ module('Integration | Component | UnsavedChangesModal', function (hooks) { assert.dom(GENERAL.modal.header('unsaved-changes')).hasText('Unsaved changes'); assert .dom(GENERAL.modal.body('unsaved-changes')) - .hasText( - `You've made changes to the following: Description Secrets duration Would you like to apply them?` - ); + .hasText(`You've made changes to the following: Description Would you like to apply them?`); }); test('it shows unsaved changes modal with custom changedFields', async function (assert) { - this.changedFields = ['Field 1', 'Field 2']; await render( hbs`` ); assert.dom(GENERAL.modal.header('unsaved-changes')).hasText('Unsaved changes'); assert .dom(GENERAL.modal.body('unsaved-changes')) - .hasText(`You've made changes to the following: Field 1 Field 2 Would you like to apply them?`); + .hasText(`You've made changes to the following: Description Would you like to apply them?`); }); }); diff --git a/ui/tests/unit/services/unsaved-changes-test.js b/ui/tests/unit/services/unsaved-changes-test.js index b0845e3b3a..424e16572f 100644 --- a/ui/tests/unit/services/unsaved-changes-test.js +++ b/ui/tests/unit/services/unsaved-changes-test.js @@ -13,20 +13,24 @@ module('Unit | Service | unsaved-changes', function (hooks) { this.unsavedChanges = this.owner.lookup('service:unsaved-changes'); }); - test('it should set properties', async function (assert) { - const initialState = { title: 'Title 1', description: 'Old description' }; - const currentState = { title: 'New Title 1', description: 'New description' }; - this.unsavedChanges.setupProperties(initialState, currentState); - assert.deepEqual(this.unsavedChanges.initialState, initialState); - assert.deepEqual(this.unsavedChanges.currentState, currentState); - }); - - test('it should update changedFields when getDiff is called', async function (assert) { + test('it should get changedFields', async function (assert) { const initialState = { title: 'Title 1', description: 'Old description' }; const currentState = { title: 'Title 1', description: 'New description' }; - this.unsavedChanges.setupProperties(initialState, currentState); - assert.deepEqual(this.unsavedChanges.changedFields, []); - this.unsavedChanges.getDiff(); + this.unsavedChanges.initialState = initialState; + this.unsavedChanges.currentState = currentState; + assert.deepEqual(this.unsavedChanges.changedFields, ['description']); }); + test('it should get hasChanges', async function (assert) { + const initialState = { title: 'Title 1', description: 'Old description' }; + const currentState = { title: 'Title 1', description: 'New description' }; + this.unsavedChanges.initialState = initialState; + this.unsavedChanges.currentState = currentState; + + assert.true(this.unsavedChanges.hasChanges, 'shows that there are unsaved changes'); + currentState.description = 'Old description'; + this.unsavedChanges.initialState = initialState; + this.unsavedChanges.currentState = currentState; + assert.false(this.unsavedChanges.hasChanges, 'shows that there are no unsaved changes'); + }); });