From 6db41584e7d1e2cfc302cadc911c65a71698dfac Mon Sep 17 00:00:00 2001 From: vdemedes Date: Thu, 22 Oct 2015 14:49:15 +0200 Subject: [PATCH] Add order parameter refs #5602 - add "order" to default browse options - parse order parameter in Base model - accept "order" option in Post, User and Tag models - add tests for posts order - add tests for tags order - add tests for users order --- core/server/api/utils.js | 3 +- core/server/models/base/index.js | 36 +++++++++++++- core/server/models/post.js | 2 +- core/server/models/tag.js | 2 +- core/server/models/user.js | 2 +- core/test/integration/api/api_posts_spec.js | 48 ++++++++++++++++++ core/test/integration/api/api_tags_spec.js | 54 +++++++++++++++++++++ core/test/integration/api/api_users_spec.js | 46 ++++++++++++++++++ core/test/unit/api_utils_spec.js | 2 +- 9 files changed, 189 insertions(+), 6 deletions(-) diff --git a/core/server/api/utils.js b/core/server/api/utils.js index d45690fd42..f7b5dfbae5 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -23,7 +23,7 @@ utils = { // ### Manual Default Options // These must be provided by the endpoint // browseDefaultOptions - valid for all browse api endpoints - browseDefaultOptions: ['page', 'limit', 'fields', 'filter'], + browseDefaultOptions: ['page', 'limit', 'fields', 'filter', 'order'], // idDefaultOptions - valid whenever an id is valid idDefaultOptions: ['id'], @@ -114,6 +114,7 @@ utils = { page: {matches: /^\d+$/}, limit: {matches: /^\d+|all$/}, fields: {matches: /^[\w, ]+$/}, + order: {matches: /^[a-z0-9_,\. ]+$/i}, name: {} }, // these values are sanitised/validated separately diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 0de08eb957..df08f6f977 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -301,7 +301,11 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ // TODO: this should just be done for all methods @ the API level options.withRelated = _.union(options.withRelated, options.include); - options.order = self.orderDefaultOptions(); + if (options.order) { + options.order = self.parseOrderOption(options.order); + } else { + options.order = self.orderDefaultOptions(); + } return itemCollection.fetchPage(options).then(function formatResponse(response) { var data = {}; @@ -455,6 +459,36 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ // Test for duplicate slugs. return checkIfSlugExists(slug); }); + }, + + parseOrderOption: function (order) { + var permittedAttributes, result, rules; + + permittedAttributes = this.prototype.permittedAttributes(); + result = {}; + rules = order.split(','); + + _.each(rules, function (rule) { + var match, field, direction; + + match = /^([a-z0-9_\.]+)\s+(asc|desc)$/i.exec(rule.trim()); + + // invalid order syntax + if (!match) { + return; + } + + field = match[1].toLowerCase(); + direction = match[2].toUpperCase(); + + if (permittedAttributes.indexOf(field) === -1) { + return; + } + + result[field] = direction; + }); + + return result; } }); diff --git a/core/server/models/post.js b/core/server/models/post.js index a8e57c9f6f..d5a70bffd6 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -388,7 +388,7 @@ Post = ghostBookshelf.Model.extend({ // these are the only options that can be passed to Bookshelf / Knex. validOptions = { findOne: ['importing', 'withRelated'], - findPage: ['page', 'limit', 'columns', 'filter', 'status', 'staticPages'], + findPage: ['page', 'limit', 'columns', 'filter', 'order', 'status', 'staticPages'], add: ['importing'] }; diff --git a/core/server/models/tag.js b/core/server/models/tag.js index 65d7b24bb5..ee38f4db98 100644 --- a/core/server/models/tag.js +++ b/core/server/models/tag.js @@ -79,7 +79,7 @@ Tag = ghostBookshelf.Model.extend({ // whitelists for the `options` hash argument on methods, by method name. // these are the only options that can be passed to Bookshelf / Knex. validOptions = { - findPage: ['page', 'limit', 'columns'] + findPage: ['page', 'limit', 'columns', 'order'] }; if (validOptions[methodName]) { diff --git a/core/server/models/user.js b/core/server/models/user.js index 04e32592d1..08ef85cd3b 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -219,7 +219,7 @@ User = ghostBookshelf.Model.extend({ findOne: ['withRelated', 'status'], setup: ['id'], edit: ['withRelated', 'id'], - findPage: ['page', 'limit', 'columns', 'status'] + findPage: ['page', 'limit', 'columns', 'order', 'status'] }; if (validOptions[methodName]) { diff --git a/core/test/integration/api/api_posts_spec.js b/core/test/integration/api/api_posts_spec.js index e2211a0b95..d15622bf84 100644 --- a/core/test/integration/api/api_posts_spec.js +++ b/core/test/integration/api/api_posts_spec.js @@ -330,6 +330,54 @@ describe('Post API', function () { done(); }).catch(done); }); + + it('can order posts using asc', function (done) { + var posts, expectedTitles; + + posts = _(testUtils.DataGenerator.Content.posts).reject('page').value(); + expectedTitles = _(posts).pluck('title').sortBy().value(); + + PostAPI.browse({context: {user: 1}, status: 'all', order: 'title asc', fields: 'title'}).then(function (results) { + should.exist(results.posts); + + var titles = _.pluck(results.posts, 'title'); + titles.should.eql(expectedTitles); + + done(); + }).catch(done); + }); + + it('can order posts using desc', function (done) { + var posts, expectedTitles; + + posts = _(testUtils.DataGenerator.Content.posts).reject('page').value(); + expectedTitles = _(posts).pluck('title').sortBy().reverse().value(); + + PostAPI.browse({context: {user: 1}, status: 'all', order: 'title DESC', fields: 'title'}).then(function (results) { + should.exist(results.posts); + + var titles = _.pluck(results.posts, 'title'); + titles.should.eql(expectedTitles); + + done(); + }).catch(done); + }); + + it('can order posts and filter disallowed attributes', function (done) { + var posts, expectedTitles; + + posts = _(testUtils.DataGenerator.Content.posts).reject('page').value(); + expectedTitles = _(posts).pluck('title').sortBy().value(); + + PostAPI.browse({context: {user: 1}, status: 'all', order: 'bunny DESC, title ASC', fields: 'title'}).then(function (results) { + should.exist(results.posts); + + var titles = _.pluck(results.posts, 'title'); + titles.should.eql(expectedTitles); + + done(); + }).catch(done); + }); }); describe('Read', function () { diff --git a/core/test/integration/api/api_tags_spec.js b/core/test/integration/api/api_tags_spec.js index c764d69005..415982d831 100644 --- a/core/test/integration/api/api_tags_spec.js +++ b/core/test/integration/api/api_tags_spec.js @@ -9,6 +9,14 @@ var testUtils = require('../../utils'), TagAPI = require('../../../server/api/tags'); +// there are some random generated tags in test database +// which can't be sorted easily using _.sortBy() +// so we filter them out and leave only pre-built fixtures +// usage: tags.filter(onlyFixtures) +function onlyFixtures(slug) { + return testUtils.DataGenerator.Content.tags.indexOf(slug) >= 0; +} + describe('Tags API', function () { // Keep the DB clean before(testUtils.teardown); @@ -259,6 +267,52 @@ describe('Tags API', function () { done(); }).catch(done); }); + + it('can browse and order by slug using asc', function (done) { + var expectedTags; + + TagAPI.browse({context: {user: 1}}) + .then(function (results) { + should.exist(results); + + expectedTags = _(results.tags).pluck('slug').filter(onlyFixtures).sortBy().value(); + + return TagAPI.browse({context: {user: 1}, order: 'slug asc'}); + }) + .then(function (results) { + var tags; + + should.exist(results); + + tags = _(results.tags).pluck('slug').filter(onlyFixtures).value(); + tags.should.eql(expectedTags); + }) + .then(done) + .catch(done); + }); + + it('can browse and order by slug using desc', function (done) { + var expectedTags; + + TagAPI.browse({context: {user: 1}}) + .then(function (results) { + should.exist(results); + + expectedTags = _(results.tags).pluck('slug').filter(onlyFixtures).sortBy().reverse().value(); + + return TagAPI.browse({context: {user: 1}, order: 'slug desc'}); + }) + .then(function (results) { + var tags; + + should.exist(results); + + tags = _(results.tags).pluck('slug').filter(onlyFixtures).value(); + tags.should.eql(expectedTags); + }) + .then(done) + .catch(done); + }); }); describe('Read', function () { diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index caf8214b2a..bd4948cff7 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -139,6 +139,52 @@ describe('Users API', function () { done(); }).catch(done); }); + + it('can browse and order by name using asc', function (done) { + var expectedUsers; + + UserAPI.browse(testUtils.context.admin) + .then(function (results) { + should.exist(results); + + expectedUsers = _(results.users).pluck('slug').sortBy().value(); + + return UserAPI.browse(_.extend({}, testUtils.context.admin, {order: 'slug asc'})); + }) + .then(function (results) { + var users; + + should.exist(results); + + users = _.pluck(results.users, 'slug'); + users.should.eql(expectedUsers); + }) + .then(done) + .catch(done); + }); + + it('can browse and order by name using desc', function (done) { + var expectedUsers; + + UserAPI.browse(testUtils.context.admin) + .then(function (results) { + should.exist(results); + + expectedUsers = _(results.users).pluck('slug').sortBy().reverse().value(); + + return UserAPI.browse(_.extend({}, testUtils.context.admin, {order: 'slug desc'})); + }) + .then(function (results) { + var users; + + should.exist(results); + + users = _.pluck(results.users, 'slug'); + users.should.eql(expectedUsers); + }) + .then(done) + .catch(done); + }); }); describe('Read', function () { diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 004cccf465..1838312887 100644 --- a/core/test/unit/api_utils_spec.js +++ b/core/test/unit/api_utils_spec.js @@ -18,7 +18,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', 'fields', 'filter']); + apiUtils.browseDefaultOptions.should.eql(['page', 'limit', 'fields', 'filter', 'order']); apiUtils.dataDefaultOptions.should.eql(['data']); apiUtils.idDefaultOptions.should.eql(['id']); });