Skip to content

Commit

Permalink
closes TryGhost#5093
Browse files Browse the repository at this point in the history
- Removing the page parameter in API operations
- Adding the offset parameter in API operations
- Fixing the tests for the new concept
  • Loading branch information
edsadr committed Jul 28, 2015
1 parent 1e630e6 commit cd9dcd2
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 30 deletions.
2 changes: 1 addition & 1 deletion core/server/api/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],

Expand Down
4 changes: 4 additions & 0 deletions core/server/controllers/frontend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion core/server/models/base/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:**
Expand Down
9 changes: 5 additions & 4 deletions core/server/models/base/pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var _ = require('lodash'),
* @property {Number|String} `limit` \- no. results per page (default: 15)
*/
defaults = {
page: 1,
offset: 0,
limit: 15
};

Expand All @@ -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;
},
Expand All @@ -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);
}
},

Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion core/server/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']
};

Expand Down
12 changes: 6 additions & 6 deletions core/test/integration/model/model_posts_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions core/test/unit/api_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
});
Expand Down Expand Up @@ -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();
Expand Down
24 changes: 12 additions & 12 deletions core/test/unit/models_pagination_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
});
});

Expand All @@ -141,7 +141,7 @@ describe('pagination', function () {
limit: 'all'
}).should.eql({
limit: 'all',
page: 1
offset: 0
});
});
});
Expand All @@ -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;
Expand Down

0 comments on commit cd9dcd2

Please sign in to comment.