From cd9dcd2eeb77fe2a6198ca10778a7dfc1dc378f8 Mon Sep 17 00:00:00 2001 From: Adrian Estrada Date: Tue, 28 Jul 2015 11:49:37 -0500 Subject: [PATCH] closes #5093 - Removing the page parameter in API operations - Adding the offset parameter in API operations - Fixing the tests for the new concept --- core/server/api/utils.js | 2 +- core/server/controllers/frontend.js | 4 ++++ core/server/models/base/index.js | 2 +- core/server/models/base/pagination.js | 9 +++---- core/server/models/post.js | 2 +- .../integration/model/model_posts_spec.js | 12 +++++----- core/test/unit/api_utils_spec.js | 10 ++++---- core/test/unit/models_pagination_spec.js | 24 +++++++++---------- 8 files changed, 35 insertions(+), 30 deletions(-) diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 6944ddb66ba2..9958b764b569 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -21,7 +21,7 @@ utils = { // ### Manual Default Options // These must be provided by the endpoint // browseDefaultOptions - valid for all browse api endpoints - browseDefaultOptions: ['page', 'limit'], + browseDefaultOptions: ['offset', 'limit'], // idDefaultOptions - valid whenever an id is valid idDefaultOptions: ['id'], diff --git a/core/server/controllers/frontend.js b/core/server/controllers/frontend.js index 380b016c149e..664daacca781 100644 --- a/core/server/controllers/frontend.js +++ b/core/server/controllers/frontend.js @@ -27,6 +27,10 @@ function getPostPage(options) { if (!isNaN(postsPerPage) && postsPerPage > 0) { options.limit = postsPerPage; } + + // translating page parameter in offset + options.offset = (options.page * options.limit) - options.limit; + options.include = 'author,tags,fields'; return api.posts.browse(options); }); diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 03ca55c51782..6949f1bc4497 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -242,7 +242,7 @@ 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 + * information about the request (offset, limit), along with the * info needed for pagination (pages, total). * * **response:** diff --git a/core/server/models/base/pagination.js b/core/server/models/base/pagination.js index 35bcea5dbee0..8ab6a840ea41 100644 --- a/core/server/models/base/pagination.js +++ b/core/server/models/base/pagination.js @@ -17,7 +17,7 @@ var _ = require('lodash'), * @property {Number|String} `limit` \- no. results per page (default: 15) */ defaults = { - page: 1, + offset: 0, limit: 15 }; @@ -40,7 +40,7 @@ paginationUtils = { options.limit = parseInt(options.limit, 10) || defaults.limit; } - options.page = parseInt(options.page, 10) || defaults.page; + options.offset = parseInt(options.offset, 10) || defaults.offset; return options; }, @@ -54,7 +54,7 @@ paginationUtils = { if (_.isNumber(options.limit)) { itemCollection .query('limit', options.limit) - .query('offset', options.limit * (options.page - 1)); + .query('offset', options.offset); } }, @@ -67,8 +67,9 @@ paginationUtils = { */ formatResponse: function formatResponse(totalItems, options) { var calcPages = Math.ceil(totalItems / options.limit) || 0, + calcCurrentPage = (options.offset / options.limit) + 1 || 1, pagination = { - page: options.page || defaults.page, + page: calcCurrentPage, limit: options.limit, pages: calcPages === 0 ? 1 : calcPages, total: totalItems, diff --git a/core/server/models/post.js b/core/server/models/post.js index 4e66b99874d0..33ab64872ac2 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -346,7 +346,7 @@ Post = ghostBookshelf.Model.extend({ validOptions = { findAll: ['withRelated'], findOne: ['importing', 'withRelated'], - findPage: ['page', 'limit', 'status', 'staticPages', 'featured'], + findPage: ['offset', 'limit', 'status', 'staticPages', 'featured'], add: ['importing'] }; diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index b1801dfc7745..dac93b4f8a34 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -138,14 +138,14 @@ describe('Post Model', function () { testUtils.fixtures.insertMorePosts().then(function () { return testUtils.fixtures.insertMorePostsTags(); }).then(function () { - return PostModel.findPage({page: 2}); + return PostModel.findPage({offset: 15, limit: 15}); }).then(function (paginationResult) { paginationResult.meta.pagination.page.should.equal(2); paginationResult.meta.pagination.limit.should.equal(15); paginationResult.meta.pagination.pages.should.equal(4); paginationResult.posts.length.should.equal(15); - return PostModel.findPage({page: 5}); + return PostModel.findPage({offset: 60, limit: 15}); }).then(function (paginationResult) { paginationResult.meta.pagination.page.should.equal(5); paginationResult.meta.pagination.limit.should.equal(15); @@ -211,7 +211,7 @@ describe('Post Model', function () { return testUtils.fixtures.insertMorePostsTags(); }).then(function () { // Test tag filter - return PostModel.findPage({page: 1, tag: 'bacon'}); + return PostModel.findPage({offset: 0, tag: 'bacon'}); }).then(function (paginationResult) { paginationResult.meta.pagination.page.should.equal(1); paginationResult.meta.pagination.limit.should.equal(15); @@ -220,7 +220,7 @@ describe('Post Model', function () { paginationResult.meta.filters.tags[0].slug.should.equal('bacon'); paginationResult.posts.length.should.equal(2); - return PostModel.findPage({page: 1, tag: 'kitchen-sink'}); + return PostModel.findPage({offset: 0, tag: 'kitchen-sink'}); }).then(function (paginationResult) { paginationResult.meta.pagination.page.should.equal(1); paginationResult.meta.pagination.limit.should.equal(15); @@ -229,7 +229,7 @@ describe('Post Model', function () { paginationResult.meta.filters.tags[0].slug.should.equal('kitchen-sink'); paginationResult.posts.length.should.equal(2); - return PostModel.findPage({page: 1, tag: 'injection'}); + return PostModel.findPage({offset: 0, tag: 'injection'}); }).then(function (paginationResult) { paginationResult.meta.pagination.page.should.equal(1); paginationResult.meta.pagination.limit.should.equal(15); @@ -238,7 +238,7 @@ describe('Post Model', function () { paginationResult.meta.filters.tags[0].slug.should.equal('injection'); paginationResult.posts.length.should.equal(15); - return PostModel.findPage({page: 2, tag: 'injection'}); + return PostModel.findPage({offset:15, tag: 'injection'}); }).then(function (paginationResult) { paginationResult.meta.pagination.page.should.equal(2); paginationResult.meta.pagination.limit.should.equal(15); diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 9381cf7f91df..18ee27fd96bf 100644 --- a/core/test/unit/api_utils_spec.js +++ b/core/test/unit/api_utils_spec.js @@ -21,7 +21,7 @@ describe('API Utils', function () { describe('Default Options', function () { it('should provide a set of default options', function () { apiUtils.globalDefaultOptions.should.eql(['context', 'include']); - apiUtils.browseDefaultOptions.should.eql(['page', 'limit']); + apiUtils.browseDefaultOptions.should.eql(['offset', 'limit']); apiUtils.dataDefaultOptions.should.eql(['data']); apiUtils.idDefaultOptions.should.eql(['id']); }); @@ -124,17 +124,17 @@ describe('API Utils', function () { }).catch(done); }); - it('should allow page & limit options when browseDefaultOptions passed', function (done) { + it('should allow offset & limit options when browseDefaultOptions passed', function (done) { apiUtils.validate('test', {opts: apiUtils.browseDefaultOptions})( - {context: 'stuff', include: 'stuff', page: 1, limit: 5} + {context: 'stuff', include: 'stuff', offset: 0, limit: 5} ).then(function (options) { options.should.not.have.ownProperty('data'); options.should.have.ownProperty('context'); options.context.should.eql('stuff'); options.should.have.ownProperty('include'); options.include.should.eql('stuff'); - options.should.have.ownProperty('page'); - options.page.should.eql(1); + options.should.have.ownProperty('offset'); + options.offset.should.eql(0); options.should.have.ownProperty('limit'); options.limit.should.eql(5); done(); diff --git a/core/test/unit/models_pagination_spec.js b/core/test/unit/models_pagination_spec.js index 0e50bd957ab4..58be3049c497 100644 --- a/core/test/unit/models_pagination_spec.js +++ b/core/test/unit/models_pagination_spec.js @@ -36,7 +36,7 @@ describe('pagination', function () { }); it('returns correct pagination object for single page', function () { - formatResponse(5, {limit: 10, page: 1}).should.eql({ + formatResponse(5, {limit: 10, offset: 0}).should.eql({ limit: 10, next: null, page: 1, @@ -47,7 +47,7 @@ describe('pagination', function () { }); it('returns correct pagination object for first page of many', function () { - formatResponse(44, {limit: 5, page: 1}).should.eql({ + formatResponse(44, {limit: 5, offset: 0}).should.eql({ limit: 5, next: 2, page: 1, @@ -58,7 +58,7 @@ describe('pagination', function () { }); it('returns correct pagination object for middle page of many', function () { - formatResponse(44, {limit: 5, page: 9}).should.eql({ + formatResponse(44, {limit: 5, offset: 40}).should.eql({ limit: 5, next: null, page: 9, @@ -69,7 +69,7 @@ describe('pagination', function () { }); it('returns correct pagination object for last page of many', function () { - formatResponse(44, {limit: 5, page: 3}).should.eql({ + formatResponse(44, {limit: 5, offset: 10}).should.eql({ limit: 5, next: 4, page: 3, @@ -112,27 +112,27 @@ describe('pagination', function () { it('should use defaults if no options are passed', function () { parseOptions().should.eql({ limit: 15, - page: 1 + offset: 0 }); }); - it('should accept numbers for limit and page', function () { + it('should accept numbers for limit and offset', function () { parseOptions({ limit: 10, - page: 2 + offset: 2 }).should.eql({ limit: 10, - page: 2 + offset: 2 }); }); it('should use defaults if bad options are passed', function () { parseOptions({ limit: 'thelma', - page: 'louise' + offset: 'louise' }).should.eql({ limit: 15, - page: 1 + offset: 0 }); }); @@ -141,7 +141,7 @@ describe('pagination', function () { limit: 'all' }).should.eql({ limit: 'all', - page: 1 + offset: 0 }); }); }); @@ -159,7 +159,7 @@ describe('pagination', function () { }); it('should add query options if limit is set', function () { - query(collection, {limit: 5, page: 1}); + query(collection, {limit: 5, offset: 0}); collection.query.calledTwice.should.be.true; collection.query.firstCall.calledWith('limit', 5).should.be.true;