Move afterCreate hook (#5829)

Luc Verdier · Jan Aagaard Meier
Commit d14615a7 authored May 18, 2016 by Luc Verdier Committed by Jan Aagaard Meier May 18, 2016
Showing with 46 additions and 14 deletions
changelog.md
lib/instance.js
test/integration/model/create/include.test.js
--- a/changelog.md
+++ b/changelog.md
@@ -7,6 +7,7 @@
 - [FIXED] Add `raw` support to `instance.get()` [#5815](https://github.com/sequelize/sequelize/issues/5815)
 - [ADDED] Compare `deletedAt` against current timestamp when using paranoid [#5880](https://github.com/sequelize/sequelize/pull/5880)
 - [FIXED] `BIGINT` gets truncated [#5176](https://github.com/sequelize/sequelize/issues/5176)
+- [FIXED] Trigger afterCreate hook after all nested includes (for hasMany or belongsToMany associations) have been created to be consistent with hasOne.
 - [REMOVED] Support for `pool:false`
 ## BC breaks:

--- a/lib/instance.js
+++ b/lib/instance.js
@@ -675,20 +675,6 @@ Instance.prototype.save = function(options) {
          return result;
        })
        .tap(function(result) {
-          // Run after hook
-          if (options.hooks) {
-            return self.Model.runHooks('after' + hook, result, options);
-          }
-        })
-        .then(function(result) {
-          options.fields.forEach(function (field) {
-            result._previousDataValues[field] = result.dataValues[field];
-            self.changed(field, false);
-          });
-          self.isNewRecord = false;
-          return result;
-        })
-        .tap(function() {
          if (!wasNewRecord) return self;
          if (!self.$options.include || !self.$options.include.length) return self;
@@ -725,6 +711,20 @@ Instance.prototype.save = function(options) {
              }
            });
          });
+        })
+        .tap(function(result) {
+          // Run after hook
+          if (options.hooks) {
+            return self.Model.runHooks('after' + hook, result, options);
+          }
+        })
+        .then(function(result) {
+          options.fields.forEach(function (field) {
+            result._previousDataValues[field] = result.dataValues[field];
+            self.changed(field, false);
+          });
+          self.isNewRecord = false;
+          return result;
        });
    });
  });

--- a/test/integration/model/create/include.test.js
+++ b/test/integration/model/create/include.test.js
@@ -13,6 +13,12 @@ describe(Support.getTestDialectTeaser('Model'), function() {
      it('should create data for BelongsTo relations', function() {
        var Product = this.sequelize.define('Product', {
          title: Sequelize.STRING
+        }, {
+          hooks: {
+            afterCreate: function (product) {
+              product.isIncludeCreatedOnAfterCreate = !!(product.User && product.User.id);
+            }
+          }
        });
        var User = this.sequelize.define('User', {
          first_name: Sequelize.STRING,
@@ -40,6 +46,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
              myOption: 'option'
            }]
          }).then(function(savedProduct) {
+            expect(savedProduct.isIncludeCreatedOnAfterCreate).to.be.true;
            expect(savedProduct.User.createOptions.myOption).to.be.equal('option');
            expect(savedProduct.User.createOptions.parentRecord).to.be.equal(savedProduct);
            return Product.findOne({
@@ -90,6 +97,15 @@ describe(Support.getTestDialectTeaser('Model'), function() {
      it('should create data for HasMany relations', function() {
        var Product = this.sequelize.define('Product', {
          title: Sequelize.STRING
+        }, {
+          hooks: {
+            afterCreate: function (product) {
+              product.areIncludesCreatedOnAfterCreate = product.Tags &&
+                product.Tags.every(function (tag) {
+                  return !!tag.id;
+                });
+            }
+          }
        });
        var Tag = this.sequelize.define('Tag', {
          name: Sequelize.STRING
@@ -117,6 +133,11 @@ describe(Support.getTestDialectTeaser('Model'), function() {
              myOption: 'option'
            }]
          }).then(function(savedProduct) {
+            expect(savedProduct.areIncludesCreatedOnAfterCreate).to.be.true;
+            expect(savedProduct.Tags[0].createOptions.myOption).to.be.equal('option');
+            expect(savedProduct.Tags[0].createOptions.parentRecord).to.be.equal(savedProduct);
+            expect(savedProduct.Tags[1].createOptions.myOption).to.be.equal('option');
+            expect(savedProduct.Tags[1].createOptions.parentRecord).to.be.equal(savedProduct);
            return Product.find({
              where: { id: savedProduct.id },
              include: [ Tag ]
@@ -224,6 +245,15 @@ describe(Support.getTestDialectTeaser('Model'), function() {
      it('should create data for BelongsToMany relations', function() {
        var User = this.sequelize.define('User', {
          username: DataTypes.STRING
+        },{
+          hooks: {
+            afterCreate: function (user) {
+              user.areIncludesCreatedOnAfterCreate = user.Tasks &&
+                user.Tasks.every(function (task) {
+                  return !!task.id;
+                });
+            }
+          }
        });
        var Task = this.sequelize.define('Task', {
@@ -253,6 +283,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
              myOption: 'option'
            }]
          }).then(function(savedUser) {
+            expect(savedUser.areIncludesCreatedOnAfterCreate).to.be.true;
            expect(savedUser.Tasks[0].createOptions.myOption).to.be.equal('option');
            expect(savedUser.Tasks[0].createOptions.parentRecord).to.be.equal(savedUser);
            expect(savedUser.Tasks[1].createOptions.myOption).to.be.equal('option');