From 7761873db7689f83b7f3569e4c28624cd750f6c7 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 17 Jun 2015 14:55:39 +0100 Subject: [PATCH] Abstract findPage & add pagination Bookshelf plugin closes #2896 - move default options / custom code into model functions - move most of the filtering logic into base/utils.filtering (to be relocated) - move the remainder of findPage back into base/index.js and remove from posts/users&tags - move pagination-specific logic to a separate 'plugin' file - pagination provides new fetchPage function, similar to fetchAll but handling pagination - findPage model method uses fetchPage - plugin is fully unit-tested and documented --- core/server/models/base/index.js | 83 +++++- core/server/models/base/pagination.js | 185 +++++++++++++ core/server/models/base/utils.js | 66 +++-- core/server/models/post.js | 232 ++++------------ core/server/models/tag.js | 99 ++----- core/server/models/user.js | 203 ++++---------- core/test/unit/models_base_utils_spec.js | 77 ------ core/test/unit/models_pagination_spec.js | 326 +++++++++++++++++++++++ 8 files changed, 757 insertions(+), 514 deletions(-) create mode 100644 core/server/models/base/pagination.js delete mode 100644 core/test/unit/models_base_utils_spec.js create mode 100644 core/test/unit/models_pagination_spec.js diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 4d08697c838..03ca55c5178 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -17,6 +17,8 @@ var _ = require('lodash'), utils = require('../../utils'), uuid = require('node-uuid'), validation = require('../../data/validation'), + baseUtils = require('./utils'), + pagination = require('./pagination'), ghostBookshelf; @@ -24,27 +26,32 @@ var _ = require('lodash'), // Initializes a new Bookshelf instance called ghostBookshelf, for reference elsewhere in Ghost. ghostBookshelf = bookshelf(config.database.knex); -// Load the registry plugin, which helps us avoid circular dependencies +// Load the Bookshelf registry plugin, which helps us avoid circular dependencies ghostBookshelf.plugin('registry'); -// ### ghostBookshelf.Model +// Load the Ghost pagination plugin, which gives us the `fetchPage` method on Models +ghostBookshelf.plugin(pagination); + +// ## ghostBookshelf.Model // The Base Model which other Ghost objects will inherit from, // including some convenience functions as static properties on the model. ghostBookshelf.Model = ghostBookshelf.Model.extend({ - + // Bookshelf `hasTimestamps` - handles created_at and updated_at properties hasTimestamps: true, - // Get permitted attributes from server/data/schema.js, which is where the DB schema is defined + // Ghost option handling - get permitted attributes from server/data/schema.js, where the DB schema is defined permittedAttributes: function permittedAttributes() { return _.keys(schema.tables[this.tableName]); }, + // Bookshelf `defaults` - default values setup on every model creation defaults: function defaults() { return { uuid: uuid.v4() }; }, + // Bookshelf `initialize` - declare a constructor-like method for model creation initialize: function initialize() { var self = this, options = arguments[1] || {}; @@ -174,7 +181,6 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ updated: function updated(attr) { return this.updatedAttributes()[attr]; } - }, { // ## Data Utility Functions @@ -221,9 +227,9 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ * @param {Object} options (optional) * @return {Promise(ghostBookshelf.Collection)} Collection of all Models */ - findAll: function findAll(options) { + findAll: function findAll(options) { options = this.filterOptions(options, 'findAll'); - return ghostBookshelf.Collection.forge([], {model: this}).fetch(options).then(function then(result) { + return this.forge().fetchAll(options).then(function then(result) { if (options.include) { _.each(result.models, function each(item) { item.include = options.include; @@ -233,6 +239,69 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ }); }, + /** + * ### Find Page + * Find results by page - returns an object containing the + * information about the request (page, limit), along with the + * info needed for pagination (pages, total). + * + * **response:** + * + * { + * posts: [ + * {...}, ... + * ], + * page: __, + * limit: __, + * pages: __, + * total: __ + * } + * + * @param {Object} options + */ + findPage: function findPage(options) { + options = options || {}; + + var self = this, + itemCollection = this.forge(), + tableName = _.result(this.prototype, 'tableName'), + filterObjects = self.setupFilters(options); + + // Filter options so that only permitted ones remain + options = this.filterOptions(options, 'findPage'); + + // Extend the model defaults + options = _.defaults(options, this.findPageDefaultOptions()); + + // Run specific conversion of model query options to where options + options = this.processOptions(itemCollection, options); + + // Prefetch filter objects + return Promise.all(baseUtils.filtering.preFetch(filterObjects)).then(function doQuery() { + // If there are `where` conditionals specified, add those to the query. + if (options.where) { + itemCollection.query('where', options.where); + } + + // Setup filter joins / queries + baseUtils.filtering.query(filterObjects, itemCollection); + + // Handle related objects + // TODO: this should just be done for all methods @ the API level + options.withRelated = _.union(options.withRelated, options.include); + + options.order = self.orderDefaultOptions(); + + return itemCollection.fetchPage(options).then(function formatResponse(response) { + var data = {}; + data[tableName] = response.collection.toJSON(options); + data.meta = {pagination: response.pagination}; + + return baseUtils.filtering.formatResponse(filterObjects, options, data); + }); + }).catch(errors.logAndThrowError); + }, + /** * ### Find One * Naive find one where data determines what to match on diff --git a/core/server/models/base/pagination.js b/core/server/models/base/pagination.js new file mode 100644 index 00000000000..637899086c0 --- /dev/null +++ b/core/server/models/base/pagination.js @@ -0,0 +1,185 @@ +// # Pagination +// +// Extends Bookshelf.Model with a `fetchPage` method. Handles everything to do with paginated requests. +var _ = require('lodash'), + Promise = require('bluebird'), + + defaults, + paginationUtils, + pagination; + +/** + * ### Default pagination values + * These are overridden via `options` passed to each function + * @typedef {Object} defaults + * @default + * @property {Number} `page` \- page in set to display (default: 1) + * @property {Number|String} `limit` \- no. results per page (default: 15) + */ +defaults = { + page: 1, + limit: 15 +}; + +/** + * ## Pagination Utils + * @api private + * @type {{parseOptions: Function, query: Function, formatResponse: Function}} + */ +paginationUtils = { + /** + * ### Parse Options + * Take the given options and ensure they are valid pagination options, else use the defaults + * @param {options} options + * @returns {options} options sanitised for pagination + */ + parseOptions: function parseOptions(options) { + options = _.defaults(options || {}, defaults); + + if (options.limit !== 'all') { + options.limit = parseInt(options.limit, 10) || defaults.limit; + } + + options.page = parseInt(options.page, 10) || defaults.page; + + return options; + }, + /** + * ### Query + * Apply the necessary parameters to paginate the query + * @param {Bookshelf.Model, Bookshelf.Collection} itemCollection + * @param {options} options + */ + query: function query(itemCollection, options) { + if (_.isNumber(options.limit)) { + itemCollection + .query('limit', options.limit) + .query('offset', options.limit * (options.page - 1)); + } + }, + + /** + * ### Format Response + * Takes the no. items returned and original options and calculates all of the pagination meta data + * @param {Number} totalItems + * @param {options} options + * @returns {pagination} pagination metadata + */ + formatResponse: function formatResponse(totalItems, options) { + var calcPages = Math.ceil(totalItems / options.limit) || 0, + pagination = { + page: options.page || defaults.page, + limit: options.limit, + pages: calcPages === 0 ? 1 : calcPages, + total: totalItems, + next: null, + 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; + } +}; + +// ## Object Definitions + +/** + * ### 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 + */ + +/** + * ### 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 + */ + +/** + * ### Fetch Page Response + * @typedef {Object} paginatedResult + * @property {Array} `collection` \- set of results + * @property {pagination} pagination \- pagination metadata + */ + +/** + * ## Pagination + * Extends `bookshelf.Model` with `fetchPage` + * @param {Bookshelf} bookshelf \- the instance to plug into + */ +pagination = function pagination(bookshelf) { + // Extend updates the first object passed to it, no need for an assignment + _.extend(bookshelf.Model.prototype, { + /** + * ### Fetch page + * A `fetch` extension to get a paginated set of items from a collection + * @param {options} options + * @returns {paginatedResult} set of results + pagination metadata + */ + fetchPage: function fetchPage(options) { + // Setup pagination options + options = paginationUtils.parseOptions(options); + + // 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 definitely using collection, + // rather than model + collection = this.constructor.collection(), + // Clone the base query & set up a promise to get the count of total items in the full set + countPromise = this.query().clone().count(tableName + '.' + idAttribute + ' as aggregate'), + collectionPromise; + + // Clone the base query into our collection + collection._knex = this.query().clone(); + + // Setup the pagination parameters so that we return the correct items from the set + paginationUtils.query(collection, options); + + // Apply ordering options if they are present + // This is an optimisation, adding order before cloning for the count query would mean the count query + // was also ordered, when that is unnecessary. + if (options.order) { + _.forOwn(options.order, function (direction, property) { + collection.query('orderBy', tableName + '.' + property, direction); + }); + } + + // 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'])); + + // 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].aggregate, options) + }; + }); + } + }); +}; + +/** + * ## Export pagination plugin + * @api public + */ +module.exports = pagination; diff --git a/core/server/models/base/utils.js b/core/server/models/base/utils.js index 6f07d6350c9..85baca7d90e 100644 --- a/core/server/models/base/utils.js +++ b/core/server/models/base/utils.js @@ -2,35 +2,49 @@ * # Utils * Parts of the model code which can be split out and unit tested */ +var _ = require('lodash'), + filtering; -/** - * 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? - * @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 = {}; +filtering = { + preFetch: function preFetch(filterObjects) { + var promises = []; + _.forOwn(filterObjects, function (obj) { + promises.push(obj.fetch()); + }); - pagination.page = options.page || 1; - pagination.limit = options.limit; - pagination.pages = calcPages === 0 ? 1 : calcPages; - pagination.total = totalItems; - pagination.next = null; - pagination.prev = null; + return promises; + }, + query: function query(filterObjects, itemCollection) { + if (filterObjects.tags) { + itemCollection + .query('join', 'posts_tags', 'posts_tags.post_id', '=', 'posts.id') + .query('where', 'posts_tags.tag_id', '=', filterObjects.tags.id); + } - 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; + if (filterObjects.author) { + itemCollection + .query('where', 'author_id', '=', filterObjects.author.id); } - } - return pagination; + if (filterObjects.roles) { + itemCollection + .query('join', 'roles_users', 'roles_users.user_id', '=', 'users.id') + .query('where', 'roles_users.role_id', '=', filterObjects.roles.id); + } + }, + formatResponse: function formatResponse(filterObjects, options, data) { + if (!_.isEmpty(filterObjects)) { + data.meta.filters = {}; + } + + _.forOwn(filterObjects, function (obj, key) { + if (!filterObjects[key].isNew()) { + data.meta.filters[key] = [filterObjects[key].toJSON(options)]; + } + }); + + return data; + } }; + +module.exports.filtering = filtering; diff --git a/core/server/models/post.js b/core/server/models/post.js index 3438fb9c97c..9f5e556435f 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -9,7 +9,6 @@ var _ = require('lodash'), ghostBookshelf = require('./base'), events = require('../events'), config = require('../config'), - paginateResponse = require('./base/utils').paginateResponse, permalinkSetting = '', getPermalinkSetting, Post, @@ -274,6 +273,65 @@ Post = ghostBookshelf.Model.extend({ return attrs; } }, { + setupFilters: function setupFilters(options) { + var filterObjects = {}; + // Deliberately switch from singular 'tag' to 'tags' and 'role' to 'roles' here + // TODO: make this consistent + if (options.tag !== undefined) { + filterObjects.tags = ghostBookshelf.model('Tag').forge({slug: options.tag}); + } + if (options.author !== undefined) { + filterObjects.author = ghostBookshelf.model('User').forge({slug: options.author}); + } + + return filterObjects; + }, + + findPageDefaultOptions: function findPageDefaultOptions() { + return { + staticPages: false, // include static pages + status: 'published', + where: {} + }; + }, + + orderDefaultOptions: function orderDefaultOptions() { + return { + status: 'ASC', + published_at: 'DESC', + updated_at: 'DESC', + id: 'DESC' + }; + }, + + processOptions: function processOptions(itemCollection, options) { + // Step 4: Setup filters (where clauses) + if (options.staticPages !== 'all') { + // convert string true/false to boolean + if (!_.isBoolean(options.staticPages)) { + options.staticPages = _.contains(['true', '1'], options.staticPages); + } + options.where.page = options.staticPages; + } + + if (options.featured) { + // convert string true/false to boolean + if (!_.isBoolean(options.featured)) { + options.featured = _.contains(['true', '1'], options.featured); + } + options.where.featured = options.featured; + } + + // Unless `all` is passed as an option, filter on + // the status provided. + if (options.status !== 'all') { + // make sure that status is valid + options.status = _.contains(['published', 'draft'], options.status) ? options.status : 'published'; + options.where.status = options.status; + } + + return options; + }, /** * Returns an array of keys permitted in a method's `options` hash, depending on the current method. @@ -332,178 +390,6 @@ Post = ghostBookshelf.Model.extend({ return ghostBookshelf.Model.findAll.call(this, options); }, - /** - * #### findPage - * Find results by page - returns an object containing the - * information about the request (page, limit), along with the - * info needed for pagination (pages, total). - * - * **response:** - * - * { - * posts: [ - * {...}, {...}, {...} - * ], - * page: __, - * limit: __, - * pages: __, - * total: __ - * } - * - * @param {Object} options - */ - findPage: function findPage(options) { - options = options || {}; - - // -- Part 0 -- - // Step 1: Setup filter models - var self = this, - tagInstance = options.tag !== undefined ? ghostBookshelf.model('Tag').forge({slug: options.tag}) : false, - authorInstance = options.author !== undefined ? ghostBookshelf.model('User').forge({slug: options.author}) : false; - - // Step 2: Setup filter model promises - function fetchTagQuery() { - if (tagInstance) { - return tagInstance.fetch(); - } - return false; - } - - function fetchAuthorQuery() { - if (authorInstance) { - return authorInstance.fetch(); - } - return false; - } - - // Step 3: Prefetch filter models - return Promise.join(fetchTagQuery(), fetchAuthorQuery()).then(function setupCollectionPromises() { - // -- Part 1 -- - var postCollection = Posts.forge(), - collectionPromise, - countPromise; - - // Step 1: Setup pagination options - if (options.limit && options.limit !== 'all') { - options.limit = parseInt(options.limit, 10) || 15; - } - - if (options.page) { - options.page = parseInt(options.page, 10) || 1; - } - - // Step 2: Filter options - options = self.filterOptions(options, 'findPage'); - - // Step 3: Extend defaults - options = _.extend({ - page: 1, // pagination page - limit: 15, - staticPages: false, // include static pages - status: 'published', - where: {} - }, options); - - // Step 4: Setup filters (where clauses) - if (options.staticPages !== 'all') { - // convert string true/false to boolean - if (!_.isBoolean(options.staticPages)) { - options.staticPages = options.staticPages === 'true' || options.staticPages === '1' ? true : false; - } - options.where.page = options.staticPages; - } - - if (options.featured) { - // convert string true/false to boolean - if (!_.isBoolean(options.featured)) { - options.featured = options.featured === 'true' || options.featured === '1' ? true : false; - } - options.where.featured = options.featured; - } - - // Unless `all` is passed as an option, filter on - // the status provided. - if (options.status !== 'all') { - // make sure that status is valid - options.status = _.indexOf(['published', 'draft'], options.status) !== -1 ? options.status : 'published'; - options.where.status = options.status; - } - - // If there are where conditionals specified, add those to the query. - if (options.where) { - postCollection.query('where', options.where); - } - - // Step 5: Setup joins - if (tagInstance) { - postCollection - .query('join', 'posts_tags', 'posts_tags.post_id', '=', 'posts.id') - .query('where', 'posts_tags.tag_id', '=', tagInstance.id); - } - - if (authorInstance) { - postCollection - .query('where', 'author_id', '=', authorInstance.id); - } - - // Step 6: Setup the counter to fetch the number of items in the set - // @TODO abstract this out - // tableName = _.result(postCollection, 'tableName'), - // idAttribute = _.result(postCollection, 'idAttribute'); - countPromise = postCollection.query().clone().count('posts.id as aggregate'); - - // -- Part 2 -- - // Add limit, offset and other query changes which aren't required when performing a count - - // Step 1: Add related objects - options.withRelated = _.union(options.withRelated, options.include); - - // Step 2: Add pagination options if needed - if (_.isNumber(options.limit)) { - postCollection - .query('limit', options.limit) - .query('offset', options.limit * (options.page - 1)); - } - - // Step 3: add order parameters - postCollection - .query('orderBy', 'status', 'ASC') - .query('orderBy', 'published_at', 'DESC') - .query('orderBy', 'updated_at', 'DESC') - .query('orderBy', 'id', 'DESC'); - - // Step 4: Setup the promise - collectionPromise = postCollection.fetch(_.omit(options, 'page', 'limit')); - - // -- Part 3 -- - // Step 1: Fetch the data - return Promise.join(collectionPromise, countPromise); - }).then(function formatResponse(results) { - var postCollection = results[0], - data = {}; - - // Step 2: Format the data - data.posts = postCollection.toJSON(options); - data.meta = {pagination: paginateResponse(results[1][0].aggregate, options)}; - - if (tagInstance) { - data.meta.filters = {}; - if (!tagInstance.isNew()) { - data.meta.filters.tags = [tagInstance.toJSON(options)]; - } - } - - if (authorInstance) { - data.meta.filters = {}; - if (!authorInstance.isNew()) { - data.meta.filters.author = authorInstance.toJSON(options); - } - } - - return data; - }).catch(errors.logAndThrowError); - }, - /** * ### Find One * @extends ghostBookshelf.Model.findOne to handle post status diff --git a/core/server/models/tag.js b/core/server/models/tag.js index 1097bf2d57d..fb96ea9b9a6 100644 --- a/core/server/models/tag.js +++ b/core/server/models/tag.js @@ -1,9 +1,6 @@ var _ = require('lodash'), - Promise = require('bluebird'), - errors = require('../errors'), ghostBookshelf = require('./base'), events = require('../events'), - paginateResponse = require('./base/utils').paginateResponse, Tag, Tags; @@ -71,6 +68,25 @@ Tag = ghostBookshelf.Model.extend({ return attrs; } }, { + setupFilters: function setupFilters() { + return {}; + }, + + findPageDefaultOptions: function findPageDefaultOptions() { + return { + where: {} + }; + }, + + orderDefaultOptions: function orderDefaultOptions() { + return {}; + }, + + processOptions: function processOptions(itemCollection, options) { + addPostCount(options, itemCollection); + return options; + }, + permittedOptions: function permittedOptions(methodName) { var options = ghostBookshelf.Model.permittedOptions(), @@ -107,83 +123,6 @@ Tag = ghostBookshelf.Model.extend({ return tag.fetch(options); }, - findPage: function findPage(options) { - options = options || {}; - - // -- Part 0 -- - // Step 1: Setup filter models - // Step 2: Setup filter model promises - // Step 3: Prefetch filter models - - // -- Part 1 -- - var tagCollection = Tags.forge(), - collectionPromise, - countPromise; - - // Step 1: Setup pagination options - if (options.limit && options.limit !== 'all') { - options.limit = parseInt(options.limit, 10) || 15; - } - - if (options.page) { - options.page = parseInt(options.page, 10) || 1; - } - - // Step 2: Filter options - options = this.filterOptions(options, 'findPage'); - - // Step 3: Extend defaults - options = _.extend({ - page: 1, // pagination page - limit: 15, - where: {} - }, options); - - // Step 4: Setup filters (where clauses) - // If there are where conditionals specified, add those to the query. - if (options.where) { - tagCollection.query('where', options.where); - } - - // Step 5: Setup joins - - // Step 6: Setup the counter to fetch the number of items in the set - // @TODO abstract this out - // tableName = _.result(postCollection, 'tableName'), - // idAttribute = _.result(postCollection, 'idAttribute'); - countPromise = tagCollection.query().clone().count('tags.id as aggregate'); - - // -- Part 2 -- - // Add limit, offset and other query changes which aren't required when performing a count - - // Step 1: Add related objects - addPostCount(options, tagCollection); - options.withRelated = _.union(options.withRelated, options.include); - - // Step 2: Add pagination options if needed - if (_.isNumber(options.limit)) { - tagCollection - .query('limit', options.limit) - .query('offset', options.limit * (options.page - 1)); - } - - // Step 3: add order parameters - - // Step 4: Setup the promise - collectionPromise = tagCollection.fetch(_.omit(options, 'page', 'limit')); - - // -- Part 3 -- - // Step 1: Fetch the data - return Promise.join(collectionPromise, countPromise).then(function formatResponse(results) { - var tagCollection = results[0], - data = {}; - // Step 2: Format the data - data.tags = tagCollection.toJSON(options); - data.meta = {pagination: paginateResponse(results[1][0].aggregate, options)}; - - return data; - }).catch(errors.logAndThrowError); - }, destroy: function destroy(options) { var id = options.id; options = this.filterOptions(options, 'destroy'); diff --git a/core/server/models/user.js b/core/server/models/user.js index 3f877623c7f..c220199b017 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -10,7 +10,6 @@ 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), @@ -163,6 +162,58 @@ User = ghostBookshelf.Model.extend({ } }, { + setupFilters: function setupFilters(options) { + var filterObjects = {}; + // Deliberately switch from singular 'tag' to 'tags' and 'role' to 'roles' here + // TODO: make this consistent + if (options.role !== undefined) { + filterObjects.roles = ghostBookshelf.model('Role').forge({name: options.role}); + } + + return filterObjects; + }, + + findPageDefaultOptions: function findPageDefaultOptions() { + return { + status: 'active', + where: {}, + whereIn: {} + }; + }, + + orderDefaultOptions: function orderDefaultOptions() { + return { + last_login: 'DESC', + name: 'ASC', + created_at: 'DESC' + }; + }, + + processOptions: function processOptions(itemCollection, options) { + // TODO: there are multiple statuses that make a user "active" or "invited" - we a way to translate/map them: + // TODO (cont'd from above): * valid "active" statuses: active, warn-1, warn-2, warn-3, warn-4, locked + // TODO (cont'd from above): * valid "invited" statuses" invited, invited-pending + + // Filter on the status. A status of 'all' translates to no filter since we want all statuses + if (options.status && options.status !== 'all') { + // make sure that status is valid + // TODO: need a better way of getting a list of statuses other than hard-coding them... + options.status = _.indexOf( + ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked', 'invited', 'inactive'], + options.status) !== -1 ? options.status : 'active'; + } + + if (options.status === 'active') { + itemCollection.query().whereIn('status', activeStates); + } else if (options.status === 'invited') { + itemCollection.query().whereIn('status', invitedStates); + } else if (options.status !== 'all') { + options.where.status = options.status; + } + + return options; + }, + /** * Returns an array of keys permitted in a method's `options` hash, depending on the current method. * @param {String} methodName The name of the method to check valid options for. @@ -200,156 +251,6 @@ User = ghostBookshelf.Model.extend({ return ghostBookshelf.Model.findAll.call(this, options); }, - /** - * #### findPage - * Find results by page - returns an object containing the - * information about the request (page, limit), along with the - * info needed for pagination (pages, total). - * - * **response:** - * - * { - * users: [ - * {...}, {...}, {...} - * ], - * meta: { - * page: __, - * limit: __, - * pages: __, - * total: __ - * } - * } - * - * @param {Object} options - */ - findPage: function findPage(options) { - options = options || {}; - - // -- Part 0 -- - // Step 1: Setup filter models - var self = this, - roleInstance = options.role !== undefined ? ghostBookshelf.model('Role').forge({name: options.role}) : false; - - // Step 2: Setup filter model promises - function fetchRoleQuery() { - if (roleInstance) { - return roleInstance.fetch(); - } - return false; - } - - // Step 3: Prefetch filter models - return Promise.resolve(fetchRoleQuery()).then(function setupCollectionPromises() { - // -- Part 1 -- - var userCollection = Users.forge(), - collectionPromise, - countPromise; - - // Step 1: Setup pagination options - if (options.limit && options.limit !== 'all') { - options.limit = parseInt(options.limit, 10) || 15; - } - - if (options.page) { - options.page = parseInt(options.page, 10) || 1; - } - - // Step 2: Filter options - options = self.filterOptions(options, 'findPage'); - - // Step 3: Extend defaults - options = _.extend({ - page: 1, // pagination page - limit: 15, - status: 'active', - where: {}, - whereIn: {} - }, options); - - // Step 4: Setup filters (where clauses) - // TODO: there are multiple statuses that make a user "active" or "invited" - we a way to translate/map them: - // TODO (cont'd from above): * valid "active" statuses: active, warn-1, warn-2, warn-3, warn-4, locked - // TODO (cont'd from above): * valid "invited" statuses" invited, invited-pending - - // Filter on the status. A status of 'all' translates to no filter since we want all statuses - if (options.status && options.status !== 'all') { - // make sure that status is valid - // TODO: need a better way of getting a list of statuses other than hard-coding them... - options.status = _.indexOf( - ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked', 'invited', 'inactive'], - options.status) !== -1 ? options.status : 'active'; - } - - if (options.status === 'active') { - userCollection.query().whereIn('status', activeStates); - } else if (options.status === 'invited') { - userCollection.query().whereIn('status', invitedStates); - } else if (options.status !== 'all') { - options.where.status = options.status; - } - - // If there are where conditionals specified, add those to the query. - if (options.where) { - userCollection.query('where', options.where); - } - - // Step 5: Setup joins - if (roleInstance) { - userCollection - .query('join', 'roles_users', 'roles_users.user_id', '=', 'users.id') - .query('where', 'roles_users.role_id', '=', roleInstance.id); - } - - // Step 6: Setup the counter to fetch the number of items in the set - // @TODO abstract this out - // tableName = _.result(userCollection, 'tableName'), - // idAttribute = _.result(userCollection, 'idAttribute'); - countPromise = userCollection.query().clone().count('users.id as aggregate'); - - // -- Part 2 -- - // Add limit, offset and other query changes which aren't required when performing a count - - // Step 1: Add related objects - options.withRelated = _.union(options.withRelated, options.include); - - // Step 2: Add pagination options if needed - if (_.isNumber(options.limit)) { - userCollection - .query('limit', options.limit) - .query('offset', options.limit * (options.page - 1)); - } - - // Step 3: add order parameters - userCollection - .query('orderBy', 'last_login', 'DESC') - .query('orderBy', 'name', 'ASC') - .query('orderBy', 'created_at', 'DESC'); - - // Step 4: Setup the promise - collectionPromise = userCollection.fetch(_.omit(options, 'page', 'limit')); - - // -- Part 3 -- - // Step 1: Fetch the data - return Promise.join(collectionPromise, countPromise); - }).then(function formatResponse(results) { - var userCollection = results[0], - data = {}; - - // Step 2: Format the data - data.users = userCollection.toJSON(options); - data.meta = {pagination: paginateResponse(results[1][0].aggregate, options)}; - - if (roleInstance) { - data.meta.filters = {}; - if (!roleInstance.isNew()) { - data.meta.filters.roles = [roleInstance.toJSON(options)]; - } - } - - return data; - }).catch(errors.logAndThrowError); - }, - /** * ### Find One * @extends ghostBookshelf.Model.findOne to include roles diff --git a/core/test/unit/models_base_utils_spec.js b/core/test/unit/models_base_utils_spec.js deleted file mode 100644 index 084c30602d3..00000000000 --- a/core/test/unit/models_base_utils_spec.js +++ /dev/null @@ -1,77 +0,0 @@ -/*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 - }); - }); -}); diff --git a/core/test/unit/models_pagination_spec.js b/core/test/unit/models_pagination_spec.js new file mode 100644 index 00000000000..73a5384ce0c --- /dev/null +++ b/core/test/unit/models_pagination_spec.js @@ -0,0 +1,326 @@ +/*globals describe, it, before, beforeEach, afterEach */ +/*jshint expr:true*/ +var should = require('should'), + sinon = require('sinon'), + Promise = require('bluebird'), + rewire = require('rewire'), + +// Thing we're testing + pagination = rewire('../../server/models/base/pagination'); + +// To stop jshint complaining +should.equal(true, true); + +describe('pagination', function () { + var sandbox, + paginationUtils; + + beforeEach(function () { + sandbox = sinon.sandbox.create(); + }); + + afterEach(function () { + sandbox.restore(); + }); + + describe('paginationUtils', function () { + before(function () { + paginationUtils = pagination.__get__('paginationUtils'); + }); + + describe('formatResponse', function () { + var formatResponse; + + before(function () { + formatResponse = paginationUtils.formatResponse; + }); + + it('returns correct pagination object for single page', function () { + formatResponse(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 () { + formatResponse(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 () { + formatResponse(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 () { + formatResponse(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 () { + formatResponse(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 () { + formatResponse(5, {limit: 'all'}).should.eql({ + limit: 'all', + next: null, + page: 1, + pages: 1, + prev: null, + total: 5 + }); + }); + }); + + describe('parseOptions', function () { + var parseOptions; + + before(function () { + parseOptions = paginationUtils.parseOptions; + }); + + it('should use defaults if no options are passed', function () { + parseOptions().should.eql({ + limit: 15, + page: 1 + }); + }); + + it('should accept numbers for limit and page', function () { + parseOptions({ + limit: 10, + page: 2 + }).should.eql({ + limit: 10, + page: 2 + }); + }); + + it('should use defaults if bad options are passed', function () { + parseOptions({ + limit: 'thelma', + page: 'louise' + }).should.eql({ + limit: 15, + page: 1 + }); + }); + + it('should permit all for limit', function () { + parseOptions({ + limit: 'all' + }).should.eql({ + limit: 'all', + page: 1 + }); + }); + }); + + describe('query', function () { + var query, + collection = {}; + + before(function () { + query = paginationUtils.query; + }); + + beforeEach(function () { + collection.query = sandbox.stub().returns(collection); + }); + + it('should add query options if limit is set', function () { + query(collection, {limit: 5, page: 1}); + + collection.query.calledTwice.should.be.true; + collection.query.firstCall.calledWith('limit', 5).should.be.true; + collection.query.secondCall.calledWith('offset', 0).should.be.true; + }); + + it('should not add query options if limit is not set', function () { + query(collection, {page: 1}); + + collection.query.called.should.be.false; + }); + }); + }); + + describe('fetchPage', function () { + var model, bookshelf, mockQuery, fetch, colQuery; + + before(function () { + paginationUtils = pagination.__get__('paginationUtils'); + }); + + beforeEach(function () { + // Stub paginationUtils + paginationUtils.parseOptions = sandbox.stub(); + paginationUtils.query = sandbox.stub(); + paginationUtils.formatResponse = sandbox.stub().returns({}); + + // Mock out bookshelf model + mockQuery = { + clone: sandbox.stub(), + count: sandbox.stub() + }; + mockQuery.clone.returns(mockQuery); + mockQuery.count.returns([{aggregate: 1}]); + + fetch = sandbox.stub().returns(Promise.resolve({})); + colQuery = sandbox.stub(); + + model = function () {}; + model.prototype.constructor = { + collection: sandbox.stub().returns({ + fetch: fetch, + query: colQuery + }) + }; + model.prototype.query = sandbox.stub(); + model.prototype.query.returns(mockQuery); + + bookshelf = {Model: model}; + + pagination(bookshelf); + }); + + it('extends Model with fetchPage', function () { + bookshelf.Model.prototype.should.have.ownProperty('fetchPage'); + bookshelf.Model.prototype.fetchPage.should.be.a.Function; + }); + + it('fetchPage calls all paginationUtils and methods', function (done) { + paginationUtils.parseOptions.returns({}); + + bookshelf.Model.prototype.fetchPage().then(function () { + sinon.assert.callOrder( + paginationUtils.parseOptions, + model.prototype.constructor.collection, + model.prototype.query, + mockQuery.clone, + mockQuery.count, + model.prototype.query, + mockQuery.clone, + paginationUtils.query, + fetch, + paginationUtils.formatResponse + ); + + paginationUtils.parseOptions.calledOnce.should.be.true; + paginationUtils.parseOptions.calledWith(undefined).should.be.true; + + paginationUtils.query.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; + + mockQuery.clone.calledTwice.should.be.true; + mockQuery.clone.firstCall.calledWith().should.be.true; + mockQuery.clone.secondCall.calledWith().should.be.true; + + mockQuery.count.calledOnce.should.be.true; + mockQuery.count.calledWith().should.be.true; + + fetch.calledOnce.should.be.true; + fetch.calledWith({}).should.be.true; + + done(); + }).catch(done); + }); + + 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.count, + model.prototype.query, + mockQuery.clone, + paginationUtils.query, + colQuery, + fetch, + paginationUtils.formatResponse + ); + + paginationUtils.parseOptions.calledOnce.should.be.true; + paginationUtils.parseOptions.calledWith(orderOptions).should.be.true; + + paginationUtils.query.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; + + mockQuery.clone.calledTwice.should.be.true; + mockQuery.clone.firstCall.calledWith().should.be.true; + mockQuery.clone.secondCall.calledWith().should.be.true; + + mockQuery.count.calledOnce.should.be.true; + mockQuery.count.calledWith().should.be.true; + + colQuery.calledOnce.should.be.true; + colQuery.calledWith('orderBy', 'undefined.id', 'DESC').should.be.true; + + fetch.calledOnce.should.be.true; + fetch.calledWith(orderOptions).should.be.true; + + done(); + }).catch(done); + }); + + it('fetchPage returns expected response', function (done) { + 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(); + }); + }); + }); +});