Merge branch 'BridgeAR-fix-update-hook'

changelog.md

@@ -4,6 +4,7 @@
 - [BUG] Fix app crash in sqlite while running in special unique constraint errors [3730](https://github.com/sequelize/sequelize/pull/3730)
 - [BUG] Fix app crash in sqlite while running in special unique constraint errors [3730](https://github.com/sequelize/sequelize/pull/3730)
 - [BUG] Fix bulkCreate: do not insert NULL for undefined values [3729](https://github.com/sequelize/sequelize/pull/3729)
 - [BUG] Fix bulkCreate: do not insert NULL for undefined values [3729](https://github.com/sequelize/sequelize/pull/3729)
 - [BUG] Fix trying to roll back a comitted transaction if an error occured while comitting resulting in an unhandled rejection [3726](https://github.com/sequelize/sequelize/pull/3726)
 - [BUG] Fix trying to roll back a comitted transaction if an error occured while comitting resulting in an unhandled rejection [3726](https://github.com/sequelize/sequelize/pull/3726)
+- [BUG] Fix regression in beforeUpdate hook where `instance.changed()` would always be false [3727](https://github.com/sequelize/sequelize/pull/3727)
 
 
 #### Backwards compatibility changes
 #### Backwards compatibility changes
 - The error that is thrown when a column is declared to be an enum but without any values used to "Values for ENUM haven't been defined" and is now "Values for ENUM have not been defined".
 - The error that is thrown when a column is declared to be an enum but without any values used to "Values for ENUM haven't been defined" and is now "Values for ENUM have not been defined".

lib/model.js

@@ -1624,7 +1624,7 @@ module.exports = (function() {
       values[this._timestampAttributes.updatedAt] = this.__getTimestamp(this._timestampAttributes.updatedAt);
       values[this._timestampAttributes.updatedAt] = this.__getTimestamp(this._timestampAttributes.updatedAt);
     }
     }
 
 
-    var daos
+    var instances
       , valuesUse;
       , valuesUse;
 
 
     return Promise.try(function() {
     return Promise.try(function() {
@@ -1659,11 +1659,11 @@ module.exports = (function() {
     }).then(function() {
     }).then(function() {
       valuesUse = values;
       valuesUse = values;
 
 
-      // Get daos and run beforeUpdate hook on each record individually
+      // Get instances and run beforeUpdate hook on each record individually
       if (options.individualHooks) {
       if (options.individualHooks) {
-        return self.findAll({where: options.where}, {transaction: options.transaction}).then(function(_daos) {
-          daos = _daos;
-          if (!daos.length) {
+        return self.findAll({where: options.where}, {transaction: options.transaction}).then(function(_instances) {
+          instances = _instances;
+          if (!instances.length) {
             return [];
             return [];
           }
           }
 
 
@@ -1672,16 +1672,22 @@ module.exports = (function() {
           var changedValues
           var changedValues
             , different = false;
             , different = false;
 
 
-          return Promise.map(daos, function(dao) {
-            // Record updates in dao's dataValues
-            Utils._.extend(dao.dataValues, values);
+          return Promise.map(instances, function(instance) {
+            // Record updates in instances dataValues
+            Utils._.extend(instance.dataValues, values);
+            // Set the changed fields on the instance
+            Utils._.forIn(valuesUse, function(newValue, attr) {
+              if (newValue !== instance._previousDataValues[attr]) {
+                instance.setDataValue(attr, newValue);
+              }
+            });
 
 
             // Run beforeUpdate hook
             // Run beforeUpdate hook
-            return self.runHooks('beforeUpdate', dao, options).then(function() {
+            return self.runHooks('beforeUpdate', instance, options).then(function() {
               if (!different) {
               if (!different) {
                 var thisChangedValues = {};
                 var thisChangedValues = {};
-                Utils._.forIn(dao.dataValues, function(newValue, attr) {
-                  if (newValue !== dao._previousDataValues[attr]) {
+                Utils._.forIn(instance.dataValues, function(newValue, attr) {
+                  if (newValue !== instance._previousDataValues[attr]) {
                     thisChangedValues[attr] = newValue;
                     thisChangedValues[attr] = newValue;
                   }
                   }
                 });
                 });
@@ -1693,10 +1699,10 @@ module.exports = (function() {
                 }
                 }
               }
               }
 
 
-              return dao;
+              return instance;
             });
             });
-          }).then(function(_daos) {
-            daos = _daos;
+          }).then(function(_instances) {
+            instances = _instances;
 
 
             if (!different) {
             if (!different) {
               // Hooks do not change values or change them uniformly
               // Hooks do not change values or change them uniformly
@@ -1708,15 +1714,15 @@ module.exports = (function() {
             } else {
             } else {
               // Hooks change values in a different way for each record
               // Hooks change values in a different way for each record
               // Do not run original query but save each record individually
               // Do not run original query but save each record individually
-              return Promise.map(daos, function(dao) {
+              return Promise.map(instances, function(instance) {
                 var individualOptions = Utils._.clone(options);
                 var individualOptions = Utils._.clone(options);
                 delete individualOptions.individualHooks;
                 delete individualOptions.individualHooks;
                 individualOptions.hooks = false;
                 individualOptions.hooks = false;
                 individualOptions.validate = false;
                 individualOptions.validate = false;
 
 
-                return dao.save(individualOptions);
-              }).tap(function(_daos) {
-                daos = _daos;
+                return instance.save(individualOptions);
+              }).tap(function(_instances) {
+                instances = _instances;
               });
               });
             }
             }
           });
           });
@@ -1734,7 +1740,7 @@ module.exports = (function() {
       // Run query to update all rows
       // Run query to update all rows
       return self.QueryInterface.bulkUpdate(self.getTableName(options), valuesUse, options.where, options, self.tableAttributes).then(function(affectedRows) {
       return self.QueryInterface.bulkUpdate(self.getTableName(options), valuesUse, options.where, options, self.tableAttributes).then(function(affectedRows) {
         if (options.returning) {
         if (options.returning) {
-          daos = affectedRows;
+          instances = affectedRows;
           return [affectedRows.length, affectedRows];
           return [affectedRows.length, affectedRows];
         }
         }
 
 
@@ -1742,10 +1748,10 @@ module.exports = (function() {
       });
       });
     }).tap(function(result) {
     }).tap(function(result) {
       if (options.individualHooks) {
       if (options.individualHooks) {
-        return Promise.map(daos, function(dao) {
-          return self.runHooks('afterUpdate', dao, options);
+        return Promise.map(instances, function(instance) {
+          return self.runHooks('afterUpdate', instance, options);
         }).then(function() {
         }).then(function() {
-          result[1] = daos;
+          result[1] = instances;
         });
         });
       }
       }
     }).tap(function() {
     }).tap(function() {
@@ -1757,7 +1763,7 @@ module.exports = (function() {
         });
         });
       }
       }
     }).then(function(result) {
     }).then(function(result) {
-      // Return result in form [affectedRows, daos] (daos missed off if options.individualHooks != true)
+      // Return result in form [affectedRows, instances] (instances missed off if options.individualHooks != true)
       return result;
       return result;
     });
     });
   };
   };

test/integration/hooks.test.js

@@ -3437,6 +3437,7 @@ describe(Support.getTestDialectTeaser('Hooks'), function() {
         });
         });
 
 
         this.User.beforeUpdate(function(user, options, fn) {
         this.User.beforeUpdate(function(user, options, fn) {
+          expect(user.changed()).to.not.be.empty;
           user.beforeHookTest = true;
           user.beforeHookTest = true;
           fn();
           fn();
         });
         });
@@ -3460,33 +3461,52 @@ describe(Support.getTestDialectTeaser('Hooks'), function() {
         });
         });
       });
       });
 
 
+      it('should run the after/before functions for each item created successfully changing some data before updating', function() {
+        var self = this;
+
+        this.User.beforeUpdate(function(user, options) {
+          expect(user.changed()).to.not.be.empty;
+          if (user.get('id') === 1) {
+            user.set('aNumber', user.get('aNumber') + 3);
+          }
+        });
+
+        return this.User.bulkCreate([
+          {aNumber: 1}, {aNumber: 1}, {aNumber: 1}
+        ]).then(function() {
+          return self.User.update({aNumber: 10}, {where: {aNumber: 1}, individualHooks: true}).spread(function(affectedRows, records) {
+            records.forEach(function(record, i) {
+              expect(record.aNumber).to.equal(10 + (record.id === 1 ? 3 : 0));
+            });
+          });
+        });
+      });
+
       it('should run the after/before functions for each item created with an error', function() {
       it('should run the after/before functions for each item created with an error', function() {
         var self = this
         var self = this
           , beforeBulk = false
           , beforeBulk = false
           , afterBulk = false;
           , afterBulk = false;
 
 
-        this.User.beforeBulkUpdate(function(options, fn) {
+        this.User.beforeBulkUpdate(function(options) {
           beforeBulk = true;
           beforeBulk = true;
-          fn();
         });
         });
 
 
-        this.User.afterBulkUpdate(function(options, fn) {
+        this.User.afterBulkUpdate(function(options) {
           afterBulk = true;
           afterBulk = true;
-          fn();
         });
         });
 
 
-        this.User.beforeUpdate(function(user, options, fn) {
-          fn(new Error('You shall not pass!'));
+        this.User.beforeUpdate(function(user, options) {
+          throw new Error('You shall not pass!');
         });
         });
 
 
-        this.User.afterUpdate(function(user, options, fn) {
+        this.User.afterUpdate(function(user, options) {
           user.username = 'User' + user.id;
           user.username = 'User' + user.id;
-          fn();
         });
         });
 
 
         return this.User.bulkCreate([{aNumber: 1}, {aNumber: 1}, {aNumber: 1}], { fields: ['aNumber'] }).then(function() {
         return this.User.bulkCreate([{aNumber: 1}, {aNumber: 1}, {aNumber: 1}], { fields: ['aNumber'] }).then(function() {
           return self.User.update({aNumber: 10}, {where: {aNumber: 1}, individualHooks: true}).catch(function(err) {
           return self.User.update({aNumber: 10}, {where: {aNumber: 1}, individualHooks: true}).catch(function(err) {
             expect(err).to.be.instanceOf(Error);
             expect(err).to.be.instanceOf(Error);
+            expect(err.message).to.be.equal('You shall not pass!');
             expect(beforeBulk).to.be.true;
             expect(beforeBulk).to.be.true;
             expect(afterBulk).to.be.false;
             expect(afterBulk).to.be.false;
           });
           });