From 5db08ee333601987af9ba54fdfaa492d5c5a306c Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 26 Jun 2014 08:52:00 +0100 Subject: [PATCH] Ember post order matches server post order fixes #3008 - this effectively breaks the sort order on the client, because the serverside order is marginally broken. --- core/client/controllers/posts.js | 45 ++++++++++++--------- core/test/functional/client/content_test.js | 21 +++++----- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/core/client/controllers/posts.js b/core/client/controllers/posts.js index 4ec82c5c02..9387ab0155 100644 --- a/core/client/controllers/posts.js +++ b/core/client/controllers/posts.js @@ -15,31 +15,40 @@ var PostsController = Ember.ArrayController.extend({ // published_at: DESC // updated_at: DESC orderBy: function (item1, item2) { - var status1 = item1.get('status'), - status2 = item2.get('status'), - updatedAt1, updatedAt2, - publishedAt1, publishedAt2; + function publishedAtCompare() { + var published1 = item1.get('published_at'), + published2 = item2.get('published_at'); - if (status1 === status2) { - if (status1 === 'draft') { - updatedAt1 = item1.get('updated_at'); - updatedAt2 = item2.get('updated_at'); - - return (new Date(updatedAt1)) < (new Date(updatedAt2)) ? 1 : -1; - } else { - publishedAt1 = item1.get('published_at'); - publishedAt2 = item2.get('published_at'); - - return (new Date(publishedAt1)) < new Date(publishedAt2) ? 1 : -1; + if (!published1 && !published2) { + return 0; } + + if (!published1 && published2) { + return -1; + } + + if (!published2 && published1) { + return 1; + } + + return Ember.compare(item1.get('published_at').valueOf(), item2.get('published_at').valueOf()); } - if (status2 === 'draft') { - return 1; + var statusResult = Ember.compare(item1.get('status'), item2.get('status')), + updatedAtResult = Ember.compare(item1.get('updated_at').valueOf(), item2.get('updated_at').valueOf()), + publishedAtResult = publishedAtCompare(); + + if (statusResult === 0) { + if (publishedAtResult === 0) { + // This should be DESC + return updatedAtResult * -1; + } + // This should be DESC + return publishedAtResult * -1; } - return -1; + return statusResult; }, // set from PostsRoute diff --git a/core/test/functional/client/content_test.js b/core/test/functional/client/content_test.js index 368f2fdffc..6f259067aa 100644 --- a/core/test/functional/client/content_test.js +++ b/core/test/functional/client/content_test.js @@ -3,7 +3,7 @@ /*globals CasperTest, casper, testPost, newUser */ -CasperTest.begin('Content screen is correct', 17, function suite(test) { +CasperTest.begin('Content screen is correct', 21, function suite(test) { // First, create a sample post for testing (this should probably be a routine) CasperTest.Routines.createTestPost.run(false); @@ -55,16 +55,15 @@ CasperTest.begin('Content screen is correct', 17, function suite(test) { test.assertExists('.post-settings-menu a.delete', 'post settings delete this post exists'); }); - // A bug is causing this to not always be activated. TODO: Uncomment when fixed #3008 -// casper.then(function testActiveItem() { -// test.assertExists('.content-list-content li:first-of-type .active', 'first item is active'); -// test.assertDoesntExist('.content-list-content li:nth-of-type(2) .active', 'second item is not active'); -// -// // Ember adds script tags into the list so we need to use nth-of-type -// }).thenClick('.content-list-content li:nth-of-type(2) a', function then() { -// test.assertDoesntExist('.content-list-content li:first-of-type .active', 'first item is not active'); -// test.assertExists('.content-list-content li:nth-of-type(2) .active', 'second item is active'); -// }); + casper.then(function testActiveItem() { + test.assertExists('.content-list-content li:first-of-type .active', 'first item is active'); + test.assertDoesntExist('.content-list-content li:nth-of-type(2) .active', 'second item is not active'); + + // Ember adds script tags into the list so we need to use nth-of-type + }).thenClick('.content-list-content li:nth-of-type(2) a', function then() { + test.assertDoesntExist('.content-list-content li:first-of-type .active', 'first item is not active'); + test.assertExists('.content-list-content li:nth-of-type(2) .active', 'second item is active'); + }); }); CasperTest.begin('Content list shows correct post status', 7, function testStaticPageStatus(test) {