From 591fa349aaebd58089c0182d6971481f77006799 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Fri, 19 Dec 2014 01:59:44 +0000 Subject: [PATCH] Finish up post count support in tags API Refs #4521 - Handle 'include' query param in tags API. - Add post_count support when fetching a tag with findOne. - Remove post_count from options.include after processing. - Extra database query no longer used to fetch post_count. --- core/server/api/tags.js | 36 ++++++++++- core/server/models/tag.js | 63 ++++++++++--------- core/test/integration/api/api_tags_spec.js | 31 ++++++++- .../test/integration/model/model_tags_spec.js | 60 ++++++++++-------- 4 files changed, 133 insertions(+), 57 deletions(-) diff --git a/core/server/api/tags.js b/core/server/api/tags.js index b5d1b40836..752a26e005 100644 --- a/core/server/api/tags.js +++ b/core/server/api/tags.js @@ -8,8 +8,17 @@ var Promise = require('bluebird'), utils = require('./utils'), docName = 'tags', + allowedIncludes = ['post_count'], tags; +// ## Helpers +function prepareInclude(include) { + include = include || ''; + include = _.intersection(include.split(','), allowedIncludes); + + return include; +} + /** * ## Tags API Methods * @@ -22,7 +31,13 @@ tags = { * @returns {Promise(Tags)} Tags Collection */ browse: function browse(options) { + options = options || {}; + return canThis(options.context).browse.tag().then(function () { + if (options.include) { + options.include = prepareInclude(options.include); + } + return dataProvider.Tag.findPage(options); }, function () { return Promise.reject(new errors.NoPermissionError('You do not have permission to browse tags.')); @@ -35,9 +50,16 @@ tags = { * @return {Promise(Tag)} Tag */ read: function read(options) { + options = options || {}; + var attrs = ['id', 'slug'], - data = _.pick(options, attrs); + data = _.pick(options, attrs); + return canThis(options.context).read.tag().then(function () { + if (options.include) { + options.include = prepareInclude(options.include); + } + return dataProvider.Tag.findOne(data, options).then(function (result) { if (result) { return {tags: [result.toJSON()]}; @@ -59,6 +81,10 @@ tags = { options = options || {}; return canThis(options.context).add.tag(object).then(function () { + if (options.include) { + options.include = prepareInclude(options.include); + } + return utils.checkObject(object, docName).then(function (checkedTagData) { return dataProvider.Tag.add(checkedTagData.tags[0], options); }).then(function (result) { @@ -80,7 +106,13 @@ tags = { * @return {Promise(Tag)} Edited Tag */ edit: function edit(object, options) { + options = options || {}; + return canThis(options.context).edit.tag(options.id).then(function () { + if (options.include) { + options.include = prepareInclude(options.include); + } + return utils.checkObject(object, docName).then(function (checkedTagData) { return dataProvider.Tag.edit(checkedTagData.tags[0], options); }).then(function (result) { @@ -105,6 +137,8 @@ tags = { * @return {Promise(Tag)} Deleted Tag */ destroy: function destroy(options) { + options = options || {}; + return canThis(options.context).destroy.tag(options.id).then(function () { return tags.read(options).then(function (result) { return dataProvider.Tag.destroy(options).then(function () { diff --git a/core/server/models/tag.js b/core/server/models/tag.js index 730f9d6fb5..a2b556a6fc 100644 --- a/core/server/models/tag.js +++ b/core/server/models/tag.js @@ -7,6 +7,17 @@ var _ = require('lodash'), Tag, Tags; +function addPostCount(options, obj) { + if (options.include && options.include.indexOf('post_count') > -1) { + obj.query('select', 'tags.*'); + obj.query('count', 'posts_tags.id as post_count'); + obj.query('leftJoin', 'posts_tags', 'tag_id', 'tags.id'); + obj.query('groupBy', 'tag_id', 'tags.id'); + + options.include = _.pull([].concat(options.include), 'post_count'); + } +} + Tag = ghostBookshelf.Model.extend({ tableName: 'tags', @@ -70,13 +81,32 @@ Tag = ghostBookshelf.Model.extend({ return options; }, + + /** + * ### Find One + * @overrides ghostBookshelf.Model.findOne + */ + findOne: function (data, options) { + options = options || {}; + + options = this.filterOptions(options, 'findOne'); + data = this.filterData(data, 'findOne'); + + var tag = this.forge(data); + + addPostCount(options, tag); + + // Add related objects + options.withRelated = _.union(options.withRelated, options.include); + + return tag.fetch(options); + }, + findPage: function (options) { options = options || {}; var tagCollection = Tags.forge(), collectionPromise, - totalPromise, - countPromise, qb; if (options.limit && options.limit !== 'all') { @@ -102,6 +132,8 @@ Tag = ghostBookshelf.Model.extend({ .query('offset', options.limit * (options.page - 1)); } + addPostCount(options, tagCollection); + collectionPromise = tagCollection.fetch(_.omit(options, 'page', 'limit')); // Find total number of tags @@ -112,22 +144,10 @@ Tag = ghostBookshelf.Model.extend({ qb.where(options.where); } - totalPromise = qb.count('tags.id as aggregate'); - - // Fetch post count information - - if (options.include) { - if (options.include.indexOf('post_count') > -1) { - qb = ghostBookshelf.knex('posts_tags'); - countPromise = qb.select('tag_id').count('* as postCount').groupBy('tag_id'); - } - } - - return Promise.join(collectionPromise, totalPromise, countPromise).then(function (results) { + return Promise.join(collectionPromise, qb.count('tags.id as aggregate')).then(function (results) { var totalTags = results[1][0].aggregate, calcPages = Math.ceil(totalTags / options.limit) || 0, tagCollection = results[0], - postsPerTagCollection = results[2], pagination = {}, meta = {}, data = {}; @@ -140,19 +160,6 @@ Tag = ghostBookshelf.Model.extend({ pagination.prev = null; data.tags = tagCollection.toJSON(); - - if (postsPerTagCollection) { - // Merge two result sets - _.each(data.tags, function (tag) { - var postsPerTag = _.find(postsPerTagCollection, function (obj) { return obj.tag_id === tag.id; }); - if (postsPerTag) { - tag.post_count = postsPerTag.postCount; - } else { - tag.post_count = 0; - } - }); - } - data.meta = meta; meta.pagination = pagination; diff --git a/core/test/integration/api/api_tags_spec.js b/core/test/integration/api/api_tags_spec.js index 294638a46a..8a693c57aa 100644 --- a/core/test/integration/api/api_tags_spec.js +++ b/core/test/integration/api/api_tags_spec.js @@ -13,7 +13,7 @@ describe('Tags API', function () { // Keep the DB clean before(testUtils.teardown); afterEach(testUtils.teardown); - beforeEach(testUtils.setup('users:roles', 'tag', 'perms:tag', 'perms:init')); + beforeEach(testUtils.setup('users:roles', 'perms:tag', 'perms:init', 'posts')); should.exist(TagAPI); @@ -161,5 +161,34 @@ describe('Tags API', function () { done(); }).catch(done); }); + + it('with include post_count', function (done) { + TagAPI.browse({context: {user: 1}, include: 'post_count'}).then(function (results) { + should.exist(results); + should.exist(results.tags); + results.tags.length.should.be.above(0); + + testUtils.API.checkResponse(results.tags[0], 'tag', 'post_count'); + should.exist(results.tags[0].post_count); + + done(); + }).catch(done); + }); + }); + + describe('Read', function () { + it('returns post_count with include post_count', function (done) { + TagAPI.read({context: {user: 1}, include: 'post_count', slug: 'kitchen-sink'}).then(function (results) { + should.exist(results); + should.exist(results.tags); + results.tags.length.should.be.above(0); + + testUtils.API.checkResponse(results.tags[0], 'tag', 'post_count'); + should.exist(results.tags[0].post_count); + results.tags[0].post_count.should.equal(2); + + done(); + }).catch(done); + }); }); }); diff --git a/core/test/integration/model/model_tags_spec.js b/core/test/integration/model/model_tags_spec.js index 97d45dc8b8..8a297a1965 100644 --- a/core/test/integration/model/model_tags_spec.js +++ b/core/test/integration/model/model_tags_spec.js @@ -37,40 +37,46 @@ describe('Tag Model', function () { }).catch(done); }); - it('can findPage with limit all', function (done) { + it('returns post_count if include post_count', function (done) { testUtils.fixtures.insertPosts().then(function () { - return TagModel.findPage({limit: 'all'}); - }).then(function (results) { - results.meta.pagination.page.should.equal(1); - results.meta.pagination.limit.should.equal('all'); - results.meta.pagination.pages.should.equal(1); - results.tags.length.should.equal(5); + TagModel.findOne({slug: 'kitchen-sink'}, {include: 'post_count'}).then(function (tag) { + should.exist(tag); + tag.get('post_count').should.equal(2); - done(); - }).catch(done); + done(); + }).catch(done); + }); }); - it('can findPage with post_count include', function (done) { - testUtils.fixtures.insertPosts().then(function () { - return TagModel.findPage({include: 'post_count'}); - }).then(function (results) { - _.each(results.tags, function (tag) { tag.should.have.property('post_count'); }); - _.findWhere(results.tags, {slug: 'kitchen-sink'}).post_count.should.equal(2); - _.findWhere(results.tags, {slug: 'pollo'}).post_count.should.equal(1); - _.findWhere(results.tags, {slug: 'injection'}).post_count.should.equal(0); + describe('findPage', function () { + beforeEach(function (done) { + testUtils.fixtures.insertPosts().then(function () { + done(); + }).catch(done); + }); - done(); - }).catch(done); - }); + it('with limit all', function (done) { + TagModel.findPage({limit: 'all'}).then(function (results) { + results.meta.pagination.page.should.equal(1); + results.meta.pagination.limit.should.equal('all'); + results.meta.pagination.pages.should.equal(1); + results.tags.length.should.equal(5); - it('can findPage with post_count include and limit less then total_tags', function (done) { - testUtils.fixtures.insertPosts().then(function () { - return TagModel.findPage({include: 'post_count', limit: 2}); - }).then(function (results) { - _.each(results.tags, function (tag) { tag.should.have.property('post_count'); }); + done(); + }).catch(done); + }); - done(); - }).catch(done); + it('with include post_count', function (done) { + TagModel.findPage({limit: 'all', include: 'post_count'}).then(function (results) { + results.meta.pagination.page.should.equal(1); + results.meta.pagination.limit.should.equal('all'); + results.meta.pagination.pages.should.equal(1); + results.tags.length.should.equal(5); + should.exist(results.tags[0].post_count); + + done(); + }).catch(done); + }); }); describe('a Post', function () {