refactor(model): change Model.destroy() signature from where, options to options

Mick Hansen
Commit 73796549 authored Sep 20, 2014 by Mick Hansen
Showing with 35 additions and 50 deletions
changelog.md
lib/model.js
test/dao-factory.test.js
--- a/changelog.md
+++ b/changelog.md
@@ -14,6 +14,7 @@
 - The naming of the join table entry for N:M association getters is now singular (like includes)
 - Signature of hooks has changed to pass options to all hooks
 - Results returned by hooks are ignored - changes to results by hooks should be made by reference
+- `Model.destroy()` signature has been changed from `(where, options)` to `(options)`, options now take a where parameter.
 # v2.0.0-dev13
 We are working our way to the first 2.0.0 release candidate.

--- a/lib/model.js
+++ b/lib/model.js
@@ -1307,8 +1307,7 @@ module.exports = (function() {
  /**
   * Delete multiple instances, or set their deletedAt timestamp to the current time if `paranoid` is enabled.
   *
-   * @param  {Object}   [where]                         Options to describe the scope of the search.
+   * @param  {Object}   [options.where]                 Filter the destroy
-   * @param  {Object}   [options]
   * @param  {Boolean}  [options.hooks=true]            Run before / after bulk destroy hooks?
   * @param  {Boolean}  [options.individualHooks=false] If set to true, destroy will find all records within the where parameter and will execute before / after bulkDestroy hooks on each row
   * @param  {Number}   [options.limit]                 How many rows to delete
@@ -1318,7 +1317,7 @@ module.exports = (function() {
   *
   * @return {Promise<undefined>}
   */
-  Model.prototype.destroy = function(where, options) {
+  Model.prototype.destroy = function(options) {
    options = Utils._.extend({
      hooks: true,
      individualHooks: false,
@@ -1329,26 +1328,22 @@ module.exports = (function() {
    options.type = QueryTypes.BULKDELETE;
    var self = this
-      , daos;
+      , instances;
    return Promise.try(function() {
      // Run before hook
      if (options.hooks) {
-        options.where = where;
+        return self.runHooks('beforeBulkDestroy', options);
-        return self.runHooks('beforeBulkDestroy', options).then(function() {
-          where = options.where;
-          delete options.where;
-        });
      }
    }).then(function() {
      // Get daos and run beforeDestroy hook on each record individually
      if (options.individualHooks) {
-        return self.findAll({where: where}, {transaction: options.transaction}).map(function(dao) {
+        return self.findAll({where: options.where}, {transaction: options.transaction}).map(function(instance) {
-          return self.runHooks('beforeDestroy', dao, options).then(function() {
+          return self.runHooks('beforeDestroy', instance, options).then(function() {
-            return dao;
+            return instance;
          });
-        }).then(function(_daos) {
+        }).then(function(_instances) {
-          daos = _daos;
+          instances = _instances;
        });
      }
    }).then(function() {
@@ -1356,24 +1351,21 @@ module.exports = (function() {
      if (self._timestampAttributes.deletedAt && !options.force) {
        var attrValueHash = {};
        attrValueHash[self._timestampAttributes.deletedAt] = Utils.now(self.modelManager.sequelize.options.dialect);
-        return self.QueryInterface.bulkUpdate(self.getTableName(), attrValueHash, where, options, self.rawAttributes);
+        return self.QueryInterface.bulkUpdate(self.getTableName(), attrValueHash, options.where, options, self.rawAttributes);
      } else {
-        return self.QueryInterface.bulkDelete(self.getTableName(), where, options, self);
+        return self.QueryInterface.bulkDelete(self.getTableName(), options.where, options, self);
      }
    }).tap(function() {
      // Run afterDestroy hook on each record individually
      if (options.individualHooks) {
-        return Promise.map(daos, function(dao) {
+        return Promise.map(instances, function(instance) {
-          return self.runHooks('afterDestroy', dao, options);
+          return self.runHooks('afterDestroy', instance, options);
        });
      }
    }).tap(function() {
      // Run after hook
      if (options.hooks) {
-        options.where = where;
+        return self.runHooks('afterBulkDestroy', options);
-        return self.runHooks('afterBulkDestroy', options).then(function() {
-          delete options.where;
-        });
      }
    }).then(function(affectedRows) {
      return affectedRows;

--- a/test/dao-factory.test.js
+++ b/test/dao-factory.test.js
@@ -970,7 +970,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        User.sync({ force: true }).success(function() {
          User.create({ username: 'foo' }).success(function() {
            sequelize.transaction().then(function(t) {
-              User.destroy({}, { transaction: t }).success(function() {
+              User.destroy({transaction: t }).success(function() {
                User.count().success(function(count1) {
                  User.count({ transaction: t }).success(function(count2) {
                    expect(count1).to.equal(1)
@@ -992,7 +992,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
                  { username: 'Bob',   secretValue: '43' }]
      this.User.bulkCreate(data).success(function() {
-        self.User.destroy({secretValue: '42'})
+        self.User.destroy({where: {secretValue: '42'}})
          .success(function() {
            self.User.findAll({order: 'id'}).success(function(users) {
              expect(users.length).to.equal(1)
@@ -1021,7 +1021,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        ParanoidUser.bulkCreate(data).success(function() {
          // since we save in UTC, let's format to UTC time
          var date = moment().utc().format('YYYY-MM-DD h:mm')
-          ParanoidUser.destroy({secretValue: '42'}).success(function() {
+          ParanoidUser.destroy({where: {secretValue: '42'}}).success(function() {
            ParanoidUser.findAll({order: 'id'}).success(function(users) {
              expect(users.length).to.equal(1)
              expect(users[0].username).to.equal("Bob")
@@ -1166,10 +1166,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
                  tobi.destroy().success(function() {
                    self.sequelize.query('SELECT * FROM paranoidusers WHERE username=\'Tobi\'', null, {raw: true, plain: true}).success(function(result) {
                      expect(result.username).to.equal('Tobi')
-                      User.destroy({username: 'Tony'}).success(function() {
+                      User.destroy({where: {username: 'Tony'}}).success(function() {
                        self.sequelize.query('SELECT * FROM paranoidusers WHERE username=\'Tony\'', null, {raw: true, plain: true}).success(function(result) {
                          expect(result.username).to.equal('Tony')
-                          User.destroy({username: ['Tony', 'Max']}, {force: true}).success(function() {
+                          User.destroy({where: {username: ['Tony', 'Max']}, force: true}).success(function() {
                            self.sequelize.query('SELECT * FROM paranoidusers', null, {raw: true}).success(function(users) {
                              expect(users).to.have.length(1)
                              expect(users[0].username).to.equal('Tobi')
@@ -1188,32 +1188,24 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      })
    })
-   it('returns the number of affected rows', function(_done) {
+    it('returns the number of affected rows', function () {
-     var self = this
+      var self = this
        , data = [{ username: 'Peter', secretValue: '42' },
                  { username: 'Paul',  secretValue: '42' },
-                  { username: 'Bob',   secretValue: '43' }]
+                  { username: 'Bob',   secretValue: '43' }];
-        , done = _.after(2, _done)
-      this.User.bulkCreate(data).success(function() {
-        self.User.destroy({secretValue: '42'}).done(function(err, affectedRows) {
-          expect(err).not.to.be.ok
-          expect(affectedRows).to.equal(2)
-          done()
-        })
-        self.User.destroy({secretValue: '44'}).done(function(err, affectedRows) {
-          expect(err).not.to.be.ok
-          expect(affectedRows).to.equal(0)
-          done()
+      return this.User.bulkCreate(data).then(function() {
-        })
+        return self.User.destroy({where: {secretValue: '42'}}).then(function(affectedRows) {
-      })
+          expect(affectedRows).to.equal(2);
-    })
+        });
+      }).then(function () {
+        return self.User.destroy({where: {secretValue: '44'}}).then(function(affectedRows) {
+          expect(affectedRows).to.equal(0);
+        });
+      });
+    });
-   it('supports table schema/prefix', function(done) {
+    it('supports table schema/prefix', function(done) {
     var self = this
       , data = [{ username: 'Peter', secretValue: '42' },
                 { username: 'Paul',  secretValue: '42' },
@@ -1223,7 +1215,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
     var run = function() {
       prefixUser.sync({ force: true }).success(function() {
         prefixUser.bulkCreate(data).success(function() {
-           prefixUser.destroy({secretValue: '42'})
+           prefixUser.destroy({where: {secretValue: '42'}})
             .success(function() {
               prefixUser.findAll({order: 'id'}).success(function(users) {
                 expect(users.length).to.equal(1)