Skip to content

Commit

Permalink
Safe post_count for Tags & Users
Browse files Browse the repository at this point in the history 
refs TryGhost#6009, TryGhost#5614

- Use the new isPublicContext method to detect whether to add extra clauses to the count
- Add count to users
  • Loading branch information
@ErisDS
ErisDS committed Nov 18, 2015
1 parent 4a25a01 commit 770f452
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 16 deletions.
2 changes: 1 addition & 1 deletion core/server/api/users.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var Promise = require('bluebird'),

docName = 'users',
// TODO: implement created_by, updated_by
allowedIncludes = ['permissions', 'roles', 'roles.permissions'],
allowedIncludes = ['post_count', 'permissions', 'roles', 'roles.permissions'],
users,
sendInviteEmail;

Expand Down
29 changes: 26 additions & 3 deletions core/server/models/plugins/include-count.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,33 @@ module.exports = function (Bookshelf) {
tags: {
posts: function addPostCountToTags(model) {
model.query('columns', 'tags.*', function (qb) {
qb.count('posts_tags.post_id')
.from('posts_tags')
.whereRaw('tag_id = tags.id')
qb.count('posts.id')
.from('posts')
.leftOuterJoin('posts_tags', 'posts.id', 'posts_tags.post_id')
.whereRaw('posts_tags.tag_id = tags.id')
.as('post_count');

if (model.isPublicContext()) {
// @TODO use the filter behavior for posts
qb.andWhere('posts.page', '=', false);
qb.andWhere('posts.status', '=', 'published');
}
});
}
},
users: {
posts: function addPostCountToTags(model) {
model.query('columns', 'users.*', function (qb) {
qb.count('posts.id')
.from('posts')
.whereRaw('posts.author_id = users.id')
.as('post_count');

if (model.isPublicContext()) {
// @TODO use the filter behavior for posts
qb.andWhere('posts.page', '=', false);
qb.andWhere('posts.status', '=', 'published');
}
});
}
}
Expand Down
90 changes: 81 additions & 9 deletions core/test/integration/api/advanced_browse_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,26 +310,98 @@ describe('Filter Param Spec', function () {
});
});

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();
describe('Count capabilities', function () {
it('can fetch `post_count` for tags (public data only)', function (done) {
TagAPI.browse({include: 'post_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 5 matching items
result.tags.should.be.an.Array.with.lengthOf(5);

// Each tag should have the correct count
_.find(result.tags, function (tag) {
return tag.name === 'Getting Started';
}).post_count.should.eql(4);

_.find(result.tags, function (tag) {
return tag.name === 'photo';
}).post_count.should.eql(4);

_.find(result.tags, function (tag) {
return tag.name === 'Video';
}).post_count.should.eql(5);

_.find(result.tags, function (tag) {
return tag.name === 'Audio';
}).post_count.should.eql(6);

_.find(result.tags, function (tag) {
return tag.name === 'No Posts';
}).post_count.should.eql(0);

// 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(5);
should.equal(result.meta.pagination.next, null);
should.equal(result.meta.pagination.prev, null);

done();
}).catch(done);
});

it('can fetch `posts.all.count` for tags (all posts)', function (done) {
it.skip('can fetch `posts.count` for tags (all data)', function (done) {
// This is tested elsewhere for now using user context
// No way to override it for public requests
done();
});

it('can fetch `posts.count` for users (published only)', function (done) {
// This could be posts.count & posts.all.count?
done();
UserAPI.browse({include: 'post_count'}).then(function (result) {
// 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 5 matching items
result.users.should.be.an.Array.with.lengthOf(2);

// Each user should have the correct count
_.find(result.users, function (user) {
return user.slug === 'leslie';
}).post_count.should.eql(15);

_.find(result.users, function (user) {
return user.slug === 'pat-smith';
}).post_count.should.eql(3);

// 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);

done();
}).catch(done);
});

it('can fetch `posts.all.count` for users (all posts)', function (done) {
it.skip('can fetch `posts.all.count` for users (all posts)', function (done) {
done();
});

it('can fetch `tags.count` for posts', function (done) {
it.skip('can fetch `tags.count` for posts', function (done) {
done();
});
});
Expand Down
40 changes: 38 additions & 2 deletions core/test/integration/api/api_users_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ describe('Users API', function () {
// Keep the DB clean
before(testUtils.teardown);

beforeEach(testUtils.setup('users:roles', 'users', 'user:token', 'perms:user', 'perms:role', 'perms:setting', 'perms:init'));
beforeEach(testUtils.setup(
'users:roles', 'users', 'user:token', 'perms:user', 'perms:role', 'perms:setting', 'perms:init', 'posts'
));
afterEach(testUtils.teardown);

function checkForErrorType(type, done) {
Expand Down Expand Up @@ -131,7 +133,6 @@ describe('Users API', function () {
testUtils.API.checkResponse(response, 'users');
should.exist(response.users);
response.users.should.have.length(7);
response.users.should.have.length(7);
testUtils.API.checkResponse(response.users[0], 'user', 'roles');
testUtils.API.checkResponse(response.users[1], 'user', 'roles');
testUtils.API.checkResponse(response.users[2], 'user', 'roles');
Expand Down Expand Up @@ -185,6 +186,41 @@ describe('Users API', function () {
.then(done)
.catch(done);
});

it('can browse with include post_count', function (done) {
UserAPI.browse(_.extend({}, testUtils.context.admin, {include: 'post_count'})).then(function (response) {
should.exist(response);
testUtils.API.checkResponse(response, 'users');
should.exist(response.users);
response.users.should.have.length(7);
response.users.should.have.length(7);

testUtils.API.checkResponse(response.users[0], 'user', 'post_count');
testUtils.API.checkResponse(response.users[1], 'user', 'post_count');
testUtils.API.checkResponse(response.users[2], 'user', 'post_count');
testUtils.API.checkResponse(response.users[3], 'user', 'post_count');
testUtils.API.checkResponse(response.users[4], 'user', 'post_count');
testUtils.API.checkResponse(response.users[5], 'user', 'post_count');
testUtils.API.checkResponse(response.users[6], 'user', 'post_count');

response.users[0].post_count.should.eql(0);
response.users[1].post_count.should.eql(0);
response.users[2].post_count.should.eql(0);
response.users[3].post_count.should.eql(7);
response.users[4].post_count.should.eql(0);
response.users[5].post_count.should.eql(0);
response.users[6].post_count.should.eql(0);

response.meta.pagination.should.have.property('page', 1);
response.meta.pagination.should.have.property('limit', 15);
response.meta.pagination.should.have.property('pages', 1);
response.meta.pagination.should.have.property('total', 7);
response.meta.pagination.should.have.property('next', null);
response.meta.pagination.should.have.property('prev', null);

done();
}).catch(done);
});
});

describe('Read', function () {
Expand Down
2 changes: 1 addition & 1 deletion core/test/utils/fixtures/filter-param/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ data.posts = [
featured: false,
page: 1,
author_id: 1,
tags: []
tags: [1, 2, 3, 4]
}
];

Expand Down

0 comments on commit 770f452

Please sign in to comment.