Skip to content

Commit

Permalink
Add filter parameter using GQL
Browse files Browse the repository at this point in the history
refs TryGhost#5604, refs TryGhost#5463

- deps: [email protected]
- 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
  • Loading branch information
ErisDS committed Oct 22, 2015
1 parent ef57d81 commit b5cebb9
Show file tree
Hide file tree
Showing 11 changed files with 998 additions and 37 deletions.
4 changes: 2 additions & 2 deletions core/server/api/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],

Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 14 additions & 3 deletions core/server/models/base/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var _ = require('lodash'),
validation = require('../../data/validation'),
baseUtils = require('./utils'),
pagination = require('./pagination'),
gql = require('ghost-gql'),

ghostBookshelf;

Expand Down Expand Up @@ -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
Expand All @@ -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);
},
Expand Down
19 changes: 14 additions & 5 deletions core/server/models/base/pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand All @@ -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)) {
Expand All @@ -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);

Expand All @@ -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)
};
});
}
Expand Down
46 changes: 45 additions & 1 deletion core/server/models/base/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
var _ = require('lodash'),
collectionQuery,
processGQLResult,
filtering,
addPostCount,
tagUpdate;
Expand All @@ -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 = [];
Expand Down Expand Up @@ -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;
8 changes: 4 additions & 4 deletions core/server/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,23 +366,23 @@ 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')) {
// convert string true/false to boolean
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
// 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;
options.where['posts.status'] = options.status;
}

return options;
Expand All @@ -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']
};

Expand Down
Loading

0 comments on commit b5cebb9

Please sign in to comment.