diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 90cef2b440..d87464e53a 100644 --- a/core/server/models/base/index.js +++ b/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 = {}; diff --git a/core/server/models/plugins/pagination.js b/core/server/models/plugins/pagination.js index 012f048adb..a9523a2d5a 100644 --- a/core/server/models/plugins/pagination.js +++ b/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) + }; + }); }); } }); diff --git a/core/server/models/post.js b/core/server/models/post.js index f819ba80af..dbe13c007d 100644 --- a/core/server/models/post.js +++ b/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 */ diff --git a/core/test/integration/api/api_posts_spec.js b/core/test/integration/api/api_posts_spec.js index 3d74b1ff8a..491ff7786c 100644 --- a/core/test/integration/api/api_posts_spec.js +++ b/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); }); diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index e668637f57..483b8e43fc 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/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); }); diff --git a/core/test/unit/models_plugins/pagination_spec.js b/core/test/unit/models_plugins/pagination_spec.js index 0efa78f7a6..cd6c4c97c3 100644 --- a/core/test/unit/models_plugins/pagination_spec.js +++ b/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(); - }); - }); }); });