From 58b635203ce7b0b1a1fec1a8407bcabab60f577d Mon Sep 17 00:00:00 2001 From: Matt Enlow Date: Mon, 14 Jul 2014 22:02:34 -0600 Subject: [PATCH] Remove minor notifications; Close persistent notifications even on error Closes #3105, Closes #3175 - Removed notification on successful post's `page` status change - Removed notification on successful post `featured` status change - Added `closePassive()` notifications on error in the post-settings-menu - Persistent notifications will close whether their `DELETE` request was successful or not. #### Misc - Added `name` attribute to `post-setting-menu.hbs` inputs to facilitate testing - Removed `return ` from action in `PostSettingsMenuController`. Actions should only return `true` - Toggling `post.featured` won't fire NProgress. --- core/client/controllers/post-settings-menu.js | 16 +-- core/client/controllers/posts/post.js | 8 +- core/client/templates/post-settings-menu.hbs | 4 +- core/client/utils/notifications.js | 3 +- core/test/functional/client/content_test.js | 108 ++++++++---------- core/test/functional/client/editor_test.js | 61 +++++----- 6 files changed, 92 insertions(+), 108 deletions(-) diff --git a/core/client/controllers/post-settings-menu.js b/core/client/controllers/post-settings-menu.js index df37d56dac..b4d5d7ce05 100644 --- a/core/client/controllers/post-settings-menu.js +++ b/core/client/controllers/post-settings-menu.js @@ -77,19 +77,16 @@ var PostSettingsMenuController = Ember.ObjectController.extend({ }, actions: { togglePage: function () { - var value = this.toggleProperty('page'), - self = this; + var self = this; + this.toggleProperty('page'); // If this is a new post. Don't save the model. Defer the save // to the user pressing the save button if (this.get('isNew')) { return; } - - return this.get('model').save(this.get('saveOptions')).then(function () { - self.showSuccess('Successfully converted to ' + (value ? 'static page' : 'post')); - return value; - }).catch(function (errors) { + + this.get('model').save(this.get('saveOptions')).catch(function (errors) { self.showErrors(errors); self.get('model').rollback(); }); @@ -205,10 +202,7 @@ var PostSettingsMenuController = Ember.ObjectController.extend({ return; } - this.get('model').save(this.get('saveOptions')).then(function () { - self.showSuccess('Publish date successfully changed to ' + - formatDate(self.get('published_at')) + '.'); - }).catch(function (errors) { + this.get('model').save(this.get('saveOptions')).catch(function (errors) { self.showErrors(errors); self.get('model').rollback(); }); diff --git a/core/client/controllers/posts/post.js b/core/client/controllers/posts/post.js index 50a9eb2135..e9bc14aeec 100644 --- a/core/client/controllers/posts/post.js +++ b/core/client/controllers/posts/post.js @@ -4,13 +4,11 @@ var PostController = Ember.ObjectController.extend({ actions: { toggleFeatured: function () { - var featured = this.toggleProperty('featured'), + var options = {disableNProgress: true}, self = this; - this.get('model').save().then(function () { - self.notifications.closePassive(); - self.notifications.showSuccess('Post successfully marked as ' + (featured ? 'featured' : 'not featured') + '.'); - }).catch(function (errors) { + this.toggleProperty('featured'); + this.get('model').save(options).catch(function (errors) { self.notifications.closePassive(); self.notifications.showErrors(errors); }); diff --git a/core/client/templates/post-settings-menu.hbs b/core/client/templates/post-settings-menu.hbs index b7b1b76d9b..2bd20993c0 100644 --- a/core/client/templates/post-settings-menu.hbs +++ b/core/client/templates/post-settings-menu.hbs @@ -6,7 +6,7 @@ - {{gh-blur-input class="post-setting-slug" id="url" value=slugValue action="updateSlug" placeholder=slugPlaceholder selectOnClick="true"}} + {{gh-blur-input class="post-setting-slug" id="url" value=slugValue name="post-setting-slug" action="updateSlug" placeholder=slugPlaceholder selectOnClick="true"}} @@ -14,7 +14,7 @@ - {{gh-blur-input class="post-setting-date" value=publishedAtValue action="setPublishedAt" placeholder=publishedAtPlaceholder}} + {{gh-blur-input class="post-setting-date" value=publishedAtValue name="post-setting-date" action="setPublishedAt" placeholder=publishedAtPlaceholder}} diff --git a/core/client/utils/notifications.js b/core/client/utils/notifications.js index 09506fb4d5..5e6aa4c5b8 100644 --- a/core/client/utils/notifications.js +++ b/core/client/utils/notifications.js @@ -68,6 +68,7 @@ var Notifications = Ember.ArrayProxy.extend({ message: message }, delayed); }, + // @Todo this function isn't referenced anywhere. Should it be removed? showWarn: function (message, delayed) { this.handleNotification({ type: 'warn', @@ -87,7 +88,7 @@ var Notifications = Ember.ArrayProxy.extend({ if (notification instanceof Notification) { notification.deleteRecord(); - notification.save().then(function () { + notification.save().finally(function () { self.removeObject(notification); }); } else { diff --git a/core/test/functional/client/content_test.js b/core/test/functional/client/content_test.js index 28b1db36ca..db9a15235e 100644 --- a/core/test/functional/client/content_test.js +++ b/core/test/functional/client/content_test.js @@ -9,7 +9,7 @@ CasperTest.begin('Content screen is correct', 21, function suite(test) { // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -71,7 +71,7 @@ CasperTest.begin('Content list shows correct post status', 7, function testStati // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -116,7 +116,7 @@ CasperTest.begin('Delete post modal', 7, function testDeleteModal(test) { // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -162,18 +162,18 @@ CasperTest.begin('Delete post modal', 7, function testDeleteModal(test) { // // Placeholder for infinite scrolling/pagination tests (will need to setup 16+ posts). // // casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { -// test.assertTitle('Ghost Admin', 'Ghost admin has no title'); +// test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); // test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); // }); //}); -CasperTest.begin('Posts can be marked as featured', 10, function suite(test) { +CasperTest.begin('Posts can be marked as featured', 8, function suite(test) { // Create a sample post CasperTest.Routines.createTestPost.run(false); // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -184,45 +184,39 @@ CasperTest.begin('Posts can be marked as featured', 10, function suite(test) { test.assert(false, 'The first post can\'t be marked as featured'); }); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - test.assertSelectorHasText('.notification-message', 'Post successfully marked as featured.'); - }, function onTimeout() { - test.assert(false, 'No success notification :('); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function (resource) { + test.assert(400 > resource.status); }); casper.waitForSelector('.content-list-content li.featured:first-of-type', function () { test.assertExists('.content-preview .featured', 'preview pane gets featured class'); test.assertExists('.content-list-content li.featured:first-of-type', 'content list got a featured star'); - this.click('.notification-success .close'); }, function onTimeout() { test.assert(false, 'No featured star appeared in the left pane'); }); // Mark as not featured - casper.waitWhileSelector('.notification-success', function waitForNoSuccess() { - this.click('.content-preview .featured'); - }, function onTimeout() { - test.assert(false, 'Success notification wont go away:('); + casper.thenClick('.content-preview .featured'); + + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function waitForSuccess(resource) { + test.assert(400 > resource.status); }); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - test.assertSelectorHasText('.notification-message', 'Post successfully marked as not featured.'); - test.assertDoesntExist('.content-preview .featured'); + casper.then(function untoggledFeaturedTest() { + test.assertDoesntExist('.content-preview .featured', 'Untoggled featured.'); test.assertDoesntExist('.content-list-content li.featured:first-of-type'); }, function onTimeout() { - test.assert(false, 'Success notification wont go away:('); + test.assert(false, 'Couldn\'t unfeature post.'); }); }); -CasperTest.begin('Post url can be changed', 7, function suite(test) { +CasperTest.begin('Post url can be changed', 5, function suite(test) { // Create a sample post CasperTest.Routines.createTestPost.run(false); // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -241,27 +235,26 @@ CasperTest.begin('Post url can be changed', 7, function suite(test) { this.click('a.post-settings'); }); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - test.assertSelectorHasText('.notification-message', 'Permalink successfully changed to new-url.'); - casper.click('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function testGoodResponse(resource) { + test.assert(400 > resource.status); }); - casper.waitWhileSelector('.notification-success', function () { - test.assert(true, 'notification cleared.'); - test.assertNotVisible('.notification-success', 'success notification should not still exist'); + casper.then(function checkValueMatches() { + //using assertField(name) checks the htmls initial "value" attribute, so have to hack around it. + var slugVal = this.evaluate(function () { + return __utils__.getFieldValue('post-setting-slug'); + }); + test.assertEqual(slugVal, 'new-url'); }); }); -CasperTest.begin('Post published date can be changed', 7, function suite(test) { +CasperTest.begin('Post published date can be changed', 5, function suite(test) { // Create a sample post - CasperTest.Routines.createTestPost.run(false); + CasperTest.Routines.createTestPost.run(false); // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -280,17 +273,16 @@ CasperTest.begin('Post published date can be changed', 7, function suite(test) { this.click('a.post-settings'); }); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - test.assertSelectorHasText('.notification-message', 'Publish date successfully changed to 22 May 14 @ 23:39.'); - casper.click('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function testGoodResponse(resource) { + test.assert(400 > resource.status); }); - casper.waitWhileSelector('.notification-success', function () { - test.assert(true, 'notification cleared.'); - test.assertNotVisible('.notification-success', 'success notification should not still exist'); + casper.then(function checkValueMatches() { + //using assertField(name) checks the htmls initial "value" attribute, so have to hack around it. + var dateVal = this.evaluate(function () { + return __utils__.getFieldValue('post-setting-date'); + }); + test.assertEqual(dateVal, '22 May 14 @ 23:39'); }); }); @@ -300,7 +292,7 @@ CasperTest.begin('Post can be changed to static page', 7, function suite(test) { // Begin test casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { - test.assertTitle('Ghost Admin', 'Ghost admin has no title'); + test.assertTitle('Ghost Admin', 'Title is "Ghost Admin"'); test.assertUrlMatch(/ghost\/\d+\/$/, 'Landed on the correct URL'); }); @@ -312,24 +304,22 @@ CasperTest.begin('Post can be changed to static page', 7, function suite(test) { casper.thenClick('.post-settings-menu .post-setting-static-page + label'); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - casper.click('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function waitForSuccess(resource) { + test.assert(400 > resource.status); }); - - casper.waitWhileSelector('.notification-success', function () { - test.assert(true, 'notification cleared.'); - test.assertNotVisible('.notification-success', 'success notification should not still exist'); + //Reload the page so the html can update to have the checked attribute + casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { + test.assertExists('.post-setting-static-page[checked=checked]', 'can turn on static page'); }); casper.thenClick('.post-settings-menu .post-setting-static-page + label'); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - casper.click('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function waitForSuccess(resource) { + test.assert(400 > resource.status); + }); + + //Reload so html can be updated to not have the checked attribute + casper.thenOpenAndWaitForPageLoad('content', function testTitleAndUrl() { + test.assertDoesntExist('.post-setting-static-page[checked=checked]', 'can turn off static page'); }); }); \ No newline at end of file diff --git a/core/test/functional/client/editor_test.js b/core/test/functional/client/editor_test.js index 4bab5425b8..7db0310b4a 100644 --- a/core/test/functional/client/editor_test.js +++ b/core/test/functional/client/editor_test.js @@ -243,7 +243,7 @@ casper.thenOpenAndWaitForPageLoad('editor', function testTitleAndUrl() { }); }); -CasperTest.begin('Post settings menu', 31, function suite(test) { +CasperTest.begin('Post settings menu', 30, function suite(test) { casper.thenOpenAndWaitForPageLoad('editor', function testTitleAndUrl() { test.assertTitle('Ghost Admin', 'Ghost admin has no title'); test.assertUrlMatch(/ghost\/editor\/$/, 'Landed on the correct URL'); @@ -337,45 +337,46 @@ CasperTest.begin('Post settings menu', 31, function suite(test) { this.click('#publish-bar a.post-settings'); }); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - test.assertSelectorHasText('.notification-success', 'Publish date successfully changed to 10 May 14 @ 00:17.'); - casper.thenClick('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function testGoodResponse(resource) { + test.assert(400 > resource.status); }); - casper.waitWhileSelector('.notification-success'); - - // Test Static Page conversion - casper.thenClick('#publish-bar a.post-settings'); - - casper.waitUntilVisible('#publish-bar .post-settings-menu', function onSuccess() { - test.assert(true, 'post settings menu should be visible after clicking post-settings icon'); + casper.then(function checkValueMatches() { + //using assertField(name) checks the htmls initial "value" attribute, so have to hack around it. + var dateVal = this.evaluate(function () { + return __utils__.getFieldValue('post-setting-date'); + }); + test.assertEqual(dateVal, '10 May 14 @ 00:17'); }); - + + // Test static page toggling casper.thenClick('.post-settings-menu .post-setting-static-page + label'); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - casper.click('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function testGoodResponse(resource) { + test.assert(400 > resource.status); }); - - casper.waitWhileSelector('.notification-success', function () { - test.assert(true, 'notification cleared.'); - test.assertNotVisible('.notification-success', 'success notification should not still exist'); + + casper.then(function staticPageIsCheckedTest() { + var checked = casper.evaluate(function evalCheckedProp() { + return document.querySelector('.post-setting-static-page').checked + }); + test.assert(checked, 'Turned post into static page.'); }); - + casper.thenClick('.post-settings-menu .post-setting-static-page + label'); - casper.waitForSelector('.notification-success', function waitForSuccess() { - test.assert(true, 'got success notification'); - casper.click('.notification-success a.close'); - }, function onTimeout() { - test.assert(false, 'No success notification'); + casper.waitForResource(/\/posts\/\d+\/\?include=tags/, function testGoodResponse(resource) { + test.assert(400 > resource.status); }); + + casper.then(function staticPageIsCheckedTest() { + var checked = casper.evaluate(function evalCheckedProp() { + return document.querySelector('.post-setting-static-page').checked + }); + test.assert(!checked, 'Turned page into post.'); + }); + + // Test Delete Post Modal casper.thenClick('.post-settings-menu a.delete');