Merge pull request #6613 from jaswilli/delete-content-refactor

Refactor content deletion

Gruntfile.js

@@ -577,8 +577,7 @@ var _              = require('lodash'),
                 migration = require('./core/server/data/migration');
 
             migration.reset().then(function () {
-                return models.init();
-            }).then(function () {
+                models.init();
                 return migration.init();
             }).then(function () {
                 done();

core/server/api/db.js

@@ -109,15 +109,22 @@ db = {
      * @returns {Promise} Success
      */
     deleteAllContent: function (options) {
-        var tasks;
+        var tasks,
+            queryOpts = {columns: 'id'};
 
         options = options || {};
 
         function deleteContent() {
-            return Promise.resolve(models.deleteAllContent())
-                .return({db: []})
+            var collections = [
+                models.Post.findAll(queryOpts),
+                models.Tag.findAll(queryOpts)
+            ];
+
+            return Promise.each(collections, function then(Collection) {
+                return Collection.invokeThen('destroy');
+            }).return({db: []})
             .catch(function (error) {
-                    return Promise.reject(new errors.InternalServerError(error.message || error));
+                throw new errors.InternalServerError(error.message || error);
             });
         }
 

core/server/api/posts.js

@@ -212,18 +212,19 @@ posts = {
         var tasks;
 
         /**
-         * ### Model Query
-         * Make the call to the Model layer
+         * @function deletePost
          * @param  {Object} options
-         * @returns {Object} options
+         * @return {Object} JSON representation of the deleted post
          */
-        function modelQuery(options) {
-            // Removing a post needs to include all posts.
-            options.status = 'all';
-            return posts.read(options).then(function (result) {
-                return dataProvider.Post.destroy(options).then(function () {
-                    return result;
-                });
+        function deletePost(options) {
+            var Post = dataProvider.Post,
+                data = _.defaults({status: 'all'}, options),
+                fetchOpts = _.defaults({require: true}, options);
+
+            return Post.findOne(data, fetchOpts).then(function (post) {
+                return post.destroy(options).return({posts: [post.toJSON()]});
+            }).catch(Post.NotFoundError, function () {
+                throw new errors.NotFoundError(i18n.t('errors.api.posts.postNotFound'));
             });
         }
 
@@ -232,20 +233,12 @@ posts = {
             utils.validate(docName, {opts: utils.idDefaultOptions}),
             utils.handlePermissions(docName, 'destroy'),
             utils.convertOptions(allowedIncludes),
-            modelQuery
+            deletePost
         ];
 
         // Pipeline calls each task passing the result of one to be the arguments for the next
-        return pipeline(tasks, options).then(function formatResponse(result) {
-            var deletedObj = result;
-
-            if (deletedObj.posts) {
-                _.each(deletedObj.posts, function (post) {
-                    post.statusChanged = true;
-                });
-            }
-
-            return deletedObj;
+        return pipeline(tasks, options).tap(function formatResponse(response) {
+            response.posts[0].statusChanged = true;
         });
     }
 };

core/server/index.js

@@ -66,7 +66,7 @@ function init(options) {
         return config.checkDeprecated();
     }).then(function () {
         // Initialise the models
-        return models.init();
+        models.init();
     }).then(function () {
         // Initialize migrations
         return migrations.init();

core/server/models/index.js

@@ -2,8 +2,7 @@
  * Dependencies
  */
 
-var Promise = require('bluebird'),
-    _ = require('lodash'),
+var _ = require('lodash'),
 
     exports,
     models;
@@ -36,32 +35,8 @@ function init() {
     models.forEach(function (name) {
         _.extend(exports, require('./' + name));
     });
-
-    return Promise.resolve();
 }
 
-/**
- * TODO: move to some other place
- */
-
-// ### deleteAllContent
-// Delete all content from the database (posts, tags, tags_posts)
-exports.deleteAllContent = function deleteAllContent() {
-    var self = this;
-
-    return self.Post.findAll().then(function then(posts) {
-        return Promise.all(_.map(posts.toJSON(), function mapper(post) {
-            return self.Post.destroy({id: post.id});
-        }));
-    }).then(function () {
-        return self.Tag.findAll().then(function then(tags) {
-            return Promise.all(_.map(tags.toJSON(), function mapper(tag) {
-                return self.Tag.destroy({id: tag.id});
-            }));
-        });
-    });
-};
-
 /**
  * Expose `init`
  */

core/server/models/post.js

@@ -82,12 +82,14 @@ Post = ghostBookshelf.Model.extend({
             }
         });
 
-        this.on('destroying', function onDestroying(model) {
+        this.on('destroying', function (model/*, attr, options*/) {
+            return model.load('tags').call('related', 'tags').call('detach').then(function then() {
                 if (model.previous('status') === 'published') {
                     model.emitChange('unpublished');
                 }
                 model.emitChange('deleted');
             });
+        });
     },
 
     saving: function saving(model, attr, options) {
@@ -373,8 +375,9 @@ Post = ghostBookshelf.Model.extend({
             // whitelists for the `options` hash argument on methods, by method name.
             // these are the only options that can be passed to Bookshelf / Knex.
             validOptions = {
-                findOne: ['importing', 'withRelated'],
+                findOne: ['importing', 'withRelated', 'require'],
                 findPage: ['page', 'limit', 'columns', 'filter', 'order', 'status', 'staticPages'],
+                findAll: ['columns'],
                 add: ['importing']
             };
 
@@ -520,44 +523,27 @@ Post = ghostBookshelf.Model.extend({
         });
     },
 
-    /**
-     * ### Destroy
-     * @extends ghostBookshelf.Model.destroy to clean up tag relations
-     * **See:** [ghostBookshelf.Model.destroy](base.js.html#destroy)
-     */
-    destroy: function destroy(options) {
-        var id = options.id;
-        options = this.filterOptions(options, 'destroy');
-
-        return this.forge({id: id}).fetch({withRelated: ['tags']}).then(function destroyTags(post) {
-            return post.related('tags').detach().then(function destroyPosts() {
-                return post.destroy(options);
-            });
-        });
-    },
-
     /**
      * ### destroyByAuthor
      * @param  {[type]} options has context and id. Context is the user doing the destroy, id is the user to destroy
      */
-    destroyByAuthor: function destroyByAuthor(options) {
+    destroyByAuthor: Promise.method(function destroyByAuthor(options) {
         var postCollection = Posts.forge(),
             authorId = options.id;
 
         options = this.filterOptions(options, 'destroyByAuthor');
-        if (authorId) {
-            return postCollection.query('where', 'author_id', '=', authorId).fetch(options).then(function destroyTags(results) {
-                return Promise.map(results.models, function mapper(post) {
-                    return post.related('tags').detach(null, options).then(function destroyPosts() {
-                        return post.destroy(options);
-                    });
-                });
-            }, function (error) {
-                return Promise.reject(new errors.InternalServerError(error.message || error));
-            });
+
+        if (!authorId) {
+            throw new errors.NotFoundError(i18n.t('errors.models.post.noUserFound'));
         }
-        return Promise.reject(new errors.NotFoundError(i18n.t('errors.models.post.noUserFound')));
-    },
+
+        return postCollection.query('where', 'author_id', '=', authorId)
+            .fetch(options)
+            .call('invokeThen', 'destroy', options)
+            .catch(function (error) {
+                throw new errors.InternalServerError(error.message || error);
+            });
+    }),
 
     permissible: function permissible(postModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
         var self = this,

core/server/models/tag.js

@@ -75,7 +75,8 @@ Tag = ghostBookshelf.Model.extend({
             // whitelists for the `options` hash argument on methods, by method name.
             // these are the only options that can be passed to Bookshelf / Knex.
             validOptions = {
-                findPage: ['page', 'limit', 'columns', 'filter', 'order']
+                findPage: ['page', 'limit', 'columns', 'filter', 'order'],
+                findAll: ['columns']
             };
 
         if (validOptions[methodName]) {

core/test/integration/model/model_posts_spec.js

@@ -819,7 +819,7 @@ describe('Post Model', function () {
                     post.tags[0].id.should.equal(firstItemData.id);
 
                     // Destroy the post
-                    return PostModel.destroy(firstItemData);
+                    return results.destroy();
                 }).then(function (response) {
                     var deleted = response.toJSON();
 
@@ -857,7 +857,7 @@ describe('Post Model', function () {
                     post.tags[0].id.should.equal(firstItemData.id);
 
                     // Destroy the post
-                    return PostModel.destroy(firstItemData);
+                    return results.destroy(firstItemData);
                 }).then(function (response) {
                     var deleted = response.toJSON();
 
@@ -894,7 +894,7 @@ describe('Post Model', function () {
                     page.page.should.be.true();
 
                     // Destroy the page
-                    return PostModel.destroy(firstItemData);
+                    return results.destroy(firstItemData);
                 }).then(function (response) {
                     var deleted = response.toJSON();
 
@@ -930,7 +930,7 @@ describe('Post Model', function () {
                     page.id.should.equal(firstItemData.id);
 
                     // Destroy the page
-                    return PostModel.destroy(firstItemData);
+                    return results.destroy(firstItemData);
                 }).then(function (response) {
                     var deleted = response.toJSON();
 

core/test/unit/middleware/auth-strategies_spec.js

@@ -31,9 +31,9 @@ var should           = require('should'),
 describe('Auth Strategies', function () {
     var next;
 
-    before(function (done) {
+    before(function () {
         // Loads all the models
-        Models.init().then(done).catch(done);
+        Models.init();
     });
 
     beforeEach(function () {

core/test/unit/middleware/oauth_spec.js

@@ -9,9 +9,9 @@ var sinon            = require('sinon'),
 describe('OAuth', function () {
     var next, req, res, sandbox;
 
-    before(function (done) {
+    before(function () {
         // Loads all the models
-        Models.init().then(done).catch(done);
+        Models.init();
     });
 
     beforeEach(function () {

core/test/unit/migration_fixture_spec.js

@@ -19,15 +19,13 @@ var should  = require('should'),
 describe('Fixtures', function () {
     var loggerStub;
 
-    beforeEach(function (done) {
+    beforeEach(function () {
         loggerStub = {
             info: sandbox.stub(),
             warn: sandbox.stub()
         };
 
-        models.init().then(function () {
-            done();
-        }).catch(done);
+        models.init();
     });
 
     afterEach(function () {

core/test/unit/migration_spec.js

@@ -976,20 +976,18 @@ describe('Migrations', function () {
     describe('FixClientSecret', function () {
         var fixClientSecret, queryStub, clientForgeStub, clientEditStub, toStringStub, cryptoStub;
 
-        beforeEach(function (done) {
+        beforeEach(function () {
             fixClientSecret = migration.__get__('fixClientSecret');
             queryStub = {
                 query: sandbox.stub().returnsThis(),
                 fetch: sandbox.stub()
             };
 
-            models.init().then(function () {
+            models.init();
             toStringStub = {toString: sandbox.stub().returns('TEST')};
             cryptoStub = sandbox.stub(crypto, 'randomBytes').returns(toStringStub);
             clientForgeStub = sandbox.stub(models.Clients, 'forge').returns(queryStub);
             clientEditStub = sandbox.stub(models.Client, 'edit');
-                done();
-            }).catch(done);
         });
 
         it('should do nothing if there are no incorrect secrets', function (done) {

core/test/unit/models_plugins/access-rules_spec.js

@@ -14,10 +14,9 @@ should.equal(true, true);
 
 describe('Access Rules', function () {
     beforeEach(function () {
-        return models.init().then(function () {
+        models.init();
         ghostBookshelf = models.Base;
     });
-    });
 
     afterEach(function () {
         sandbox.restore();

core/test/unit/models_plugins/filter_spec.js

@@ -12,10 +12,9 @@ var should = require('should'),
 
 describe('Filter', function () {
     before(function () {
-        return models.init().then(function () {
+        models.init();
         ghostBookshelf = models.Base;
     });
-    });
 
     beforeEach(function () {
         // re-initialise the plugin with the rewired version

core/test/unit/permissions_spec.js

@@ -19,8 +19,8 @@ describe('Permissions', function () {
     });
 
     describe('actions map', function () {
-        before(function (done) {
-            Models.init().then(done).catch(done);
+        before(function () {
+            Models.init();
         });
 
         beforeEach(function () {

core/test/utils/index.js

@@ -471,9 +471,9 @@ setup = function setup() {
         args = arguments;
 
     return function (done) {
-        return Models.init().then(function () {
-            return initFixtures.apply(self, args);
-        }).then(function () {
+        Models.init();
+
+        return initFixtures.apply(self, args).then(function () {
             done();
         }).catch(done);
     };