fix(n:m): fix issue with otherKey inferration for N:M self associations, closes #3084

changelog.md

 # Next
 # Next
 - [BUG] Fixed issue with empty `include.where`
 - [BUG] Fixed issue with empty `include.where`
+- [BUG] Fixed issue with otherKey generation for self-association N:M
 
 
 # 2.0.0
 # 2.0.0
 - [BUG] Fixed `field` support for `increment` and `decrement`.
 - [BUG] Fixed `field` support for `increment` and `decrement`.

lib/associations/belongs-to-many.js

@@ -38,6 +38,9 @@ module.exports = (function() {
       this.isSelfAssociation ? (this.as || this.target.tableName) : this.target.tableName
       this.isSelfAssociation ? (this.as || this.target.tableName) : this.target.tableName
     );
     );
 
 
+    /*
+     * Normalize through
+     */
     if (this.through === undefined) {
     if (this.through === undefined) {
       this.through = true;
       this.through = true;
     }
     }
@@ -46,6 +49,26 @@ module.exports = (function() {
       deprecated('belongsToMany: not defining through or defining through as true has been deprecated, please provide a string or a model');
       deprecated('belongsToMany: not defining through or defining through as true has been deprecated, please provide a string or a model');
     }
     }
 
 
+    if (this.through !== null && !this.through.model) {
+      this.through = {
+        model: this.through
+      };
+    }
+
+    /*
+     * If self association, this is the target association - Unless we find a pairing association
+     */
+    if (this.isSelfAssociation) {
+      if (!this.as) {
+        throw new Error('\'as\' must be defined for many-to-many self-associations');
+      }
+
+      this.targetAssociation = this;
+    }
+
+    /*
+     * Default/generated foreign/other keys
+     */
     if (Utils._.isObject(this.options.foreignKey)) {
     if (Utils._.isObject(this.options.foreignKey)) {
       this.foreignKeyAttribute = this.options.foreignKey;
       this.foreignKeyAttribute = this.options.foreignKey;
       this.foreignKey = this.foreignKeyAttribute.name || this.foreignKeyAttribute.fieldName;
       this.foreignKey = this.foreignKeyAttribute.name || this.foreignKeyAttribute.fieldName;
@@ -75,19 +98,18 @@ module.exports = (function() {
       this.otherKeyAttribute = {};
       this.otherKeyAttribute = {};
       this.otherKey = this.options.otherKey || Utils._.camelizeIf(
       this.otherKey = this.options.otherKey || Utils._.camelizeIf(
         [
         [
-          Utils._.underscoredIf(this.target.options.name.singular, this.target.options.underscored),
+          Utils._.underscoredIf(
+            this.isSelfAssociation ?
+              Utils.singularize(this.as) :
+              this.target.options.name.singular,
+            this.target.options.underscored
+          ),
           this.target.primaryKeyAttribute
           this.target.primaryKeyAttribute
         ].join('_'),
         ].join('_'),
         !this.target.options.underscored
         !this.target.options.underscored
       );
       );
     }
     }
 
 
-    if (this.through !== null && !this.through.model) {
-      this.through = {
-        model: this.through
-      };
-    }
-
     /*
     /*
      * Determine associationAccessor, especially for include options to identify the correct model
      * Determine associationAccessor, especially for include options to identify the correct model
      */
      */
@@ -104,18 +126,6 @@ module.exports = (function() {
     }
     }
 
 
     /*
     /*
-     * If self association, this is the target association - Unless we find a pairing association
-     */
-    if (this.isSelfAssociation) {
-      // check 'as' is defined for many-to-many self-association
-      if (this.through && this.through.model !== true && !this.as) {
-        throw new Error('\'as\' must be defined for many-to-many self-associations');
-      }
-
-      this.targetAssociation = this;
-    }
-
-    /*
      * If we are double linked, and through is either default or a string, we create the through model and set it on both associations
      * If we are double linked, and through is either default or a string, we create the through model and set it on both associations
      */
      */
     if (this.through.model === true) {
     if (this.through.model === true) {

test/integration/associations/belongs-to-many.test.js

@@ -1707,4 +1707,30 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
         .throw ("Naming collision between attribute 'user' and association 'user' on model user. To remedy this, change either foreignKey or as in your association definition");
         .throw ("Naming collision between attribute 'user' and association 'user' on model user. To remedy this, change either foreignKey or as in your association definition");
     });
     });
   });
   });
-});
+
+  describe('selfAssociations', function () {
+    it('should setup correct foreign keys', function () {
+      /* camcelCase */
+      var Person = this.sequelize.define('Person')
+        , PersonChildren = this.sequelize.define('PersonChildren')
+        , Children;
+
+      Children = Person.belongsToMany(Person, { as: 'Children', through: PersonChildren});
+
+      expect(Children.foreignKey).to.equal('PersonId');
+      expect(Children.otherKey).to.equal('ChildId');
+      expect(PersonChildren.rawAttributes[Children.foreignKey]).to.be.ok;
+      expect(PersonChildren.rawAttributes[Children.otherKey]).to.be.ok;
+
+      /* underscored */
+      Person = this.sequelize.define('Person', {}, {underscored: true});
+      PersonChildren = this.sequelize.define('PersonChildren', {}, {underscored: true});
+      Children = Person.belongsToMany(Person, { as: 'Children', through: PersonChildren});
+
+      expect(Children.foreignKey).to.equal('person_id');
+      expect(Children.otherKey).to.equal('child_id');
+      expect(PersonChildren.rawAttributes[Children.foreignKey]).to.be.ok;
+      expect(PersonChildren.rawAttributes[Children.otherKey]).to.be.ok;
+    });
+  });
+});
\ No newline at end of file