From 7ac6ebb9204b6ce65035a4d0093c50407422b7d0 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 18 Jun 2015 22:56:18 +0100 Subject: [PATCH 1/3] Refactor notifications service & components issue #5409 - change persistent/passive notification status to alert/notification - replace showSuccess/Info/Warn/Error with showNotification/showAlert - fix and clean up notification/alert components --- core/client/app/components/gh-alert.js | 26 +- core/client/app/components/gh-alerts.js | 12 +- core/client/app/components/gh-notification.js | 31 +- .../client/app/components/gh-notifications.js | 7 +- core/client/app/components/gh-user-invited.js | 8 +- .../app/controllers/modals/delete-all.js | 2 +- .../app/controllers/modals/delete-post.js | 3 +- .../app/controllers/modals/delete-tag.js | 5 +- .../app/controllers/modals/delete-user.js | 3 +- .../app/controllers/modals/invite-new-user.js | 8 +- .../app/controllers/modals/leave-editor.js | 2 +- core/client/app/controllers/modals/signin.js | 3 +- .../app/controllers/modals/transfer-owner.js | 2 +- core/client/app/controllers/modals/upload.js | 2 - .../app/controllers/post-settings-menu.js | 4 - core/client/app/controllers/reset.js | 2 +- .../controllers/settings/code-injection.js | 5 +- .../app/controllers/settings/general.js | 1 - core/client/app/controllers/settings/labs.js | 9 +- .../app/controllers/settings/navigation.js | 8 +- core/client/app/controllers/settings/tags.js | 2 +- core/client/app/controllers/setup/three.js | 8 +- core/client/app/controllers/signin.js | 7 +- core/client/app/controllers/signup.js | 2 +- core/client/app/controllers/team/user.js | 5 +- .../app/mixins/editor-base-controller.js | 7 +- .../app/mixins/pagination-controller.js | 2 +- core/client/app/models/notification.js | 1 - core/client/app/router.js | 2 +- core/client/app/routes/application.js | 17 +- core/client/app/routes/reset.js | 2 +- core/client/app/routes/signup.js | 6 +- core/client/app/services/notifications.js | 140 +++------ .../app/styles/components/notifications.css | 10 + core/client/bower.json | 1 + core/client/package.json | 1 + .../tests/unit/components/gh-alert-test.js | 71 +++++ .../tests/unit/components/gh-alerts-test.js | 63 ++++ .../unit/components/gh-notification-test.js | 82 +++++ .../unit/components/gh-notifications-test.js | 47 +++ .../tests/unit/services/notifications-test.js | 286 ++++++++++++++++++ core/server/api/notifications.js | 4 +- core/server/controllers/admin.js | 2 +- core/test/functional/base.js | 4 +- core/test/functional/client/editor_test.js | 48 ++- core/test/functional/client/psm_test.js | 12 +- core/test/functional/client/settings_test.js | 42 ++- core/test/functional/client/signin_test.js | 38 +-- core/test/functional/client/team_test.js | 82 ++--- .../routes/api/notifications_spec.js | 4 +- core/test/functional/setup/setup_test.js | 23 +- .../integration/api/api_notifications_spec.js | 2 +- 52 files changed, 797 insertions(+), 369 deletions(-) create mode 100644 core/client/tests/unit/components/gh-alert-test.js create mode 100644 core/client/tests/unit/components/gh-alerts-test.js create mode 100644 core/client/tests/unit/components/gh-notification-test.js create mode 100644 core/client/tests/unit/components/gh-notifications-test.js create mode 100644 core/client/tests/unit/services/notifications-test.js diff --git a/core/client/app/components/gh-alert.js b/core/client/app/components/gh-alert.js index 1ac91e4b39..e5fbad5c28 100644 --- a/core/client/app/components/gh-alert.js +++ b/core/client/app/components/gh-alert.js @@ -2,7 +2,7 @@ import Ember from 'ember'; export default Ember.Component.extend({ tagName: 'article', - classNames: ['gh-alert', 'gh-alert-blue'], + classNames: ['gh-alert'], classNameBindings: ['typeClass'], notifications: Ember.inject.service(), @@ -10,22 +10,18 @@ export default Ember.Component.extend({ typeClass: Ember.computed(function () { var classes = '', message = this.get('message'), - type, - dismissible; + type = Ember.get(message, 'type'), + typeMapping; - // Check to see if we're working with a DS.Model or a plain JS object - if (typeof message.toJSON === 'function') { - type = message.get('type'); - dismissible = message.get('dismissible'); - } else { - type = message.type; - dismissible = message.dismissible; - } + typeMapping = { + success: 'green', + error: 'red', + warn: 'yellow', + info: 'blue' + }; - classes += 'notification-' + type; - - if (type === 'success' && dismissible !== false) { - classes += ' notification-passive'; + if (typeMapping[type] !== undefined) { + classes += 'gh-alert-' + typeMapping[type]; } return classes; diff --git a/core/client/app/components/gh-alerts.js b/core/client/app/components/gh-alerts.js index 5fd6459163..6e04c4dfb4 100644 --- a/core/client/app/components/gh-alerts.js +++ b/core/client/app/components/gh-alerts.js @@ -1,18 +1,14 @@ import Ember from 'ember'; -var AlertsComponent = Ember.Component.extend({ + +export default Ember.Component.extend({ tagName: 'aside', classNames: 'gh-alerts', - messages: Ember.computed.filter('notifications', function (notification) { - var displayStatus = (typeof notification.toJSON === 'function') ? - notification.get('status') : notification.status; + notifications: Ember.inject.service(), - return displayStatus === 'persistent'; - }), + messages: Ember.computed.alias('notifications.alerts'), messageCountObserver: Ember.observer('messages.[]', function () { this.sendAction('notify', this.get('messages').length); }) }); - -export default AlertsComponent; diff --git a/core/client/app/components/gh-notification.js b/core/client/app/components/gh-notification.js index 60f67744e9..55cb48b731 100644 --- a/core/client/app/components/gh-notification.js +++ b/core/client/app/components/gh-notification.js @@ -2,7 +2,7 @@ import Ember from 'ember'; export default Ember.Component.extend({ tagName: 'article', - classNames: ['gh-notification', 'gh-notification-green'], + classNames: ['gh-notification', 'gh-notification-passive'], classNameBindings: ['typeClass'], message: null, @@ -12,22 +12,17 @@ export default Ember.Component.extend({ typeClass: Ember.computed(function () { var classes = '', message = this.get('message'), - type, - dismissible; + type = Ember.get(message, 'type'), + typeMapping; - // Check to see if we're working with a DS.Model or a plain JS object - if (typeof message.toJSON === 'function') { - type = message.get('type'); - dismissible = message.get('dismissible'); - } else { - type = message.type; - dismissible = message.dismissible; - } + typeMapping = { + success: 'green', + error: 'red', + warn: 'yellow' + }; - classes += 'notification-' + type; - - if (type === 'success' && dismissible !== false) { - classes += ' notification-passive'; + if (typeMapping[type] !== undefined) { + classes += 'gh-notification-' + typeMapping[type]; } return classes; @@ -38,11 +33,15 @@ export default Ember.Component.extend({ self.$().on('animationend webkitAnimationEnd oanimationend MSAnimationEnd', function (event) { if (event.originalEvent.animationName === 'fade-out') { - self.get('notifications').removeObject(self.get('message')); + self.get('notifications').closeNotification(self.get('message')); } }); }, + willDestroyElement: function () { + this.$().off('animationend webkitAnimationEnd oanimationend MSAnimationEnd'); + }, + actions: { closeNotification: function () { this.get('notifications').closeNotification(this.get('message')); diff --git a/core/client/app/components/gh-notifications.js b/core/client/app/components/gh-notifications.js index 03ce6e859f..a8c4c81288 100644 --- a/core/client/app/components/gh-notifications.js +++ b/core/client/app/components/gh-notifications.js @@ -6,10 +6,5 @@ export default Ember.Component.extend({ notifications: Ember.inject.service(), - messages: Ember.computed.filter('notifications.content', function (notification) { - var displayStatus = (typeof notification.toJSON === 'function') ? - notification.get('status') : notification.status; - - return displayStatus === 'passive'; - }) + messages: Ember.computed.alias('notifications.notifications') }); diff --git a/core/client/app/components/gh-user-invited.js b/core/client/app/components/gh-user-invited.js index 14fdab8059..8aaf83563c 100644 --- a/core/client/app/components/gh-user-invited.js +++ b/core/client/app/components/gh-user-invited.js @@ -24,10 +24,10 @@ export default Ember.Component.extend({ // If sending the invitation email fails, the API will still return a status of 201 // but the user's status in the response object will be 'invited-pending'. if (result.users[0].status === 'invited-pending') { - notifications.showWarn('Invitation email was not sent. Please try resending.'); + notifications.showAlert('Invitation email was not sent. Please try resending.', {type: 'error'}); } else { user.set('status', result.users[0].status); - notifications.showSuccess(notificationText); + notifications.showNotification(notificationText); } }).catch(function (error) { notifications.showAPIError(error); @@ -46,14 +46,14 @@ export default Ember.Component.extend({ user.destroyRecord().then(function () { var notificationText = 'Invitation revoked. (' + email + ')'; - notifications.showSuccess(notificationText, false); + notifications.showNotification(notificationText); }).catch(function (error) { notifications.showAPIError(error); }); } else { // if the user is no longer marked as "invited", then show a warning and reload the route self.sendAction('reload'); - notifications.showError('This user has already accepted the invitation.', {delayed: 500}); + notifications.showAlert('This user has already accepted the invitation.', {type: 'error', delayed: true}); } }); } diff --git a/core/client/app/controllers/modals/delete-all.js b/core/client/app/controllers/modals/delete-all.js index 30c10f3179..4ea73dd6e4 100644 --- a/core/client/app/controllers/modals/delete-all.js +++ b/core/client/app/controllers/modals/delete-all.js @@ -12,7 +12,7 @@ export default Ember.Controller.extend({ ajax(this.get('ghostPaths.url').api('db'), { type: 'DELETE' }).then(function () { - self.get('notifications').showSuccess('All content deleted from database.'); + self.get('notifications').showAlert('All content deleted from database.', {type: 'success'}); self.store.unloadAll('post'); self.store.unloadAll('tag'); }).catch(function (response) { diff --git a/core/client/app/controllers/modals/delete-post.js b/core/client/app/controllers/modals/delete-post.js index 1f334c4437..8b87628122 100644 --- a/core/client/app/controllers/modals/delete-post.js +++ b/core/client/app/controllers/modals/delete-post.js @@ -15,9 +15,8 @@ export default Ember.Controller.extend({ model.destroyRecord().then(function () { self.get('dropdown').closeDropdowns(); self.transitionToRoute('posts.index'); - self.get('notifications').showSuccess('Your post has been deleted.', {delayed: true}); }, function () { - self.get('notifications').showError('Your post could not be deleted. Please try again.'); + self.get('notifications').showAlert('Your post could not be deleted. Please try again.', {type: 'error'}); }); }, diff --git a/core/client/app/controllers/modals/delete-tag.js b/core/client/app/controllers/modals/delete-tag.js index 507eacd077..f94908c4b3 100644 --- a/core/client/app/controllers/modals/delete-tag.js +++ b/core/client/app/controllers/modals/delete-tag.js @@ -10,14 +10,11 @@ export default Ember.Controller.extend({ actions: { confirmAccept: function () { var tag = this.get('model'), - name = tag.get('name'), self = this; this.send('closeMenus'); - tag.destroyRecord().then(function () { - self.get('notifications').showSuccess('Deleted ' + name); - }).catch(function (error) { + tag.destroyRecord().catch(function (error) { self.get('notifications').showAPIError(error); }); }, diff --git a/core/client/app/controllers/modals/delete-user.js b/core/client/app/controllers/modals/delete-user.js index e446b62954..c3ea0ff24c 100644 --- a/core/client/app/controllers/modals/delete-user.js +++ b/core/client/app/controllers/modals/delete-user.js @@ -31,9 +31,8 @@ export default Ember.Controller.extend({ user.destroyRecord().then(function () { self.store.unloadAll('post'); self.transitionToRoute('team'); - self.get('notifications').showSuccess('The user has been deleted.', {delayed: true}); }, function () { - self.get('notifications').showError('The user could not be deleted. Please try again.'); + self.get('notifications').showAlert('The user could not be deleted. Please try again.', {type: 'error'}); }); }, diff --git a/core/client/app/controllers/modals/invite-new-user.js b/core/client/app/controllers/modals/invite-new-user.js index 4af00cc725..cf8fdd95ca 100644 --- a/core/client/app/controllers/modals/invite-new-user.js +++ b/core/client/app/controllers/modals/invite-new-user.js @@ -55,9 +55,9 @@ export default Ember.Controller.extend({ if (invitedUser) { if (invitedUser.get('status') === 'invited' || invitedUser.get('status') === 'invited-pending') { - self.get('notifications').showWarn('A user with that email address was already invited.'); + self.get('notifications').showAlert('A user with that email address was already invited.', {type: 'warn'}); } else { - self.get('notifications').showWarn('A user with that email address already exists.'); + self.get('notifications').showAlert('A user with that email address already exists.', {type: 'warn'}); } } else { newUser = self.store.createRecord('user', { @@ -72,9 +72,9 @@ export default Ember.Controller.extend({ // If sending the invitation email fails, the API will still return a status of 201 // but the user's status in the response object will be 'invited-pending'. if (newUser.get('status') === 'invited-pending') { - self.get('notifications').showWarn('Invitation email was not sent. Please try resending.'); + self.get('notifications').showAlert('Invitation email was not sent. Please try resending.', {type: 'error'}); } else { - self.get('notifications').showSuccess(notificationText); + self.get('notifications').showAlert(notificationText, {type: 'success'}); } }).catch(function (errors) { newUser.deleteRecord(); diff --git a/core/client/app/controllers/modals/leave-editor.js b/core/client/app/controllers/modals/leave-editor.js index d455ead29d..2b188f5287 100644 --- a/core/client/app/controllers/modals/leave-editor.js +++ b/core/client/app/controllers/modals/leave-editor.js @@ -19,7 +19,7 @@ export default Ember.Controller.extend({ } if (!transition || !editorController) { - this.get('notifications').showError('Sorry, there was an error in the application. Please let the Ghost team know what happened.'); + this.get('notifications').showNotification('Sorry, there was an error in the application. Please let the Ghost team know what happened.', {type: 'error'}); return true; } diff --git a/core/client/app/controllers/modals/signin.js b/core/client/app/controllers/modals/signin.js index 89a438ef7f..ada9b10280 100644 --- a/core/client/app/controllers/modals/signin.js +++ b/core/client/app/controllers/modals/signin.js @@ -22,7 +22,6 @@ export default Ember.Controller.extend(ValidationEngine, { this.get('session').authenticate(authStrategy, data).then(function () { self.send('closeModal'); - self.get('notifications').showSuccess('Login successful.'); self.set('password', ''); }).catch(function () { // if authentication fails a rejected promise will be returned. @@ -41,7 +40,7 @@ export default Ember.Controller.extend(ValidationEngine, { $('#login').find('input').trigger('change'); this.validate({format: false}).then(function () { - self.get('notifications').closePassive(); + self.get('notifications').closeNotifications(); self.send('authenticate'); }).catch(function (errors) { self.get('notifications').showErrors(errors); diff --git a/core/client/app/controllers/modals/transfer-owner.js b/core/client/app/controllers/modals/transfer-owner.js index f721713962..40372adaee 100644 --- a/core/client/app/controllers/modals/transfer-owner.js +++ b/core/client/app/controllers/modals/transfer-owner.js @@ -33,7 +33,7 @@ export default Ember.Controller.extend({ }); } - self.get('notifications').showSuccess('Ownership successfully transferred to ' + user.get('name')); + self.get('notifications').showAlert('Ownership successfully transferred to ' + user.get('name'), {type: 'success'}); }).catch(function (error) { self.get('notifications').showAPIError(error); }); diff --git a/core/client/app/controllers/modals/upload.js b/core/client/app/controllers/modals/upload.js index f186205826..a288447d56 100644 --- a/core/client/app/controllers/modals/upload.js +++ b/core/client/app/controllers/modals/upload.js @@ -10,8 +10,6 @@ export default Ember.Controller.extend({ var notifications = this.get('notifications'); this.get('model').save().then(function (model) { - notifications.showSuccess('Saved'); - return model; }).catch(function (err) { notifications.showErrors(err); diff --git a/core/client/app/controllers/post-settings-menu.js b/core/client/app/controllers/post-settings-menu.js index d40f05ee69..58051e9210 100644 --- a/core/client/app/controllers/post-settings-menu.js +++ b/core/client/app/controllers/post-settings-menu.js @@ -193,10 +193,6 @@ export default Ember.Controller.extend(SettingsMenuMixin, { this.get('notifications').showErrors(errors); }, - showSuccess: function (message) { - this.get('notifications').showSuccess(message); - }, - actions: { togglePage: function () { var self = this; diff --git a/core/client/app/controllers/reset.js b/core/client/app/controllers/reset.js index a428b30b5c..92304f0c80 100644 --- a/core/client/app/controllers/reset.js +++ b/core/client/app/controllers/reset.js @@ -43,7 +43,7 @@ export default Ember.Controller.extend(ValidationEngine, { } }).then(function (resp) { self.toggleProperty('submitting'); - self.get('notifications').showSuccess(resp.passwordreset[0].message, true); + self.get('notifications').showAlert(resp.passwordreset[0].message, {type: 'warn', delayed: true}); self.get('session').authenticate('simple-auth-authenticator:oauth2-password-grant', { identification: self.get('email'), password: credentials.newPassword diff --git a/core/client/app/controllers/settings/code-injection.js b/core/client/app/controllers/settings/code-injection.js index 9cfa4307d0..043c1efd14 100644 --- a/core/client/app/controllers/settings/code-injection.js +++ b/core/client/app/controllers/settings/code-injection.js @@ -8,12 +8,11 @@ export default Ember.Controller.extend({ var notifications = this.get('notifications'); return this.get('model').save().then(function (model) { - notifications.closePassive(); - notifications.showSuccess('Settings successfully saved.'); + notifications.closeNotifications(); return model; }).catch(function (errors) { - notifications.closePassive(); + notifications.closeNotifications(); notifications.showErrors(errors); }); } diff --git a/core/client/app/controllers/settings/general.js b/core/client/app/controllers/settings/general.js index 8a829c2185..0a97ad1b8d 100644 --- a/core/client/app/controllers/settings/general.js +++ b/core/client/app/controllers/settings/general.js @@ -69,7 +69,6 @@ export default Ember.Controller.extend({ return this.get('model').save().then(function (model) { config.set('blogTitle', model.get('title')); - notifications.showSuccess('Settings successfully saved.'); return model; }).catch(function (errors) { diff --git a/core/client/app/controllers/settings/labs.js b/core/client/app/controllers/settings/labs.js index 58c48dc9d7..c1e3629572 100644 --- a/core/client/app/controllers/settings/labs.js +++ b/core/client/app/controllers/settings/labs.js @@ -36,7 +36,7 @@ export default Ember.Controller.extend({ this.set('uploadButtonText', 'Importing'); this.set('importErrors', ''); - notifications.closePassive(); + notifications.closeNotifications(); formData.append('importfile', file); @@ -52,13 +52,14 @@ export default Ember.Controller.extend({ self.store.unloadAll(); // Reload currentUser and set session self.set('session.user', self.store.find('user', currentUserId)); - notifications.showSuccess('Import successful.'); + // TODO: keep as notification, add link to view content + notifications.showNotification('Import successful.'); }).catch(function (response) { if (response && response.jqXHR && response.jqXHR.responseJSON && response.jqXHR.responseJSON.errors) { self.set('importErrors', response.jqXHR.responseJSON.errors); } - notifications.showError('Import Failed'); + notifications.showAlert('Import Failed', {type: 'error'}); }).finally(function () { self.set('uploadButtonText', 'Import'); }); @@ -82,7 +83,7 @@ export default Ember.Controller.extend({ ajax(this.get('ghostPaths.url').api('mail', 'test'), { type: 'POST' }).then(function () { - notifications.showSuccess('Check your email for the test message.'); + notifications.showAlert('Check your email for the test message.', {type: 'info'}); }).catch(function (error) { if (typeof error.jqXHR !== 'undefined') { notifications.showAPIError(error); diff --git a/core/client/app/controllers/settings/navigation.js b/core/client/app/controllers/settings/navigation.js index bcb7e91ae7..1807d0da6a 100644 --- a/core/client/app/controllers/settings/navigation.js +++ b/core/client/app/controllers/settings/navigation.js @@ -108,7 +108,7 @@ export default Ember.Controller.extend({ // Don't save if there's a blank label. if (navItems.find(function (item) {return !item.get('isComplete') && !item.get('last');})) { - notifications.showErrors([message.htmlSafe()]); + notifications.showAlert(message.htmlSafe(), {type: 'error'}); return; } @@ -148,11 +148,9 @@ export default Ember.Controller.extend({ // we need to have navigationItems recomputed. this.get('model').notifyPropertyChange('navigation'); - notifications.closePassive(); + notifications.closeNotifications(); - this.get('model').save().then(function () { - notifications.showSuccess('Navigation items saved.'); - }).catch(function (err) { + this.get('model').save().catch(function (err) { notifications.showErrors(err); }); } diff --git a/core/client/app/controllers/settings/tags.js b/core/client/app/controllers/settings/tags.js index fefe3ea4ad..e1b9e01ac5 100644 --- a/core/client/app/controllers/settings/tags.js +++ b/core/client/app/controllers/settings/tags.js @@ -59,7 +59,7 @@ export default Ember.Controller.extend(PaginationMixin, SettingsMenuMixin, { activeTag.set(propKey, newValue); - this.get('notifications').closePassive(); + this.get('notifications').closeNotifications(); activeTag.save().catch(function (errors) { self.showErrors(errors); diff --git a/core/client/app/controllers/setup/three.js b/core/client/app/controllers/setup/three.js index 29dd0df2a2..5c8fb1ab7f 100644 --- a/core/client/app/controllers/setup/three.js +++ b/core/client/app/controllers/setup/three.js @@ -104,20 +104,21 @@ export default Ember.Controller.extend({ if (erroredEmails.length > 0) { message = 'Failed to send ' + erroredEmails.length + ' invitations: '; message += erroredEmails.join(', '); - notifications.showError(message, {delayed: successCount > 0}); + notifications.showAlert(message, {type: 'error', delayed: successCount > 0}); } if (successCount > 0) { // pluralize invitationsString = successCount > 1 ? 'invitations' : 'invitation'; - notifications.showSuccess(successCount + ' ' + invitationsString + ' sent!', {delayed: true}); + notifications.showAlert(successCount + ' ' + invitationsString + ' sent!', {type: 'success', delayed: true}); self.transitionTo('posts.index'); } }); }); } else if (users.length === 0) { - notifications.showError('No users to invite.'); + // TODO: switch to inline-validation + notifications.showAlert('No users to invite.', {type: 'error'}); } else { errorMessages = validationErrors.map(function (error) { // Only one error type here so far, but one day the errors might be more detailed @@ -127,6 +128,7 @@ export default Ember.Controller.extend({ } }); + // TODO: switch to inline-validation notifications.showErrors(errorMessages); } } diff --git a/core/client/app/controllers/signin.js b/core/client/app/controllers/signin.js index b578d975d6..b74494950a 100644 --- a/core/client/app/controllers/signin.js +++ b/core/client/app/controllers/signin.js @@ -31,7 +31,7 @@ export default Ember.Controller.extend(ValidationEngine, { $('#login').find('input').trigger('change'); this.validate({format: false}).then(function () { - self.get('notifications').closePassive(); + self.get('notifications').closeNotifications(); self.send('authenticate'); }).catch(function (errors) { if (errors) { @@ -46,7 +46,8 @@ export default Ember.Controller.extend(ValidationEngine, { self = this; if (!email) { - return notifications.showError('Enter email address to reset password.'); + // TODO: Switch to in-line validation + return notifications.showNotification('Enter email address to reset password.', {type: 'error'}); } self.set('submitting', true); @@ -61,7 +62,7 @@ export default Ember.Controller.extend(ValidationEngine, { } }).then(function () { self.set('submitting', false); - notifications.showSuccess('Please check your email for instructions.'); + notifications.showAlert('Please check your email for instructions.', {type: 'info'}); }).catch(function (resp) { self.set('submitting', false); notifications.showAPIError(resp, {defaultErrorText: 'There was a problem with the reset, please try again.'}); diff --git a/core/client/app/controllers/signup.js b/core/client/app/controllers/signup.js index 77ea44a6be..cbb981d5d7 100644 --- a/core/client/app/controllers/signup.js +++ b/core/client/app/controllers/signup.js @@ -18,7 +18,7 @@ export default Ember.Controller.extend(ValidationEngine, { data = model.getProperties('name', 'email', 'password', 'token'), notifications = this.get('notifications'); - notifications.closePassive(); + notifications.closeNotifications(); this.toggleProperty('submitting'); this.validate({format: false}).then(function () { diff --git a/core/client/app/controllers/team/user.js b/core/client/app/controllers/team/user.js index 6ad63f3f0d..3cb9032a16 100644 --- a/core/client/app/controllers/team/user.js +++ b/core/client/app/controllers/team/user.js @@ -105,8 +105,6 @@ export default Ember.Controller.extend({ var currentPath, newPath; - self.get('notifications').showSuccess('Settings successfully saved.'); - // If the user's slug has changed, change the URL and replace // the history so refresh and back button still work if (slugChanged) { @@ -142,13 +140,14 @@ export default Ember.Controller.extend({ ne2Password: '' }); - self.get('notifications').showSuccess('Password updated.'); + self.get('notifications').showAlert('Password updated.', {type: 'success'}); return model; }).catch(function (errors) { self.get('notifications').showAPIError(errors); }); } else { + // TODO: switch to in-line validation self.get('notifications').showErrors(user.get('passwordValidationErrors')); } }, diff --git a/core/client/app/mixins/editor-base-controller.js b/core/client/app/mixins/editor-base-controller.js index 718167d017..7fa44730f4 100644 --- a/core/client/app/mixins/editor-base-controller.js +++ b/core/client/app/mixins/editor-base-controller.js @@ -209,6 +209,7 @@ export default Ember.Mixin.create({ } }, + // TODO: Update for new notification click-action API showSaveNotification: function (prevStatus, status, delay) { var message = this.messageMap.success.post[prevStatus][status], path = this.get('model.absoluteUrl'), @@ -219,7 +220,7 @@ export default Ember.Mixin.create({ message += ` View ${type}`; } - notifications.showSuccess(message.htmlSafe(), {delayed: delay}); + notifications.showNotification(message.htmlSafe(), {delayed: delay}); }, showErrorNotification: function (prevStatus, status, errors, delay) { @@ -229,7 +230,7 @@ export default Ember.Mixin.create({ message += '
' + error; - notifications.showError(message.htmlSafe(), {delayed: delay}); + notifications.showAlert(message.htmlSafe(), {type: 'error', delayed: delay}); }, actions: { @@ -263,7 +264,7 @@ export default Ember.Mixin.create({ this.set('timedSaveId', null); } - notifications.closePassive(); + notifications.closeNotifications(); // Set the properties that are indirected // set markdown equal to what's in the editor, minus the image markers. diff --git a/core/client/app/mixins/pagination-controller.js b/core/client/app/mixins/pagination-controller.js index 7957781561..4b417d9ee8 100644 --- a/core/client/app/mixins/pagination-controller.js +++ b/core/client/app/mixins/pagination-controller.js @@ -25,7 +25,7 @@ export default Ember.Mixin.create({ message += '.'; } - this.get('notifications').showError(message); + this.get('notifications').showAlert(message, {type: 'error'}); }, actions: { diff --git a/core/client/app/models/notification.js b/core/client/app/models/notification.js index 98cdc175e1..93e8bb506e 100644 --- a/core/client/app/models/notification.js +++ b/core/client/app/models/notification.js @@ -1,7 +1,6 @@ import DS from 'ember-data'; var Notification = DS.Model.extend({ dismissible: DS.attr('boolean'), - location: DS.attr('string'), status: DS.attr('string'), type: DS.attr('string'), message: DS.attr('string') diff --git a/core/client/app/router.js b/core/client/app/router.js index d5253a5771..de02a58899 100644 --- a/core/client/app/router.js +++ b/core/client/app/router.js @@ -11,7 +11,7 @@ var Router = Ember.Router.extend({ clearNotifications: Ember.on('didTransition', function () { var notifications = this.get('notifications'); - notifications.closePassive(); + notifications.closeNotifications(); notifications.displayDelayed(); }) }); diff --git a/core/client/app/routes/application.js b/core/client/app/routes/application.js index db4e3f0ede..7a12f7075e 100644 --- a/core/client/app/routes/application.js +++ b/core/client/app/routes/application.js @@ -66,7 +66,7 @@ export default Ember.Route.extend(ApplicationRouteMixin, ShortcutsRoute, { this.get('notifications').showErrors(error.errors); } else { // connection errors don't return proper status message, only req.body - this.get('notifications').showError('There was a problem on the server.'); + this.get('notifications').showAlert('There was a problem on the server.', {type: 'error'}); } }, @@ -91,7 +91,7 @@ export default Ember.Route.extend(ApplicationRouteMixin, ShortcutsRoute, { }, sessionInvalidationFailed: function (error) { - this.get('notifications').showError(error.message); + this.get('notifications').showAlert(error.message, {type: 'error'}); }, openModal: function (modalName, model, type) { @@ -152,19 +152,6 @@ export default Ember.Route.extend(ApplicationRouteMixin, ShortcutsRoute, { } }, - handleErrors: function (errors) { - var notifications = this.get('notifications'); - - notifications.clear(); - errors.forEach(function (errorObj) { - notifications.showError(errorObj.message || errorObj); - - if (errorObj.hasOwnProperty('el')) { - errorObj.el.addClass('input-error'); - } - }); - }, - // noop default for unhandled save (used from shortcuts) save: Ember.K } diff --git a/core/client/app/routes/reset.js b/core/client/app/routes/reset.js index 9497f6d121..e2e98a6d12 100644 --- a/core/client/app/routes/reset.js +++ b/core/client/app/routes/reset.js @@ -9,7 +9,7 @@ export default Ember.Route.extend(styleBody, { beforeModel: function () { if (this.get('session').isAuthenticated) { - this.get('notifications').showWarn('You can\'t reset your password while you\'re signed in.', {delayed: true}); + this.get('notifications').showAlert('You can\'t reset your password while you\'re signed in.', {type: 'warn', delayed: true}); this.transitionTo(Configuration.routeAfterAuthentication); } }, diff --git a/core/client/app/routes/signup.js b/core/client/app/routes/signup.js index 7527de9916..b7a900c746 100644 --- a/core/client/app/routes/signup.js +++ b/core/client/app/routes/signup.js @@ -12,7 +12,7 @@ export default Ember.Route.extend(styleBody, { beforeModel: function () { if (this.get('session').isAuthenticated) { - this.get('notifications').showWarn('You need to sign out to register as a new user.', {delayed: true}); + this.get('notifications').showAlert('You need to sign out to register as a new user.', {type: 'warn', delayed: true}); this.transitionTo(Configuration.routeAfterAuthentication); } }, @@ -26,7 +26,7 @@ export default Ember.Route.extend(styleBody, { return new Ember.RSVP.Promise(function (resolve) { if (!re.test(params.token)) { - self.get('notifications').showError('Invalid token.', {delayed: true}); + self.get('notifications').showAlert('Invalid token.', {type: 'error', delayed: true}); return resolve(self.transitionTo('signin')); } @@ -47,7 +47,7 @@ export default Ember.Route.extend(styleBody, { } }).then(function (response) { if (response && response.invitation && response.invitation[0].valid === false) { - self.get('notifications').showError('The invitation does not exist or is no longer valid.', {delayed: true}); + self.get('notifications').showAlert('The invitation does not exist or is no longer valid.', {type: 'warn', delayed: true}); return resolve(self.transitionTo('signin')); } diff --git a/core/client/app/services/notifications.js b/core/client/app/services/notifications.js index 4e4c10d6b4..f178ee36ca 100644 --- a/core/client/app/services/notifications.js +++ b/core/client/app/services/notifications.js @@ -1,138 +1,94 @@ import Ember from 'ember'; -import Notification from 'ghost/models/notification'; export default Ember.Service.extend({ delayedNotifications: Ember.A(), content: Ember.A(), - timeout: 3000, - pushObject: function (object) { - // object can be either a DS.Model or a plain JS object, so when working with - // it, we need to handle both cases. + alerts: Ember.computed.filter('content', function (notification) { + var status = Ember.get(notification, 'status'); + return status === 'alert'; + }), - // make sure notifications have all the necessary properties set. - if (typeof object.toJSON === 'function') { - // working with a DS.Model - - if (object.get('location') === '') { - object.set('location', 'bottom'); - } - } else { - if (!object.location) { - object.location = 'bottom'; - } - } - - this._super(object); - }, + notifications: Ember.computed.filter('content', function (notification) { + var status = Ember.get(notification, 'status'); + return status === 'notification'; + }), handleNotification: function (message, delayed) { - if (typeof message.toJSON === 'function') { - // If this is a persistent message from the server, treat it as html safe - if (message.get('status') === 'persistent') { - message.set('message', message.get('message').htmlSafe()); - } + // If this is an alert message from the server, treat it as html safe + if (typeof message.toJSON === 'function' && message.get('status') === 'alert') { + message.set('message', message.get('message').htmlSafe()); + } - if (!message.get('status')) { - message.set('status', 'passive'); - } - } else { - if (!message.status) { - message.status = 'passive'; - } + if (!Ember.get(message, 'status')) { + Ember.set(message, 'status', 'notification'); } if (!delayed) { this.get('content').pushObject(message); } else { - this.delayedNotifications.pushObject(message); + this.get('delayedNotifications').pushObject(message); } }, - showError: function (message, options) { + showAlert: function (message, options) { options = options || {}; - if (!options.doNotClosePassive) { - this.closePassive(); - } - this.handleNotification({ - type: 'error', - message: message + message: message, + status: 'alert', + type: options.type }, options.delayed); }, + showNotification: function (message, options) { + options = options || {}; + + if (!options.doNotCloseNotifications) { + this.closeNotifications(); + } + + this.handleNotification({ + message: message, + status: 'notification', + type: options.type + }, options.delayed); + }, + + // TODO: review whether this can be removed once no longer used by validations showErrors: function (errors, options) { options = options || {}; - if (!options.doNotClosePassive) { - this.closePassive(); + if (!options.doNotCloseNotifications) { + this.closeNotifications(); } for (var i = 0; i < errors.length; i += 1) { - this.showError(errors[i].message || errors[i], {doNotClosePassive: true}); + this.showNotification(errors[i].message || errors[i], {type: 'error', doNotCloseNotifications: true}); } }, showAPIError: function (resp, options) { options = options || {}; + options.type = options.type || 'error'; - if (!options.doNotClosePassive) { - this.closePassive(); + if (!options.doNotCloseNotifications) { + this.closeNotifications(); } options.defaultErrorText = options.defaultErrorText || 'There was a problem on the server, please try again.'; if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.error) { - this.showError(resp.jqXHR.responseJSON.error, options); + this.showAlert(resp.jqXHR.responseJSON.error, options); } else if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.errors) { this.showErrors(resp.jqXHR.responseJSON.errors, options); } else if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.message) { - this.showError(resp.jqXHR.responseJSON.message, options); + this.showAlert(resp.jqXHR.responseJSON.message, options); } else { - this.showError(options.defaultErrorText, {doNotClosePassive: true}); + this.showAlert(options.defaultErrorText, {type: options.type, doNotCloseNotifications: true}); } }, - showInfo: function (message, options) { - options = options || {}; - - if (!options.doNotClosePassive) { - this.closePassive(); - } - - this.handleNotification({ - type: 'info', - message: message - }, options.delayed); - }, - - showSuccess: function (message, options) { - options = options || {}; - - if (!options.doNotClosePassive) { - this.closePassive(); - } - - this.handleNotification({ - type: 'success', - message: message - }, options.delayed); - }, - - showWarn: function (message, options) { - options = options || {}; - - if (!options.doNotClosePassive) { - this.closePassive(); - } - - this.handleNotification({ - type: 'warn', - message: message - }, options.delayed); - }, - displayDelayed: function () { var self = this; @@ -145,7 +101,7 @@ export default Ember.Service.extend({ closeNotification: function (notification) { var content = this.get('content'); - if (notification instanceof Notification) { + if (typeof notification.toJSON === 'function') { notification.deleteRecord(); notification.save().finally(function () { content.removeObject(notification); @@ -155,12 +111,8 @@ export default Ember.Service.extend({ } }, - closePassive: function () { - this.set('content', this.get('content').rejectBy('status', 'passive')); - }, - - closePersistent: function () { - this.set('content', this.get('content').rejectBy('status', 'persistent')); + closeNotifications: function () { + this.set('content', this.get('content').rejectBy('status', 'notification')); }, closeAll: function () { diff --git a/core/client/app/styles/components/notifications.css b/core/client/app/styles/components/notifications.css index c06884a672..8895860afa 100644 --- a/core/client/app/styles/components/notifications.css +++ b/core/client/app/styles/components/notifications.css @@ -63,6 +63,16 @@ color: var(--red); } +.gh-notification-passive { + animation: fade-out; + animation-delay: 5s; + animation-iteration-count: 1; +} + +.gh-notification-passive:hover { + animation: fade-in; +} + /* Red notification /* ---------------------------------------------------------- */ diff --git a/core/client/bower.json b/core/client/bower.json index 3b8eda1210..415b1a5679 100644 --- a/core/client/bower.json +++ b/core/client/bower.json @@ -26,6 +26,7 @@ "password-generator": "git://github.com/bermi/password-generator#49accd7", "rangyinputs": "1.2.0", "showdown-ghost": "0.3.6", + "sinonjs": "1.14.1", "validator-js": "3.39.0", "xregexp": "2.0.0" } diff --git a/core/client/package.json b/core/client/package.json index 7932ed4e64..02ec8d7121 100644 --- a/core/client/package.json +++ b/core/client/package.json @@ -37,6 +37,7 @@ "ember-data": "1.0.0-beta.18", "ember-export-application-global": "^1.0.2", "ember-myth": "0.1.0", + "ember-sinon": "0.2.1", "fs-extra": "0.16.3", "glob": "^4.0.5" }, diff --git a/core/client/tests/unit/components/gh-alert-test.js b/core/client/tests/unit/components/gh-alert-test.js new file mode 100644 index 0000000000..58de121abd --- /dev/null +++ b/core/client/tests/unit/components/gh-alert-test.js @@ -0,0 +1,71 @@ +/* jshint expr:true */ +import { expect } from 'chai'; +import { + describeComponent, + it +} +from 'ember-mocha'; +import sinon from 'sinon'; + +describeComponent( + 'gh-alert', + 'GhAlertComponent', { + // specify the other units that are required for this test + // needs: ['component:foo', 'helper:bar'] + }, + function () { + it('renders', function () { + // creates the component instance + var component = this.subject(); + expect(component._state).to.equal('preRender'); + + component.set('message', {message: 'Test message', type: 'success'}); + + // renders the component on the page + this.render(); + expect(component._state).to.equal('inDOM'); + + expect(this.$().prop('tagName')).to.equal('ARTICLE'); + expect(this.$().hasClass('gh-alert')).to.be.true; + expect(this.$().text()).to.match(/Test message/); + }); + + it('maps success alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'success'}); + expect(this.$().hasClass('gh-alert-green')).to.be.true; + }); + + it('maps error alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'error'}); + expect(this.$().hasClass('gh-alert-red')).to.be.true; + }); + + it('maps warn alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'warn'}); + expect(this.$().hasClass('gh-alert-yellow')).to.be.true; + }); + + it('maps info alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'info'}); + expect(this.$().hasClass('gh-alert-blue')).to.be.true; + }); + + it('closes notification through notifications service', function () { + var component = this.subject(), + notifications = {}, + notification = {message: 'Test close', type: 'success'}; + + notifications.closeNotification = sinon.spy(); + component.set('notifications', notifications); + component.set('message', notification); + + this.$().find('button').click(); + + expect(notifications.closeNotification.calledWith(notification)).to.be.true; + }); + } +); diff --git a/core/client/tests/unit/components/gh-alerts-test.js b/core/client/tests/unit/components/gh-alerts-test.js new file mode 100644 index 0000000000..c1bdd142b4 --- /dev/null +++ b/core/client/tests/unit/components/gh-alerts-test.js @@ -0,0 +1,63 @@ +/* jshint expr:true */ +import Ember from 'ember'; +import { expect } from 'chai'; +import { + describeComponent, + it +} +from 'ember-mocha'; +import sinon from 'sinon'; + +describeComponent( + 'gh-alerts', + 'GhAlertsComponent', { + // specify the other units that are required for this test + needs: ['component:gh-alert'] + }, + function () { + beforeEach(function () { + // Stub the notifications service + var notifications = Ember.Object.create(); + notifications.alerts = Ember.A(); + notifications.alerts.pushObject({message: 'First', type: 'error'}); + notifications.alerts.pushObject({message: 'Second', type: 'warn'}); + + this.subject().set('notifications', notifications); + }); + + it('renders', function () { + // creates the component instance + var component = this.subject(); + expect(component._state).to.equal('preRender'); + + // renders the component on the page + this.render(); + expect(component._state).to.equal('inDOM'); + + expect(this.$().prop('tagName')).to.equal('ASIDE'); + expect(this.$().hasClass('gh-alerts')).to.be.true; + expect(this.$().children().length).to.equal(2); + + Ember.run(function () { + component.set('notifications.alerts', Ember.A()); + }); + + expect(this.$().children().length).to.equal(0); + }); + + it('triggers "notify" action when message count changes', function () { + var component = this.subject(); + + component.sendAction = sinon.spy(); + + component.get('notifications.alerts') + .pushObject({message: 'New alert', type: 'info'}); + + expect(component.sendAction.calledWith('notify', 3)).to.be.true; + + component.set('notifications.alerts', Ember.A()); + + expect(component.sendAction.calledWith('notify', 0)).to.be.true; + }); + } +); diff --git a/core/client/tests/unit/components/gh-notification-test.js b/core/client/tests/unit/components/gh-notification-test.js new file mode 100644 index 0000000000..e8b35eec82 --- /dev/null +++ b/core/client/tests/unit/components/gh-notification-test.js @@ -0,0 +1,82 @@ +/* jshint expr:true */ +import { expect } from 'chai'; +import { + describeComponent, + it +} +from 'ember-mocha'; +import sinon from 'sinon'; + +describeComponent( + 'gh-notification', + 'GhNotificationComponent', { + // specify the other units that are required for this test + // needs: ['component:foo', 'helper:bar'] + }, + function () { + it('renders', function () { + // creates the component instance + var component = this.subject(); + expect(component._state).to.equal('preRender'); + + component.set('message', {message: 'Test message', type: 'success'}); + + // renders the component on the page + this.render(); + expect(component._state).to.equal('inDOM'); + + expect(this.$().prop('tagName')).to.equal('ARTICLE'); + expect(this.$().is('.gh-notification, .gh-notification-passive')).to.be.true; + expect(this.$().text()).to.match(/Test message/); + }); + + it('maps success alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'success'}); + expect(this.$().hasClass('gh-notification-green')).to.be.true; + }); + + it('maps error alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'error'}); + expect(this.$().hasClass('gh-notification-red')).to.be.true; + }); + + it('maps warn alert type to correct class', function () { + var component = this.subject(); + component.set('message', {message: 'Test message', type: 'warn'}); + expect(this.$().hasClass('gh-notification-yellow')).to.be.true; + }); + + it('closes notification through notifications service', function () { + var component = this.subject(), + notifications = {}, + notification = {message: 'Test close', type: 'success'}; + + notifications.closeNotification = sinon.spy(); + component.set('notifications', notifications); + component.set('message', notification); + + this.$().find('button').click(); + + expect(notifications.closeNotification.calledWith(notification)).to.be.true; + }); + + it('closes notification when animationend event is triggered', function (done) { + var component = this.subject(), + notifications = {}, + notification = {message: 'Test close', type: 'success'}; + + notifications.closeNotification = sinon.spy(); + component.set('notifications', notifications); + component.set('message', notification); + + // shorten the animation delay to speed up test + this.$().css('animation-delay', '0.1s'); + setTimeout(function () { + expect(notifications.closeNotification.calledWith(notification)).to.be.true; + done(); + }, 150); + }); + } +); diff --git a/core/client/tests/unit/components/gh-notifications-test.js b/core/client/tests/unit/components/gh-notifications-test.js new file mode 100644 index 0000000000..e6cd6550d8 --- /dev/null +++ b/core/client/tests/unit/components/gh-notifications-test.js @@ -0,0 +1,47 @@ +/* jshint expr:true */ +import Ember from 'ember'; +import { expect } from 'chai'; +import { + describeComponent, + it +} +from 'ember-mocha'; + +describeComponent( + 'gh-notifications', + 'GhNotificationsComponent', { + // specify the other units that are required for this test + needs: ['component:gh-notification'] + }, + function () { + beforeEach(function () { + // Stub the notifications service + var notifications = Ember.Object.create(); + notifications.notifications = Ember.A(); + notifications.notifications.pushObject({message: 'First', type: 'error'}); + notifications.notifications.pushObject({message: 'Second', type: 'warn'}); + + this.subject().set('notifications', notifications); + }); + + it('renders', function () { + // creates the component instance + var component = this.subject(); + expect(component._state).to.equal('preRender'); + + // renders the component on the page + this.render(); + expect(component._state).to.equal('inDOM'); + + expect(this.$().prop('tagName')).to.equal('ASIDE'); + expect(this.$().hasClass('gh-notifications')).to.be.true; + expect(this.$().children().length).to.equal(2); + + Ember.run(function () { + component.set('notifications.notifications', Ember.A()); + }); + + expect(this.$().children().length).to.equal(0); + }); + } +); diff --git a/core/client/tests/unit/services/notifications-test.js b/core/client/tests/unit/services/notifications-test.js new file mode 100644 index 0000000000..852a5f6a75 --- /dev/null +++ b/core/client/tests/unit/services/notifications-test.js @@ -0,0 +1,286 @@ +/* jshint expr:true */ +import Ember from 'ember'; +import sinon from 'sinon'; +import { expect } from 'chai'; +import { + describeModule, + it +} from 'ember-mocha'; + +describeModule( + 'service:notifications', + 'NotificationsService', + { + // Specify the other units that are required for this test. + // needs: ['model:notification'] + }, + function () { + beforeEach(function () { + this.subject().set('content', Ember.A()); + this.subject().set('delayedNotifications', Ember.A()); + }); + + it('filters alerts/notifications', function () { + var notifications = this.subject(); + + notifications.set('content', [ + {message: 'Alert', status: 'alert'}, + {message: 'Notification', status: 'notification'} + ]); + + expect(notifications.get('alerts')) + .to.deep.equal([{message: 'Alert', status: 'alert'}]); + + expect(notifications.get('notifications')) + .to.deep.equal([{message: 'Notification', status: 'notification'}]); + }); + + it('#handleNotification deals with DS.Notification notifications', function () { + var notifications = this.subject(), + notification = Ember.Object.create({message: '

Test

', status: 'alert'}); + + notification.toJSON = function () {}; + + notifications.handleNotification(notification); + + notification = notifications.get('alerts')[0]; + + // alerts received from the server should be marked html safe + expect(notification.get('message')).to.have.property('toHTML'); + }); + + it('#handleNotification defaults to notification if no status supplied', function () { + var notifications = this.subject(); + + notifications.handleNotification({message: 'Test'}, false); + + expect(notifications.get('content')) + .to.deep.include({message: 'Test', status: 'notification'}); + }); + + it('#showAlert adds POJO alerts', function () { + var notifications = this.subject(); + + notifications.showAlert('Test Alert', {type: 'error'}); + + expect(notifications.get('alerts')) + .to.deep.include({message: 'Test Alert', status: 'alert', type: 'error'}); + }); + + it('#showAlert adds delayed notifications', function () { + var notifications = this.subject(); + + notifications.showNotification('Test Alert', {type: 'error', delayed: true}); + + expect(notifications.get('delayedNotifications')) + .to.deep.include({message: 'Test Alert', status: 'notification', type: 'error'}); + }); + + it('#showNotification adds POJO notifications', function () { + var notifications = this.subject(); + + notifications.showNotification('Test Notification', {type: 'success'}); + + expect(notifications.get('notifications')) + .to.deep.include({message: 'Test Notification', status: 'notification', type: 'success'}); + }); + + it('#showNotification adds delayed notifications', function () { + var notifications = this.subject(); + + notifications.showNotification('Test Notification', {delayed: true}); + + expect(notifications.get('delayedNotifications')) + .to.deep.include({message: 'Test Notification', status: 'notification', type: undefined}); + }); + + it('#showNotification clears existing notifications', function () { + var notifications = this.subject(); + + notifications.showNotification('First'); + notifications.showNotification('Second'); + + expect(notifications.get('content.length')).to.equal(1); + expect(notifications.get('content')) + .to.deep.equal([{message: 'Second', status: 'notification', type: undefined}]); + }); + + it('#showNotification keeps existing notifications if doNotCloseNotifications option passed', function () { + var notifications = this.subject(); + + notifications.showNotification('First'); + notifications.showNotification('Second', {doNotCloseNotifications: true}); + + expect(notifications.get('content.length')).to.equal(2); + }); + + // TODO: review whether this can be removed once it's no longer used by validations + it('#showErrors adds multiple notifications', function () { + var notifications = this.subject(); + + notifications.showErrors([ + {message: 'First'}, + {message: 'Second'} + ]); + + expect(notifications.get('content')).to.deep.equal([ + {message: 'First', status: 'notification', type: 'error'}, + {message: 'Second', status: 'notification', type: 'error'} + ]); + }); + + it('#showAPIError adds single json response error', function () { + var notifications = this.subject(), + resp = {jqXHR: {responseJSON: {error: 'Single error'}}}; + + notifications.showAPIError(resp); + + expect(notifications.get('content')).to.deep.equal([ + {message: 'Single error', status: 'alert', type: 'error'} + ]); + }); + + // used to display validation errors returned from the server + it('#showAPIError adds multiple json response errors', function () { + var notifications = this.subject(), + resp = {jqXHR: {responseJSON: {errors: ['First error', 'Second error']}}}; + + notifications.showAPIError(resp); + + expect(notifications.get('content')).to.deep.equal([ + {message: 'First error', status: 'notification', type: 'error'}, + {message: 'Second error', status: 'notification', type: 'error'} + ]); + }); + + it('#showAPIError adds single json response message', function () { + var notifications = this.subject(), + resp = {jqXHR: {responseJSON: {message: 'Single message'}}}; + + notifications.showAPIError(resp); + + expect(notifications.get('content')).to.deep.equal([ + {message: 'Single message', status: 'alert', type: 'error'} + ]); + }); + + it('#showAPIError displays default error text if response has no error/message', function () { + var notifications = this.subject(), + resp = {}; + + notifications.showAPIError(resp); + expect(notifications.get('content')).to.deep.equal([ + {message: 'There was a problem on the server, please try again.', status: 'alert', type: 'error'} + ]); + + notifications.set('content', Ember.A()); + + notifications.showAPIError(resp, {defaultErrorText: 'Overridden default'}); + expect(notifications.get('content')).to.deep.equal([ + {message: 'Overridden default', status: 'alert', type: 'error'} + ]); + }); + + it('#displayDelayed moves delayed notifications into content', function () { + var notifications = this.subject(); + + notifications.showNotification('First', {delayed: true}); + notifications.showNotification('Second', {delayed: true}); + notifications.showNotification('Third', {delayed: false}); + + notifications.displayDelayed(); + + expect(notifications.get('content')).to.deep.equal([ + {message: 'Third', status: 'notification', type: undefined}, + {message: 'First', status: 'notification', type: undefined}, + {message: 'Second', status: 'notification', type: undefined} + ]); + }); + + it('#closeNotification removes POJO notifications', function () { + var notification = {message: 'Close test', status: 'notification'}, + notifications = this.subject(); + + notifications.handleNotification(notification); + + expect(notifications.get('notifications')) + .to.include(notification); + + notifications.closeNotification(notification); + + expect(notifications.get('notifications')) + .to.not.include(notification); + }); + + it('#closeNotification removes and deletes DS.Notification records', function () { + var notification = Ember.Object.create({message: 'Close test', status: 'alert'}), + notifications = this.subject(); + + notification.toJSON = function () {}; + notification.deleteRecord = function () {}; + sinon.spy(notification, 'deleteRecord'); + notification.save = function () { + return { + finally: function (callback) { return callback(notification); } + }; + }; + sinon.spy(notification, 'save'); + + notifications.handleNotification(notification); + expect(notifications.get('alerts')).to.include(notification); + + notifications.closeNotification(notification); + + expect(notification.deleteRecord.calledOnce).to.be.true; + expect(notification.save.calledOnce).to.be.true; + + // wrap in runloop so filter updates + Ember.run.next(function () { + expect(notifications.get('alerts')).to.not.include(notification); + }); + }); + + it('#closeNotifications only removes notifications', function () { + var notifications = this.subject(); + + notifications.showAlert('First alert'); + notifications.showNotification('First notification'); + notifications.showNotification('Second notification', {doNotCloseNotifications: true}); + + expect(notifications.get('alerts.length')).to.equal(1); + expect(notifications.get('notifications.length')).to.equal(2); + + notifications.closeNotifications(); + + // wrap in runloop so filter updates + Ember.run.next(function () { + expect(notifications.get('alerts.length')).to.equal(1); + expect(notifications.get('notifications.length')).to.equal(1); + }); + }); + + it('#closeAll removes everything without deletion', function () { + var notifications = this.subject(), + notificationModel = Ember.Object.create({message: 'model'}); + + notificationModel.toJSON = function () {}; + notificationModel.deleteRecord = function () {}; + sinon.spy(notificationModel, 'deleteRecord'); + notificationModel.save = function () { + return { + finally: function (callback) { return callback(notificationModel); } + }; + }; + sinon.spy(notificationModel, 'save'); + + notifications.handleNotification(notificationModel); + notifications.handleNotification({message: 'pojo'}); + + notifications.closeAll(); + + expect(notifications.get('content')).to.be.empty; + expect(notificationModel.deleteRecord.called).to.be.false; + expect(notificationModel.save.called).to.be.false; + }); + } +); diff --git a/core/server/api/notifications.js b/core/server/api/notifications.js index 97ce9971d3..8bcf859e32 100644 --- a/core/server/api/notifications.js +++ b/core/server/api/notifications.js @@ -50,7 +50,7 @@ notifications = { var defaults = { dismissible: true, location: 'bottom', - status: 'persistent' + status: 'alert' }, addedNotifications = []; @@ -61,7 +61,7 @@ notifications = { notification = _.assign(defaults, notification, { id: notificationCounter - // status: 'persistent' + // status: 'alert' }); notificationsStore.push(notification); diff --git a/core/server/controllers/admin.js b/core/server/controllers/admin.js index 832d365a10..825dd8ae3c 100644 --- a/core/server/controllers/admin.js +++ b/core/server/controllers/admin.js @@ -36,7 +36,7 @@ adminControllers = { type: 'upgrade', location: 'settings-about-upgrade', dismissible: false, - status: 'persistent', + status: 'alert', message: 'Ghost ' + updateVersion + ' is available! Hot Damn. Click here to upgrade.' }; diff --git a/core/test/functional/base.js b/core/test/functional/base.js index 5129912e69..8dd1e53f86 100644 --- a/core/test/functional/base.js +++ b/core/test/functional/base.js @@ -411,9 +411,9 @@ CasperTest.Routines = (function () { casper.captureScreenshot('setting_up2.png'); - casper.waitForSelectorTextChange('.notification-error', function onSuccess() { + casper.waitForSelectorTextChange('.gh-alert-success', function onSuccess() { var errorText = casper.evaluate(function () { - return document.querySelector('.notification-error').innerText; + return document.querySelector('.gh-alert').innerText; }); casper.echoConcise('Setup failed. Error text: ' + errorText); }, function onTimeout() { diff --git a/core/test/functional/client/editor_test.js b/core/test/functional/client/editor_test.js index 6ad5f6e5cc..e647b27bf6 100644 --- a/core/test/functional/client/editor_test.js +++ b/core/test/functional/client/editor_test.js @@ -26,7 +26,7 @@ CasperTest.begin('Ghost editor functions correctly', 16, function suite(test) { casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success', function onSuccess() { + casper.waitForSelector('.gh-notification', function onSuccess() { test.assert(true, 'Can save with no title.'); test.assertEvalEquals(function () { return document.getElementById('entry-title').value; @@ -58,7 +58,7 @@ CasperTest.begin('Ghost editor functions correctly', 16, function suite(test) { casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success', function onSuccess() { + casper.waitForSelector('.gh-notification', function onSuccess() { test.assertUrlMatch(/ghost\/editor\/\d+\/$/, 'got an id on our URL'); test.assertEvalEquals(function () { return document.querySelector('#entry-title').value; @@ -223,7 +223,7 @@ CasperTest.begin('Image Uploads', 23, function suite(test) { // Save the post with the image casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success', function onSuccess() { + casper.waitForSelector('.gh-notification', function onSuccess() { test.assertUrlMatch(/ghost\/editor\/\d+\/$/, 'got an id on our URL'); }, casper.failOnTimeout(test, 'Post was not successfully created')); @@ -385,7 +385,7 @@ CasperTest.begin('Publish menu - existing post', 23, function suite(test) { // Create a post in draft status casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success', function checkPostWasCreated() { + casper.waitForSelector('.gh-notification', function checkPostWasCreated() { test.assertUrlMatch(/ghost\/editor\/\d+\/$/, 'got an id on our URL'); }); @@ -429,7 +429,7 @@ CasperTest.begin('Publish menu - existing post', 23, function suite(test) { // Do publish casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success', function checkPostWasCreated() { + casper.waitForSelector('.gh-notification', function checkPostWasCreated() { test.assertUrlMatch(/ghost\/editor\/\d+\/$/, 'got an id on our URL'); }); @@ -461,7 +461,7 @@ CasperTest.begin('Publish menu - existing post', 23, function suite(test) { // Do unpublish casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success', function checkPostWasCreated() { + casper.waitForSelector('.gh-notification', function checkPostWasCreated() { // ... check status, label, class casper.waitForSelector('.js-publish-splitbutton', function onSuccess() { test.assertExists('.js-publish-button.btn-blue', 'Publish button should have .btn-blue'); @@ -472,7 +472,7 @@ CasperTest.begin('Publish menu - existing post', 23, function suite(test) { }); }); -CasperTest.begin('Publish menu - delete post', 7, function testDeleteModal(test) { +CasperTest.begin('Publish menu - delete post', 6, function testDeleteModal(test) { // Create a post that can be deleted CasperTest.Routines.createTestPost.run(false); @@ -515,15 +515,8 @@ CasperTest.begin('Publish menu - delete post', 7, function testDeleteModal(test) // Delete the post this.click('.modal-content .js-button-accept'); - casper.waitForSelector('.notification-success', function onSuccess() { - test.assert(true, 'Got success notification from delete post'); - test.assertSelectorHasText( - '.gh-notification-content', - 'Your post has been deleted.', - '.gh-notification-content has correct text' - ); - }, function onTimeout() { - test.fail('No success notification from delete post'); + casper.waitWhileVisible('.modal-container', function onSuccess() { + test.assert(true, 'clicking delete button should close the delete post modal'); }); }); }); @@ -579,7 +572,7 @@ CasperTest.begin('Publish menu - existing post status is correct after failed sa // save casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success'); + casper.waitForSelector('.gh-notification'); casper.then(function updateTitle() { casper.sendKeys('#entry-title', new Array(160).join('y')); @@ -607,15 +600,14 @@ CasperTest.begin('Publish menu - existing post status is correct after failed sa casper.thenClick('.js-publish-button'); // ... check status, label, class - // TODO: re-implement these once #5933 is merged - // casper.waitForSelector('.notification-error', function onSuccess() { - // test.assertExists('.js-publish-button.btn-blue', 'Update button should have .btn-blue'); - // // wait for button to settle - // casper.wait(500); - // test.assertSelectorHasText('.js-publish-button', 'Save Draft', '.js-publish-button says Save Draft'); - // }, function onTimeout() { - // test.assert(false, 'Saving post with invalid title should trigger an error'); - // }); + casper.waitForSelector('.gh-alert-red', function onSuccess() { + test.assertExists('.js-publish-button.btn-blue', 'Update button should have .btn-blue'); + // wait for button to settle + casper.wait(500); + test.assertSelectorHasText('.js-publish-button', 'Save Draft', '.js-publish-button says Save Draft'); + }, function onTimeout() { + test.assert(false, 'Saving post with invalid title should trigger an error'); + }); }); // test the markdown help modal @@ -660,7 +652,7 @@ CasperTest.begin('Title input is set correctly after using the Post-Settings-Men // save draft casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success'); + casper.waitForSelector('.gh-notification'); // change the title casper.then(function updateTitle() { @@ -707,7 +699,7 @@ CasperTest.begin('Editor content is set correctly after using the Post-Settings- // save draft casper.thenClick('.js-publish-button'); - casper.waitForSelector('.notification-success'); + casper.waitForSelector('.gh-notification'); // change the content casper.then(function updateContent() { diff --git a/core/test/functional/client/psm_test.js b/core/test/functional/client/psm_test.js index c77db3cb5a..b91f5db62a 100644 --- a/core/test/functional/client/psm_test.js +++ b/core/test/functional/client/psm_test.js @@ -3,7 +3,7 @@ /*globals CasperTest, casper, __utils__ */ -CasperTest.begin('Post settings menu', 10, function suite(test) { +CasperTest.begin('Post settings menu', 8, function suite(test) { casper.thenOpenAndWaitForPageLoad('editor', function testTitleAndUrl() { test.assertTitle('Editor - Test Blog', 'Ghost admin has incorrect title'); test.assertUrlMatch(/ghost\/editor\/$/, 'Landed on the correct URL'); @@ -24,16 +24,6 @@ CasperTest.begin('Post settings menu', 10, function suite(test) { casper.thenClick('.js-publish-button'); }); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - test.assertSelectorHasText('.notification-success', 'Saved.', '.notification-success has correct text'); - casper.click('.gh-notification-close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); - }); - - casper.waitWhileSelector('.notification-success'); - casper.thenClick('.post-settings'); casper.waitForOpaque('.settings-menu', function onSuccess() { diff --git a/core/test/functional/client/settings_test.js b/core/test/functional/client/settings_test.js index dbc88256e2..5f237dd3b6 100644 --- a/core/test/functional/client/settings_test.js +++ b/core/test/functional/client/settings_test.js @@ -21,7 +21,7 @@ CasperTest.begin('Settings screen is correct', 5, function suite(test) { }); // ## General settings tests -CasperTest.begin('General settings pane is correct', 7, function suite(test) { +CasperTest.begin('General settings pane is correct', 4, function suite(test) { casper.thenOpenAndWaitForPageLoad('settings.general', function testTitleAndUrl() { test.assertUrlMatch(/ghost\/settings\/general\/$/, 'Landed on the correct URL'); }); @@ -29,9 +29,6 @@ CasperTest.begin('General settings pane is correct', 7, function suite(test) { function assertImageUploaderModalThenClose() { test.assertSelectorHasText('.description', 'Add image', '.description has the correct text'); casper.click('.modal-container .js-button-accept'); - casper.waitForSelector('.notification-success', function onSuccess() { - test.assert(true, 'Got success notification'); - }, casper.failOnTimeout(test, 'No success notification')); } // Ensure image upload modals display correctly @@ -65,22 +62,17 @@ CasperTest.begin('General settings pane is correct', 7, function suite(test) { // Ensure can save casper.waitForSelector('header .btn-blue').then(function () { - casper.thenClick('header .btn-blue').waitFor(function successNotification() { - return this.evaluate(function () { - return document.querySelectorAll('.gh-notification').length > 0; - }); + casper.thenClick('header .btn-blue'); + casper.waitForResource('settings/', function onSuccess() { + test.assert(true, 'Settings were saved'); }, function doneWaiting() { - test.pass('Waited for notification'); - }, casper.failOnTimeout(test, 'Saving the general pane did not result in a notification')); + test.fail('Settings were not saved'); + }); }); - casper.then(function checkSettingsWereSaved() { + casper.then(function stopListeningForRequests() { casper.removeListener('resource.requested', handleSettingsRequest); }); - - casper.waitForSelector('.notification-success', function onSuccess() { - test.assert(true, 'Got success notification'); - }, casper.failOnTimeout(test, 'No success notification :(')); }); // ## General settings validations tests @@ -96,24 +88,24 @@ CasperTest.begin('General settings validation is correct', 4, function suite(tes 'general[title]': new Array(152).join('a') }); - // TODO: re-implement once #5933 is merged - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'too long', '.notification-error has correct text'); - // }, casper.failOnTimeout(test, 'Blog title length error did not appear'), 2000); + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { + test.assertSelectorHasText('.gh-notification-red', 'too long', '.gh-notification-red has correct text'); + }, casper.failOnTimeout(test, 'Blog title length error did not appear'), 2000); - casper.thenClick('.gh-notification-close'); + casper.thenClick('.gh-alert-close'); // Ensure general blog description field length validation casper.fillAndSave('form#settings-general', { 'general[description]': new Array(202).join('a') }); - // TODO: re-implement once #5933 is merged - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'too long', '.notification-error has correct text'); - // }, casper.failOnTimeout(test, 'Blog description length error did not appear')); + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { + test.assertSelectorHasText('.gh-notification-red', 'too long', '.gh-notification-red has correct text'); + }, casper.failOnTimeout(test, 'Blog description length error did not appear')); - casper.thenClick('.gh-notification-close'); + casper.thenClick('.gh-alert-close'); // TODO move these to ember tests, note: async issues - field will be often be null without a casper.wait // Check postsPerPage autocorrect diff --git a/core/test/functional/client/signin_test.js b/core/test/functional/client/signin_test.js index a5154cd5f9..67e2f0af59 100644 --- a/core/test/functional/client/signin_test.js +++ b/core/test/functional/client/signin_test.js @@ -52,7 +52,7 @@ CasperTest.begin('Login limit is in place', 4, function suite(test) { casper.waitForText('remaining', function onSuccess() { test.assert(true, 'The login limit is in place.'); - test.assertSelectorDoesntHaveText('.notification-error', '[object Object]'); + test.assertSelectorDoesntHaveText('.gh-alert', '[object Object]'); }, function onTimeout() { test.assert(false, 'We did not trip the login limit.'); }); @@ -129,21 +129,23 @@ CasperTest.begin('Ensure email field form validation', 2, function suite(test) { test.fail('Login form didn\'t fade in.'); }); - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'Invalid Email', '.notification-error text is correct'); - // }, function onTimeout() { - // test.fail('Email validation error did not appear'); - // }, 2000); - // - // casper.then(function testMissingEmail() { - // this.fillAndSave('form.gh-signin', { - // identification: '' - // }); - // }); - // - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'Please enter an email', '.notification-error text is correct'); - // }, function onTimeout() { - // test.fail('Missing Email validation error did not appear'); - // }, 2000); + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { + test.assertSelectorHasText('.gh-notification-red', 'Invalid Email', '.gh-notification-red text is correct'); + }, function onTimeout() { + test.fail('Email validation error did not appear'); + }, 2000); + + casper.then(function testMissingEmail() { + this.fillAndSave('form.gh-signin', { + identification: '' + }); + }); + + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { + test.assertSelectorHasText('.gh-notification-red', 'Please enter an email', '.gh-notification-red text is correct'); + }, function onTimeout() { + test.fail('Missing Email validation error did not appear'); + }, 2000); }, true); diff --git a/core/test/functional/client/team_test.js b/core/test/functional/client/team_test.js index f347198bc6..2d17942a06 100644 --- a/core/test/functional/client/team_test.js +++ b/core/test/functional/client/team_test.js @@ -71,7 +71,7 @@ CasperTest.begin('Users screen is correct', 9, function suite(test) { }); // ### User settings tests -CasperTest.begin('Can save settings', 7, function suite(test) { +CasperTest.begin('Can save settings', 5, function suite(test) { casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() { test.assertTitle('Team - User - Test Blog', 'Ghost Admin title is correct'); test.assertUrlMatch(/ghost\/team\/test\/$/, 'team.user has correct URL'); @@ -96,22 +96,16 @@ CasperTest.begin('Can save settings', 7, function suite(test) { }); casper.thenClick('.btn-blue'); - casper.waitFor(function successNotification() { - return this.evaluate(function () { - return document.querySelectorAll('.gh-notification').length > 0; - }); + casper.waitForResource(/\/users\/\d\/\?include=roles/, function onSuccess() { + test.assert(true, 'Saving the user pane triggered a save request'); }, function doneWaiting() { - test.pass('Waited for notification'); - }, casper.failOnTimeout(test, 'Saving the user pane did not result in a notification')); + test.fail('Saving the user pane did not trigger a save request'); + }); casper.then(function checkUserWasSaved() { casper.removeListener('resource.requested', handleUserRequest); }); - casper.waitForSelector('.notification-success', function onSuccess() { - test.assert(true, 'Got success notification'); - }, casper.failOnTimeout(test, 'No success notification :(')); - casper.thenClick('.gh-nav-settings-general').then(function testTransitionToGeneral() { casper.waitForSelector(generalTabDetector, function then() { casper.on('resource.requested', handleSettingsRequest); @@ -122,22 +116,17 @@ CasperTest.begin('Can save settings', 7, function suite(test) { casper.failOnTimeout(test, 'waitForSelector `usersTabDetector` timed out')); }); - casper.thenClick('.btn-blue').waitFor(function successNotification() { - return this.evaluate(function () { - return document.querySelectorAll('.gh-notification').length > 0; - }); + casper.thenClick('.btn-blue'); + casper.waitForResource(/\/users\/\d\/\?include=roles/, function onSuccess() { + test.assert(true, 'Saving the user pane triggered a save request'); }, function doneWaiting() { - test.pass('Waited for notification'); - }, casper.failOnTimeout(test, 'Saving the general pane did not result in a notification')); + test.fail('Saving the user pane did not trigger a save request'); + }); casper.then(function checkSettingsWereSaved() { casper.removeListener('resource.requested', handleSettingsRequest); }); - casper.waitForSelector('.notification-success', function onSuccess() { - test.assert(true, 'Got success notification'); - }, casper.failOnTimeout(test, 'No success notification :(')); - CasperTest.beforeDone(function () { casper.removeListener('resource.requested', handleUserRequest); casper.removeListener('resource.requested', handleSettingsRequest); @@ -203,8 +192,7 @@ CasperTest.begin('User settings screen change slug handles duplicate slug', 4, f }); }); -// TODO: Change number of tests back to 6 once the commented-out tests are fixed -CasperTest.begin('User settings screen validates email', 4, function suite(test) { +CasperTest.begin('User settings screen validates email', 6, function suite(test) { var email; casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() { @@ -230,11 +218,11 @@ CasperTest.begin('User settings screen validates email', 4, function suite(test) casper.waitForResource('/team/'); - // TODO: Re-implement after inlin-errors is merged - // casper.waitForSelector('.notification-error', function onSuccess() { - // test.assert(true, 'Got error notification'); - // test.assertSelectorDoesntHaveText('.notification-error', '[object Object]', 'notification text is not broken'); - // }, casper.failOnTimeout(test, 'No error notification :(')); + // TODO: review once inline-validations are implemented + casper.waitForSelector('.gh-notification', function onSuccess() { + test.assert(true, 'Got error notification'); + test.assertSelectorDoesntHaveText('.gh-notification', '[object Object]', 'notification text is not broken'); + }, casper.failOnTimeout(test, 'No error notification :(')); casper.then(function resetEmailToValid() { casper.fillSelectors('.user-profile', { @@ -245,11 +233,6 @@ CasperTest.begin('User settings screen validates email', 4, function suite(test) casper.thenClick('.view-actions .btn-blue'); casper.waitForResource(/users/); - - casper.waitForSelector('.notification-success', function onSuccess() { - test.assert(true, 'Got success notification'); - test.assertSelectorDoesntHaveText('.notification-success', '[object Object]', 'notification text is not broken'); - }, casper.failOnTimeout(test, 'No success notification :(')); }); // TODO: user needs to be loaded whenever it is edited (multi user) @@ -278,8 +261,7 @@ CasperTest.begin('User settings screen shows remaining characters for Bio proper }); }); -// TODO: Change number of tests back to 3 once the commented-out tests are fixed -CasperTest.begin('Ensure user bio field length validation', 2, function suite(test) { +CasperTest.begin('Ensure user bio field length validation', 3, function suite(test) { casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() { test.assertTitle('Team - User - Test Blog', 'Ghost admin has incorrect title'); test.assertUrlMatch(/ghost\/team\/test\/$/, 'Ghost doesn\'t require login this time'); @@ -293,14 +275,13 @@ CasperTest.begin('Ensure user bio field length validation', 2, function suite(te casper.thenClick('.view-actions .btn-blue'); - // TODO: re-implement after inline-errors is complete - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'is too long', '.notification-error text is correct'); - // }, casper.failOnTimeout(test, 'Bio field length error did not appear', 2000)); + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification', function onSuccess() { + test.assertSelectorHasText('.gh-notification', 'is too long', '.gh-notification text is correct'); + }, casper.failOnTimeout(test, 'Bio field length error did not appear', 2000)); }); -// TODO: Change number of tests back to 3 once the commented-out tests are fixed -CasperTest.begin('Ensure user url field validation', 2, function suite(test) { +CasperTest.begin('Ensure user url field validation', 3, function suite(test) { casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() { test.assertTitle('Team - User - Test Blog', 'Ghost admin has incorrect title'); test.assertUrlMatch(/ghost\/team\/test\/$/, 'Ghost doesn\'t require login this time'); @@ -314,14 +295,13 @@ CasperTest.begin('Ensure user url field validation', 2, function suite(test) { casper.thenClick('.view-actions .btn-blue'); - // TODO: re-implement after inline-errors is complete - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'not a valid url', '.notification-error text is correct'); - // }, casper.failOnTimeout(test, 'Url validation error did not appear', 2000)); + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification', function onSuccess() { + test.assertSelectorHasText('.gh-notification', 'not a valid url', '.gh-notification text is correct'); + }, casper.failOnTimeout(test, 'Url validation error did not appear', 2000)); }); -// TODO: Change number of tests back to 3 once the commented-out tests are fixed -CasperTest.begin('Ensure user location field length validation', 2, function suite(test) { +CasperTest.begin('Ensure user location field length validation', 3, function suite(test) { casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() { test.assertTitle('Team - User - Test Blog', 'Ghost admin has incorrect title'); test.assertUrlMatch(/ghost\/team\/test\/$/, 'Ghost doesn\'t require login this time'); @@ -335,8 +315,8 @@ CasperTest.begin('Ensure user location field length validation', 2, function sui casper.thenClick('.view-actions .btn-blue'); - // TODO: re-implement after inline-errors is complete - // casper.waitForSelectorTextChange('.notification-error', function onSuccess() { - // test.assertSelectorHasText('.notification-error', 'is too long', '.notification-error text is correct'); - // }, casper.failOnTimeout(test, 'Location field length error did not appear', 2000)); + // TODO: review once inline-validations are implemented + casper.waitForSelectorTextChange('.gh-notification', function onSuccess() { + test.assertSelectorHasText('.gh-notification', 'is too long', '.gh-notification text is correct'); + }, casper.failOnTimeout(test, 'Location field length error did not appear', 2000)); }); diff --git a/core/test/functional/routes/api/notifications_spec.js b/core/test/functional/routes/api/notifications_spec.js index cd93bf7686..e7464ca393 100644 --- a/core/test/functional/routes/api/notifications_spec.js +++ b/core/test/functional/routes/api/notifications_spec.js @@ -55,7 +55,7 @@ describe('Notifications API', function () { jsonResponse.notifications[0].type.should.equal(newNotification.type); jsonResponse.notifications[0].message.should.equal(newNotification.message); - jsonResponse.notifications[0].status.should.equal('persistent'); + jsonResponse.notifications[0].status.should.equal('alert'); done(); }); @@ -66,7 +66,7 @@ describe('Notifications API', function () { var newNotification = { type: 'info', message: 'test notification', - status: 'persistent' + status: 'alert' }; it('deletes a notification', function (done) { diff --git a/core/test/functional/setup/setup_test.js b/core/test/functional/setup/setup_test.js index 20366b1314..e2a0f91345 100644 --- a/core/test/functional/setup/setup_test.js +++ b/core/test/functional/setup/setup_test.js @@ -2,8 +2,7 @@ /*global CasperTest, casper, email, user, password */ -// TODO: change test number to 12 after inline-errors are fixed -CasperTest.begin('Ghost setup fails properly', 10, function suite(test) { +CasperTest.begin('Ghost setup fails properly', 12, function suite(test) { casper.thenOpenAndWaitForPageLoad('setup', function then() { test.assertUrlMatch(/ghost\/setup\/one\/$/, 'Landed on the correct URL'); }); @@ -14,12 +13,12 @@ CasperTest.begin('Ghost setup fails properly', 10, function suite(test) { // TODO: Fix tests to support inline validation // should now throw a short password error - // casper.waitForSelector('.notification-error', function onSuccess() { - // test.assert(true, 'Got error notification'); - // test.assertSelectorHasText('.notification-error', 'Password must be at least 8 characters long'); - // }, function onTimeout() { - // test.assert(false, 'No error notification :('); - // }); + casper.waitForSelector('.gh-notification', function onSuccess() { + test.assert(true, 'Got error notification'); + test.assertSelectorHasText('.gh-notification', 'Password must be at least 8 characters long'); + }, function onTimeout() { + test.assert(false, 'No error notification :('); + }); casper.then(function setupWithLongPassword() { casper.fillAndAdd('#setup', {'blog-title': 'ghost', name: 'slimer', email: email, password: password}); @@ -32,9 +31,9 @@ CasperTest.begin('Ghost setup fails properly', 10, function suite(test) { casper.thenClick('.gh-flow-content .btn'); }); - casper.waitForSelector('.notification-error', function onSuccess() { + casper.waitForSelector('.gh-alert', function onSuccess() { test.assert(true, 'Got error notification'); - test.assertSelectorHasText('.notification-error', 'No users to invite.'); + test.assertSelectorHasText('.gh-alert', 'No users to invite.'); test.assertExists('.gh-flow-content .btn-minor', 'Submit button is not minor'); test.assertSelectorHasText('.gh-flow-content .btn', 'Invite some users', 'Submit button has wrong text'); @@ -58,9 +57,9 @@ CasperTest.begin('Ghost setup fails properly', 10, function suite(test) { casper.wait(5000); // These invitations will fail, because Casper can't send emails - casper.waitForSelector('.notification-error', function onSuccess() { + casper.waitForSelector('.gh-alert', function onSuccess() { test.assert(true, 'Got error notification'); - test.assertSelectorHasText('.notification-error', 'Failed to send 2 invitations: test@example.com, test2@example.com'); + test.assertSelectorHasText('.gh-alert', 'Failed to send 2 invitations: test@example.com, test2@example.com'); }, function onTimeout() { test.assert(false, 'No error notification after invite.'); }); diff --git a/core/test/integration/api/api_notifications_spec.js b/core/test/integration/api/api_notifications_spec.js index 0143746a63..fd5ff6f996 100644 --- a/core/test/integration/api/api_notifications_spec.js +++ b/core/test/integration/api/api_notifications_spec.js @@ -74,7 +74,7 @@ describe('Notifications API', function () { notification.id.should.be.a.Number; notification.id.should.not.equal(99); should.exist(notification.status); - notification.status.should.equal('persistent'); + notification.status.should.equal('alert'); done(); }).catch(done); From 1bcd7fd333f7ff1e615d80297ebf84cd948a83c2 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 7 Jul 2015 18:14:23 +0100 Subject: [PATCH 2/3] Replace validation notifications with inline validations issue #5409 & #5336 - update settings/general - update signin - update signup - update edit user - update reset password - update setup/three - remove `formatErrors` function from validationEngine mixin (it's no longer needed as inline validations should handle this instead) --- .../app/components/gh-trim-focus-input.js | 9 ++- .../app/controllers/settings/general.js | 10 +++- core/client/app/controllers/setup/three.js | 15 +++-- core/client/app/controllers/signin.js | 50 ++++++++--------- core/client/app/controllers/signup.js | 11 ++-- core/client/app/controllers/team/user.js | 6 +- core/client/app/mixins/validation-engine.js | 56 ++----------------- core/client/app/styles/layouts/flow.css | 1 + core/client/app/templates/reset.hbs | 21 +++++-- .../client/app/templates/settings/general.hbs | 24 ++++---- core/client/app/templates/setup/three.hbs | 1 + core/client/app/templates/signin.hbs | 14 +++-- core/client/app/templates/signup.hbs | 23 ++++---- core/client/app/templates/team/user.hbs | 47 ++++++++++------ core/client/app/validators/reset.js | 5 +- core/client/app/validators/setting.js | 2 +- core/client/app/validators/signin.js | 1 + core/client/app/validators/user.js | 5 +- core/test/functional/client/editor_test.js | 3 +- core/test/functional/client/settings_test.js | 29 +++++----- core/test/functional/client/signin_test.js | 21 +++---- core/test/functional/client/team_test.js | 46 +++++++-------- core/test/functional/setup/setup_test.js | 30 +++++----- 23 files changed, 208 insertions(+), 222 deletions(-) diff --git a/core/client/app/components/gh-trim-focus-input.js b/core/client/app/components/gh-trim-focus-input.js index b75f4f9844..f1627bf7f6 100644 --- a/core/client/app/components/gh-trim-focus-input.js +++ b/core/client/app/components/gh-trim-focus-input.js @@ -13,19 +13,18 @@ var TrimFocusInput = Ember.TextField.extend({ return false; }), - didInsertElement: function () { + focusField: Ember.on('didInsertElement', function () { // This fix is required until Mobile Safari has reliable // autofocus, select() or focus() support if (this.get('focus') && !device.ios()) { this.$().val(this.$().val()).focus(); } - }, + }), - focusOut: function () { + trimValue: Ember.on('focusOut', function () { var text = this.$().val(); - this.$().val(text.trim()); - } + }) }); export default TrimFocusInput; diff --git a/core/client/app/controllers/settings/general.js b/core/client/app/controllers/settings/general.js index 0a97ad1b8d..68a67779a9 100644 --- a/core/client/app/controllers/settings/general.js +++ b/core/client/app/controllers/settings/general.js @@ -63,6 +63,10 @@ export default Ember.Controller.extend({ }), actions: { + validate: function () { + this.get('model').validate(arguments); + }, + save: function () { var notifications = this.get('notifications'), config = this.get('config'); @@ -71,8 +75,10 @@ export default Ember.Controller.extend({ config.set('blogTitle', model.get('title')); return model; - }).catch(function (errors) { - notifications.showErrors(errors); + }).catch(function (error) { + if (error) { + notifications.showAPIError(error); + } }); }, diff --git a/core/client/app/controllers/setup/three.js b/core/client/app/controllers/setup/three.js index 5c8fb1ab7f..4c56a58d96 100644 --- a/core/client/app/controllers/setup/three.js +++ b/core/client/app/controllers/setup/three.js @@ -1,7 +1,9 @@ import Ember from 'ember'; +import DS from 'ember-data'; export default Ember.Controller.extend({ notifications: Ember.inject.service(), + errors: DS.Errors.create(), users: '', usersArray: Ember.computed('users', function () { var users = this.get('users').split('\n').filter(function (email) { @@ -62,10 +64,11 @@ export default Ember.Controller.extend({ var self = this, validationErrors = this.get('validateUsers'), users = this.get('usersArray'), - errorMessages, notifications = this.get('notifications'), invitationsString; + this.get('errors').clear(); + if (validationErrors === true && users.length > 0) { this.get('authorRole').then(function (authorRole) { Ember.RSVP.Promise.all( @@ -117,19 +120,15 @@ export default Ember.Controller.extend({ }); }); } else if (users.length === 0) { - // TODO: switch to inline-validation - notifications.showAlert('No users to invite.', {type: 'error'}); + this.get('errors').add('users', 'No users to invite.'); } else { - errorMessages = validationErrors.map(function (error) { + validationErrors.forEach(function (error) { // Only one error type here so far, but one day the errors might be more detailed switch (error.error) { case 'email': - return {message: error.user + ' is not a valid email.'}; + self.get('errors').add('users', error.user + ' is not a valid email.'); } }); - - // TODO: switch to inline-validation - notifications.showErrors(errorMessages); } } } diff --git a/core/client/app/controllers/signin.js b/core/client/app/controllers/signin.js index b74494950a..76c352f915 100644 --- a/core/client/app/controllers/signin.js +++ b/core/client/app/controllers/signin.js @@ -3,13 +3,14 @@ import ValidationEngine from 'ghost/mixins/validation-engine'; import {request as ajax} from 'ic-ajax'; export default Ember.Controller.extend(ValidationEngine, { - validationType: 'signin', - submitting: false, ghostPaths: Ember.inject.service('ghost-paths'), notifications: Ember.inject.service(), + // ValidationEngine settings + validationType: 'signin', + actions: { authenticate: function () { var model = this.get('model'), @@ -30,12 +31,12 @@ export default Ember.Controller.extend(ValidationEngine, { // browsers and password managers that don't send proper events on autofill $('#login').find('input').trigger('change'); - this.validate({format: false}).then(function () { + this.validate().then(function () { self.get('notifications').closeNotifications(); self.send('authenticate'); - }).catch(function (errors) { - if (errors) { - self.get('notifications').showErrors(errors); + }).catch(function (error) { + if (error) { + self.get('notifications').showAPIError(error); } }); }, @@ -45,27 +46,24 @@ export default Ember.Controller.extend(ValidationEngine, { notifications = this.get('notifications'), self = this; - if (!email) { - // TODO: Switch to in-line validation - return notifications.showNotification('Enter email address to reset password.', {type: 'error'}); - } + this.validate({property: 'identification'}).then(function () { + self.set('submitting', true); - self.set('submitting', true); - - ajax({ - url: self.get('ghostPaths.url').api('authentication', 'passwordreset'), - type: 'POST', - data: { - passwordreset: [{ - email: email - }] - } - }).then(function () { - self.set('submitting', false); - notifications.showAlert('Please check your email for instructions.', {type: 'info'}); - }).catch(function (resp) { - self.set('submitting', false); - notifications.showAPIError(resp, {defaultErrorText: 'There was a problem with the reset, please try again.'}); + ajax({ + url: self.get('ghostPaths.url').api('authentication', 'passwordreset'), + type: 'POST', + data: { + passwordreset: [{ + email: email + }] + } + }).then(function () { + self.set('submitting', false); + notifications.showAlert('Please check your email for instructions.', {type: 'info'}); + }).catch(function (resp) { + self.set('submitting', false); + notifications.showAPIError(resp, {defaultErrorText: 'There was a problem with the reset, please try again.'}); + }); }); } } diff --git a/core/client/app/controllers/signup.js b/core/client/app/controllers/signup.js index cbb981d5d7..b348109dda 100644 --- a/core/client/app/controllers/signup.js +++ b/core/client/app/controllers/signup.js @@ -20,8 +20,8 @@ export default Ember.Controller.extend(ValidationEngine, { notifications.closeNotifications(); - this.toggleProperty('submitting'); - this.validate({format: false}).then(function () { + this.validate().then(function () { + this.toggleProperty('submitting'); ajax({ url: self.get('ghostPaths.url').api('authentication', 'invitation'), type: 'POST', @@ -43,10 +43,9 @@ export default Ember.Controller.extend(ValidationEngine, { self.toggleProperty('submitting'); notifications.showAPIError(resp); }); - }).catch(function (errors) { - self.toggleProperty('submitting'); - if (errors) { - notifications.showErrors(errors); + }).catch(function (error) { + if (error) { + notifications.showAPIError(error); } }); } diff --git a/core/client/app/controllers/team/user.js b/core/client/app/controllers/team/user.js index 3cb9032a16..0598d2bcf4 100644 --- a/core/client/app/controllers/team/user.js +++ b/core/client/app/controllers/team/user.js @@ -2,8 +2,12 @@ import Ember from 'ember'; import SlugGenerator from 'ghost/models/slug-generator'; import isNumber from 'ghost/utils/isNumber'; import boundOneWay from 'ghost/utils/bound-one-way'; +import ValidationEngine from 'ghost/mixins/validation-engine'; + +export default Ember.Controller.extend(ValidationEngine, { + // ValidationEngine settings + validationType: 'user', -export default Ember.Controller.extend({ ghostPaths: Ember.inject.service('ghost-paths'), notifications: Ember.inject.service(), diff --git a/core/client/app/mixins/validation-engine.js b/core/client/app/mixins/validation-engine.js index 3c4287588f..be1370cfc4 100644 --- a/core/client/app/mixins/validation-engine.js +++ b/core/client/app/mixins/validation-engine.js @@ -15,49 +15,6 @@ import TagSettingsValidator from 'ghost/validators/tag-settings'; // our extensions to the validator library ValidatorExtensions.init(); -// This is here because it is used by some things that format errors from api responses -// This function should be removed in the notifications refactor -// format errors to be used in `notifications.showErrors`. -// result is [{message: 'concatenated error messages'}] -function formatErrors(errors, opts) { - var message = 'There was an error'; - - opts = opts || {}; - - if (opts.wasSave && opts.validationType) { - message += ' saving this ' + opts.validationType; - } - - if (Ember.isArray(errors)) { - // get the validator's error messages from the array. - // normalize array members to map to strings. - message = errors.map(function (error) { - var errorMessage; - if (typeof error === 'string') { - errorMessage = error; - } else { - errorMessage = error.message; - } - - return Ember.Handlebars.Utils.escapeExpression(errorMessage); - }).join('
').htmlSafe(); - } else if (errors instanceof Error) { - message += errors.message || '.'; - } else if (typeof errors === 'object') { - // Get messages from server response - message += ': ' + getRequestErrorMessage(errors, true); - } else if (typeof errors === 'string') { - message += ': ' + errors; - } else { - message += '.'; - } - - // set format for notifications.showErrors - message = [{message: message}]; - - return message; -} - /** * The class that gets this mixin will receive these properties and functions. * It will be able to validate any properties on itself (or the model it passes to validate()) @@ -163,15 +120,10 @@ export default Ember.Mixin.create({ return this.validate(options).then(function () { return _super.call(self, options); }).catch(function (result) { - // server save failed - validate() would have given back an array - if (!Ember.isArray(result)) { - if (options.format !== false) { - // concatenate all errors into an array with a single object: [{message: 'concatted message'}] - result = formatErrors(result, options); - } else { - // return the array of errors from the server - result = getRequestErrorMessage(result); - } + // server save failed or validator type doesn't exist + if (result && !Ember.isArray(result)) { + // return the array of errors from the server + result = getRequestErrorMessage(result); } return Ember.RSVP.reject(result); diff --git a/core/client/app/styles/layouts/flow.css b/core/client/app/styles/layouts/flow.css index fd880b17ba..9e019741bb 100644 --- a/core/client/app/styles/layouts/flow.css +++ b/core/client/app/styles/layouts/flow.css @@ -377,6 +377,7 @@ } .gh-flow-content .gh-flow-invite { + position: relative; margin: 0 auto; max-width: 400px; width: 100%; diff --git a/core/client/app/templates/reset.hbs b/core/client/app/templates/reset.hbs index 3e98a6ca13..b630b5e532 100644 --- a/core/client/app/templates/reset.hbs +++ b/core/client/app/templates/reset.hbs @@ -2,12 +2,21 @@
diff --git a/core/client/app/templates/settings/general.hbs b/core/client/app/templates/settings/general.hbs index d33fbe7b0d..5ea6b51960 100644 --- a/core/client/app/templates/settings/general.hbs +++ b/core/client/app/templates/settings/general.hbs @@ -10,21 +10,22 @@
-
+ {{#gh-form-group errors=model.errors property="title"}} - {{input id="blog-title" class="gh-input" name="general[title]" type="text" value=model.title}} + {{gh-input id="blog-title" class="gh-input" name="general[title]" type="text" value=model.title focusOut=(action "validate" "title")}} + {{gh-error-message errors=model.errors property="title"}}

The name of your blog

-
+ {{/gh-form-group}} -
+ {{#gh-form-group class="description-container" errors=model.errors property="description"}} - {{textarea id="blog-description" class="gh-input" name="general[description]" value=model.description}} + {{gh-textarea id="blog-description" class="gh-input" name="general[description]" value=model.description focusOut=(action "validate" "description")}} + {{gh-error-message errors=model.errors property="description"}}

Describe what your blog is about {{gh-count-characters model.description}}

- -
+ {{/gh-form-group}}
@@ -52,7 +53,7 @@
{{! `pattern` brings up numeric keypad allowing any number of digits}} - {{input id="postsPerPage" class="gh-input" name="general[postsPerPage]" focus-out="checkPostsPerPage" value=model.postsPerPage min="1" max="1000" type="number" pattern="[0-9]*"}} + {{gh-input id="postsPerPage" class="gh-input" name="general[postsPerPage]" focus-out="checkPostsPerPage" value=model.postsPerPage min="1" max="1000" type="number" pattern="[0-9]*"}}

How many posts should be displayed on each page

@@ -92,10 +93,11 @@
{{#if model.isPrivate}} -
- {{input name="general[password]" type="text" value=model.password}} + {{#gh-form-group errors=model.errors property="password"}} + {{gh-input name="general[password]" type="text" value=model.password focusOut=(action "validate" "password")}} + {{gh-error-message errors=model.errors property="password"}}

This password will be needed to access your blog. All search engine optimization and social features are now disabled. This password is stored in plaintext.

-
+ {{/gh-form-group}} {{/if}}
diff --git a/core/client/app/templates/setup/three.hbs b/core/client/app/templates/setup/three.hbs index 9b57f21eae..ce9359d4db 100644 --- a/core/client/app/templates/setup/three.hbs +++ b/core/client/app/templates/setup/three.hbs @@ -8,6 +8,7 @@
{{textarea class="gh-input" name="users" value=users required="required"}} + {{gh-error-message errors=errors property="users"}}
-
+ {{gh-error-message errors=model.errors property="password"}} + {{/gh-form-group}} diff --git a/core/client/app/templates/signup.hbs b/core/client/app/templates/signup.hbs index 374f959f6b..de601d81c8 100644 --- a/core/client/app/templates/signup.hbs +++ b/core/client/app/templates/signup.hbs @@ -14,26 +14,28 @@ -
+ {{#gh-form-group errors=model.errors property="email"}} - {{input class="gh-input" type="email" name="email" autocorrect="off" value=model.email }} + {{gh-input type="email" name="email" placeholder="Eg. john@example.com" class="gh-input" autofocus="autofocus" autocorrect="off" value=model.email focusOut=(action "validate" "email")}} -
-
+ {{gh-error-message errors=model.errors property="email"}} + {{/gh-form-group}} + {{#gh-form-group errors=model.errors property="name"}} - {{gh-trim-focus-input class="gh-input" type="text" name="name" autofocus="autofocus" autocorrect="off" value=model.name }} + {{gh-input type="text" name="name" placeholder="Eg. John H. Watson" class="gh-input" autofocus="autofocus" autocorrect="off" value=model.name focusOut=(action "validate" "name")}} -
-
+ {{gh-error-message errors=model.errors property="name"}} + {{/gh-form-group}} + {{#gh-form-group errors=model.errors property="password"}} - {{input class="gh-input" type="password" name="password" autofocus="autofocus" autocorrect="off" value=model.password }} + {{input class="gh-input" type="password" name="password" autofocus="autofocus" autocorrect="off" value=model.password focusOut=(action "validate" "password")}}
@@ -42,7 +44,8 @@
-
+ {{gh-error-message errors=model.errors property="password"}} + {{/gh-form-group}} diff --git a/core/client/app/templates/team/user.hbs b/core/client/app/templates/team/user.hbs index 00a18cef51..24ffd76392 100644 --- a/core/client/app/templates/team/user.hbs +++ b/core/client/app/templates/team/user.hbs @@ -53,32 +53,39 @@ -
+ {{#gh-form-group class="first-form-group" errors=user.errors property="name"}} - {{input value=user.name id="user-name" class="gh-input user-name" placeholder="Full Name" autocorrect="off"}} -

Use your real name so people can recognise you

-
+ {{input value=user.name id="user-name" class="gh-input user-name" placeholder="Full Name" autocorrect="off" focusOut=(action "validate" "name")}} + {{#if user.errors.name}} + {{gh-error-message errors=user.errors property="name"}} + {{else}} +

Use your real name so people can recognise you

+ {{/if}} + {{/gh-form-group}}
-
+ {{#gh-form-group errors=user.errors property="slug"}} {{gh-input class="gh-input user-name" id="user-slug" value=slugValue name="user" focus-out="updateSlug" placeholder="Slug" selectOnClick="true" autocorrect="off"}}

{{gh-blog-url}}/author/{{slugValue}}

-
+ {{gh-error-message errors=user.errors property="slug"}} + {{/gh-form-group}} -
+ {{#gh-form-group errors=user.errors property="email"}} {{!-- Administrators only see text of Owner's email address but not input --}} {{#unless isAdminUserOnOwnerProfile}} - {{input type="email" value=user.email id="user-email" class="gh-input" placeholder="Email Address" autocapitalize="off" autocorrect="off" autocomplete="off"}} + {{input type="email" value=user.email id="user-email" name="email" class="gh-input" placeholder="Email Address" autocapitalize="off" autocorrect="off" autocomplete="off" focusOut=(action "validate" "email")}} + {{gh-error-message errors=user.errors property="email"}} {{else}} {{user.email}} {{/unless}}

Used for notifications

-
+ {{/gh-form-group}} + {{#if rolesDropdownIsVisible}}
@@ -94,26 +101,30 @@

What permissions should this user have?

{{/if}} -
+ + {{#gh-form-group errors=user.errors property="location"}} - {{input type="text" value=user.location id="user-location" class="gh-input"}} + {{input type="text" value=user.location id="user-location" class="gh-input" focusOut=(action "validate" "location")}} + {{gh-error-message errors=user.errors property="location"}}

Where in the world do you live?

-
+ {{/gh-form-group}} -
+ {{#gh-form-group errors=user.errors property="website"}} - {{input type="url" value=user.website id="user-website" class="gh-input" autocapitalize="off" autocorrect="off" autocomplete="off"}} + {{input type="url" value=user.website id="user-website" class="gh-input" autocapitalize="off" autocorrect="off" autocomplete="off" focusOut=(action "validate" "website")}} + {{gh-error-message errors=user.errors property="website"}}

Have a website or blog other than this one? Link it!

-
+ {{/gh-form-group}} -
+ {{#gh-form-group class="bio-container" errors=user.errors property="bio"}} - {{textarea id="user-bio" class="gh-input" value=user.bio}} + {{textarea id="user-bio" class="gh-input" value=user.bio focusOut=(action "validate" "bio")}} + {{gh-error-message errors=user.errors property="bio"}}

Write about you, in 200 characters or less. {{gh-count-characters user.bio}}

-
+ {{/gh-form-group}}
diff --git a/core/client/app/validators/reset.js b/core/client/app/validators/reset.js index b69cd856cd..d80a675afe 100644 --- a/core/client/app/validators/reset.js +++ b/core/client/app/validators/reset.js @@ -5,7 +5,10 @@ var ResetValidator = BaseValidator.create({ var p1 = model.get('newPassword'), p2 = model.get('ne2Password'); - if (!validator.isLength(p1, 8)) { + if (validator.empty(p1)) { + model.get('errors').add('newPassword', 'Please enter a password.'); + this.invalidate(); + } else if (!validator.isLength(p1, 8)) { model.get('errors').add('newPassword', 'The password is not long enough.'); this.invalidate(); } else if (!validator.equals(p1, p2)) { diff --git a/core/client/app/validators/setting.js b/core/client/app/validators/setting.js index b0795a5f1d..44e6a29c95 100644 --- a/core/client/app/validators/setting.js +++ b/core/client/app/validators/setting.js @@ -20,7 +20,7 @@ var SettingValidator = BaseValidator.create({ }, password: function (model) { var isPrivate = model.get('isPrivate'), - password = this.get('password'); + password = model.get('password'); if (isPrivate && password === '') { model.get('errors').add('password', 'Password must be supplied'); diff --git a/core/client/app/validators/signin.js b/core/client/app/validators/signin.js index 868509ed13..b44350922d 100644 --- a/core/client/app/validators/signin.js +++ b/core/client/app/validators/signin.js @@ -7,6 +7,7 @@ var SigninValidator = BaseValidator.create({ if (validator.empty(id)) { model.get('errors').add('identification', 'Please enter an email'); + this.invalidate(); } else if (!validator.isEmail(id)) { model.get('errors').add('identification', 'Invalid email'); this.invalidate(); diff --git a/core/client/app/validators/user.js b/core/client/app/validators/user.js index 9b615452c0..4f842eb49f 100644 --- a/core/client/app/validators/user.js +++ b/core/client/app/validators/user.js @@ -10,7 +10,10 @@ var UserValidator = BaseValidator.create({ var name = model.get('name'); if (this.isActive(model)) { - if (!validator.isLength(name, 0, 150)) { + if (validator.empty(name)) { + model.get('errors').add('name', 'Please enter a name.'); + this.invalidate(); + } else if (!validator.isLength(name, 0, 150)) { model.get('errors').add('name', 'Name is too long'); this.invalidate(); } diff --git a/core/test/functional/client/editor_test.js b/core/test/functional/client/editor_test.js index e647b27bf6..dc4df0a0e8 100644 --- a/core/test/functional/client/editor_test.js +++ b/core/test/functional/client/editor_test.js @@ -556,8 +556,7 @@ CasperTest.begin('Publish menu - new post status is correct after failed save', }); }); -// TODO: Change number of tests back to 6 once the commented-out tests are fixed -CasperTest.begin('Publish menu - existing post status is correct after failed save', 4, function suite(test) { +CasperTest.begin('Publish menu - existing post status is correct after failed save', 6, function suite(test) { casper.thenOpenAndWaitForPageLoad('editor', function testTitleAndUrl() { test.assertTitle('Editor - Test Blog', 'Ghost admin has incorrect title'); test.assertUrlMatch(/ghost\/editor\/$/, 'Landed on the correct URL'); diff --git a/core/test/functional/client/settings_test.js b/core/test/functional/client/settings_test.js index 5f237dd3b6..2e5128c899 100644 --- a/core/test/functional/client/settings_test.js +++ b/core/test/functional/client/settings_test.js @@ -76,8 +76,7 @@ CasperTest.begin('General settings pane is correct', 4, function suite(test) { }); // ## General settings validations tests -// // TODO: Change number of tests back to 6 once the commented-out tests are fixed -CasperTest.begin('General settings validation is correct', 4, function suite(test) { +CasperTest.begin('General settings validation is correct', 7, function suite(test) { casper.thenOpenAndWaitForPageLoad('settings.general', function testTitleAndUrl() { test.assertTitle('Settings - General - Test Blog', 'Ghost admin has incorrect title'); test.assertUrlMatch(/ghost\/settings\/general\/$/, 'Landed on the correct URL'); @@ -88,25 +87,19 @@ CasperTest.begin('General settings validation is correct', 4, function suite(tes 'general[title]': new Array(152).join('a') }); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { - test.assertSelectorHasText('.gh-notification-red', 'too long', '.gh-notification-red has correct text'); - }, casper.failOnTimeout(test, 'Blog title length error did not appear'), 2000); - - casper.thenClick('.gh-alert-close'); + casper.waitForText('Title is too long', function onSuccess() { + test.assert(true, 'Blog title length error was shown'); + }, casper.failOnTimeout(test, 'Blog title length error did not appear')); // Ensure general blog description field length validation casper.fillAndSave('form#settings-general', { 'general[description]': new Array(202).join('a') }); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { - test.assertSelectorHasText('.gh-notification-red', 'too long', '.gh-notification-red has correct text'); + casper.waitForText('Description is too long', function onSuccess() { + test.assert(true, 'Blog description length error was shown'); }, casper.failOnTimeout(test, 'Blog description length error did not appear')); - casper.thenClick('.gh-alert-close'); - // TODO move these to ember tests, note: async issues - field will be often be null without a casper.wait // Check postsPerPage autocorrect casper.fillAndSave('form#settings-general', { @@ -128,4 +121,14 @@ CasperTest.begin('General settings validation is correct', 4, function suite(tes casper.then(function checkSlugInputValue() { test.assertField('general[postsPerPage]', '5', 'posts per page is set correctly'); }); + + // Ensure private blog password validation + casper.fillAndSave('form#settings-general', { + 'general[isPrivate]': '1', + 'general[password]': '' + }); + + casper.waitForText('Password must be supplied', function onSuccess() { + test.assert(true, 'Password required error was shown'); + }, casper.failOnTimeout(test, 'Password required error did not appear')); }); diff --git a/core/test/functional/client/signin_test.js b/core/test/functional/client/signin_test.js index 67e2f0af59..551766ec94 100644 --- a/core/test/functional/client/signin_test.js +++ b/core/test/functional/client/signin_test.js @@ -110,8 +110,7 @@ CasperTest.begin('Authenticated user is redirected', 6, function suite(test) { }); }, true); -// TODO: Change number of tests back to 4 once the commented-out tests are fixed -CasperTest.begin('Ensure email field form validation', 2, function suite(test) { +CasperTest.begin('Ensure email field form validation', 4, function suite(test) { CasperTest.Routines.signout.run(test); casper.thenOpenAndWaitForPageLoad('signin', function testTitleAndUrl() { @@ -129,12 +128,9 @@ CasperTest.begin('Ensure email field form validation', 2, function suite(test) { test.fail('Login form didn\'t fade in.'); }); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { - test.assertSelectorHasText('.gh-notification-red', 'Invalid Email', '.gh-notification-red text is correct'); - }, function onTimeout() { - test.fail('Email validation error did not appear'); - }, 2000); + casper.waitForText('Invalid email', function onSuccess() { + test.assert(true, 'Invalid email error was shown'); + }, casper.failOnTimeout(test, 'Invalid email error was not shown')); casper.then(function testMissingEmail() { this.fillAndSave('form.gh-signin', { @@ -142,10 +138,7 @@ CasperTest.begin('Ensure email field form validation', 2, function suite(test) { }); }); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification-red', function onSuccess() { - test.assertSelectorHasText('.gh-notification-red', 'Please enter an email', '.gh-notification-red text is correct'); - }, function onTimeout() { - test.fail('Missing Email validation error did not appear'); - }, 2000); + casper.waitForText('Please enter an email', function onSuccess() { + test.assert(true, 'Missing email error was shown'); + }, casper.failOnTimeout(test, 'Missing email error was not shown')); }, true); diff --git a/core/test/functional/client/team_test.js b/core/test/functional/client/team_test.js index 2d17942a06..2d2b57fa5b 100644 --- a/core/test/functional/client/team_test.js +++ b/core/test/functional/client/team_test.js @@ -192,7 +192,7 @@ CasperTest.begin('User settings screen change slug handles duplicate slug', 4, f }); }); -CasperTest.begin('User settings screen validates email', 6, function suite(test) { +CasperTest.begin('User settings screen validates email', 4, function suite(test) { var email; casper.thenOpenAndWaitForPageLoad('team.user', function testTitleAndUrl() { @@ -208,21 +208,14 @@ CasperTest.begin('User settings screen validates email', 6, function suite(test) casper.then(function setEmailToInvalid() { var brokenEmail = email.replace('.', '-'); - - casper.fillSelectors('.user-profile', { - '#user-email': brokenEmail - }, false); + this.fillAndSave('.user-profile', { + email: brokenEmail + }); }); - casper.thenClick('.btn-blue'); - - casper.waitForResource('/team/'); - - // TODO: review once inline-validations are implemented - casper.waitForSelector('.gh-notification', function onSuccess() { - test.assert(true, 'Got error notification'); - test.assertSelectorDoesntHaveText('.gh-notification', '[object Object]', 'notification text is not broken'); - }, casper.failOnTimeout(test, 'No error notification :(')); + casper.waitForText('Please supply a valid email address', function onSuccess() { + test.assert(true, 'Invalid email error was shown'); + }, casper.failOnTimeout(test, 'Invalid email error was not shown')); casper.then(function resetEmailToValid() { casper.fillSelectors('.user-profile', { @@ -230,6 +223,10 @@ CasperTest.begin('User settings screen validates email', 6, function suite(test) }, false); }); + casper.then(function checkEmailErrorWasCleared() { + test.assertTextDoesntExist('Please supply a valid email address', 'Invalid email error was not cleared'); + }); + casper.thenClick('.view-actions .btn-blue'); casper.waitForResource(/users/); @@ -275,10 +272,9 @@ CasperTest.begin('Ensure user bio field length validation', 3, function suite(te casper.thenClick('.view-actions .btn-blue'); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification', function onSuccess() { - test.assertSelectorHasText('.gh-notification', 'is too long', '.gh-notification text is correct'); - }, casper.failOnTimeout(test, 'Bio field length error did not appear', 2000)); + casper.waitForText('Bio is too long', function onSuccess() { + test.assert(true, 'Bio too long error was shown'); + }, casper.failOnTimeout(test, 'Bio too long error was not shown')); }); CasperTest.begin('Ensure user url field validation', 3, function suite(test) { @@ -295,10 +291,9 @@ CasperTest.begin('Ensure user url field validation', 3, function suite(test) { casper.thenClick('.view-actions .btn-blue'); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification', function onSuccess() { - test.assertSelectorHasText('.gh-notification', 'not a valid url', '.gh-notification text is correct'); - }, casper.failOnTimeout(test, 'Url validation error did not appear', 2000)); + casper.waitForText('Website is not a valid url', function onSuccess() { + test.assert(true, 'Website invalid error was shown'); + }, casper.failOnTimeout(test, 'Website invalid error was not shown')); }); CasperTest.begin('Ensure user location field length validation', 3, function suite(test) { @@ -315,8 +310,7 @@ CasperTest.begin('Ensure user location field length validation', 3, function sui casper.thenClick('.view-actions .btn-blue'); - // TODO: review once inline-validations are implemented - casper.waitForSelectorTextChange('.gh-notification', function onSuccess() { - test.assertSelectorHasText('.gh-notification', 'is too long', '.gh-notification text is correct'); - }, casper.failOnTimeout(test, 'Location field length error did not appear', 2000)); + casper.waitForText('Location is too long', function onSuccess() { + test.assert(true, 'Location too long error was shown'); + }, casper.failOnTimeout(test, 'Location too long error was not shown')); }); diff --git a/core/test/functional/setup/setup_test.js b/core/test/functional/setup/setup_test.js index e2a0f91345..544cf09558 100644 --- a/core/test/functional/setup/setup_test.js +++ b/core/test/functional/setup/setup_test.js @@ -2,7 +2,7 @@ /*global CasperTest, casper, email, user, password */ -CasperTest.begin('Ghost setup fails properly', 12, function suite(test) { +CasperTest.begin('Ghost setup fails properly', 11, function suite(test) { casper.thenOpenAndWaitForPageLoad('setup', function then() { test.assertUrlMatch(/ghost\/setup\/one\/$/, 'Landed on the correct URL'); }); @@ -11,14 +11,10 @@ CasperTest.begin('Ghost setup fails properly', 12, function suite(test) { casper.fillAndAdd('#setup', {'blog-title': 'ghost', name: 'slimer', email: email, password: 'short'}); }); - // TODO: Fix tests to support inline validation - // should now throw a short password error - casper.waitForSelector('.gh-notification', function onSuccess() { - test.assert(true, 'Got error notification'); - test.assertSelectorHasText('.gh-notification', 'Password must be at least 8 characters long'); - }, function onTimeout() { - test.assert(false, 'No error notification :('); - }); + // should now show a short password error + casper.waitForText('Password must be at least 8 characters long', function onSuccess() { + test.assert(true, 'Short password error was shown'); + }, casper.failOnTimeout(test, 'Short password error was not shown')); casper.then(function setupWithLongPassword() { casper.fillAndAdd('#setup', {'blog-title': 'ghost', name: 'slimer', email: email, password: password}); @@ -31,16 +27,24 @@ CasperTest.begin('Ghost setup fails properly', 12, function suite(test) { casper.thenClick('.gh-flow-content .btn'); }); - casper.waitForSelector('.gh-alert', function onSuccess() { - test.assert(true, 'Got error notification'); - test.assertSelectorHasText('.gh-alert', 'No users to invite.'); + casper.waitForText('No users to invite.', function onSuccess() { + test.assert(true, 'Got error message'); test.assertExists('.gh-flow-content .btn-minor', 'Submit button is not minor'); test.assertSelectorHasText('.gh-flow-content .btn', 'Invite some users', 'Submit button has wrong text'); }, function onTimeout() { - test.assert(false, 'No error notification for empty invitation list'); + test.assert(false, 'No error message for empty invitation list'); }); + casper.then(function fillInvalidEmail() { + casper.fill('form.gh-flow-invite', {users: 'test'}); + casper.thenClick('.gh-flow-content .btn'); + }); + + casper.waitForText('test is not a valid email.', function onSuccess() { + test.assert(true, 'Got invalid email error'); + }, casper.failOnTimeout(test, 'Invalid email error not shown')); + casper.then(function fillInvitationForm() { casper.fill('form.gh-flow-invite', {users: 'test@example.com'}); test.assertSelectorHasText('.gh-flow-content .btn', 'Invite 1 user', 'One invitation button text is incorrect'); From e21d7ed1f591889acbee9dd1a0ac3ddfd315be0a Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 21 Jul 2015 18:56:17 +0100 Subject: [PATCH 3/3] WIP: review uses of notifications.showErrors issue #5409 `notifications.showErrors` was historically used to display multiple error notifications whether from validation errors or responses form the API. This usage needs to be reviewed as inline validations should handle the validation side and we should be displaying alerts for actual errors. Eventually `notifications.showErrors` should be left unused and therefore removed. --- core/client/app/controllers/modals/delete-all.js | 2 +- core/client/app/controllers/modals/invite-new-user.js | 4 ++++ core/client/app/controllers/modals/upload.js | 2 +- core/client/app/controllers/reset.js | 7 ++----- core/client/app/controllers/settings/code-injection.js | 7 ++----- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/core/client/app/controllers/modals/delete-all.js b/core/client/app/controllers/modals/delete-all.js index 4ea73dd6e4..dc19efda5c 100644 --- a/core/client/app/controllers/modals/delete-all.js +++ b/core/client/app/controllers/modals/delete-all.js @@ -16,7 +16,7 @@ export default Ember.Controller.extend({ self.store.unloadAll('post'); self.store.unloadAll('tag'); }).catch(function (response) { - self.get('notifications').showErrors(response); + self.get('notifications').showAPIError(response); }); }, diff --git a/core/client/app/controllers/modals/invite-new-user.js b/core/client/app/controllers/modals/invite-new-user.js index cf8fdd95ca..5f8e66f511 100644 --- a/core/client/app/controllers/modals/invite-new-user.js +++ b/core/client/app/controllers/modals/invite-new-user.js @@ -78,6 +78,10 @@ export default Ember.Controller.extend({ } }).catch(function (errors) { newUser.deleteRecord(); + // TODO: user model includes ValidationEngine mixin so + // save is overridden in order to validate, we probably + // want to use inline-validations here and only show an + // alert if we have an actual error self.get('notifications').showErrors(errors); }); } diff --git a/core/client/app/controllers/modals/upload.js b/core/client/app/controllers/modals/upload.js index a288447d56..f11abae874 100644 --- a/core/client/app/controllers/modals/upload.js +++ b/core/client/app/controllers/modals/upload.js @@ -12,7 +12,7 @@ export default Ember.Controller.extend({ this.get('model').save().then(function (model) { return model; }).catch(function (err) { - notifications.showErrors(err); + notifications.showAPIError(err); }); }, diff --git a/core/client/app/controllers/reset.js b/core/client/app/controllers/reset.js index 92304f0c80..4f0a2d6307 100644 --- a/core/client/app/controllers/reset.js +++ b/core/client/app/controllers/reset.js @@ -33,8 +33,8 @@ export default Ember.Controller.extend(ValidationEngine, { var credentials = this.getProperties('newPassword', 'ne2Password', 'token'), self = this; - this.toggleProperty('submitting'); - this.validate({format: false}).then(function () { + this.validate().then(function () { + self.toggleProperty('submitting'); ajax({ url: self.get('ghostPaths.url').api('authentication', 'passwordreset'), type: 'PUT', @@ -52,9 +52,6 @@ export default Ember.Controller.extend(ValidationEngine, { self.get('notifications').showAPIError(response); self.toggleProperty('submitting'); }); - }).catch(function (error) { - self.toggleProperty('submitting'); - self.get('notifications').showErrors(error); }); } } diff --git a/core/client/app/controllers/settings/code-injection.js b/core/client/app/controllers/settings/code-injection.js index 043c1efd14..c68c63ee95 100644 --- a/core/client/app/controllers/settings/code-injection.js +++ b/core/client/app/controllers/settings/code-injection.js @@ -8,12 +8,9 @@ export default Ember.Controller.extend({ var notifications = this.get('notifications'); return this.get('model').save().then(function (model) { - notifications.closeNotifications(); - return model; - }).catch(function (errors) { - notifications.closeNotifications(); - notifications.showErrors(errors); + }).catch(function (error) { + notifications.showAPIError(error); }); } }