OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #6005 from ErisDS/old-filter-clean

Browse Source 
Remove featured, tag, author & role API params
This commit is contained in:
Sebastian Gierlinger 2015-10-27 13:05:12 +01:00
commit 9dc4efca13
14 changed files with 76 additions and 257 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/server/api/posts.js
View File

@ -36,7 +36,7 @@ posts = {
* @returns {Promise<Posts>} Posts Collection with Meta
*/
browse: function browse(options) {
var extraOptions = ['tag', 'author', 'status', 'staticPages', 'featured'],
var extraOptions = ['status', 'staticPages'],
permittedOptions = utils.browseDefaultOptions.concat(extraOptions),
tasks;

2
core/server/api/users.js
View File

@ -73,7 +73,7 @@ users = {
* @returns {Promise<Users>} Users Collection
*/
browse: function browse(options) {
var extraOptions = ['role', 'status'],
var extraOptions = ['status'],
permittedOptions = utils.browseDefaultOptions.concat(extraOptions),
tasks;

8
core/server/helpers/get.js
View File

@ -74,14 +74,6 @@ function resolvePaths(data, value) {
* @returns {*}
*/
function parseOptions(data, options) {
if (_.isArray(options.tag)) {
options.tag = _.pluck(options.tag, 'slug').join(',');
}
if (_.isObject(options.author)) {
options.author = options.author.slug;
}
if (_.isString(options.filter)) {
options.filter = resolvePaths(data, options.filter);
}

59
core/server/models/base/index.js
View File

@ -266,8 +266,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
var self = this,
itemCollection = this.forge(),
tableName = _.result(this.prototype, 'tableName'),
filterObjects = self.setupFilters(options);
tableName = _.result(this.prototype, 'tableName');
// Filter options so that only permitted ones remain
options = this.filterOptions(options, 'findPage');
@ -283,39 +282,33 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
options.columns = _.intersection(options.columns, this.prototype.permittedAttributes());
}
// Prefetch filter objects
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);
}
// 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.oldFiltering.query(filterObjects, itemCollection);
// Handle related objects
// TODO: this should just be done for all methods @ the API level
options.withRelated = _.union(options.withRelated, options.include);
options.order = self.orderDefaultOptions();
return itemCollection.fetchPage(options).then(function formatResponse(response) {
var data = {};
data[tableName] = response.collection.toJSON(options);
data.meta = {pagination: response.pagination};
return baseUtils.oldFiltering.formatResponse(filterObjects, options, data);
// Apply FILTER
if (options.filter) {
options.filter = gql.parse(options.filter);
itemCollection.query(function (qb) {
gql.knexify(qb, options.filter);
});
baseUtils.processGQLResult(itemCollection, options);
}
// Handle related objects
// TODO: this should just be done for all methods @ the API level
options.withRelated = _.union(options.withRelated, options.include);
options.order = self.orderDefaultOptions();
return itemCollection.fetchPage(options).then(function formatResponse(response) {
var data = {};
data[tableName] = response.collection.toJSON(options);
data.meta = {pagination: response.pagination};
return data;
}).catch(errors.logAndThrowError);
},

48
core/server/models/base/utils.js
View File

@ -5,7 +5,6 @@
var _ = require('lodash'),
collectionQuery,
processGQLResult,
filtering,
addPostCount,
tagUpdate;
@ -64,52 +63,6 @@ processGQLResult = function processGQLResult(itemCollection, options) {
}
};
/**
* 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 = [];
_.forOwn(filterObjects, function (obj) {
promises.push(obj.fetch());
});
return promises;
},
query: function query(filterObjects, itemCollection) {
if (filterObjects.tags) {
itemCollection
.query('join', 'posts_tags', 'posts_tags.post_id', '=', 'posts.id')
.query('where', 'posts_tags.tag_id', '=', filterObjects.tags.id);
}
if (filterObjects.author) {
itemCollection
.query('where', 'author_id', '=', filterObjects.author.id);
}
if (filterObjects.roles) {
itemCollection
.query('join', 'roles_users', 'roles_users.user_id', '=', 'users.id')
.query('where', 'roles_users.role_id', '=', filterObjects.roles.id);
}
},
formatResponse: function formatResponse(filterObjects, options, data) {
if (!_.isEmpty(filterObjects)) {
data.meta.filters = {};
}
_.forOwn(filterObjects, function (obj, key) {
if (!filterObjects[key].isNew()) {
data.meta.filters[key] = [filterObjects[key].toJSON(options)];
}
});
return data;
}
};
tagUpdate = {
fetchCurrentPost: function fetchCurrentPost(PostModel, id, options) {
return PostModel.forge({id: id}).fetch(_.extend({}, options, {withRelated: ['tags']}));
@ -172,7 +125,6 @@ tagUpdate = {
}
};
module.exports.oldFiltering = filtering;
module.exports.processGQLResult = processGQLResult;
module.exports.collectionQuery = collectionQuery;
module.exports.addPostCount = addPostCount;

16
core/server/models/post.js
View File

@ -330,20 +330,6 @@ Post = ghostBookshelf.Model.extend({
return attrs;
}
}, {
setupFilters: function setupFilters(options) {
var filterObjects = {};
// Deliberately switch from singular 'tag' to 'tags' and 'role' to 'roles' here
// TODO: make this consistent
if (options.tag !== undefined) {
filterObjects.tags = ghostBookshelf.model('Tag').forge({slug: options.tag});
}
if (options.author !== undefined) {
filterObjects.author = ghostBookshelf.model('User').forge({slug: options.author});
}
return filterObjects;
},
findPageDefaultOptions: function findPageDefaultOptions() {
return {
staticPages: false, // include static pages
@ -402,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', 'featured'],
findPage: ['page', 'limit', 'columns', 'filter', 'status', 'staticPages'],
add: ['importing']
};

4
core/server/models/tag.js
View File

@ -59,10 +59,6 @@ Tag = ghostBookshelf.Model.extend({
return attrs;
}
}, {
setupFilters: function setupFilters() {
return {};
},
findPageDefaultOptions: function findPageDefaultOptions() {
return {
where: {}

11
core/server/models/user.js
View File

@ -164,17 +164,6 @@ User = ghostBookshelf.Model.extend({
}
}, {
setupFilters: function setupFilters(options) {
var filterObjects = {};
// Deliberately switch from singular 'tag' to 'tags' and 'role' to 'roles' here
// TODO: make this consistent
if (options.role !== undefined) {
filterObjects.roles = ghostBookshelf.model('Role').forge({name: options.role});
}
return filterObjects;
},
findPageDefaultOptions: function findPageDefaultOptions() {
return {
status: 'active',

2
core/test/functional/routes/api/posts_spec.js
View File

@ -125,7 +125,7 @@ describe('Post API', function () {
});
it('can retrieve just featured posts', function (done) {
request.get(testUtils.API.getApiQuery('posts/?featured=true'))
request.get(testUtils.API.getApiQuery('posts/?filter=featured:true'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)

64
core/test/integration/api/advanced_browse_spec.js
View File

@ -259,38 +259,6 @@ describe('Filter Param Spec', function () {
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;
@ -323,38 +291,6 @@ describe('Filter Param Spec', function () {
}).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;

28
core/test/integration/api/api_posts_spec.js
View File

@ -21,7 +21,7 @@ describe('Post API', function () {
describe('Browse', function () {
it('can fetch featured posts', function (done) {
PostAPI.browse({context: {user: 1}, featured: true}).then(function (results) {
PostAPI.browse({context: {user: 1}, filter: 'featured:true'}).then(function (results) {
should.exist(results.posts);
results.posts.length.should.eql(4);
results.posts[0].featured.should.eql(true);
@ -31,7 +31,7 @@ describe('Post API', function () {
});
it('can exclude featured posts', function (done) {
PostAPI.browse({context: {user: 1}, status: 'all', featured: false}).then(function (results) {
PostAPI.browse({context: {user: 1}, status: 'all', filter: 'featured:false'}).then(function (results) {
should.exist(results.posts);
results.posts.length.should.eql(1);
results.posts[0].featured.should.eql(false);
@ -202,26 +202,34 @@ describe('Post API', function () {
});
it('can fetch all posts for a tag', function (done) {
PostAPI.browse({context: {user: 1}, status: 'all', tag: 'kitchen-sink'}).then(function (results) {
PostAPI.browse({context: {user: 1}, status: 'all', filter: 'tags:kitchen-sink', include: 'tags'}).then(function (results) {
results.posts.length.should.be.eql(2);
results.meta.filters.tags[0].slug.should.eql('kitchen-sink');
_.each(results.posts, function (post) {
var slugs = _.pluck(post.tags, 'slug');
slugs.should.containEql('kitchen-sink');
});
done();
}).catch(done);
});
it('can fetch all posts for an author', function (done) {
PostAPI.browse({context: {user: 1}, status: 'all', author: 'joe-bloggs'}).then(function (results) {
PostAPI.browse({context: {user: 1}, status: 'all', filter: 'author:joe-bloggs', include: 'author'}).then(function (results) {
should.exist(results.posts);
results.posts.length.should.eql(5);
results.meta.filters.author[0].slug.should.eql('joe-bloggs');
_.each(results.posts, function (post) {
post.author.slug.should.eql('joe-bloggs');
});
done();
}).catch(done);
});
it('cannot fetch all posts for a tag with invalid slug', function (done) {
PostAPI.browse({tag: 'invalid!'}).then(function () {
// @TODO: ensure filters are fully validated
it.skip('cannot fetch all posts for a tag with invalid slug', function (done) {
PostAPI.browse({filter: 'tags:invalid!'}).then(function () {
done(new Error('Should not return a result with invalid tag'));
}).catch(function (err) {
should.exist(err);
@ -231,8 +239,8 @@ describe('Post API', function () {
});
});
it('cannot fetch all posts for an author with invalid slug', function (done) {
PostAPI.browse({author: 'invalid!'}).then(function () {
it.skip('cannot fetch all posts for an author with invalid slug', function (done) {
PostAPI.browse({filter: 'author:invalid!'}).then(function () {
done(new Error('Should not return a result with invalid author'));
}).catch(function (err) {
should.exist(err);

20
core/test/integration/model/model_posts_spec.js
View File

@ -176,7 +176,7 @@ describe('Post Model', function () {
paginationResult.posts.length.should.equal(1);
// Test featured pages
return PostModel.findPage({limit: 10, featured: true});
return PostModel.findPage({limit: 10, filter: 'featured:true'});
}).then(function (paginationResult) {
paginationResult.meta.pagination.page.should.equal(1);
paginationResult.meta.pagination.limit.should.equal(10);
@ -184,7 +184,7 @@ describe('Post Model', function () {
paginationResult.posts.length.should.equal(10);
// Test both boolean formats for featured pages
return PostModel.findPage({limit: 10, featured: '1'});
return PostModel.findPage({limit: 10, filter: 'featured:1'});
}).then(function (paginationResult) {
paginationResult.meta.pagination.page.should.equal(1);
paginationResult.meta.pagination.limit.should.equal(10);
@ -211,40 +211,32 @@ describe('Post Model', function () {
return testUtils.fixtures.insertMorePostsTags();
}).then(function () {
// Test tag filter
return PostModel.findPage({page: 1, tag: 'bacon'});
return PostModel.findPage({page: 1, filter: 'tags:bacon'});
}).then(function (paginationResult) {
paginationResult.meta.pagination.page.should.equal(1);
paginationResult.meta.pagination.limit.should.equal(15);
paginationResult.meta.pagination.pages.should.equal(1);
paginationResult.meta.filters.tags[0].name.should.equal('bacon');
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({page: 1, filter: 'tags:kitchen-sink'});
}).then(function (paginationResult) {
paginationResult.meta.pagination.page.should.equal(1);
paginationResult.meta.pagination.limit.should.equal(15);
paginationResult.meta.pagination.pages.should.equal(1);
paginationResult.meta.filters.tags[0].name.should.equal('kitchen sink');
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({page: 1, filter: 'tags:injection'});
}).then(function (paginationResult) {
paginationResult.meta.pagination.page.should.equal(1);
paginationResult.meta.pagination.limit.should.equal(15);
paginationResult.meta.pagination.pages.should.equal(2);
paginationResult.meta.filters.tags[0].name.should.equal('injection');
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({page: 2, filter: 'tags:injection'});
}).then(function (paginationResult) {
paginationResult.meta.pagination.page.should.equal(2);
paginationResult.meta.pagination.limit.should.equal(15);
paginationResult.meta.pagination.pages.should.equal(2);
paginationResult.meta.filters.tags[0].name.should.equal('injection');
paginationResult.meta.filters.tags[0].slug.should.equal('injection');
paginationResult.posts.length.should.equal(10);
done();

5
core/test/integration/model/model_users_spec.js
View File

@ -236,7 +236,10 @@ describe('User Model', function run() {
}).catch(done);
});
it('can findPage by role', function (done) {
/**
* Removed in favour of filters, but this relation hasn't been re-added yet
*/
it.skip('can findPage by role', function (done) {
return testUtils.fixtures.createExtraUsers().then(function () {
return UserModel.findPage({role: 'Administrator'});
}).then(function (results) {

64
core/test/unit/server_helpers/get_spec.js
View File

@ -34,7 +34,7 @@ describe('{{#get}} helper', function () {
});
describe('posts', function () {
var browseStub, readStub, testPostsArr = [
var browsePostsStub, readPostsStub, readTagsStub, readUsersStub, testPostsArr = [
{id: 1, title: 'Test Post 1', author: {slug: 'cameron'}},
{id: 2, title: 'Test Post 2', author: {slug: 'cameron'}, featured: true},
{id: 3, title: 'Test Post 3', tags: [{slug: 'test'}]},
@ -42,17 +42,19 @@ describe('{{#get}} helper', function () {
],
meta = {pagination: {}};
beforeEach(function () {
browseStub = sandbox.stub(api.posts, 'browse');
readStub = sandbox.stub(api.posts, 'read');
browsePostsStub = sandbox.stub(api.posts, 'browse');
readPostsStub = sandbox.stub(api.posts, 'read');
readTagsStub = sandbox.stub(api.tags, 'read').returns(new Promise.resolve({tags: []}));
readUsersStub = sandbox.stub(api.users, 'read').returns(new Promise.resolve({users: []}));
browseStub.returns(new Promise.resolve({posts: testPostsArr}));
browseStub.withArgs({limit: '3'}).returns(new Promise.resolve({posts: testPostsArr.slice(0, 3), meta: meta}));
browseStub.withArgs({limit: '1'}).returns(new Promise.resolve({posts: testPostsArr.slice(0, 1)}));
browseStub.withArgs({tag: 'test'}).returns(new Promise.resolve({posts: testPostsArr.slice(2, 3)}));
browseStub.withArgs({tag: 'none'}).returns(new Promise.resolve({posts: []}));
browseStub.withArgs({author: 'cameron'}).returns(new Promise.resolve({posts: testPostsArr.slice(0, 2)}));
browseStub.withArgs({featured: 'true'}).returns(new Promise.resolve({posts: testPostsArr.slice(2, 3)}));
readStub.withArgs({id: '2'}).returns(new Promise.resolve({posts: testPostsArr.slice(1, 2)}));
browsePostsStub.returns(new Promise.resolve({posts: testPostsArr}));
browsePostsStub.withArgs({limit: '3'}).returns(new Promise.resolve({posts: testPostsArr.slice(0, 3), meta: meta}));
browsePostsStub.withArgs({limit: '1'}).returns(new Promise.resolve({posts: testPostsArr.slice(0, 1)}));
browsePostsStub.withArgs({filter: 'tags:test'}).returns(new Promise.resolve({posts: testPostsArr.slice(2, 3)}));
browsePostsStub.withArgs({filter: 'tags:none'}).returns(new Promise.resolve({posts: []}));
browsePostsStub.withArgs({filter: 'author:cameron'}).returns(new Promise.resolve({posts: testPostsArr.slice(0, 2)}));
browsePostsStub.withArgs({filter: 'featured:true'}).returns(new Promise.resolve({posts: testPostsArr.slice(2, 3)}));
readPostsStub.withArgs({id: '2'}).returns(new Promise.resolve({posts: testPostsArr.slice(1, 2)}));
});
it('should handle default browse posts call', function (done) {
@ -123,22 +125,7 @@ describe('{{#get}} helper', function () {
helpers.get.call(
{},
'posts',
{hash: {tag: 'test'}, fn: fn, inverse: inverse}
).then(function () {
fn.calledOnce.should.be.true;
fn.firstCall.args[0].should.be.an.Object.with.property('posts');
fn.firstCall.args[0].posts.should.have.lengthOf(1);
fn.firstCall.args[0].posts.should.eql(testPostsArr.slice(2, 3));
inverse.called.should.be.false;
done();
}).catch(done);
});
it('should handle browse post call with relative tag', function (done) {
helpers.get.call(
{},
'posts',
{hash: {tag: [{slug: 'test'}]}, fn: fn, inverse: inverse}
{hash: {filter: 'tags:test'}, fn: fn, inverse: inverse}
).then(function () {
fn.calledOnce.should.be.true;
fn.firstCall.args[0].should.be.an.Object.with.property('posts');
@ -153,22 +140,7 @@ describe('{{#get}} helper', function () {
helpers.get.call(
{},
'posts',
{hash: {author: 'cameron'}, fn: fn, inverse: inverse}
).then(function () {
fn.calledOnce.should.be.true;
fn.firstCall.args[0].should.be.an.Object.with.property('posts');
fn.firstCall.args[0].posts.should.have.lengthOf(2);
fn.firstCall.args[0].posts.should.eql(testPostsArr.slice(0, 2));
inverse.called.should.be.false;
done();
}).catch(done);
});
it('should handle browse post call with relative author', function (done) {
helpers.get.call(
{},
'posts',
{hash: {author: {slug: 'cameron'}}, fn: fn, inverse: inverse}
{hash: {filter: 'author:cameron'}, fn: fn, inverse: inverse}
).then(function () {
fn.calledOnce.should.be.true;
fn.firstCall.args[0].should.be.an.Object.with.property('posts');
@ -183,7 +155,7 @@ describe('{{#get}} helper', function () {
helpers.get.call(
{},
'posts',
{hash: {featured: 'true'}, fn: fn, inverse: inverse}
{hash: {filter: 'featured:true'}, fn: fn, inverse: inverse}
).then(function () {
fn.calledOnce.should.be.true;
fn.firstCall.args[0].should.be.an.Object.with.property('posts');
@ -214,7 +186,7 @@ describe('{{#get}} helper', function () {
helpers.get.call(
{},
'posts',
{hash: {tag: 'none'}, fn: fn, inverse: inverse}
{hash: {filter: 'tags:none'}, fn: fn, inverse: inverse}
).then(function () {
fn.called.should.be.false;
inverse.calledOnce.should.be.true;
@ -247,7 +219,7 @@ describe('{{#get}} helper', function () {
helpers.get.call(
{},
'posts',
{hash: {tag: 'thing!'}, fn: fn, inverse: inverse}
{hash: {status: 'thing!'}, fn: fn, inverse: inverse}
).then(function () {
fn.called.should.be.false;
inverse.calledOnce.should.be.true;