Skip to content

Commit

Permalink
[ui] Warn users when they leave an edited but unsaved variable page (#…
Browse files Browse the repository at this point in the history
…14665)

* Warning on attempt to leave

* Lintfix

* Only router.off once

* Dont warn on transition when only updating queryparams

* Remove double-push and queryparam-only issues, thanks @lgfa29

* Acceptance tests

* Changelog
  • Loading branch information
philrenaud committed Sep 23, 2022
1 parent 63098f1 commit a34c241
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/14665.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: warn a user before they leave a Variables form page with unsaved information
```
65 changes: 62 additions & 3 deletions ui/app/components/variable-form.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @ts-check

import Component from '@glimmer/component';
import { action } from '@ember/object';
import { action, computed } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import { inject as service } from '@ember/service';
import { trimPath } from '../helpers/trim-path';
Expand All @@ -12,6 +12,7 @@ import MutableArray from '@ember/array/mutable';
import { A } from '@ember/array';
import { stringifyObject } from 'nomad-ui/helpers/stringify-object';
import notifyConflict from 'nomad-ui/utils/notify-conflict';
import isEqual from 'lodash.isequal';

const EMPTY_KV = {
key: '',
Expand Down Expand Up @@ -43,6 +44,7 @@ export default class VariableFormComponent extends Component {
constructor() {
super(...arguments);
set(this, 'path', this.args.model.path);
this.addExitHandler();
}
@action
Expand Down Expand Up @@ -93,7 +95,7 @@ export default class VariableFormComponent extends Component {
if (!this.args.model?.isNew) {
keyValues.pushObject(copy(EMPTY_KV));
}
this.keyValues = keyValues;
set(this, 'keyValues', keyValues);

this.JSONItems = stringifyObject([
this.keyValues.reduce((acc, { key, value }) => {
Expand Down Expand Up @@ -182,7 +184,7 @@ export default class VariableFormComponent extends Component {
if (!nonEmptyItems.length) {
throw new Error('Please provide at least one key/value pair.');
} else {
this.keyValues = nonEmptyItems;
set(this, 'keyValues', nonEmptyItems);
}

if (this.args.model?.isNew) {
Expand All @@ -206,6 +208,7 @@ export default class VariableFormComponent extends Component {
destroyOnClick: false,
timeout: 5000,
});
this.removeExitHandler();
this.router.transitionTo('variables.variable', this.args.model.id);
} catch (error) {
notifyConflict(this)(error);
Expand Down Expand Up @@ -326,4 +329,60 @@ export default class VariableFormComponent extends Component {
this.args.model.pathLinkedEntities?.task
);
}

//#region Unsaved Changes Confirmation

hasRemovedExitHandler = false;

@computed(
'args.model.{keyValues,path}',
'keyValues.@each.{key,value}',
'path'
)
get hasUserModifiedAttributes() {
const compactedBasicKVs = this.keyValues
.map((kv) => ({ key: kv.key, value: kv.value }))
.filter((kv) => kv.key || kv.value);
const compactedPassedKVs = this.args.model.keyValues.filter(
(kv) => kv.key || kv.value
);
const unequal =
!isEqual(compactedBasicKVs, compactedPassedKVs) ||
!isEqual(this.path, this.args.model.path);
return unequal;
}

addExitHandler() {
this.router.on('routeWillChange', this, this.confirmExit);
}

removeExitHandler() {
if (!this.hasRemovedExitHandler) {
this.router.off('routeWillChange', this, this.confirmExit);
this.hasRemovedExitHandler = true;
}
}

confirmExit(transition) {
if (transition.isAborted || transition.queryParamsOnly) return;

if (this.hasUserModifiedAttributes) {
if (
!confirm(
'Your variable has unsaved changes. Are you sure you want to leave?'
)
) {
transition.abort();
} else {
this.removeExitHandler();
}
}
}

willDestroy() {
super.willDestroy(...arguments);
this.removeExitHandler();
}

//#endregion Unsaved Changes Confirmation
}
2 changes: 1 addition & 1 deletion ui/app/templates/variables/variable/edit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
Edit
{{this.model.path}}
<Toggle
data-test-memory-toggle
data-test-json-toggle
@isActive={{eq this.view "json"}}
@onToggle={{action this.toggleView}}
title="JSON"
Expand Down
67 changes: 67 additions & 0 deletions ui/tests/acceptance/variables-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,73 @@ module('Acceptance | variables', function (hooks) {
// Reset Token
window.localStorage.nomadTokenSecret = null;
});

test('warns you if you try to leave with an unsaved form', async function (assert) {
// Arrange Test Set-up
allScenarios.variableTestCluster(server);
const variablesToken = server.db.tokens.find(VARIABLE_TOKEN_ID);
window.localStorage.nomadTokenSecret = variablesToken.secretId;

const originalWindowConfirm = window.confirm;
let confirmFired = false;
let leave = true;
window.confirm = function () {
confirmFired = true;
return leave;
};
// End Test Set-up

await Variables.visitConflicting();
document.querySelector('[data-test-var-key]').value = ''; // clear current input
await typeIn('[data-test-var-key]', 'buddy');
await typeIn('[data-test-var-value]', 'pal');
await click('[data-test-gutter-link="jobs"]');
assert.ok(confirmFired, 'Confirm fired when leaving with unsaved form');
assert.equal(
currentURL(),
'/jobs?namespace=*',
'Opted to leave, ended up on desired page'
);

// Reset checks
confirmFired = false;
leave = false;

await Variables.visitConflicting();
document.querySelector('[data-test-var-key]').value = ''; // clear current input
await typeIn('[data-test-var-key]', 'buddy');
await typeIn('[data-test-var-value]', 'pal');
await click('[data-test-gutter-link="jobs"]');
assert.ok(confirmFired, 'Confirm fired when leaving with unsaved form');
assert.equal(
currentURL(),
'/variables/var/Auto-conflicting%20Variable@default/edit',
'Opted to stay, did not leave page'
);

// Reset checks
confirmFired = false;

await Variables.visitConflicting();
document.querySelector('[data-test-var-key]').value = ''; // clear current input
await typeIn('[data-test-var-key]', 'buddy');
await typeIn('[data-test-var-value]', 'pal');
await click('[data-test-json-toggle]');
assert.notOk(
confirmFired,
'Confirm did not fire when only transitioning queryParams'
);
assert.equal(
currentURL(),
'/variables/var/Auto-conflicting%20Variable@default/edit?view=json',
'Stayed on page, queryParams changed'
);

// Reset Token
window.localStorage.nomadTokenSecret = null;
// Restore the original window.confirm implementation
window.confirm = originalWindowConfirm;
});
});

module('delete flow', function () {
Expand Down

0 comments on commit a34c241

Please sign in to comment.