Skip to content

Commit

Permalink
Move pagination formatting into a util
Browse files Browse the repository at this point in the history 
refs TryGhost#2896

- moves repeated code out of models
- creates a new file for unit-testable code (this should be moved in future)
- adds a default for `page` as that seems sensible
- adds 100% test coverage for the new file
  • Loading branch information
@ErisDS
ErisDS committed Jun 15, 2015
1 parent b6cbd2d commit 16f98ee
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 84 deletions.
36 changes: 36 additions & 0 deletions core/server/models/base/utils.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* # Utils
* Parts of the model code which can be split out and unit tested
*/

/**
* Takes the number of items returned and original options and calculates all of the pagination meta data
* TODO: Could be moved to either middleware or a bookshelf plugin?

This comment has been minimized.

Sign in to view

Copy link
@ErisDS

ErisDS Jun 15, 2015

Author Owner

I don't think this will be the final resting place for this code, but as this refactor progresses I think homes for things will become more apparent. In the meantime, I find it useful to split out unit-testable code from the denseness of the model layer.
* @param {Number} totalItems
* @param {Object} options
* @returns {Object} pagination
*/
module.exports.paginateResponse = function paginateResponse(totalItems, options) {
var calcPages = Math.ceil(totalItems / options.limit) || 0,
pagination = {};

pagination.page = options.page || 1;
pagination.limit = options.limit;
pagination.pages = calcPages === 0 ? 1 : calcPages;
pagination.total = totalItems;
pagination.next = null;
pagination.prev = null;

if (pagination.pages > 1) {
if (pagination.page === 1) {
pagination.next = pagination.page + 1;
} else if (pagination.page === pagination.pages) {
pagination.prev = pagination.page - 1;
} else {
pagination.next = pagination.page + 1;
pagination.prev = pagination.page - 1;
}
}

return pagination;
};
39 changes: 8 additions & 31 deletions core/server/models/post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ var _ = require('lodash'),
converter = new Showdown.converter({extensions: ['ghostgfm', 'footnotes', 'highlight']}),
ghostBookshelf = require('./base'),
events = require('../events'),

config = require('../config'),
config = require('../config'),
paginateResponse = require('./base/utils').paginateResponse,
permalinkSetting = '',
getPermalinkSetting,
Post,
Expand Down Expand Up @@ -483,46 +483,23 @@ Post = ghostBookshelf.Model.extend({

return Promise.join(collectionPromise, countPromise);
}).then(function then(results) {
var totalPosts = parseInt(results[1][0].aggregate, 10),
calcPages = Math.ceil(totalPosts / options.limit) || 0,
postCollection = results[0],
pagination = {},
meta = {},
var postCollection = results[0],
data = {};

pagination.page = options.page;
pagination.limit = options.limit;
pagination.pages = calcPages === 0 ? 1 : calcPages;
pagination.total = totalPosts;
pagination.next = null;
pagination.prev = null;

data.posts = postCollection.toJSON(options);
data.meta = meta;
meta.pagination = pagination;

if (pagination.pages > 1) {
if (pagination.page === 1) {
pagination.next = pagination.page + 1;
} else if (pagination.page === pagination.pages) {
pagination.prev = pagination.page - 1;
} else {
pagination.next = pagination.page + 1;
pagination.prev = pagination.page - 1;
}
}
data.meta = {pagination: paginateResponse(results[1][0].aggregate, options)};

if (tagInstance) {
meta.filters = {};
data.meta.filters = {};
if (!tagInstance.isNew()) {
meta.filters.tags = [tagInstance.toJSON(options)];
data.meta.filters.tags = [tagInstance.toJSON(options)];
}
}

if (authorInstance) {
meta.filters = {};
data.meta.filters = {};
if (!authorInstance.isNew()) {
meta.filters.author = authorInstance.toJSON(options);
data.meta.filters.author = authorInstance.toJSON(options);
}
}

Expand Down
29 changes: 3 additions & 26 deletions core/server/models/tag.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var _ = require('lodash'),
errors = require('../errors'),
ghostBookshelf = require('./base'),
events = require('../events'),
paginateResponse = require('./base/utils').paginateResponse,

Tag,
Tags;
Expand Down Expand Up @@ -141,42 +142,18 @@ Tag = ghostBookshelf.Model.extend({
collectionPromise = tagCollection.fetch(_.omit(options, 'page', 'limit'));

// Find total number of tags

qb = ghostBookshelf.knex('tags');

if (options.where) {
qb.where(options.where);
}

return Promise.join(collectionPromise, qb.count('tags.id as aggregate')).then(function then(results) {
var totalTags = results[1][0].aggregate,
calcPages = Math.ceil(totalTags / options.limit) || 0,
tagCollection = results[0],
pagination = {},
meta = {},
var tagCollection = results[0],
data = {};

pagination.page = options.page;
pagination.limit = options.limit;
pagination.pages = calcPages === 0 ? 1 : calcPages;
pagination.total = totalTags;
pagination.next = null;
pagination.prev = null;

data.tags = tagCollection.toJSON(options);
data.meta = meta;
meta.pagination = pagination;

if (pagination.pages > 1) {
if (pagination.page === 1) {
pagination.next = pagination.page + 1;
} else if (pagination.page === pagination.pages) {
pagination.prev = pagination.page - 1;
} else {
pagination.next = pagination.page + 1;
pagination.prev = pagination.page - 1;
}
}
data.meta = {pagination: paginateResponse(results[1][0].aggregate, options)};

return data;
})
Expand Down
32 changes: 5 additions & 27 deletions core/server/models/user.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var _ = require('lodash'),
validation = require('../data/validation'),
config = require('../config'),
events = require('../events'),
paginateResponse = require('./base/utils').paginateResponse,

bcryptGenSalt = Promise.promisify(bcrypt.genSalt),
bcryptHash = Promise.promisify(bcrypt.hash),
Expand Down Expand Up @@ -331,38 +332,15 @@ User = ghostBookshelf.Model.extend({
})
// Format response of data
.then(function then(results) {
var totalUsers = parseInt(results[1][0].aggregate, 10),
calcPages = Math.ceil(totalUsers / options.limit) || 0,
pagination = {},
meta = {},
data = {};

pagination.page = options.page;
pagination.limit = options.limit;
pagination.pages = calcPages === 0 ? 1 : calcPages;
pagination.total = totalUsers;
pagination.next = null;
pagination.prev = null;
var data = {};

data.users = userCollection.toJSON(options);
data.meta = meta;
meta.pagination = pagination;

if (pagination.pages > 1) {
if (pagination.page === 1) {
pagination.next = pagination.page + 1;
} else if (pagination.page === pagination.pages) {
pagination.prev = pagination.page - 1;
} else {
pagination.next = pagination.page + 1;
pagination.prev = pagination.page - 1;
}
}
data.meta = {pagination: paginateResponse(results[1][0].aggregate, options)};

if (roleInstance) {
meta.filters = {};
data.meta.filters = {};
if (!roleInstance.isNew()) {
meta.filters.roles = [roleInstance.toJSON(options)];
data.meta.filters.roles = [roleInstance.toJSON(options)];
}
}

Expand Down
77 changes: 77 additions & 0 deletions core/test/unit/models_base_utils_spec.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*globals describe, it*/
/*jshint expr:true*/
var should = require('should'),

// Thing we're testing
utils = require('../../server/models/base/utils');

// To stop jshint complaining
should.equal(true, true);

describe('paginateResponse', function () {
it('returns correct pagination object for single page', function () {
utils.paginateResponse(5, {limit: 10, page: 1}).should.eql({
limit: 10,
next: null,
page: 1,
pages: 1,
prev: null,
total: 5
});
});

it('returns correct pagination object for first page of many', function () {
utils.paginateResponse(44, {limit: 5, page: 1}).should.eql({
limit: 5,
next: 2,
page: 1,
pages: 9,
prev: null,
total: 44
});
});

it('returns correct pagination object for middle page of many', function () {
utils.paginateResponse(44, {limit: 5, page: 9}).should.eql({
limit: 5,
next: null,
page: 9,
pages: 9,
prev: 8,
total: 44
});
});

it('returns correct pagination object for last page of many', function () {
utils.paginateResponse(44, {limit: 5, page: 3}).should.eql({
limit: 5,
next: 4,
page: 3,
pages: 9,
prev: 2,
total: 44
});
});

it('returns correct pagination object when page not set', function () {
utils.paginateResponse(5, {limit: 10}).should.eql({
limit: 10,
next: null,
page: 1,
pages: 1,
prev: null,
total: 5
});
});

it('returns correct pagination object for limit all', function () {
utils.paginateResponse(5, {limit: 'all'}).should.eql({
limit: 'all',
next: null,
page: 1,
pages: 1,
prev: null,
total: 5
});
});
});

0 comments on commit 16f98ee

Please sign in to comment.