From b5cebb9ec6c27f7c772f37c29cbca26de1511cf7 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 28 Jul 2015 01:27:54 +0100 Subject: [PATCH] Add `filter` parameter using GQL refs #5604, refs #5463 - deps: ghost-gql@0.0.2 - adds code to wire up the filtering to a paginated query - updated pagination plugin count query to use 'distinct' so it's more robust - rename paginationUtils.query to addLimitAndOffset to be more explicit and make the code clearer - add a new 'advanced browsing spec' set of tests for tracking these features as they are built out --- core/server/api/utils.js | 4 +- core/server/models/base/index.js | 17 +- core/server/models/base/pagination.js | 19 +- core/server/models/base/utils.js | 46 +- core/server/models/post.js | 8 +- .../integration/api/advanced_browse_spec.js | 580 ++++++++++++++++++ core/test/unit/api_utils_spec.js | 2 +- core/test/unit/models_pagination_spec.js | 42 +- .../test/utils/fixtures/filter-param/index.js | 312 ++++++++++ core/test/utils/index.js | 4 +- package.json | 1 + 11 files changed, 998 insertions(+), 37 deletions(-) create mode 100644 core/test/integration/api/advanced_browse_spec.js create mode 100644 core/test/utils/fixtures/filter-param/index.js diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 1d3ca57df7..767c600215 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'], + browseDefaultOptions: ['page', 'limit', 'fields', 'filter'], // idDefaultOptions - valid whenever an id is valid idDefaultOptions: ['id'], @@ -117,7 +117,7 @@ utils = { name: {} }, // these values are sanitised/validated separately - noValidation = ['data', 'context', 'include'], + noValidation = ['data', 'context', 'include', 'filter'], errors = []; _.each(options, function (value, key) { diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 08841d7c1f..b26a8ba751 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -19,6 +19,7 @@ var _ = require('lodash'), validation = require('../../data/validation'), baseUtils = require('./utils'), pagination = require('./pagination'), + gql = require('ghost-gql'), ghostBookshelf; @@ -278,14 +279,24 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ options = this.processOptions(itemCollection, options); // Prefetch filter objects - return Promise.all(baseUtils.filtering.preFetch(filterObjects)).then(function doQuery() { + return Promise.all(baseUtils.oldFiltering.preFetch(filterObjects)).then(function doQuery() { // If there are `where` conditionals specified, add those to the query. if (options.where) { itemCollection.query('where', options.where); } + // Apply FILTER + if (options.filter) { + options.filter = gql.parse(options.filter); + itemCollection.query(function (qb) { + gql.knexify(qb, options.filter); + }); + + baseUtils.processGQLResult(itemCollection, options); + } + // Setup filter joins / queries - baseUtils.filtering.query(filterObjects, itemCollection); + baseUtils.oldFiltering.query(filterObjects, itemCollection); // Handle related objects // TODO: this should just be done for all methods @ the API level @@ -298,7 +309,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ data[tableName] = response.collection.toJSON(options); data.meta = {pagination: response.pagination}; - return baseUtils.filtering.formatResponse(filterObjects, options, data); + return baseUtils.oldFiltering.formatResponse(filterObjects, options, data); }); }).catch(errors.logAndThrowError); }, diff --git a/core/server/models/base/pagination.js b/core/server/models/base/pagination.js index 4560c8407c..b093e6baa3 100644 --- a/core/server/models/base/pagination.js +++ b/core/server/models/base/pagination.js @@ -48,10 +48,10 @@ paginationUtils = { /** * ### Query * Apply the necessary parameters to paginate the query - * @param {Bookshelf.Model, Bookshelf.Collection} itemCollection + * @param {Bookshelf.Model|Bookshelf.Collection} itemCollection * @param {options} options */ - query: function query(itemCollection, options) { + addLimitAndOffset: function addLimitAndOffset(itemCollection, options) { if (_.isNumber(options.limit)) { itemCollection .query('limit', options.limit) @@ -151,7 +151,10 @@ pagination = function pagination(bookshelf) { // Add any where or join clauses which need to be included with the aggregate query // 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'); + // Due to lack of support for count distinct, this is pretty complex. + countPromise = this.query().clone().select( + bookshelf.knex.raw('count(distinct ' + tableName + '.' + idAttribute + ') as aggregate') + ); // Clone the base query into our collection collection._knex = this.query().clone(); @@ -160,7 +163,7 @@ pagination = function pagination(bookshelf) { // Add any where or join clauses which need to NOT be included with the aggregate query // Setup the pagination parameters so that we return the correct items from the set - paginationUtils.query(collection, options); + paginationUtils.addLimitAndOffset(collection, options); // Apply ordering options if they are present if (options.order && !_.isEmpty(options.order)) { @@ -169,6 +172,12 @@ pagination = function pagination(bookshelf) { }); } + if (options.groups && !_.isEmpty(options.groups)) { + _.each(options.groups, function (group) { + collection.query('groupBy', group); + }); + } + // Apply count options if they are present baseUtils.collectionQuery.count(collection, options); @@ -195,7 +204,7 @@ pagination = function pagination(bookshelf) { // Format the collection & count result into `{collection: [], pagination: {}}` return { collection: results[0], - pagination: paginationUtils.formatResponse(results[1][0].aggregate, options) + pagination: paginationUtils.formatResponse(results[1][0] ? results[1][0].aggregate : 0, options) }; }); } diff --git a/core/server/models/base/utils.js b/core/server/models/base/utils.js index fae15e42e5..16ee1dcd8f 100644 --- a/core/server/models/base/utils.js +++ b/core/server/models/base/utils.js @@ -4,6 +4,7 @@ */ var _ = require('lodash'), collectionQuery, + processGQLResult, filtering, addPostCount, tagUpdate; @@ -25,6 +26,48 @@ collectionQuery = { } }; +processGQLResult = function processGQLResult(itemCollection, options) { + var joinTables = options.filter.joins, + tagsHasIn = false; + + if (joinTables && joinTables.indexOf('tags') > -1) { + // We need to use leftOuterJoin to insure we still include posts which don't have tags in the result + // The where clause should restrict which items are returned + itemCollection + .query('leftOuterJoin', 'posts_tags', 'posts_tags.post_id', '=', 'posts.id') + .query('leftOuterJoin', 'tags', 'posts_tags.tag_id', '=', 'tags.id'); + + // The order override should ONLY happen if we are doing an "IN" query + // TODO move the order handling to the query building that is currently inside pagination + // TODO make the order handling in pagination handle orderByRaw + // TODO extend this handling to all joins + _.each(options.filter.statements, function (statement) { + if (statement.op === 'IN' && statement.prop.match(/tags/)) { + tagsHasIn = true; + } + }); + + if (tagsHasIn) { + // TODO make this count the number of MATCHING tags, not just the number of tags + itemCollection.query('orderByRaw', 'count(tags.id) DESC'); + } + + // We need to add a group by to counter the double left outer join + // TODO improve on th group by handling + options.groups = options.groups || []; + options.groups.push('posts.id'); + } + + if (joinTables && joinTables.indexOf('author') > -1) { + itemCollection + .query('join', 'users as author', 'author.id', '=', 'posts.author_id'); + } +}; + +/** + * All of this can be removed once the filter parameter is in place + * And the current filtering methods are removed + */ filtering = { preFetch: function preFetch(filterObjects) { var promises = []; @@ -129,7 +172,8 @@ tagUpdate = { } }; -module.exports.filtering = filtering; +module.exports.oldFiltering = filtering; +module.exports.processGQLResult = processGQLResult; module.exports.collectionQuery = collectionQuery; module.exports.addPostCount = addPostCount; module.exports.tagUpdate = tagUpdate; diff --git a/core/server/models/post.js b/core/server/models/post.js index 07653b0c3c..8869ccff84 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -366,7 +366,7 @@ Post = ghostBookshelf.Model.extend({ if (!_.isBoolean(options.staticPages)) { options.staticPages = _.contains(['true', '1'], options.staticPages); } - options.where.page = options.staticPages; + options.where['posts.page'] = options.staticPages; } if (_.has(options, 'featured')) { @@ -374,7 +374,7 @@ Post = ghostBookshelf.Model.extend({ if (!_.isBoolean(options.featured)) { options.featured = _.contains(['true', '1'], options.featured); } - options.where.featured = options.featured; + options.where['posts.featured'] = options.featured; } // Unless `all` is passed as an option, filter on @@ -382,7 +382,7 @@ Post = ghostBookshelf.Model.extend({ 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; + options.where['posts.status'] = options.status; } return options; @@ -400,7 +400,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', 'status', 'staticPages', 'featured'], + findPage: ['page', 'limit', 'columns', 'filter', 'status', 'staticPages', 'featured'], add: ['importing'] }; diff --git a/core/test/integration/api/advanced_browse_spec.js b/core/test/integration/api/advanced_browse_spec.js new file mode 100644 index 0000000000..d47851763c --- /dev/null +++ b/core/test/integration/api/advanced_browse_spec.js @@ -0,0 +1,580 @@ +/*globals describe, before, after, it */ +/*jshint expr:true*/ +var testUtils = require('../../utils'), + should = require('should'), + _ = require('lodash'), + +// Stuff we are testing + PostAPI = require('../../../server/api/posts'), + TagAPI = require('../../../server/api/tags'), + UserAPI = require('../../../server/api/users'); + +describe('Filter Param Spec', function () { + // Initialise the DB just once, the tests are fetch-only + before(testUtils.teardown); + before(testUtils.setup('filter')); + after(testUtils.teardown); + + should.exist(PostAPI); + + describe('Advanced Use Cases', function () { + describe('1. Posts - filter: "tags: [photo, video] + id: -4", limit: "3", include: "tags"', function () { + it('Will fetch 3 posts with tags which match `photo` or `video` and are not the post with id 4.', function (done) { + PostAPI.browse({filter: 'tags: [photo, video] + id: -4', limit: 3, include: 'tags'}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 3 items according to the limit property + result.posts.should.be.an.Array.with.lengthOf(3); + + // None of the items returned should be the post with id 4, as that was excluded + ids = _.pluck(result.posts, 'id'); + ids.should.not.containEql(4); + + // Should not contain draft + ids.should.not.containEql(19); + + // The ordering specifies that any post which matches both tags should be first + // Post 2 is the first in the list to have both tags + ids[0].should.eql(2); + + // Each post should have a tag which matches either 'photo' or 'video' + _.each(result.posts, function (post) { + var slugs = _.pluck(post.tags, 'slug'); + slugs.should.matchAny(/photo|video/); + }); + + // TODO: match order, followed by publish date + // This isn't finished yet, as the 'special rule' ordering for matching 'in' requests hasn't been + // implemented properly. + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(3); + result.meta.pagination.pages.should.eql(3); + result.meta.pagination.total.should.eql(7); + result.meta.pagination.next.should.eql(2); + should.equal(result.meta.pagination.prev, null); + + done(); + }).catch(done); + }); + }); + + describe('2. Posts - filter: "tag:photo,featured:true,image:-null", include: "tags"', function () { + it('Will fetch posts which have either a tag of `photo`, are marked `featured` or have an image.', function (done) { + PostAPI.browse({filter: 'tag:photo,featured:true,image:-null', include: 'tags'}).then(function (result) { + var ids; + + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(10); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([15, 14, 11, 9, 8, 7, 6, 5, 3, 2]); + + // TODO: Should be in published order + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(10); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + done(); + }).catch(done); + }); + }); + + describe.skip('3. Tags - filter="post.count:>=1" order="posts.count DESC" limit="all"', function () { + it('Will fetch all tags, ordered by post count, where the post count is at least 1.', function (done) { + TagAPI.browse({filter: 'post.count:>=1', order: 'posts.count DESC', limit: 'all', include: 'posts.count'}).then(function (result) { + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('tags'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 3 matching items + result.tags.should.be.an.Array.with.lengthOf(3); + + // TODO: add the ordering + // TODO: manage the count + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + // TODO complete meta data assertions + + done(); + }).catch(done); + }); + }); + + describe('4. Posts - filter="author:[leslie,pat]+(featured:true,tag:audio)"', function () { + // Note that `pat` doesn't exist (it's `pat-smith`) + it('Will fetch posts by the author `leslie` or `pat` which are either featured or have tag `audio`.', function (done) { + PostAPI.browse({filter: 'author:[leslie,pat]+(featured:true,tag:audio)', include: 'author,tags'}).then(function (result) { + var ids, authors; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 2 matching items + result.posts.should.be.an.Array.with.lengthOf(4); + + // Each post must either have the author 'leslie' or 'pat' + authors = _.map(result.posts, function (post) { + return post.author.slug; + }); + authors.should.matchAny(/leslie|pat/); + + // Each post must either be featured or have the tag 'audio' + _.each(result.posts, function (post) { + var tags; + // This construct ensures we get an assertion or a failure + if (post.featured === 'true') { + post.featured.should.be.true; + } else { + tags = _.pluck(post.tags, 'slug'); + tags.should.containEql('audio'); + } + }); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([14, 12, 9, 8]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(4); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + done(); + }).catch(done); + }); + }); + + describe('5. Users - filter="posts.tags:photo" order="posts.count DESC" limit="3"', function () { + it('Will fetch the 3 most prolific users who write posts with the tag `photo` ordered by most posts.', function (done) { + UserAPI.browse({filter: 'posts.tags:photo', order: 'posts.count DESC', limit: 3}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('users'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 2 matching items + result.users.should.be.an.Array.with.lengthOf(2); + + ids = _.pluck(result.users, 'id'); + ids.should.eql([1, 2]); + + // TODO: add the order + // TODO: manage the count + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(3); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(2); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + done(); + }).catch(done); + }); + }); + + describe.skip('6. Posts filter="published_at:>\'2015-07-20\'" limit="5"}}', function () { + it('Will fetch 5 posts after a given date.', function (done) { + PostAPI.browse({filter: 'published_at:>\'2015-07-20\'', limit: 5, include: 'tags'}).then(function (result) { + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // TODO: make dates work + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + // TODO complete meta data assertions + + done(); + }).catch(done); + }); + }); + }); + + describe.skip('Count capabilities', function () { + it('can fetch `posts.count` for tags (published only)', function (done) { + // This could be posts.count & posts.all.count? + done(); + }); + + it('can fetch `posts.all.count` for tags (all posts)', function (done) { + done(); + }); + + it('can fetch `posts.count` for users (published only)', function (done) { + // This could be posts.count & posts.all.count? + done(); + }); + + it('can fetch `posts.all.count` for users (all posts)', function (done) { + done(); + }); + + it('can fetch `tags.count` for posts', function (done) { + done(); + }); + }); + + describe('Old Use Cases', function () { + // Please note: these tests are mostly here to help prove certain things whilst building out new behaviour + describe('Old post "filters"', function () { + it('OLD STYLE TAG FILTER For checking against.. to be removed', function (done) { + PostAPI.browse({tag: 'photo', include: 'tag,author'}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 4 matching items + result.posts.should.be.an.Array.with.lengthOf(4); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([11, 9, 3, 2]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(4); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + // NOTE: old query has meta filter + result.meta.should.have.property('filters'); + + done(); + }).catch(done); + }); + + it('Will fetch posts with a given tag', function (done) { + PostAPI.browse({filter: 'tag:photo', include: 'tag,author'}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 4 matching items + result.posts.should.be.an.Array.with.lengthOf(4); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([11, 9, 3, 2]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(4); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + // NOTE: new query does not have meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + + it('OLD STYLE AUTHOR FILTER For checking against.. to be removed', function (done) { + PostAPI.browse({author: 'leslie', include: 'tag,author', limit: 5, page: 2}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(5); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([13, 12, 11, 10, 9]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(2); + result.meta.pagination.limit.should.eql(5); + result.meta.pagination.pages.should.eql(3); + result.meta.pagination.total.should.eql(15); + result.meta.pagination.next.should.eql(3); + result.meta.pagination.prev.should.eql(1); + + // NOTE: old query has meta filter + result.meta.should.have.property('filters'); + + done(); + }).catch(done); + }); + + it('Will fetch posts with a given author', function (done) { + PostAPI.browse({filter: 'author:leslie', include: 'tag,author', limit: 5, page: 2}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(5); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([13, 12, 11, 10, 9]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(2); + result.meta.pagination.limit.should.eql(5); + result.meta.pagination.pages.should.eql(3); + result.meta.pagination.total.should.eql(15); + result.meta.pagination.next.should.eql(3); + result.meta.pagination.prev.should.eql(1); + + // NOTE: old query has meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + }); + + describe('Handling "featured"', function () { + it('Will fetch all posts regardless of featured status by default', function (done) { + PostAPI.browse({}).then(function (result) { + var ids; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(15); + + ids = _.pluck(result.posts, 'id'); + ids.should.eql([20, 18, 17, 16, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(2); + result.meta.pagination.total.should.eql(18); + result.meta.pagination.next.should.eql(2); + should.equal(result.meta.pagination.prev, null); + + // NOTE: old query has meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + + it('Will fetch only featured posts when requested', function (done) { + PostAPI.browse({filter: 'featured:true'}).then(function (result) { + var ids, featured; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(3); + + // All posts should be marked as featured 'true' + featured = _.pluck(result.posts, 'featured'); + featured.should.matchEach(true); + + // Match exact items + ids = _.pluck(result.posts, 'id'); + ids.should.eql([14, 8, 5]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(3); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + // NOTE: old query has meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + + it('Will fetch only non-featured posts when requested', function (done) { + PostAPI.browse({filter: 'featured:false'}).then(function (result) { + var ids, featured; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(15); + + // All posts should be marked as featured 'false' + featured = _.pluck(result.posts, 'featured'); + featured.should.matchEach(false); + + // Match exact items + ids = _.pluck(result.posts, 'id'); + ids.should.eql([20, 18, 17, 16, 13, 12, 11, 10, 9, 7, 6, 4, 3, 2, 1]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(15); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + // NOTE: old query has meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + }); + + describe('Handling "page" (staticPages)', function () { + it('Will return only posts by default', function (done) { + PostAPI.browse({limit: 'all'}).then(function (result) { + var ids, page; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(18); + + // All posts should be marked as page 'false' + page = _.pluck(result.posts, 'page'); + page.should.matchEach(false); + + // Match exact items + ids = _.pluck(result.posts, 'id'); + ids.should.eql([20, 18, 17, 16, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql('all'); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(18); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + // NOTE: old query has meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + + // TODO: determine if this should be supported via filter, or whether it should only be available via a 'PageAPI' + it.skip('Will return only pages when requested', function (done) { + PostAPI.browse({filter: 'page:true'}).then(function (result) { + var ids, page; + // 1. Result should have the correct base structure + should.exist(result); + result.should.have.property('posts'); + result.should.have.property('meta'); + + // 2. The data part of the response should be correct + // We should have 5 matching items + result.posts.should.be.an.Array.with.lengthOf(2); + + // All posts should be marked as page 'true' + page = _.pluck(result.posts, 'page'); + page.should.matchEach(true); + + // Match exact items + ids = _.pluck(result.posts, 'id'); + ids.should.eql([21, 15]); + + // 3. The meta object should contain the right details + result.meta.should.have.property('pagination'); + result.meta.pagination.should.be.an.Object.with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']); + result.meta.pagination.page.should.eql(1); + result.meta.pagination.limit.should.eql(15); + result.meta.pagination.pages.should.eql(1); + result.meta.pagination.total.should.eql(2); + should.equal(result.meta.pagination.next, null); + should.equal(result.meta.pagination.prev, null); + + // NOTE: old query has meta filter + result.meta.should.not.have.property('filters'); + + done(); + }).catch(done); + }); + + it.skip('Will NOT return both posts and pages from post API', function (done) { + done(); + }); + }); + }); +}); diff --git a/core/test/unit/api_utils_spec.js b/core/test/unit/api_utils_spec.js index 8a3a398685..004cccf465 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']); + apiUtils.browseDefaultOptions.should.eql(['page', 'limit', 'fields', 'filter']); apiUtils.dataDefaultOptions.should.eql(['data']); apiUtils.idDefaultOptions.should.eql(['id']); }); diff --git a/core/test/unit/models_pagination_spec.js b/core/test/unit/models_pagination_spec.js index 0e50bd957a..7cd08c12f4 100644 --- a/core/test/unit/models_pagination_spec.js +++ b/core/test/unit/models_pagination_spec.js @@ -146,12 +146,12 @@ describe('pagination', function () { }); }); - describe('query', function () { - var query, + describe('addLimitAndOffset', function () { + var addLimitAndOffset, collection = {}; before(function () { - query = paginationUtils.query; + addLimitAndOffset = paginationUtils.addLimitAndOffset; }); beforeEach(function () { @@ -159,7 +159,7 @@ describe('pagination', function () { }); it('should add query options if limit is set', function () { - query(collection, {limit: 5, page: 1}); + addLimitAndOffset(collection, {limit: 5, page: 1}); collection.query.calledTwice.should.be.true; collection.query.firstCall.calledWith('limit', 5).should.be.true; @@ -167,7 +167,7 @@ describe('pagination', function () { }); it('should not add query options if limit is not set', function () { - query(collection, {page: 1}); + addLimitAndOffset(collection, {page: 1}); collection.query.called.should.be.false; }); @@ -175,7 +175,7 @@ describe('pagination', function () { }); describe('fetchPage', function () { - var model, bookshelf, on, mockQuery, fetch, colQuery; + var model, bookshelf, knex, on, mockQuery, fetch, colQuery; before(function () { paginationUtils = pagination.__get__('paginationUtils'); @@ -184,16 +184,16 @@ describe('pagination', function () { beforeEach(function () { // Stub paginationUtils paginationUtils.parseOptions = sandbox.stub(); - paginationUtils.query = sandbox.stub(); + paginationUtils.addLimitAndOffset = sandbox.stub(); paginationUtils.formatResponse = sandbox.stub().returns({}); // Mock out bookshelf model mockQuery = { clone: sandbox.stub(), - count: sandbox.stub() + select: sandbox.stub() }; mockQuery.clone.returns(mockQuery); - mockQuery.count.returns([{aggregate: 1}]); + mockQuery.select.returns([{aggregate: 1}]); fetch = sandbox.stub().returns(Promise.resolve({})); colQuery = sandbox.stub(); @@ -212,7 +212,9 @@ describe('pagination', function () { model.prototype.resetQuery = sandbox.stub(); model.prototype.query.returns(mockQuery); - bookshelf = {Model: model}; + knex = {raw: sandbox.stub().returns(Promise.resolve())}; + + bookshelf = {Model: model, knex: knex}; pagination(bookshelf); }); @@ -231,10 +233,10 @@ describe('pagination', function () { model.prototype.constructor.collection, model.prototype.query, mockQuery.clone, - mockQuery.count, + mockQuery.select, model.prototype.query, mockQuery.clone, - paginationUtils.query, + paginationUtils.addLimitAndOffset, on, on, fetch, @@ -244,7 +246,7 @@ describe('pagination', function () { paginationUtils.parseOptions.calledOnce.should.be.true; paginationUtils.parseOptions.calledWith(undefined).should.be.true; - paginationUtils.query.calledOnce.should.be.true; + paginationUtils.addLimitAndOffset.calledOnce.should.be.true; paginationUtils.formatResponse.calledOnce.should.be.true; model.prototype.constructor.collection.calledOnce.should.be.true; @@ -258,8 +260,8 @@ describe('pagination', function () { 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; + mockQuery.select.calledOnce.should.be.true; + mockQuery.select.calledWith().should.be.true; on.calledTwice.should.be.true; on.firstCall.calledWith('fetching').should.be.true; @@ -282,10 +284,10 @@ describe('pagination', function () { model.prototype.constructor.collection, model.prototype.query, mockQuery.clone, - mockQuery.count, + mockQuery.select, model.prototype.query, mockQuery.clone, - paginationUtils.query, + paginationUtils.addLimitAndOffset, colQuery, on, on, @@ -296,7 +298,7 @@ describe('pagination', function () { paginationUtils.parseOptions.calledOnce.should.be.true; paginationUtils.parseOptions.calledWith(orderOptions).should.be.true; - paginationUtils.query.calledOnce.should.be.true; + paginationUtils.addLimitAndOffset.calledOnce.should.be.true; paginationUtils.formatResponse.calledOnce.should.be.true; model.prototype.constructor.collection.calledOnce.should.be.true; @@ -310,8 +312,8 @@ describe('pagination', function () { 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; + mockQuery.select.calledOnce.should.be.true; + mockQuery.select.calledWith().should.be.true; colQuery.calledOnce.should.be.true; colQuery.calledWith('orderBy', 'undefined.id', 'DESC').should.be.true; diff --git a/core/test/utils/fixtures/filter-param/index.js b/core/test/utils/fixtures/filter-param/index.js new file mode 100644 index 0000000000..d3e957b9b8 --- /dev/null +++ b/core/test/utils/fixtures/filter-param/index.js @@ -0,0 +1,312 @@ +/** + * These fixtures are just for testing the filter spec + */ +var _ = require('lodash'), + config = require('../../../../server/config'), + data = {}; + +data.tags = [ + { + name: 'Getting Started', + slug: 'getting-started', + created_by: 1 + }, + { + name: 'photo', + slug: 'photo', + created_by: 2 + }, + { + name: 'Video', + slug: 'video', + created_by: 1 + }, + { + name: 'Audio', + slug: 'audio', + created_by: 1 + }, + { + name: 'No Posts', + slug: 'no-posts', + created_by: 2 + } +]; + +// Password = Sl1m3rson +data.users = [ + { + name: 'Leslie Jones', + slug: 'leslie', + email: 'ljones@nothere.com', + password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6' + }, + { + name: 'Pat Smith', + slug: 'pat-smith', + email: 'pat-smith@nothere.com', + password: '$2a$10$.pZeeBE0gHXd0PTnbT/ph.GEKgd0Wd3q2pWna3ynTGBkPKnGIKZL6' + } +]; + +data.posts = [ + { + title: 'First Post', + slug: 'first-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [1] + }, + { + title: 'Second Post', + slug: 'second-post', + markdown: 'Hello World!', + featured: false, + author_id: 2, + tags: [2, 3, 4] + }, + { + title: 'Third Post', + slug: 'third-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [2] + }, + { + title: 'Fourth Post', + slug: 'fourth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [3] + }, + { + title: 'Fifth Post', + slug: 'fifth-post', + markdown: 'Hello World!', + featured: true, + author_id: 2, + tags: [] + }, + { + title: 'Sixth Post', + slug: 'sixth-post', + markdown: 'Hello World!', + featured: false, + author_id: 2, + image: 'some/image/path.jpg', + tags: [1, 4] + }, + { + title: 'Seventh Post', + slug: 'seventh-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + image: 'some/image/path.jpg', + tags: [1, 3] + }, + { + title: 'Eighth Post', + slug: 'eighth-post', + markdown: 'Hello World!', + featured: true, + author_id: 1, + tags: [1, 3, 4] + }, + { + title: 'Ninth Post', + slug: 'ninth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [2, 4] + }, + { + title: 'Tenth Post', + slug: 'tenth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [3] + }, + { + title: 'Eleventh Post', + slug: 'eleventh-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + image: 'some/image/path.jpg', + tags: [2] + }, + { + title: 'Twelfth Post', + slug: 'twelfth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [4] + }, + { + title: 'Thirteenth Post', + slug: 'thirteenth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [] + }, + { + title: 'Fourteenth Post', + slug: 'fourteenth-post', + markdown: 'Hello World!', + featured: true, + author_id: 1, + tags: [4] + }, + { + title: 'Fifteenth Post', + slug: 'fifteenth-post', + markdown: 'Hello World! I am a featured page', + featured: true, + page: 1, + author_id: 1, + tags: [] + }, + { + title: 'Sixteenth Post', + slug: 'sixteenth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [] + }, + { + title: 'Seventeenth Post', + slug: 'seventeenth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [] + }, + { + title: 'Eighteenth Post', + slug: 'eighteenth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [] + }, + { + title: 'Nineteenth Post', + slug: 'nineteenth-post', + markdown: 'Hello World!', + featured: false, + status: 'draft', + author_id: 1, + tags: [1, 2, 3, 4] + }, + { + title: 'Twentieth Post', + slug: 'twentieth-post', + markdown: 'Hello World!', + featured: false, + author_id: 1, + tags: [] + }, + { + title: 'About Page', + slug: 'about', + markdown: 'About Me!', + featured: false, + page: 1, + author_id: 1, + tags: [] + } +]; + +function fixDataIndexes(origData, storedData) { + var indexedData = {}; + _.each(origData, function (orig, index) { + indexedData[index + 1] = _.find(storedData, function (stored) { + return stored.slug === orig.slug; + }); + }); + + return indexedData; +} + +function writeFetchFix(knex, resource) { + return knex(resource).insert(data[resource]).then(function () { + return knex(resource).select(); + }).then(function (stored) { + return fixDataIndexes(data[resource], stored); + }); +} + +function createUsers(knex, DataGenerator) { + // First, loop through and prep the data + data.users = _.map(data.users, function (user) { + return DataGenerator.forKnex.createUser(user); + }); + + // Next, insert it into the database & return the correctly indexed data + return writeFetchFix(knex, 'users'); +} + +function createTags(knex, DataGenerator, created) { + data.tags = _.map(data.tags, function (tag) { + tag = DataGenerator.forKnex.createBasic(tag); + tag.created_by = created.users[tag.created_by].id; + return tag; + }); + + // Next, insert it into the database & return the correctly indexed data + return writeFetchFix(knex, 'tags'); +} + +function createPosts(knex, DataGenerator, created) { + var postsTags = []; + data.posts = _.map(data.posts, function (post, index) { + post = DataGenerator.forKnex.createPost(post); + post.created_by = created.users[post.author_id].id; + post.author_id = created.users[post.author_id].id; + _.each(post.tags, function (tagId) { + postsTags.push({post_id: index + 1, tag_id: created.tags[tagId].id}); + }); + delete post.tags; + return post; + }); + + // Next, insert it into the database & return the correctly indexed data + return writeFetchFix(knex, 'posts').then(function (createdPosts) { + // Handle post tags + postsTags = _.map(postsTags, function (postTag) { + postTag.post_id = createdPosts[postTag.post_id].id; + return postTag; + }); + + return knex('posts_tags').insert(postsTags).then(function () { + return createdPosts; + }); + }); +} + +module.exports = function (DataGenerator) { + var knex = config.database.knex, + created = {}; + // Create users first + return createUsers(knex, DataGenerator).then(function (createdUsers) { + created.users = createdUsers; + // Next create tags + return createTags(knex, DataGenerator, created); + }).then(function (createdTags) { + created.tags = createdTags; + // Finally, setup posts with the right authors and tags + return createPosts(knex, DataGenerator, created); + }).then(function (createdPosts) { + created.posts = createdPosts; + return created; + }); +}; diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 03a1ef6b33..5cd0748563 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -10,6 +10,7 @@ var Promise = require('bluebird'), permissions = require('../../server/permissions'), permsFixtures = require('../../server/data/fixtures/permissions/permissions.json'), DataGenerator = require('./fixtures/data-generator'), + filterData = require('./fixtures/filter-param'), API = require('./api'), fork = require('./fork'), config = require('../../server/config'), @@ -419,7 +420,8 @@ toDoList = { perms: function permissionsFor(obj) { return function permissionsForObj() { return fixtures.permissionsFor(obj); }; }, - clients: function insertClients() { return fixtures.insertClients(); } + clients: function insertClients() { return fixtures.insertClients(); }, + filter: function createFilterParamFixtures() { return filterData(DataGenerator); } }; /** diff --git a/package.json b/package.json index 702aab3942..af2bc3aec3 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "express-hbs": "0.8.4", "extract-zip": "1.1.1", "fs-extra": "0.24.0", + "ghost-gql": "0.0.2", "glob": "5.0.15", "html-to-text": "1.3.2", "intl": "1.0.0",