From d1fea9ec0af8fadbffc5f62b1f6cba939701f996 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 29 Aug 2018 19:14:31 +0100 Subject: [PATCH] UI: Simplify/refactor the actions/notification layer (#4572) + (#4573) * Move notification texts to a slightly different layer (#4572) * Further Simplify/refactor the actions/notification layer (#4573) 1. Move the 'with-feedback' actions to a 'with-blocking-action' mixin which better describes what it does 2. Additional set of unit tests almost over the entire layer to prove things work/add confidence for further changes The multiple 'with-action' mixins used for every 'index/edit' combo are now reduced down to only contain the functionality related to their specific routes, i.e. where to redirect. The actual functionality to block and carry out the action and then notify are 'almost' split out so that their respective classes/objects do one thing and one thing 'well'. Mixins are chosen for the moment as the decoration approach used by mixins feels better than multiple levels of inheritence, but I would like to take this fuether in the future to a 'compositional' based approach. There is still possible further work to be done here, but I'm a lot happier now this is reduced down into separate parts. --- ui-v2/app/mixins/acl/with-actions.js | 100 +++------------ ui-v2/app/mixins/intention/with-actions.js | 75 ++--------- ui-v2/app/mixins/kv/with-actions.js | 95 ++++---------- ui-v2/app/mixins/with-blocking-actions.js | 117 ++++++++++++++++++ ui-v2/app/mixins/with-feedback.js | 17 --- ui-v2/app/routes/dc/nodes/show.js | 22 ++-- ui-v2/app/routes/settings.js | 28 +---- ui-v2/app/services/feedback.js | 27 ++-- ui-v2/app/templates/components/app-view.hbs | 3 +- .../app/templates/dc/acls/-notifications.hbs | 32 +++++ ui-v2/app/templates/dc/acls/edit.hbs | 3 + ui-v2/app/templates/dc/acls/index.hbs | 3 + ui-v2/app/templates/dc/intentions/edit.hbs | 3 + ui-v2/app/templates/dc/intentions/index.hbs | 3 + .../templates/dc/intentions/notifications.hbs | 22 ++++ ui-v2/app/templates/dc/kv/-notifications.hbs | 20 +++ ui-v2/app/templates/dc/kv/edit.hbs | 7 +- ui-v2/app/templates/dc/kv/index.hbs | 3 + .../app/templates/dc/nodes/-notifications.hbs | 8 ++ ui-v2/app/templates/dc/nodes/show.hbs | 4 + ui-v2/app/templates/dc/services/index.hbs | 4 + ui-v2/app/templates/settings.hbs | 15 +++ ui-v2/ember-cli-build.js | 3 + ui-v2/package.json | 1 + ui-v2/tests/acceptance/dc/acls/update.feature | 24 ++-- ui-v2/tests/acceptance/dc/acls/use.feature | 4 + .../acceptance/dc/intentions/update.feature | 41 ++++++ .../dc/kvs/sessions/invalidate.feature | 32 +++++ ui-v2/tests/acceptance/dc/kvs/update.feature | 27 +++- .../dc/nodes/sessions/invalidate.feature | 14 ++- ui-v2/tests/acceptance/deleting.feature | 39 +++++- .../tests/acceptance/settings/update.feature | 3 + .../steps/dc/intentions/update-steps.js | 10 ++ .../steps/dc/kvs/sessions/invalidate-steps.js | 10 ++ .../tests/lib/page-object/createDeletable.js | 9 +- ui-v2/tests/pages/dc/kv/edit.js | 1 + ui-v2/tests/steps.js | 12 +- .../unit/mixins/acl/with-actions-test.js | 87 +++++++++++-- .../mixins/intention/with-actions-test.js | 36 ++++-- .../tests/unit/mixins/kv/with-actions-test.js | 48 +++++-- .../unit/mixins/with-blocking-actions-test.js | 74 +++++++++++ ui-v2/tests/unit/mixins/with-feedback-test.js | 20 --- ui-v2/yarn.lock | 7 ++ 43 files changed, 769 insertions(+), 344 deletions(-) create mode 100644 ui-v2/app/mixins/with-blocking-actions.js delete mode 100644 ui-v2/app/mixins/with-feedback.js create mode 100644 ui-v2/app/templates/dc/acls/-notifications.hbs create mode 100644 ui-v2/app/templates/dc/intentions/notifications.hbs create mode 100644 ui-v2/app/templates/dc/kv/-notifications.hbs create mode 100644 ui-v2/app/templates/dc/nodes/-notifications.hbs create mode 100644 ui-v2/tests/acceptance/dc/intentions/update.feature create mode 100644 ui-v2/tests/acceptance/dc/kvs/sessions/invalidate.feature create mode 100644 ui-v2/tests/acceptance/steps/dc/intentions/update-steps.js create mode 100644 ui-v2/tests/acceptance/steps/dc/kvs/sessions/invalidate-steps.js create mode 100644 ui-v2/tests/unit/mixins/with-blocking-actions-test.js delete mode 100644 ui-v2/tests/unit/mixins/with-feedback-test.js diff --git a/ui-v2/app/mixins/acl/with-actions.js b/ui-v2/app/mixins/acl/with-actions.js index 90e8c4b38..54a09c8ed 100644 --- a/ui-v2/app/mixins/acl/with-actions.js +++ b/ui-v2/app/mixins/acl/with-actions.js @@ -1,92 +1,32 @@ import Mixin from '@ember/object/mixin'; import { get } from '@ember/object'; import { inject as service } from '@ember/service'; -import WithFeedback from 'consul-ui/mixins/with-feedback'; +import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; -export default Mixin.create(WithFeedback, { +export default Mixin.create(WithBlockingActions, { settings: service('settings'), actions: { - create: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .persist(item) - .then(item => { - return this.transitionTo('dc.acls'); - }); - }, - `Your ACL token has been added.`, - `There was an error adding your ACL token.` - ); - }, - update: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .persist(item) - .then(() => { - return this.transitionTo('dc.acls'); - }); - }, - `Your ACL token was saved.`, - `There was an error saving your ACL token.` - ); - }, - delete: function(item) { - get(this, 'feedback').execute( - () => { - return ( - get(this, 'repo') - // ember-changeset doesn't support `get` - // and `data` returns an object not a model - .remove(item) - .then(() => { - switch (this.routeName) { - case 'dc.acls.index': - return this.refresh(); - default: - return this.transitionTo('dc.acls'); - } - }) - ); - }, - `Your ACL token was deleted.`, - `There was an error deleting your ACL token.` - ); - }, - cancel: function(item) { - this.transitionTo('dc.acls'); - }, use: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'settings') - .persist({ token: get(item, 'ID') }) - .then(() => { - this.transitionTo('dc.services'); - }); - }, - `Now using new ACL token`, - `There was an error using that ACL token` - ); + return get(this, 'feedback').execute(() => { + return get(this, 'settings') + .persist({ token: get(item, 'ID') }) + .then(() => { + return this.transitionTo('dc.services'); + }); + }, 'use'); }, clone: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .clone(item) - .then(item => { - switch (this.routeName) { - case 'dc.acls.index': - return this.refresh(); - default: - return this.transitionTo('dc.acls'); - } - }); - }, - `Your ACL token was cloned.`, - `There was an error cloning your ACL token.` - ); + return get(this, 'feedback').execute(() => { + return get(this, 'repo') + .clone(item) + .then(item => { + // cloning is similar to delete in that + // if you clone from the listing page, stay on the listing page + // whereas if you clone form another token, take me back to the listing page + // so I can see it + return this.afterDelete(...arguments); + }); + }, 'clone'); }, }, }); diff --git a/ui-v2/app/mixins/intention/with-actions.js b/ui-v2/app/mixins/intention/with-actions.js index f56866607..08653a076 100644 --- a/ui-v2/app/mixins/intention/with-actions.js +++ b/ui-v2/app/mixins/intention/with-actions.js @@ -1,70 +1,17 @@ import Mixin from '@ember/object/mixin'; -import { get } from '@ember/object'; -import WithFeedback from 'consul-ui/mixins/with-feedback'; +import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; import { INTERNAL_SERVER_ERROR as HTTP_INTERNAL_SERVER_ERROR } from 'consul-ui/utils/http/status'; -export default Mixin.create(WithFeedback, { - actions: { - create: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .persist(item) - .then(item => { - return this.transitionTo('dc.intentions'); - }); - }, - `Your intention has been added.`, - function(e) { - if (e.errors && e.errors[0]) { - const error = e.errors[0]; - if (parseInt(error.status) === HTTP_INTERNAL_SERVER_ERROR) { - if (error.detail.indexOf('duplicate intention found:') === 0) { - return `An intention already exists for this Source-Destination pair. Please enter a different combination of Services, or search the intentions to edit an existing intention.`; - } - } - } - return `There was an error adding your intention.`; +export default Mixin.create(WithBlockingActions, { + errorCreate: function(type, e) { + if (e.errors && e.errors[0]) { + const error = e.errors[0]; + if (parseInt(error.status) === HTTP_INTERNAL_SERVER_ERROR) { + if (error.detail.indexOf('duplicate intention found:') === 0) { + return 'exists'; } - ); - }, - update: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .persist(item) - .then(() => { - return this.transitionTo('dc.intentions'); - }); - }, - `Your intention was saved.`, - `There was an error saving your intention.` - ); - }, - delete: function(item) { - get(this, 'feedback').execute( - () => { - return ( - get(this, 'repo') - // ember-changeset doesn't support `get` - // and `data` returns an object not a model - .remove(item) - .then(() => { - switch (this.routeName) { - case 'dc.intentions.index': - return this.refresh(); - default: - return this.transitionTo('dc.intentions'); - } - }) - ); - }, - `Your intention was deleted.`, - `There was an error deleting your intention.` - ); - }, - cancel: function(item) { - this.transitionTo('dc.intentions'); - }, + } + } + return type; }, }); diff --git a/ui-v2/app/mixins/kv/with-actions.js b/ui-v2/app/mixins/kv/with-actions.js index e9abd5786..b281ef2c5 100644 --- a/ui-v2/app/mixins/kv/with-actions.js +++ b/ui-v2/app/mixins/kv/with-actions.js @@ -1,80 +1,35 @@ import Mixin from '@ember/object/mixin'; import { get, set } from '@ember/object'; -import WithFeedback from 'consul-ui/mixins/with-feedback'; +import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; -const transitionToList = function(key = '/', transitionTo) { - if (key === '/') { - return transitionTo('dc.kv.index'); - } else { - return transitionTo('dc.kv.folder', key); - } -}; - -export default Mixin.create(WithFeedback, { +export default Mixin.create(WithBlockingActions, { + // afterCreate just calls afterUpdate + afterUpdate: function(item, parent) { + const key = get(parent, 'Key'); + if (key === '/') { + return this.transitionTo('dc.kv.index'); + } else { + return this.transitionTo('dc.kv.folder', key); + } + }, + afterDelete: function(item, parent) { + if (this.routeName === 'dc.kv.folder') { + return this.refresh(); + } + return this._super(...arguments); + }, actions: { - create: function(item, parent) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .persist(item) - .then(item => { - return transitionToList(get(parent, 'Key'), this.transitionTo.bind(this)); - }); - }, - `Your key has been added.`, - `There was an error adding your key.` - ); - }, - update: function(item, parent) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .persist(item) - .then(() => { - return transitionToList(get(parent, 'Key'), this.transitionTo.bind(this)); - }); - }, - `Your key has been saved.`, - `There was an error saving your key.` - ); - }, - delete: function(item, parent) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo') - .remove(item) - .then(() => { - switch (this.routeName) { - case 'dc.kv.folder': - case 'dc.kv.index': - return this.refresh(); - default: - return transitionToList(get(parent, 'Key'), this.transitionTo.bind(this)); - } - }); - }, - `Your key was deleted.`, - `There was an error deleting your key.` - ); - }, - cancel: function(item, parent) { - return transitionToList(get(parent, 'Key'), this.transitionTo.bind(this)); - }, invalidateSession: function(item) { const controller = this.controller; const repo = get(this, 'sessionRepo'); - get(this, 'feedback').execute( - () => { - return repo.remove(item).then(() => { - const item = get(controller, 'item'); - set(item, 'Session', null); - delete item.Session; - set(controller, 'session', null); - }); - }, - `The session was invalidated.`, - `There was an error invalidating the session.` - ); + return get(this, 'feedback').execute(() => { + return repo.remove(item).then(() => { + const item = get(controller, 'item'); + set(item, 'Session', null); + delete item.Session; + set(controller, 'session', null); + }); + }, 'delete'); }, }, }); diff --git a/ui-v2/app/mixins/with-blocking-actions.js b/ui-v2/app/mixins/with-blocking-actions.js new file mode 100644 index 000000000..5cfca5d0e --- /dev/null +++ b/ui-v2/app/mixins/with-blocking-actions.js @@ -0,0 +1,117 @@ +import Mixin from '@ember/object/mixin'; +import { inject as service } from '@ember/service'; +import { get, set } from '@ember/object'; +/** With Blocking Actions + * This mixin contains common write actions (Create Update Delete) for routes. + * It could also be an Route to extend but decoration seems to be more sense right now. + * + * Each 'blocking action' (blocking in terms of showing some sort of blocking loader) is + * wrapped in the functionality to signal that the page should be blocked + * (currently via the 'feedback' service) as well as some sane default hooks for where the page + * should go when the action has finished. + * + * Hooks can and are being overwritten for custom redirects/error handling on a route by route basis. + * + * Notifications are part of the injectable 'feedback' service, meaning different ones + * could be easily swapped in an out as need be in the future. + * + */ +export default Mixin.create({ + _feedback: service('feedback'), + init: function() { + this._super(...arguments); + const feedback = get(this, '_feedback'); + const route = this; + set(this, 'feedback', { + execute: function(cb, type, error) { + return feedback.execute(cb, type, error, route.controller); + }, + }); + }, + afterCreate: function(item) { + // do the same as update + return this.afterUpdate(...arguments); + }, + afterUpdate: function(item) { + // e.g. dc.intentions.index + const parts = this.routeName.split('.'); + // e.g. index or edit + parts.pop(); + // e.g. dc.intentions, essentially go to the listings page + return this.transitionTo(parts.join('.')); + }, + afterDelete: function(item) { + // e.g. dc.intentions.index + const parts = this.routeName.split('.'); + // e.g. index or edit + const page = parts.pop(); + switch (page) { + case 'index': + // essentially if I'm on the index page, stay there + return this.refresh(); + default: + // e.g. dc.intentions essentially do to the listings page + return this.transitionTo(parts.join('.')); + } + }, + errorCreate: function(type, e) { + return type; + }, + errorUpdate: function(type, e) { + return type; + }, + errorDelete: function(type, e) { + return type; + }, + actions: { + cancel: function() { + // do the same as an update, or override + return this.afterUpdate(...arguments); + }, + create: function(item) { + return get(this, 'feedback').execute( + () => { + return get(this, 'repo') + .persist(item) + .then(item => { + return this.afterCreate(...arguments); + }); + }, + 'create', + (type, e) => { + return this.errorCreate(type, e); + } + ); + }, + update: function(item, parent) { + return get(this, 'feedback').execute( + () => { + return get(this, 'repo') + .persist(item) + .then(() => { + return this.afterUpdate(...arguments); + }); + }, + 'update', + (type, e) => { + return this.errorUpdate(type, e); + } + ); + }, + delete: function(item, parent) { + return get(this, 'feedback').execute( + () => { + return get(this, 'repo') + .remove(item) + .then(() => { + return this.afterDelete(...arguments); + }); + }, + 'delete', + (type, e) => { + return this.errorDelete(type, e); + } + ); + }, + }, +}); diff --git a/ui-v2/app/mixins/with-feedback.js b/ui-v2/app/mixins/with-feedback.js deleted file mode 100644 index 4038e70c6..000000000 --- a/ui-v2/app/mixins/with-feedback.js +++ /dev/null @@ -1,17 +0,0 @@ -import Mixin from '@ember/object/mixin'; -import { inject as service } from '@ember/service'; -import { get, set } from '@ember/object'; - -export default Mixin.create({ - _feedback: service('feedback'), - init: function() { - this._super(...arguments); - const feedback = get(this, '_feedback'); - const route = this; - set(this, 'feedback', { - execute: function() { - feedback.execute(...[...arguments, route.controller]); - }, - }); - }, -}); diff --git a/ui-v2/app/routes/dc/nodes/show.js b/ui-v2/app/routes/dc/nodes/show.js index 75da2ab0d..b13402edc 100644 --- a/ui-v2/app/routes/dc/nodes/show.js +++ b/ui-v2/app/routes/dc/nodes/show.js @@ -5,11 +5,11 @@ import { get, set } from '@ember/object'; import distance from 'consul-ui/utils/distance'; import tomographyFactory from 'consul-ui/utils/tomography'; -import WithFeedback from 'consul-ui/mixins/with-feedback'; +import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; const tomography = tomographyFactory(distance); -export default Route.extend(WithFeedback, { +export default Route.extend(WithBlockingActions, { repo: service('nodes'), sessionRepo: service('session'), queryParams: { @@ -49,18 +49,14 @@ export default Route.extend(WithFeedback, { const dc = this.modelFor('dc').dc.Name; const controller = this.controller; const repo = get(this, 'sessionRepo'); - get(this, 'feedback').execute( - () => { - const node = get(item, 'Node'); - return repo.remove(item).then(() => { - return repo.findByNode(node, dc).then(function(sessions) { - set(controller, 'sessions', sessions); - }); + return get(this, 'feedback').execute(() => { + const node = get(item, 'Node'); + return repo.remove(item).then(() => { + return repo.findByNode(node, dc).then(function(sessions) { + set(controller, 'sessions', sessions); }); - }, - `The session was invalidated.`, - `There was an error invalidating the session.` - ); + }); + }, 'delete'); }, }, }); diff --git a/ui-v2/app/routes/settings.js b/ui-v2/app/routes/settings.js index cffbeb8aa..837219325 100644 --- a/ui-v2/app/routes/settings.js +++ b/ui-v2/app/routes/settings.js @@ -3,8 +3,8 @@ import { inject as service } from '@ember/service'; import { hash } from 'rsvp'; import { get } from '@ember/object'; -import WithFeedback from 'consul-ui/mixins/with-feedback'; -export default Route.extend(WithFeedback, { +import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; +export default Route.extend(WithBlockingActions, { dcRepo: service('dc'), repo: service('settings'), model: function(params) { @@ -24,24 +24,8 @@ export default Route.extend(WithFeedback, { this._super(...arguments); controller.setProperties(model); }, - actions: { - update: function(item) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo').persist(item); - }, - `Your settings were saved.`, - `There was an error saving your settings.` - ); - }, - delete: function(key) { - get(this, 'feedback').execute( - () => { - return get(this, 'repo').remove(key); - }, - `You settings have been reset.`, - `There was an error resetting your settings.` - ); - }, - }, + // overwrite afterUpdate and afterDelete hooks + // to avoid the default 'return to listing page' + afterUpdate: function() {}, + afterDelete: function() {}, }); diff --git a/ui-v2/app/services/feedback.js b/ui-v2/app/services/feedback.js index cff3cd9ce..2be104ef8 100644 --- a/ui-v2/app/services/feedback.js +++ b/ui-v2/app/services/feedback.js @@ -2,36 +2,41 @@ import Service, { inject as service } from '@ember/service'; import { get, set } from '@ember/object'; import callableType from 'consul-ui/utils/callable-type'; +const TYPE_SUCCESS = 'success'; +const TYPE_ERROR = 'error'; +const defaultStatus = function(type, obj) { + return type; +}; export default Service.extend({ notify: service('flashMessages'), logger: service('logger'), - execute: function(handle, success, error, controller) { + execute: function(handle, action, status = defaultStatus, controller) { set(controller, 'isLoading', true); - const displaySuccess = callableType(success); - const displayError = callableType(error); + const getAction = callableType(action); + const getStatus = callableType(status); const notify = get(this, 'notify'); return ( handle() - //TODO: pass this through to display success.. - .then(() => { + //TODO: pass this through to getAction.. + .then(target => { notify.add({ - type: 'success', + type: getStatus(TYPE_SUCCESS), // here.. - message: displaySuccess(), + action: getAction(), }); }) .catch(e => { get(this, 'logger').execute(e); if (e.name === 'TransitionAborted') { notify.add({ - type: 'success', + type: getStatus(TYPE_SUCCESS), // and here - message: displaySuccess(), + action: getAction(), }); } else { notify.add({ - type: 'error', - message: displayError(e), + type: getStatus(TYPE_ERROR, e), + action: getAction(), }); } }) diff --git a/ui-v2/app/templates/components/app-view.hbs b/ui-v2/app/templates/components/app-view.hbs index be624ad5a..37417d333 100644 --- a/ui-v2/app/templates/components/app-view.hbs +++ b/ui-v2/app/templates/components/app-view.hbs @@ -4,7 +4,8 @@ {{#each flashMessages.queue as |flash|}} {{#flash-message flash=flash as |component flash|}} {{! flashes automatically ucfirst the type }} -

{{if (eq component.flashType 'Success') 'Success!' 'Error!'}} {{flash.message}}

+ +

{{if (eq component.flashType 'Success') 'Success!' 'Error!'}} {{#yield-slot 'notification' (block-params (lowercase component.flashType) (lowercase flash.action) )}}{{yield}}{{/yield-slot}}

{{/flash-message}} {{/each}}
diff --git a/ui-v2/app/templates/dc/acls/-notifications.hbs b/ui-v2/app/templates/dc/acls/-notifications.hbs new file mode 100644 index 000000000..1b577c6ef --- /dev/null +++ b/ui-v2/app/templates/dc/acls/-notifications.hbs @@ -0,0 +1,32 @@ +{{#if (eq type 'create')}} + {{#if (eq status 'success') }} + Your ACL token has been added. + {{else}} + There was an error adding your ACL token. + {{/if}} +{{else if (eq type 'update') }} + {{#if (eq status 'success') }} + Your ACL token has been saved. + {{else}} + There was an error saving your ACL token. + {{/if}} +{{ else if (eq type 'delete')}} + {{#if (eq status 'success') }} + Your ACL token was deleted. + {{else}} + There was an error deleting your ACL token. + {{/if}} +{{ else if (eq type 'use')}} + {{#if (eq status 'success') }} + Now using new ACL token. + {{else}} + There was an error using that ACL token. + {{/if}} +{{ else if (eq type 'clone')}} + {{#if (eq status 'success') }} + Your ACL token was cloned. + {{else}} + There was an error cloning your ACL token. + {{/if}} +{{/if}} + diff --git a/ui-v2/app/templates/dc/acls/edit.hbs b/ui-v2/app/templates/dc/acls/edit.hbs index 4a6f70301..dde086202 100644 --- a/ui-v2/app/templates/dc/acls/edit.hbs +++ b/ui-v2/app/templates/dc/acls/edit.hbs @@ -1,4 +1,7 @@ {{#app-view class="acl edit" loading=isLoading}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/acls/notifications'}} + {{/block-slot}} {{#block-slot 'breadcrumbs'}}
  1. All Tokens
    2. diff --git a/ui-v2/app/templates/dc/acls/index.hbs b/ui-v2/app/templates/dc/acls/index.hbs index 7587d4f2c..27bedb2ae 100644 --- a/ui-v2/app/templates/dc/acls/index.hbs +++ b/ui-v2/app/templates/dc/acls/index.hbs @@ -1,4 +1,7 @@ {{#app-view class="acl list" loading=isLoading}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/acls/notifications'}} + {{/block-slot}} {{#block-slot 'header'}}

    ACL Tokens diff --git a/ui-v2/app/templates/dc/intentions/edit.hbs b/ui-v2/app/templates/dc/intentions/edit.hbs index 3fc2ba45d..8ee321c36 100644 --- a/ui-v2/app/templates/dc/intentions/edit.hbs +++ b/ui-v2/app/templates/dc/intentions/edit.hbs @@ -1,4 +1,7 @@ {{#app-view class="acl edit" loading=isLoading}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/intentions/notifications'}} + {{/block-slot}} {{#block-slot 'breadcrumbs'}}
    1. All Intentions
      2. diff --git a/ui-v2/app/templates/dc/intentions/index.hbs b/ui-v2/app/templates/dc/intentions/index.hbs index dfde3056f..2236f7d79 100644 --- a/ui-v2/app/templates/dc/intentions/index.hbs +++ b/ui-v2/app/templates/dc/intentions/index.hbs @@ -1,4 +1,7 @@ {{#app-view class="intention list"}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/intentions/notifications'}} + {{/block-slot}} {{#block-slot 'header'}}

      Intentions diff --git a/ui-v2/app/templates/dc/intentions/notifications.hbs b/ui-v2/app/templates/dc/intentions/notifications.hbs new file mode 100644 index 000000000..cdef9d5f2 --- /dev/null +++ b/ui-v2/app/templates/dc/intentions/notifications.hbs @@ -0,0 +1,22 @@ +{{#if (eq type 'create')}} + {{#if (eq status 'success') }} + Your intention has been added. + {{else if (eq status 'exists') }} + An intention already exists for this Source-Destination pair. Please enter a different combination of Services, or search the intentions to edit an existing intention. + {{else}} + There was an error adding your intention. + {{/if}} +{{else if (eq type 'update') }} + {{#if (eq status 'success') }} + Your intention has been saved. + {{else}} + There was an error saving your intention. + {{/if}} +{{ else if (eq type 'delete')}} + {{#if (eq status 'success') }} + Your intention was deleted. + {{else}} + There was an error deleting your intention. + {{/if}} +{{/if}} + diff --git a/ui-v2/app/templates/dc/kv/-notifications.hbs b/ui-v2/app/templates/dc/kv/-notifications.hbs new file mode 100644 index 000000000..7399dc2cd --- /dev/null +++ b/ui-v2/app/templates/dc/kv/-notifications.hbs @@ -0,0 +1,20 @@ +{{#if (eq type 'create')}} + {{#if (eq status 'success') }} + Your key has been added. + {{else}} + There was an error adding your key. + {{/if}} +{{else if (eq type 'update') }} + {{#if (eq status 'success') }} + Your key has been saved. + {{else}} + There was an error saving your key. + {{/if}} +{{ else if (eq type 'delete')}} + {{#if (eq status 'success') }} + Your key was deleted. + {{else}} + There was an error deleting your key. + {{/if}} +{{/if}} + diff --git a/ui-v2/app/templates/dc/kv/edit.hbs b/ui-v2/app/templates/dc/kv/edit.hbs index 22c7b3973..7bf538afb 100644 --- a/ui-v2/app/templates/dc/kv/edit.hbs +++ b/ui-v2/app/templates/dc/kv/edit.hbs @@ -1,4 +1,7 @@ {{#app-view class="kv edit" loading=isLoading}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/kv/notifications'}} + {{/block-slot}} {{#block-slot 'breadcrumbs'}}
      1. Key / Values
        2. @@ -24,7 +27,7 @@ {{/if}} {{partial 'dc/kv/form'}} {{#if session}} -
        +

        Lock Session

        Name
        @@ -54,7 +57,7 @@
        {{#confirmation-dialog message='Are you sure you want to invalidate this session?'}} {{#block-slot 'action' as |confirm|}} - + {{/block-slot}} {{#block-slot 'dialog' as |execute cancel message|}}

        diff --git a/ui-v2/app/templates/dc/kv/index.hbs b/ui-v2/app/templates/dc/kv/index.hbs index 0733bef0c..c403241ae 100644 --- a/ui-v2/app/templates/dc/kv/index.hbs +++ b/ui-v2/app/templates/dc/kv/index.hbs @@ -1,4 +1,7 @@ {{#app-view class="kv list" loading=isLoading}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/kv/notifications'}} + {{/block-slot}} {{#block-slot 'breadcrumbs'}}

          {{#if (not-eq parent.Key '/') }} diff --git a/ui-v2/app/templates/dc/nodes/-notifications.hbs b/ui-v2/app/templates/dc/nodes/-notifications.hbs new file mode 100644 index 000000000..70f90530b --- /dev/null +++ b/ui-v2/app/templates/dc/nodes/-notifications.hbs @@ -0,0 +1,8 @@ +{{#if (eq type 'delete')}} + {{#if (eq status 'success') }} + The session was invalidated. + {{else}} + There was an error invalidating the session. + {{/if}} +{{/if}} + diff --git a/ui-v2/app/templates/dc/nodes/show.hbs b/ui-v2/app/templates/dc/nodes/show.hbs index ee143d882..9a63296b8 100644 --- a/ui-v2/app/templates/dc/nodes/show.hbs +++ b/ui-v2/app/templates/dc/nodes/show.hbs @@ -1,4 +1,8 @@ {{#app-view class="node show"}} + {{#block-slot 'notification' as |status type|}} + {{!TODO: Move sessions to its own folder within nodes }} + {{partial 'dc/nodes/notifications'}} + {{/block-slot}} {{#block-slot 'breadcrumbs'}}
          1. All Nodes
            2. diff --git a/ui-v2/app/templates/dc/services/index.hbs b/ui-v2/app/templates/dc/services/index.hbs index 5267078b6..64c9a54e3 100644 --- a/ui-v2/app/templates/dc/services/index.hbs +++ b/ui-v2/app/templates/dc/services/index.hbs @@ -1,4 +1,8 @@ {{#app-view class="service list"}} + {{!TODO: Look at the item passed through to figure what partial to show, also move into its own service partial, for the moment keeping here for visibility}} + {{#block-slot 'notification' as |status type|}} + {{partial 'dc/acls/notifications'}} + {{/block-slot}} {{#block-slot 'header'}}

            Services diff --git a/ui-v2/app/templates/settings.hbs b/ui-v2/app/templates/settings.hbs index 8406d57a0..97390b6a6 100644 --- a/ui-v2/app/templates/settings.hbs +++ b/ui-v2/app/templates/settings.hbs @@ -1,5 +1,20 @@ {{#hashicorp-consul id="wrapper" dcs=dcs dc=dc}} {{#app-view class="settings show"}} + {{#block-slot 'notification' as |status type|}} + {{#if (eq type 'update')}} + {{#if (eq status 'success') }} + Your settings were saved. + {{else}} + There was an error saving your settings. + {{/if}} + {{ else if (eq type 'delete')}} + {{#if (eq status 'success') }} + You settings have been reset. + {{else}} + There was an error resetting your settings. + {{/if}} + {{/if}} + {{/block-slot}} {{#block-slot 'header'}}

            Settings diff --git a/ui-v2/ember-cli-build.js b/ui-v2/ember-cli-build.js index f1a9b6d14..c097b895c 100644 --- a/ui-v2/ember-cli-build.js +++ b/ui-v2/ember-cli-build.js @@ -45,6 +45,9 @@ module.exports = function(defaults) { "ie 11" ] }, + 'ember-cli-string-helpers': { + only: ['lowercase'] + } }); // Use `app.import` to add additional libraries to the generated // output files. diff --git a/ui-v2/package.json b/ui-v2/package.json index 68caedaa4..7f822a76d 100644 --- a/ui-v2/package.json +++ b/ui-v2/package.json @@ -59,6 +59,7 @@ "ember-cli-sass": "^7.1.4", "ember-cli-shims": "^1.2.0", "ember-cli-sri": "^2.1.0", + "ember-cli-string-helpers": "^1.9.0", "ember-cli-uglify": "^2.0.0", "ember-cli-yadda": "^0.4.0", "ember-collection": "^1.0.0-alpha.7", diff --git a/ui-v2/tests/acceptance/dc/acls/update.feature b/ui-v2/tests/acceptance/dc/acls/update.feature index f79d637c7..42a77b3e2 100644 --- a/ui-v2/tests/acceptance/dc/acls/update.feature +++ b/ui-v2/tests/acceptance/dc/acls/update.feature @@ -1,6 +1,6 @@ @setupApplicationTest Feature: dc / acls / update: ACL Update - Scenario: Update to [Name], [Type], [Rules] + Background: Given 1 datacenter model with the value "datacenter" And 1 acl model from yaml --- @@ -12,6 +12,7 @@ Feature: dc / acls / update: ACL Update acl: key --- Then the url should be /datacenter/acls/key + Scenario: Update to [Name], [Type], [Rules] Then I fill in with yaml --- name: [Name] @@ -23,6 +24,9 @@ Feature: dc / acls / update: ACL Update Name: [Name] Type: [Type] --- + Then the url should be /datacenter/acls + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class Where: ---------------------------------------------------------- | Name | Type | Rules | @@ -31,9 +35,15 @@ Feature: dc / acls / update: ACL Update | key%20name | client | node "0" {policy = "read"} | | utf8? | management | node "0" {policy = "write"} | ---------------------------------------------------------- -@ignore - Scenario: Rules can be edited/updated - Then ok -@ignore - Scenario: The feedback dialog says success or failure - Then ok + Scenario: There was an error saving the key + Given the url "/v1/acl/update" responds with a 500 status + And I submit + Then the url should be /datacenter/acls/key + Then "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "error" class +# @ignore + # Scenario: Rules can be edited/updated + # Then ok +# @ignore + # Scenario: The feedback dialog says success or failure + # Then ok diff --git a/ui-v2/tests/acceptance/dc/acls/use.feature b/ui-v2/tests/acceptance/dc/acls/use.feature index 6ad722487..cfc1e0eb0 100644 --- a/ui-v2/tests/acceptance/dc/acls/use.feature +++ b/ui-v2/tests/acceptance/dc/acls/use.feature @@ -18,6 +18,8 @@ Feature: dc / acls / use: Using an ACL token And I click actions on the acls And I click use on the acls And I click confirmUse on the acls + Then "[data-notification]" has the "notification-use" class + And "[data-notification]" has the "success" class Then I have settings like yaml --- token: token @@ -34,6 +36,8 @@ Feature: dc / acls / use: Using an ACL token --- And I click use And I click confirmUse + Then "[data-notification]" has the "notification-use" class + And "[data-notification]" has the "success" class Then I have settings like yaml --- token: token diff --git a/ui-v2/tests/acceptance/dc/intentions/update.feature b/ui-v2/tests/acceptance/dc/intentions/update.feature new file mode 100644 index 000000000..7d46b636a --- /dev/null +++ b/ui-v2/tests/acceptance/dc/intentions/update.feature @@ -0,0 +1,41 @@ +@setupApplicationTest +Feature: dc / intentions / update: Intention Update + Background: + Given 1 datacenter model with the value "datacenter" + And 1 intention model from yaml + --- + ID: intention-id + --- + When I visit the intention page for yaml + --- + dc: datacenter + intention: intention-id + --- + Then the url should be /datacenter/intentions/intention-id + Scenario: Update to [Description], [Action], [Rules] + Then I fill in with yaml + --- + Description: [Name] + --- + And I click "[value=[Action]]" + And I submit + Then a PUT request is made to "/v1/connect/intentions/intention-id?dc=datacenter" with the body from yaml + --- + Description: [Name] + Action: [Action] + --- + Then the url should be /datacenter/intentions + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class + Where: + ------------------------------ + | Description | Action | + | Desc | allow | + ------------------------------ + Scenario: There was an error saving the intention + Given the url "/v1/connect/intentions/intention-id" responds with a 500 status + And I submit + Then the url should be /datacenter/intentions/intention-id + Then "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "error" class + diff --git a/ui-v2/tests/acceptance/dc/kvs/sessions/invalidate.feature b/ui-v2/tests/acceptance/dc/kvs/sessions/invalidate.feature new file mode 100644 index 000000000..01cf8918d --- /dev/null +++ b/ui-v2/tests/acceptance/dc/kvs/sessions/invalidate.feature @@ -0,0 +1,32 @@ +@setupApplicationTest +Feature: dc / kvs / sessions / invalidate: Invalidate Lock Sessions + In order to invalidate a lock session + As a user + I should be able to invalidate a lock session by clicking a button and confirming + Background: + Given 1 datacenter model with the value "datacenter" + And 1 kv model from yaml + --- + Key: key + --- + When I visit the kv page for yaml + --- + dc: datacenter + kv: key + --- + Then the url should be /datacenter/kv/key/edit + + Scenario: Invalidating the lock session + And I click delete on the session + And I click confirmDelete on the session + Then the last PUT request was made to "/v1/session/destroy/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=datacenter" + Then the url should be /datacenter/kv/key/edit + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "success" class + Scenario: Invalidating a lock session and receiving an error + Given the url "/v1/session/destroy/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=datacenter" responds with a 500 status + And I click delete on the session + And I click confirmDelete on the session + Then the url should be /datacenter/kv/key/edit + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "error" class diff --git a/ui-v2/tests/acceptance/dc/kvs/update.feature b/ui-v2/tests/acceptance/dc/kvs/update.feature index ba54ebf6c..970d3ef61 100644 --- a/ui-v2/tests/acceptance/dc/kvs/update.feature +++ b/ui-v2/tests/acceptance/dc/kvs/update.feature @@ -19,6 +19,8 @@ Feature: dc / kvs / update: KV Update --- And I submit Then a PUT request is made to "/v1/kv/[Name]?dc=datacenter" with the body "[Value]" + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class Where: -------------------------------------------- | Name | Value | @@ -43,6 +45,9 @@ Feature: dc / kvs / update: KV Update --- And I submit Then a PUT request is made to "/v1/kv/key?dc=datacenter" with the body " " + Then the url should be /datacenter/kv + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class Scenario: Update to a key change value to '' And 1 kv model from yaml --- @@ -60,6 +65,9 @@ Feature: dc / kvs / update: KV Update --- And I submit Then a PUT request is made to "/v1/kv/key?dc=datacenter" with no body + Then the url should be /datacenter/kv + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class Scenario: Update to a key when the value is empty And 1 kv model from yaml --- @@ -74,9 +82,22 @@ Feature: dc / kvs / update: KV Update Then the url should be /datacenter/kv/key/edit And I submit Then a PUT request is made to "/v1/kv/key?dc=datacenter" with no body -@ignore - Scenario: The feedback dialog says success or failure - Then ok + Then the url should be /datacenter/kv + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class + Scenario: There was an error saving the key + When I visit the kv page for yaml + --- + dc: datacenter + kv: key + --- + Then the url should be /datacenter/kv/key/edit + + Given the url "/v1/kv/key" responds with a 500 status + And I submit + Then the url should be /datacenter/kv/key/edit + Then "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "error" class @ignore Scenario: KV's with spaces are saved correctly Then ok diff --git a/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature b/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature index 786b9e4fd..ae77c20f0 100644 --- a/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature +++ b/ui-v2/tests/acceptance/dc/nodes/sessions/invalidate.feature @@ -3,7 +3,7 @@ Feature: dc / nodes / sessions / invalidate: Invalidate Lock Sessions In order to invalidate a lock session As a user I should be able to invalidate a lock session by clicking a button and confirming - Scenario: Given 2 lock sessions + Background: Given 1 datacenter model with the value "dc1" And 1 node model from yaml --- @@ -19,8 +19,20 @@ Feature: dc / nodes / sessions / invalidate: Invalidate Lock Sessions dc: dc1 node: node-0 --- + Then the url should be /dc1/nodes/node-0 And I click lockSessions on the tabs Then I see lockSessionsIsSelected on the tabs + Scenario: Invalidating the lock session And I click delete on the sessions And I click confirmDelete on the sessions Then a PUT request is made to "/v1/session/destroy/7bbbd8bb-fff3-4292-b6e3-cfedd788546a?dc=dc1" + Then the url should be /dc1/nodes/node-0 + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "success" class + Scenario: Invalidating a lock session and receiving an error + Given the url "/v1/session/destroy/7bbbd8bb-fff3-4292-b6e3-cfedd788546a?dc=dc1" responds with a 500 status + And I click delete on the sessions + And I click confirmDelete on the sessions + Then the url should be /dc1/nodes/node-0 + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "error" class diff --git a/ui-v2/tests/acceptance/deleting.feature b/ui-v2/tests/acceptance/deleting.feature index 5f7132881..19fabce57 100644 --- a/ui-v2/tests/acceptance/deleting.feature +++ b/ui-v2/tests/acceptance/deleting.feature @@ -1,8 +1,9 @@ @setupApplicationTest -Feature: deleting: Deleting from the listing and the detail page with confirmation - Scenario: Deleting a [Model] from the [Model] listing page +Feature: deleting: Deleting items with confirmations, success and error notifications + Background: Given 1 datacenter model with the value "datacenter" - And 1 [Model] model from json + Scenario: Deleting a [Model] from the [Model] listing page + Given 1 [Model] model from json --- [Data] --- @@ -14,6 +15,26 @@ Feature: deleting: Deleting from the listing and the detail page with confirmati And I click delete on the [Model]s And I click confirmDelete on the [Model]s Then a [Method] request is made to "[URL]" + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "success" class + When I visit the [Model] page for yaml + --- + dc: datacenter + [Slug] + --- + Given the url "[URL]" responds with a 500 status + And I click delete + And I click confirmDelete + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "error" class + Where: + --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + | Model | Method | URL | Data | Slug | + | acl | PUT | /v1/acl/destroy/something?dc=datacenter | {"Name": "something", "ID": "something"} | acl: something | + | kv | DELETE | /v1/kv/key-name?dc=datacenter | ["key-name"] | kv: key-name | + | intention | DELETE | /v1/connect/intentions/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=datacenter | {"SourceName": "name", "ID": "ee52203d-989f-4f7a-ab5a-2bef004164ca"} | intention: ee52203d-989f-4f7a-ab5a-2bef004164ca | + --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + Scenario: Deleting a [Model] from the [Model] detail page When I visit the [Model] page for yaml --- dc: datacenter @@ -22,6 +43,18 @@ Feature: deleting: Deleting from the listing and the detail page with confirmati And I click delete And I click confirmDelete Then a [Method] request is made to "[URL]" + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "success" class + When I visit the [Model] page for yaml + --- + dc: datacenter + [Slug] + --- + Given the url "[URL]" responds with a 500 status + And I click delete + And I click confirmDelete + And "[data-notification]" has the "notification-delete" class + And "[data-notification]" has the "error" class Where: --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | Model | Method | URL | Data | Slug | diff --git a/ui-v2/tests/acceptance/settings/update.feature b/ui-v2/tests/acceptance/settings/update.feature index 300a9123a..46d5b8feb 100644 --- a/ui-v2/tests/acceptance/settings/update.feature +++ b/ui-v2/tests/acceptance/settings/update.feature @@ -16,4 +16,7 @@ Feature: settings / update: Update Settings --- token: '' --- + And the url should be /settings + And "[data-notification]" has the "notification-update" class + And "[data-notification]" has the "success" class diff --git a/ui-v2/tests/acceptance/steps/dc/intentions/update-steps.js b/ui-v2/tests/acceptance/steps/dc/intentions/update-steps.js new file mode 100644 index 000000000..a7eff3228 --- /dev/null +++ b/ui-v2/tests/acceptance/steps/dc/intentions/update-steps.js @@ -0,0 +1,10 @@ +import steps from '../../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui-v2/tests/acceptance/steps/dc/kvs/sessions/invalidate-steps.js b/ui-v2/tests/acceptance/steps/dc/kvs/sessions/invalidate-steps.js new file mode 100644 index 000000000..9bfbe9ac9 --- /dev/null +++ b/ui-v2/tests/acceptance/steps/dc/kvs/sessions/invalidate-steps.js @@ -0,0 +1,10 @@ +import steps from '../../../steps'; + +// step definitions that are shared between features should be moved to the +// tests/acceptance/steps/steps.js file + +export default function(assert) { + return steps(assert).then('I should find a file', function() { + assert.ok(true, this.step); + }); +} diff --git a/ui-v2/tests/lib/page-object/createDeletable.js b/ui-v2/tests/lib/page-object/createDeletable.js index cc3aff4c5..88a328d96 100644 --- a/ui-v2/tests/lib/page-object/createDeletable.js +++ b/ui-v2/tests/lib/page-object/createDeletable.js @@ -1,10 +1,13 @@ export default function(clickable) { - return function(obj) { + return function(obj, scope = '') { + if (scope !== '') { + scope = scope + ' '; + } return { ...obj, ...{ - delete: clickable('[data-test-delete]'), - confirmDelete: clickable('button.type-delete'), + delete: clickable(scope + '[data-test-delete]'), + confirmDelete: clickable(scope + 'button.type-delete'), }, }; }; diff --git a/ui-v2/tests/pages/dc/kv/edit.js b/ui-v2/tests/pages/dc/kv/edit.js index b644876cc..0e7b9ea1a 100644 --- a/ui-v2/tests/pages/dc/kv/edit.js +++ b/ui-v2/tests/pages/dc/kv/edit.js @@ -3,6 +3,7 @@ export default function(visitable, submitable, deletable, cancelable) { cancelable( deletable({ visit: visitable(['/:dc/kv/:kv/edit', '/:dc/kv/create'], str => str), + session: deletable({}, '[data-test-session]'), }) ) ); diff --git a/ui-v2/tests/steps.js b/ui-v2/tests/steps.js index 287c0900a..f2a3bd671 100644 --- a/ui-v2/tests/steps.js +++ b/ui-v2/tests/steps.js @@ -60,7 +60,7 @@ export default function(assert) { ) // TODO: Abstract this away from HTTP .given(['the url "$url" responds with a $status status'], function(url, status) { - return api.server.respondWithStatus(url, parseInt(status)); + return api.server.respondWithStatus(url.split('?')[0], parseInt(status)); }) // interactions .when('I visit the $name page', function(name) { @@ -390,10 +390,16 @@ export default function(assert) { // TODO: These should be mergeable .then(['"$selector" has the "$class" class'], function(selector, cls) { // because `find` doesn't work, guessing its sandboxed to ember's container - assert.ok(document.querySelector(selector).classList.contains(cls)); + assert.ok( + document.querySelector(selector).classList.contains(cls), + `Expected [class] to contain ${cls} on ${selector}` + ); }) .then(['"$selector" doesn\'t have the "$class" class'], function(selector, cls) { - assert.ok(!document.querySelector(selector).classList.contains(cls)); + assert.ok( + !document.querySelector(selector).classList.contains(cls), + `Expected [class] not to contain ${cls} on ${selector}` + ); }) .then('ok', function() { assert.ok(true); diff --git a/ui-v2/tests/unit/mixins/acl/with-actions-test.js b/ui-v2/tests/unit/mixins/acl/with-actions-test.js index d92d1c86f..ade74b60f 100644 --- a/ui-v2/tests/unit/mixins/acl/with-actions-test.js +++ b/ui-v2/tests/unit/mixins/acl/with-actions-test.js @@ -1,15 +1,25 @@ -import EmberObject from '@ember/object'; -import AclWithActionsMixin from 'consul-ui/mixins/acl/with-actions'; -import { moduleFor, test } from 'ember-qunit'; +import { moduleFor } from 'ember-qunit'; +import test from 'ember-sinon-qunit/test-support/test'; +import { getOwner } from '@ember/application'; +import Route from 'consul-ui/routes/dc/acls/index'; +import Service from '@ember/service'; + +import Mixin from 'consul-ui/mixins/acl/with-actions'; moduleFor('mixin:acl/with-actions', 'Unit | Mixin | acl/with actions', { // Specify the other units that are required for this test. - needs: ['mixin:with-feedback'], + needs: [ + 'mixin:with-blocking-actions', + 'service:feedback', + 'service:flashMessages', + 'service:logger', + 'service:settings', + 'service:acls', + ], subject: function() { - const AclWithActionsObject = EmberObject.extend(AclWithActionsMixin); - this.register('test-container:acl/with-actions-object', AclWithActionsObject); - // TODO: May need to actually get this from the container - return AclWithActionsObject; + const MixedIn = Route.extend(Mixin); + this.register('test-container:acl/with-actions-object', MixedIn); + return getOwner(this).lookup('test-container:acl/with-actions-object'); }, }); @@ -18,3 +28,64 @@ test('it works', function(assert) { const subject = this.subject(); assert.ok(subject); }); +test('use persists the token and calls transitionTo correctly', function(assert) { + assert.expect(4); + this.register( + 'service:feedback', + Service.extend({ + execute: function(cb, name) { + assert.equal(name, 'use'); + return cb(); + }, + }) + ); + const item = { ID: 'id' }; + this.register( + 'service:settings', + Service.extend({ + persist: function(actual) { + assert.equal(actual.token, item.ID); + return Promise.resolve(actual); + }, + }) + ); + const subject = this.subject(); + const expected = 'dc.services'; + const transitionTo = this.stub(subject, 'transitionTo').returnsArg(0); + return subject.actions.use + .bind(subject)(item) + .then(function(actual) { + assert.ok(transitionTo.calledOnce); + assert.equal(actual, expected); + }); +}); +test('clone clones the token and calls afterDelete correctly', function(assert) { + assert.expect(4); + this.register( + 'service:feedback', + Service.extend({ + execute: function(cb, name) { + assert.equal(name, 'clone'); + return cb(); + }, + }) + ); + const expected = { ID: 'id' }; + this.register( + 'service:acls', + Service.extend({ + clone: function(actual) { + assert.deepEqual(actual, expected); + return Promise.resolve(actual); + }, + }) + ); + const subject = this.subject(); + const afterDelete = this.stub(subject, 'afterDelete').returnsArg(0); + return subject.actions.clone + .bind(subject)(expected) + .then(function(actual) { + assert.ok(afterDelete.calledOnce); + assert.equal(actual, expected); + }); +}); diff --git a/ui-v2/tests/unit/mixins/intention/with-actions-test.js b/ui-v2/tests/unit/mixins/intention/with-actions-test.js index 30f86de97..a33881eda 100644 --- a/ui-v2/tests/unit/mixins/intention/with-actions-test.js +++ b/ui-v2/tests/unit/mixins/intention/with-actions-test.js @@ -1,15 +1,21 @@ -import EmberObject from '@ember/object'; -import IntentionWithActionsMixin from 'consul-ui/mixins/intention/with-actions'; -import { moduleFor, test } from 'ember-qunit'; +import { moduleFor } from 'ember-qunit'; +import test from 'ember-sinon-qunit/test-support/test'; +import { getOwner } from '@ember/application'; +import Route from '@ember/routing/route'; +import Mixin from 'consul-ui/mixins/intention/with-actions'; moduleFor('mixin:intention/with-actions', 'Unit | Mixin | intention/with actions', { // Specify the other units that are required for this test. - needs: ['service:feedback'], + needs: [ + 'mixin:with-blocking-actions', + 'service:feedback', + 'service:flashMessages', + 'service:logger', + ], subject: function() { - const IntentionWithActionsObject = EmberObject.extend(IntentionWithActionsMixin); - this.register('test-container:intention/with-actions-object', IntentionWithActionsObject); - // TODO: May need to actually get this from the container - return IntentionWithActionsObject; + const MixedIn = Route.extend(Mixin); + this.register('test-container:intention/with-actions-object', MixedIn); + return getOwner(this).lookup('test-container:intention/with-actions-object'); }, }); @@ -18,3 +24,17 @@ test('it works', function(assert) { const subject = this.subject(); assert.ok(subject); }); +test('errorCreate returns a different status code if a duplicate intention is found', function(assert) { + const subject = this.subject(); + const expected = 'exists'; + const actual = subject.errorCreate('error', { + errors: [{ status: '500', detail: 'duplicate intention found:' }], + }); + assert.equal(actual, expected); +}); +test('errorCreate returns the same code if there is no error', function(assert) { + const subject = this.subject(); + const expected = 'error'; + const actual = subject.errorCreate('error', {}); + assert.equal(actual, expected); +}); diff --git a/ui-v2/tests/unit/mixins/kv/with-actions-test.js b/ui-v2/tests/unit/mixins/kv/with-actions-test.js index eccc3a141..d42d3c0e8 100644 --- a/ui-v2/tests/unit/mixins/kv/with-actions-test.js +++ b/ui-v2/tests/unit/mixins/kv/with-actions-test.js @@ -1,15 +1,21 @@ -import EmberObject from '@ember/object'; -import KvWithActionsMixin from 'consul-ui/mixins/kv/with-actions'; -import { moduleFor, test } from 'ember-qunit'; +import { moduleFor, skip } from 'ember-qunit'; +import test from 'ember-sinon-qunit/test-support/test'; +import { getOwner } from '@ember/application'; +import Route from '@ember/routing/route'; +import Mixin from 'consul-ui/mixins/kv/with-actions'; moduleFor('mixin:kv/with-actions', 'Unit | Mixin | kv/with actions', { // Specify the other units that are required for this test. - needs: ['mixin:with-feedback'], + needs: [ + 'mixin:with-blocking-actions', + 'service:feedback', + 'service:flashMessages', + 'service:logger', + ], subject: function() { - const KvWithActionsObject = EmberObject.extend(KvWithActionsMixin); - this.register('test-container:kv/with-actions-object', KvWithActionsObject); - // TODO: May need to actually get this from the container - return KvWithActionsObject; + const MixedIn = Route.extend(Mixin); + this.register('test-container:kv/with-actions-object', MixedIn); + return getOwner(this).lookup('test-container:kv/with-actions-object'); }, }); @@ -18,3 +24,29 @@ test('it works', function(assert) { const subject = this.subject(); assert.ok(subject); }); +test('afterUpdate calls transitionTo index when the key is a single slash', function(assert) { + const subject = this.subject(); + const expected = 'dc.kv.index'; + const transitionTo = this.stub(subject, 'transitionTo').returnsArg(0); + const actual = subject.afterUpdate({}, { Key: '/' }); + assert.equal(actual, expected); + assert.ok(transitionTo.calledOnce); +}); +test('afterUpdate calls transitionTo folder when the key is not a single slash', function(assert) { + const subject = this.subject(); + const expected = 'dc.kv.folder'; + const transitionTo = this.stub(subject, 'transitionTo').returnsArg(0); + ['', '/key', 'key/name'].forEach(item => { + const actual = subject.afterUpdate({}, { Key: item }); + assert.equal(actual, expected); + }); + assert.ok(transitionTo.calledThrice); +}); +test('afterDelete calls refresh folder when the routeName is `folder`', function(assert) { + const subject = this.subject(); + subject.routeName = 'dc.kv.folder'; + const refresh = this.stub(subject, 'refresh'); + subject.afterDelete({}, {}); + assert.ok(refresh.calledOnce); +}); +skip('action invalidateSession test'); diff --git a/ui-v2/tests/unit/mixins/with-blocking-actions-test.js b/ui-v2/tests/unit/mixins/with-blocking-actions-test.js new file mode 100644 index 000000000..73ee2aa8e --- /dev/null +++ b/ui-v2/tests/unit/mixins/with-blocking-actions-test.js @@ -0,0 +1,74 @@ +import { moduleFor, skip } from 'ember-qunit'; +import test from 'ember-sinon-qunit/test-support/test'; +import { getOwner } from '@ember/application'; +import Route from '@ember/routing/route'; +import Mixin from 'consul-ui/mixins/with-blocking-actions'; + +moduleFor('mixin:with-blocking-actions', 'Unit | Mixin | with blocking actions', { + // Specify the other units that are required for this test. + needs: ['service:feedback', 'service:flashMessages', 'service:logger'], + subject: function() { + const MixedIn = Route.extend(Mixin); + this.register('test-container:with-blocking-actions-object', MixedIn); + return getOwner(this).lookup('test-container:with-blocking-actions-object'); + }, +}); + +// Replace this with your real tests. +test('it works', function(assert) { + const subject = this.subject(); + assert.ok(subject); +}); +skip('init sets up feedback properly'); +test('afterCreate just calls afterUpdate', function(assert) { + const subject = this.subject(); + const expected = [1, 2, 3, 4]; + const afterUpdate = this.stub(subject, 'afterUpdate').returns(expected); + const actual = subject.afterCreate(expected); + assert.deepEqual(actual, expected); + assert.ok(afterUpdate.calledOnce); +}); +test('afterUpdate calls transitionTo without the last part of the current route name', function(assert) { + const subject = this.subject(); + const expected = 'dc.kv'; + subject.routeName = expected + '.edit'; + const transitionTo = this.stub(subject, 'transitionTo').returnsArg(0); + const actual = subject.afterUpdate(); + assert.equal(actual, expected); + assert.ok(transitionTo.calledOnce); +}); +test('afterDelete calls transitionTo without the last part of the current route name if the last part is not `index`', function(assert) { + const subject = this.subject(); + const expected = 'dc.kv'; + subject.routeName = expected + '.edit'; + const transitionTo = this.stub(subject, 'transitionTo').returnsArg(0); + const actual = subject.afterDelete(); + assert.equal(actual, expected); + assert.ok(transitionTo.calledOnce); +}); +test('afterDelete calls refresh if the last part is `index`', function(assert) { + const subject = this.subject(); + subject.routeName = 'dc.kv.index'; + const expected = 'refresh'; + const refresh = this.stub(subject, 'refresh').returns(expected); + const actual = subject.afterDelete(); + assert.equal(actual, expected); + assert.ok(refresh.calledOnce); +}); +test('the error hooks return type', function(assert) { + const subject = this.subject(); + const expected = 'success'; + ['errorCreate', 'errorUpdate', 'errorDelete'].forEach(function(item) { + const actual = subject[item](expected, {}); + assert.equal(actual, expected); + }); +}); +test('action cancel just calls afterUpdate', function(assert) { + const subject = this.subject(); + const expected = [1, 2, 3, 4]; + const afterUpdate = this.stub(subject, 'afterUpdate').returns(expected); + // TODO: unsure as to whether ember testing should actually bind this for you? + const actual = subject.actions.cancel.bind(subject)(expected); + assert.deepEqual(actual, expected); + assert.ok(afterUpdate.calledOnce); +}); diff --git a/ui-v2/tests/unit/mixins/with-feedback-test.js b/ui-v2/tests/unit/mixins/with-feedback-test.js deleted file mode 100644 index 6e79cf62f..000000000 --- a/ui-v2/tests/unit/mixins/with-feedback-test.js +++ /dev/null @@ -1,20 +0,0 @@ -import EmberObject from '@ember/object'; -import WithFeedbackMixin from 'consul-ui/mixins/with-feedback'; -import { moduleFor, test } from 'ember-qunit'; - -moduleFor('mixin:with-feedback', 'Unit | Mixin | with feedback', { - // Specify the other units that are required for this test. - needs: ['service:feedback'], - subject: function() { - const WithFeedbackObject = EmberObject.extend(WithFeedbackMixin); - this.register('test-container:with-feedback-object', WithFeedbackObject); - // TODO: May need to actually get this from the container - return WithFeedbackObject; - }, -}); - -// Replace this with your real tests. -test('it works', function(assert) { - const subject = this.subject(); - assert.ok(subject); -}); diff --git a/ui-v2/yarn.lock b/ui-v2/yarn.lock index 5c91f370f..7b11ec92c 100644 --- a/ui-v2/yarn.lock +++ b/ui-v2/yarn.lock @@ -3360,6 +3360,13 @@ ember-cli-sri@^2.1.0: dependencies: broccoli-sri-hash "^2.1.0" +ember-cli-string-helpers@^1.9.0: + version "1.9.0" + resolved "https://registry.yarnpkg.com/ember-cli-string-helpers/-/ember-cli-string-helpers-1.9.0.tgz#2c1605bc5768ff58cecd2fa1bd0d13d81e47f3d3" + dependencies: + broccoli-funnel "^1.0.1" + ember-cli-babel "^6.6.0" + ember-cli-string-utils@^1.0.0, ember-cli-string-utils@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/ember-cli-string-utils/-/ember-cli-string-utils-1.1.0.tgz#39b677fc2805f55173735376fcef278eaa4452a1"