TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-04 17:04:59 +03:00
Code Issues Projects Releases Wiki Activity

Merge pull request #6037 from ErisDS/pagination

Browse Source 
Pagination cleanup & improvements
This commit is contained in:
Sebastian Gierlinger 2015-11-03 13:53:18 +01:00
commit c4f9cde008
4 changed files with 119 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
core/server/models/base/pagination.js
View File

@ -48,12 +48,12 @@ paginationUtils = {
/**
* ### Query
* Apply the necessary parameters to paginate the query
* @param {Bookshelf.Model|Bookshelf.Collection} itemCollection
* @param {bookshelf.Model} model
* @param {options} options
*/
addLimitAndOffset: function addLimitAndOffset(itemCollection, options) {
addLimitAndOffset: function addLimitAndOffset(model, options) {
if (_.isNumber(options.limit)) {
itemCollection
model
.query('limit', options.limit)
.query('offset', options.limit * (options.page - 1));
}
@ -97,26 +97,26 @@ paginationUtils = {
/**
* ### Pagination Object
* @typedef {Object} pagination
* @property {Number} `page` \- page in set to display
* @property {Number|String} `limit` \- no. results per page, or 'all'
* @property {Number} `pages` \- total no. pages in the full set
* @property {Number} `total` \- total no. items in the full set
* @property {Number|null} `next` \- next page
* @property {Number|null} `prev` \- previous page
* @property {Number} page \- page in set to display
* @property {Number|String} limit \- no. results per page, or 'all'
* @property {Number} pages \- total no. pages in the full set
* @property {Number} total \- total no. items in the full set
* @property {Number|null} next \- next page
* @property {Number|null} prev \- previous page
*/
/**
* ### Fetch Page Options
* @typedef {Object} options
* @property {Number} `page` \- page in set to display
* @property {Number|String} `limit` \- no. results per page, or 'all'
* @property {Object} `order` \- set of order by params and directions
* @property {Number} page \- page in set to display
* @property {Number|String} limit \- no. results per page, or 'all'
* @property {Object} order \- set of order by params and directions
*/
/**
* ### Fetch Page Response
* @typedef {Object} paginatedResult
* @property {Array} `collection` \- set of results
* @property {Array} collection \- set of results
* @property {pagination} pagination \- pagination metadata
*/
@ -141,8 +141,6 @@ 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'),
// Create a new collection for running `this` query, ensuring we're using collection, rather than model
collection = this.constructor.collection(),
countPromise,
collectionPromise,
self = this;
@ -156,48 +154,31 @@ pagination = function pagination(bookshelf) {
bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate')
);
// Clone the base query into our collection
collection._knex = this.query().clone();
// #### 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(collection, options);
paginationUtils.addLimitAndOffset(self, options);
// Apply ordering options if they are present
if (options.order && !_.isEmpty(options.order)) {
_.forOwn(options.order, function (direction, property) {
collection.query('orderBy', tableName + '.' + property, direction);
self.query('orderBy', tableName + '.' + property, direction);
});
}
if (options.groups && !_.isEmpty(options.groups)) {
_.each(options.groups, function (group) {
collection.query('groupBy', group);
self.query('groupBy', group);
});
}
// Apply count options if they are present
baseUtils.collectionQuery.count(collection, options);
this.resetQuery();
if (this.relatedData) {
collection.relatedData = this.relatedData;
}
// ensure that our model (self) gets the correct events fired upon it
collection
.on('fetching', function (collection, columns, options) {
return self.triggerThen('fetching:collection', collection, columns, options);
})
.on('fetched', function (collection, resp, options) {
return self.triggerThen('fetched:collection', collection, resp, options);
});
baseUtils.collectionQuery.count(self, 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
collectionPromise = collection.fetch(_.omit(options, ['page', 'limit']));
collectionPromise = self.fetchAll(_.omit(options, ['page', 'limit']));
// Resolve the two promises
return Promise.join(collectionPromise, countPromise).then(function formatResponse(results) {

8
core/server/models/base/utils.js
View File

@ -8,9 +8,9 @@ var _ = require('lodash'),
addPostCount,
tagUpdate;
addPostCount = function addPostCount(options, itemCollection) {
addPostCount = function addPostCount(options, model) {
if (options.include && options.include.indexOf('post_count') > -1) {
itemCollection.query('columns', 'tags.*', function (qb) {
model.query('columns', 'tags.*', function (qb) {
qb.count('posts_tags.post_id').from('posts_tags').whereRaw('tag_id = tags.id').as('post_count');
});
@ -20,8 +20,8 @@ addPostCount = function addPostCount(options, itemCollection) {
};
collectionQuery = {
count: function count(collection, options) {
addPostCount(options, collection);
count: function count(model, options) {
addPostCount(options, model);
}
};

30
core/test/integration/api/advanced_browse_spec.js
View File

@ -512,5 +512,35 @@ describe('Filter Param Spec', function () {
done();
});
});
describe('Empty results', function () {
it('Will return empty result if tag has no posts', function (done) {
PostAPI.browse({filter: 'tag:no-posts', include: 'tag,author'}).then(function (result) {
// 1. Result should have the correct base structure
should.exist(result);
result.should.have.property('posts');
result.should.have.property('meta');
// 2. The data part of the response should be correct
// We should have 4 matching items
result.posts.should.be.an.Array.with.lengthOf(0);
// 3. The meta object should contain the right details
result.meta.should.have.property('pagination');
result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']);
result.meta.pagination.page.should.eql(1);
result.meta.pagination.limit.should.eql(15);
result.meta.pagination.pages.should.eql(1);
result.meta.pagination.total.should.eql(0);
should.equal(result.meta.pagination.next, null);
should.equal(result.meta.pagination.prev, null);
// NOTE: new query does not have meta filter
result.meta.should.not.have.property('filters');
done();
}).catch(done);
});
});
});
});

121
core/test/unit/models_pagination_spec.js
View File

@ -175,7 +175,7 @@ describe('pagination', function () {
});
describe('fetchPage', function () {
var model, bookshelf, knex, on, mockQuery, fetch, colQuery;
var model, bookshelf, knex, mockQuery;
before(function () {
paginationUtils = pagination.__get__('paginationUtils');
@ -195,21 +195,10 @@ describe('pagination', function () {
mockQuery.clone.returns(mockQuery);
mockQuery.select.returns([{aggregate: 1}]);
fetch = sandbox.stub().returns(Promise.resolve({}));
colQuery = sandbox.stub();
on = function () { return this; };
on = sandbox.spy(on);
model = function () {};
model.prototype.constructor = {
collection: sandbox.stub().returns({
on: on,
fetch: fetch,
query: colQuery
})
};
model.prototype.fetchAll = sandbox.stub().returns(Promise.resolve({}));
model.prototype.query = sandbox.stub();
model.prototype.resetQuery = sandbox.stub();
model.prototype.query.returns(mockQuery);
knex = {raw: sandbox.stub().returns(Promise.resolve())};
@ -230,16 +219,11 @@ describe('pagination', function () {
bookshelf.Model.prototype.fetchPage().then(function () {
sinon.assert.callOrder(
paginationUtils.parseOptions,
model.prototype.constructor.collection,
model.prototype.query,
mockQuery.clone,
mockQuery.select,
model.prototype.query,
mockQuery.clone,
paginationUtils.addLimitAndOffset,
on,
on,
fetch,
model.prototype.fetchAll,
paginationUtils.formatResponse
);
@ -249,26 +233,17 @@ describe('pagination', function () {
paginationUtils.addLimitAndOffset.calledOnce.should.be.true;
paginationUtils.formatResponse.calledOnce.should.be.true;
model.prototype.constructor.collection.calledOnce.should.be.true;
model.prototype.constructor.collection.calledWith().should.be.true;
model.prototype.query.calledTwice.should.be.true;
model.prototype.query.calledOnce.should.be.true;
model.prototype.query.firstCall.calledWith().should.be.true;
model.prototype.query.secondCall.calledWith().should.be.true;
mockQuery.clone.calledTwice.should.be.true;
mockQuery.clone.calledOnce.should.be.true;
mockQuery.clone.firstCall.calledWith().should.be.true;
mockQuery.clone.secondCall.calledWith().should.be.true;
mockQuery.select.calledOnce.should.be.true;
mockQuery.select.calledWith().should.be.true;
on.calledTwice.should.be.true;
on.firstCall.calledWith('fetching').should.be.true;
on.secondCall.calledWith('fetched').should.be.true;
fetch.calledOnce.should.be.true;
fetch.calledWith({}).should.be.true;
model.prototype.fetchAll.calledOnce.should.be.true;
model.prototype.fetchAll.calledWith({}).should.be.true;
done();
}).catch(done);
@ -276,22 +251,17 @@ describe('pagination', function () {
it('fetchPage calls all paginationUtils and methods when order set', function (done) {
var orderOptions = {order: {id: 'DESC'}};
paginationUtils.parseOptions.returns(orderOptions);
bookshelf.Model.prototype.fetchPage(orderOptions).then(function () {
sinon.assert.callOrder(
paginationUtils.parseOptions,
model.prototype.constructor.collection,
model.prototype.query,
mockQuery.clone,
mockQuery.select,
model.prototype.query,
mockQuery.clone,
paginationUtils.addLimitAndOffset,
colQuery,
on,
on,
fetch,
model.prototype.query,
model.prototype.fetchAll,
paginationUtils.formatResponse
);
@ -301,29 +271,57 @@ describe('pagination', function () {
paginationUtils.addLimitAndOffset.calledOnce.should.be.true;
paginationUtils.formatResponse.calledOnce.should.be.true;
model.prototype.constructor.collection.calledOnce.should.be.true;
model.prototype.constructor.collection.calledWith().should.be.true;
model.prototype.query.calledTwice.should.be.true;
model.prototype.query.firstCall.calledWith().should.be.true;
model.prototype.query.secondCall.calledWith().should.be.true;
model.prototype.query.secondCall.calledWith('orderBy', 'undefined.id', 'DESC').should.be.true;
mockQuery.clone.calledTwice.should.be.true;
mockQuery.clone.calledOnce.should.be.true;
mockQuery.clone.firstCall.calledWith().should.be.true;
mockQuery.clone.secondCall.calledWith().should.be.true;
mockQuery.select.calledOnce.should.be.true;
mockQuery.select.calledWith().should.be.true;
colQuery.calledOnce.should.be.true;
colQuery.calledWith('orderBy', 'undefined.id', 'DESC').should.be.true;
model.prototype.fetchAll.calledOnce.should.be.true;
model.prototype.fetchAll.calledWith(orderOptions).should.be.true;
on.calledTwice.should.be.true;
on.firstCall.calledWith('fetching').should.be.true;
on.secondCall.calledWith('fetched').should.be.true;
done();
}).catch(done);
});
fetch.calledOnce.should.be.true;
fetch.calledWith(orderOptions).should.be.true;
it('fetchPage calls all paginationUtils and methods when group by set', function (done) {
var groupOptions = {groups: ['posts.id']};
paginationUtils.parseOptions.returns(groupOptions);
bookshelf.Model.prototype.fetchPage(groupOptions).then(function () {
sinon.assert.callOrder(
paginationUtils.parseOptions,
model.prototype.query,
mockQuery.clone,
mockQuery.select,
paginationUtils.addLimitAndOffset,
model.prototype.query,
model.prototype.fetchAll,
paginationUtils.formatResponse
);
paginationUtils.parseOptions.calledOnce.should.be.true;
paginationUtils.parseOptions.calledWith(groupOptions).should.be.true;
paginationUtils.addLimitAndOffset.calledOnce.should.be.true;
paginationUtils.formatResponse.calledOnce.should.be.true;
model.prototype.query.calledTwice.should.be.true;
model.prototype.query.firstCall.calledWith().should.be.true;
model.prototype.query.secondCall.calledWith('groupBy', 'posts.id').should.be.true;
mockQuery.clone.calledOnce.should.be.true;
mockQuery.clone.firstCall.calledWith().should.be.true;
mockQuery.select.calledOnce.should.be.true;
mockQuery.select.calledWith().should.be.true;
model.prototype.fetchAll.calledOnce.should.be.true;
model.prototype.fetchAll.calledWith(groupOptions).should.be.true;
done();
}).catch(done);
@ -340,5 +338,20 @@ describe('pagination', function () {
done();
});
});
it('fetchPage returns expected response even when aggregate is empty', function (done) {
// override aggregate response
mockQuery.select.returns([]);
paginationUtils.parseOptions.returns({});
bookshelf.Model.prototype.fetchPage().then(function (result) {
result.should.have.ownProperty('collection');
result.should.have.ownProperty('pagination');
result.collection.should.be.an.Object;
result.pagination.should.be.an.Object;
done();
});
});
});
});