Merge pull request #1194 from mickhansen/has-many-through-self-association-fix

Add test and fix for hasMany selfAssociation with through option (Fixes #1184)

lib/associations/has-many-double-linked.js

@@ -21,7 +21,7 @@ module.exports = (function() {
         , options = _options || {}
         , queryOptions = {}
         , targetAssociation = self.association.targetAssociation
-
+        
       //fully qualify
       var instancePrimaryKeys = Object.keys(self.instance.daoFactory.primaryKeys)
         , instancePrimaryKey = instancePrimaryKeys.length > 0 ? instancePrimaryKeys[0] : 'id'

lib/associations/has-many.js

@@ -56,31 +56,38 @@ module.exports = (function() {
     }
 
     /*
-     * Find partner DAOFactory if present, to identify double linked association
+     * If self association, this association is target association
      */
-     if (this.through) {
-      _.each(this.target.associations, function (association, accessor) {
-        if (self.source === association.target) {
-          var paired = false
-
-          // If through is default, we determine pairing by the accesor value (i.e. DAOFactory's using as won't pair, but regular ones will)
-          if (self.through === true && accessor === self.associationAccessor) {
-            paired = true
-          }
-          // If through is not default, determine pairing by through value (model/string)
-          if (self.through !== true && self.options.through === association.options.through) {
-            paired = true
-          }
-          // If paired, set properties identifying both associations as double linked, and allow them to each eachtoerh
-          if (paired) {
-            self.doubleLinked = true
-            association.doubleLinked = true
-            
-            self.targetAssociation = association
-            association.targetAssociation = self
-          }
+    if (this.isSelfAssociation) {
+      this.targetAssociation = this
+    }
+
+    /*
+     * Else find partner DAOFactory if present, to identify double linked association
+     */
+    else if (this.through) {
+    _.each(this.target.associations, function (association, accessor) {
+      if (self.source === association.target) {
+        var paired = false
+
+        // If through is default, we determine pairing by the accesor value (i.e. DAOFactory's using as won't pair, but regular ones will)
+        if (self.through === true && accessor === self.associationAccessor) {
+          paired = true
         }
-      })
+        // If through is not default, determine pairing by through value (model/string)
+        if (self.through !== true && self.options.through === association.options.through) {
+          paired = true
+        }
+        // If paired, set properties identifying both associations as double linked, and allow them to each eachtoerh
+        if (paired) {
+          self.doubleLinked = true
+          association.doubleLinked = true
+          
+          self.targetAssociation = association
+          association.targetAssociation = self
+        }
+      }
+    })
     }
 
     /*
@@ -90,13 +97,14 @@ module.exports = (function() {
       if (this.through === true) {
         this.through = this.combinedTableName
       }
-      if (typeof this.through === "string") {
-        this.through = this.sequelize.define(this.through, {}, _.extend(this.options, {
-          tableName: this.through
-        }))
+    }
 
-        this.targetAssociation.through = this.through
-      }
+    if (typeof this.through === "string") {
+      this.through = this.sequelize.define(this.through, {}, _.extend(this.options, {
+        tableName: this.through
+      }))
+
+      if (this.targetAssociation) this.targetAssociation.through = this.through
     }
 
     this.options.tableName = this.combinedName = (this.through === Object(this.through) ? this.through.tableName : this.through)
@@ -238,7 +246,7 @@ module.exports = (function() {
       return new Utils.CustomEventEmitter(function(emitter) {
         instance[self.accessors.get]()
           .success(function(oldAssociatedObjects) {
-            var Class = self.doubleLinked ? HasManyMultiLinked : HasManySingleLinked
+            var Class = Object(self.through) === self.through ? HasManyMultiLinked : HasManySingleLinked
             new Class(self, instance).injectSetter(emitter, oldAssociatedObjects, newAssociatedObjects, defaultAttributes)
           })
           .proxy(emitter, {events: ['error', 'sql']})
@@ -258,7 +266,7 @@ module.exports = (function() {
           .proxy(emitter, {events: ['error', 'sql']})
           .success(function(currentAssociatedObjects) {
             if (currentAssociatedObjects.length === 0 || Object(self.through) === self.through) {
-              var Class = self.doubleLinked ? HasManyMultiLinked : HasManySingleLinked
+              var Class = Object(self.through) === self.through ? HasManyMultiLinked : HasManySingleLinked
               new Class(self, instance).injectAdder(emitter, newAssociatedObject, additionalAttributes, !!currentAssociatedObjects.length)
             } else {
               emitter.emit('success', newAssociatedObject);

lib/dialects/abstract/query-generator.js

@@ -399,7 +399,7 @@ module.exports = (function() {
 
           optAttributes = optAttributes.concat(attributes)
 
-          if (include.association.doubleLinked) {
+          if (Object(include.association.through) === include.association.through) {
             var primaryKeysSource = Object.keys(include.association.source.primaryKeys)
               , tableSource       = parentTable
               , identSource       = include.association.identifier

test/associations/has-many.test.js

@@ -429,6 +429,19 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
         })
       })
     }) // end optimization using bulk create, destroy and update
+
+    describe('selfAssociations', function () {
+      it('should work', function (done) {
+        var Group = this.sequelize.define('Group', {})
+
+        Group.hasMany(Group, { through: 'groups_outsourcing_companies', as: 'OutsourcingCompanies'});
+
+        this.sequelize.sync().done(function (err) {
+          expect(err).not.to.be.ok
+          done()
+        })
+      })
+    })
   })
 
   describe('(N:M)', function() {

test/include.test.js

@@ -478,6 +478,40 @@ describe(Support.getTestDialectTeaser("Include"), function () {
         })
       })
     })
+
+    it('should support self associated hasMany (with through) include', function (done) {
+      var Group = this.sequelize.define('Group', {
+        name: DataTypes.STRING
+      })
+
+      Group.hasMany(Group, { through: 'groups_outsourcing_companies', as: 'OutsourcingCompanies'});
+
+      this.sequelize.sync().done(function (err) {
+        Group.bulkCreate([
+          {name: 'SoccerMoms'},
+          {name: 'Coca Cola'},
+          {name: 'Dell'},
+          {name: 'Pepsi'}
+        ]).done(function () {
+          Group.findAll().done(function (err, groups) {
+            groups[0].setOutsourcingCompanies(groups.slice(1)).done(function (err) {
+              expect(err).not.to.be.ok
+
+              Group.find({
+                where: {
+                  id: groups[0].id,
+                },
+                include: [{model: Group, as: 'OutsourcingCompanies'}]
+              }).done(function (err, group) {
+                expect(err).not.to.be.ok
+                expect(group.outsourcingCompanies.length).to.equal(3)
+                done()
+              })
+            })
+          })
+        })
+      })
+    })
   })
 
   describe('findAll', function () {