change default order of posts (#7096)

closes #6932
- new default order of posts: scheduled, draft, published
- invent orderDefaultRaw fn for each model
- each model is able to create a default raw order query
- separate count and fetch query for fetchPage, because the count query where group/order statements attached

core/server/models/base/index.js

@@ -348,9 +348,12 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
 
         if (options.order) {
             options.order = self.parseOrderOption(options.order, options.include);
+        } else if (self.orderDefaultRaw) {
+            options.orderRaw = self.orderDefaultRaw();
         } else {
             options.order = self.orderDefaultOptions();
         }
+
         return itemCollection.fetchPage(options).then(function formatResponse(response) {
             var data = {};
 

core/server/models/plugins/pagination.js

@@ -2,8 +2,6 @@
 //
 // Extends Bookshelf.Model with a `fetchPage` method. Handles everything to do with paginated requests.
 var _          = require('lodash'),
-    Promise    = require('bluebird'),
-
     defaults,
     paginationUtils,
     pagination;
@@ -130,6 +128,11 @@ pagination = function pagination(bookshelf) {
         /**
          * ### Fetch page
          * A `fetch` extension to get a paginated set of items from a collection
+         *
+         * We trigger two queries:
+         * 1. count query to know how many pages left (important: we don't attach any group/order statements!)
+         * 2. the actualy fetch query with limit and page property
+         *
          * @param {options} options
          * @returns {paginatedResult} set of results + pagination metadata
          */
@@ -140,57 +143,58 @@ pagination = function pagination(bookshelf) {
             // Get the table name and idAttribute for this model
             var tableName = _.result(this.constructor.prototype, 'tableName'),
                 idAttribute = _.result(this.constructor.prototype, 'idAttribute'),
-                countPromise,
-                collectionPromise,
-                self = this;
+                self = this,
+                countPromise = this.query().clone().select(
+                    bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate')
+                );
+
+            // the debug flag doesn't work for the raw knex count query!
+            if (this.debug) {
+                console.log('COUNT', countPromise.toQuery());
+            }
 
             // #### Pre count clauses
             // Add any where or join clauses which need to be included with the aggregate query
 
             // Clone the base query & set up a promise to get the count of total items in the full set
             // Due to lack of support for count distinct, this is pretty complex.
-            countPromise = this.query().clone().select(
-                bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate')
-            );
+            return countPromise.then(function (countResult) {
+                // #### Post count clauses
+                // Add any where or join clauses which need to NOT be included with the aggregate query
 
-            // #### Post count clauses
-            // Add any where or join clauses which need to NOT be included with the aggregate query
+                // Setup the pagination parameters so that we return the correct items from the set
+                paginationUtils.addLimitAndOffset(self, options);
 
-            // Setup the pagination parameters so that we return the correct items from the set
-            paginationUtils.addLimitAndOffset(self, options);
+                // Apply ordering options if they are present
+                if (options.order && !_.isEmpty(options.order)) {
+                    _.forOwn(options.order, function (direction, property) {
+                        if (property === 'count.posts') {
+                            self.query('orderBy', 'count__posts', direction);
+                        } else {
+                            self.query('orderBy', tableName + '.' + property, direction);
+                        }
+                    });
+                } else if (options.orderRaw) {
+                    self.query(function (qb) {
+                        qb.orderByRaw(options.orderRaw);
+                    });
+                }
 
-            // Apply ordering options if they are present
-            if (options.order && !_.isEmpty(options.order)) {
-                _.forOwn(options.order, function (direction, property) {
-                    if (property === 'count.posts') {
-                        self.query('orderBy', 'count__posts', direction);
-                    } else {
-                        self.query('orderBy', tableName + '.' + property, direction);
-                    }
-                });
-            }
+                if (options.groups && !_.isEmpty(options.groups)) {
+                    _.each(options.groups, function (group) {
+                        self.query('groupBy', group);
+                    });
+                }
 
-            if (options.groups && !_.isEmpty(options.groups)) {
-                _.each(options.groups, function (group) {
-                    self.query('groupBy', group);
-                });
-            }
-
-            if (this.debug) {
-                console.log('COUNT', countPromise.toQuery());
-            }
-
-            // Setup the promise to do a fetch on our collection, running the specified query
-            // @TODO: ensure option handling is done using an explicit pick elsewhere
-            collectionPromise = self.fetchAll(_.omit(options, ['page', 'limit']));
-
-            // Resolve the two promises
-            return Promise.join(collectionPromise, countPromise).then(function formatResponse(results) {
-                // Format the collection & count result into `{collection: [], pagination: {}}`
-                return {
-                    collection: results[0],
-                    pagination: paginationUtils.formatResponse(results[1][0] ? results[1][0].aggregate : 0, options)
-                };
+                // Setup the promise to do a fetch on our collection, running the specified query
+                // @TODO: ensure option handling is done using an explicit pick elsewhere
+                return self.fetchAll(_.omit(options, ['page', 'limit']))
+                    .then(function (fetchResult) {
+                        return {
+                            collection: fetchResult,
+                            pagination: paginationUtils.formatResponse(countResult[0] ? countResult[0].aggregate : 0, options)
+                        };
+                    });
             });
         }
     });

core/server/models/post.js

@@ -429,6 +429,16 @@ Post = ghostBookshelf.Model.extend({
         };
     },
 
+    orderDefaultRaw: function () {
+        return '' +
+            'CASE WHEN posts.status = \'scheduled\' THEN 1 ' +
+            'WHEN posts.status = \'draft\' THEN 2 ' +
+            'ELSE 3 END ASC,' +
+            'posts.published_at DESC,' +
+            'posts.updated_at DESC,' +
+            'posts.id DESC';
+    },
+
     /**
      * @deprecated in favour of filter
      */

core/test/integration/api/api_posts_spec.js

@@ -21,6 +21,7 @@ describe('Post API', function () {
             done();
         }).catch(done);
     });
+
     beforeEach(function (done) {
         Promise.mapSeries(testUtils.DataGenerator.forKnex.tags, function (tag) {
             return models.Tag.add(tag, {context: {internal:true}});
@@ -28,6 +29,7 @@ describe('Post API', function () {
             done();
         }).catch(done);
     });
+
     beforeEach(function (done) {
         db.knex('posts_tags').insert(testUtils.DataGenerator.forKnex.posts_tags)
             .then(function () {
@@ -43,17 +45,45 @@ describe('Post API', function () {
     should.exist(PostAPI);
 
     describe('Browse', function () {
-        it('can fetch featured posts', function (done) {
-            PostAPI.browse({context: {user: 1}, filter: 'featured:true'}).then(function (results) {
+        it('can fetch all posts with internal context in correct order', function (done) {
+            PostAPI.browse({context: {internal: true}}).then(function (results) {
                 should.exist(results.posts);
-                results.posts.length.should.eql(4);
-                results.posts[0].featured.should.eql(true);
+                results.posts.length.should.eql(8);
+
+                results.posts[0].status.should.eql('scheduled');
+
+                results.posts[1].status.should.eql('draft');
+                results.posts[2].status.should.eql('draft');
+
+                results.posts[3].status.should.eql('published');
+                results.posts[4].status.should.eql('published');
+                results.posts[5].status.should.eql('published');
+                results.posts[6].status.should.eql('published');
+                results.posts[7].status.should.eql('published');
 
                 done();
             }).catch(done);
         });
 
-        it('can exclude featured posts', function (done) {
+        it('can fetch featured posts for user 1', function (done) {
+            PostAPI.browse({context: {user: 1}, filter: 'featured:true'}).then(function (results) {
+                should.exist(results.posts);
+                results.posts.length.should.eql(4);
+                results.posts[0].featured.should.eql(true);
+                done();
+            }).catch(done);
+        });
+
+        it('can fetch featured posts for user 2', function (done) {
+            PostAPI.browse({context: {user: 2}, filter: 'featured:true'}).then(function (results) {
+                should.exist(results.posts);
+                results.posts.length.should.eql(4);
+                results.posts[0].featured.should.eql(true);
+                done();
+            }).catch(done);
+        });
+
+        it('can exclude featured posts for user 1', function (done) {
             PostAPI.browse({context: {user: 1}, status: 'all', filter: 'featured:false'}).then(function (results) {
                 should.exist(results.posts);
                 results.posts.length.should.eql(1);
@@ -135,7 +165,8 @@ describe('Post API', function () {
             PostAPI.browse({context: {user: 1}, page: 1, limit: 2, status: 'all'}).then(function (results) {
                 should.exist(results.posts);
                 results.posts.length.should.eql(2);
-                results.posts[0].slug.should.eql('unfinished');
+                results.posts[0].slug.should.eql('scheduled-post');
+                results.posts[1].slug.should.eql('unfinished');
                 results.meta.pagination.page.should.eql(1);
                 results.meta.pagination.next.should.eql(2);
 
@@ -147,7 +178,8 @@ describe('Post API', function () {
             PostAPI.browse({context: {user: 1}, page: 2, limit: 2, status: 'all'}).then(function (results) {
                 should.exist(results.posts);
                 results.posts.length.should.eql(2);
-                results.posts[0].slug.should.eql('short-and-sweet');
+                results.posts[0].slug.should.eql('not-so-short-bit-complex');
+                results.posts[1].slug.should.eql('short-and-sweet');
                 results.meta.pagination.page.should.eql(2);
                 results.meta.pagination.next.should.eql(3);
                 results.meta.pagination.prev.should.eql(1);
@@ -217,8 +249,10 @@ describe('Post API', function () {
 
         it('can include tags', function (done) {
             PostAPI.browse({context: {user: 1}, status: 'all', include: 'tags'}).then(function (results) {
-                should.exist(results.posts[0].tags[0].name);
-                results.posts[0].tags[0].name.should.eql('pollo');
+                results.posts[0].tags.length.should.eql(0);
+                results.posts[1].tags.length.should.eql(1);
+
+                results.posts[1].tags[0].name.should.eql('pollo');
                 done();
             }).catch(done);
         });

core/test/integration/model/model_posts_spec.js

@@ -77,7 +77,6 @@ describe('Post Model', function () {
                 PostModel.findAll().then(function (results) {
                     should.exist(results);
                     results.length.should.be.above(1);
-
                     done();
                 }).catch(done);
             });

core/test/unit/models_plugins/pagination_spec.js

@@ -192,7 +192,7 @@ describe('pagination', function () {
                 toQuery: sandbox.stub()
             };
             mockQuery.clone.returns(mockQuery);
-            mockQuery.select.returns([{aggregate: 1}]);
+            mockQuery.select.returns(Promise.resolve([{aggregate: 1}]));
 
             model = function () {};
 
@@ -340,7 +340,7 @@ describe('pagination', function () {
 
         it('returns expected response even when aggregate is empty', function (done) {
             // override aggregate response
-            mockQuery.select.returns([]);
+            mockQuery.select.returns(Promise.resolve([]));
             paginationUtils.parseOptions.returns({});
 
             bookshelf.Model.prototype.fetchPage().then(function (result) {
@@ -352,18 +352,5 @@ describe('pagination', function () {
                 done();
             });
         });
-
-        it('will output sql statements in debug mode', function (done) {
-            model.prototype.debug = true;
-            mockQuery.select.returns({toQuery: function () {}});
-            paginationUtils.parseOptions.returns({});
-
-            var consoleSpy = sandbox.spy(console, 'log');
-
-            bookshelf.Model.prototype.fetchPage().then(function () {
-                consoleSpy.calledOnce.should.be.true();
-                done();
-            });
-        });
     });
 });