Refactor polymorphic relations, fix inverse #215

See #215 - when creating a related item through a the inverse of a
polymorphic HABTM relation,
the through-model was not created correctly. By refactoring the
specifics into the `polymorphic`
property of a RelationDefinition, it's now possible to handle this
correctly.

lib/relation-definition.js

@@ -98,8 +98,8 @@ function RelationDefinition(definition) {
   this.keyFrom = definition.keyFrom;
   this.modelTo = definition.modelTo;
   this.keyTo = definition.keyTo;
-  this.discriminator = definition.discriminator;
+  this.polymorphic = definition.polymorphic;
-  if (!this.discriminator) {
+  if (typeof this.polymorphic !== 'object') {
     assert(this.modelTo, 'Target model is required');
   }
   this.modelThrough = definition.modelThrough;
@@ -137,8 +137,13 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) {
   filter = filter || {};
   filter.where = filter.where || {};
   if ((this.type !== 'belongsTo' || this.type === 'hasOne')
-    && typeof this.discriminator === 'string') { // polymorphic
+    && typeof this.polymorphic === 'object') { // polymorphic
-    filter.where[this.discriminator] = this.modelFrom.modelName;
+    var discriminator = this.polymorphic.discriminator;
+    if (this.polymorphic.invert) {
+      filter.where[discriminator] = this.modelTo.modelName;
+    } else {
+      filter.where[discriminator] = this.modelFrom.modelName;
+    }
   }
   if (typeof this.scope === 'function') {
     var scope = this.scope.call(this, modelInstance, filter);
@@ -172,8 +177,13 @@ RelationDefinition.prototype.applyProperties = function(modelInstance, obj) {
     }
   }
   if ((this.type !== 'belongsTo' || this.type === 'hasOne')
-    && typeof this.discriminator === 'string') { // polymorphic
+    && typeof this.polymorphic === 'object') { // polymorphic
-    target[this.discriminator] = this.modelFrom.modelName;
+    var discriminator = this.polymorphic.discriminator;
+    if (this.polymorphic.invert) {
+      target[discriminator] = this.modelTo.modelName;
+    } else {
+      target[discriminator] = this.modelFrom.modelName;
+    }
   }
 };
 
@@ -461,10 +471,11 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
   var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true);
 
   var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id';
-  var discriminator;
+  var discriminator, polymorphic;
   
   if (params.polymorphic) {
-    var polymorphic = polymorphicParams(params.polymorphic);
+    polymorphic = polymorphicParams(params.polymorphic);
+    polymorphic.invert = !!params.invert;
     discriminator = polymorphic.discriminator;
     if (!params.invert) {
       fk = polymorphic.foreignKey;
@@ -480,12 +491,12 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
     modelFrom: modelFrom,
     keyFrom: idName,
     keyTo: fk,
-    discriminator: discriminator,
     modelTo: modelTo,
     multiple: true,
     properties: params.properties,
     scope: params.scope,
-    options: params.options
+    options: params.options,
+    polymorphic: polymorphic
   });
   
   definition.modelThrough = params.through;
@@ -700,9 +711,13 @@ var throughKeys = function(definition) {
   var modelThrough = definition.modelThrough;
   var pk2 = definition.modelTo.definition.idName();
   
-  if (definition.discriminator) { // polymorphic
+  if (typeof definition.polymorphic === 'object') { // polymorphic
     var fk1 = definition.keyTo;
-    var fk2 = definition.keyThrough;
+    if (definition.polymorphic.invert) {
+      var fk2 = definition.polymorphic.foreignKey;
+    } else {
+      var fk2 = definition.keyThrough;
+    }
   } else {
     var fk1 = findBelongsTo(modelThrough, definition.modelFrom,
       definition.keyFrom);
@@ -1004,11 +1019,11 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) {
     modelFrom: modelFrom,
     keyFrom: fk,
     keyTo: idName,
-    discriminator: discriminator,
     modelTo: modelTo,
     properties: params.properties,
     scope: params.scope,
-    options: params.options
+    options: params.options,
+    polymorphic: polymorphic
   });
   
   // Define a property for the scope so that we have 'this' for the scoped methods
@@ -1086,10 +1101,10 @@ BelongsTo.prototype.related = function (refresh, params) {
   var self = this;
   var modelFrom = this.definition.modelFrom;
   var modelTo = this.definition.modelTo;
-  var discriminator = this.definition.discriminator;
   var pk = this.definition.keyTo;
   var fk = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
+  var discriminator;
   
   if (arguments.length === 1) {
     params = refresh;
@@ -1098,6 +1113,10 @@ BelongsTo.prototype.related = function (refresh, params) {
     throw new Error('Method can\'t be called with more than two arguments');
   }
   
+  if (typeof this.definition.polymorphic === 'object') {
+    discriminator = this.definition.polymorphic.discriminator;
+  }
+  
   var cachedValue;
   if (!refresh) {
     cachedValue = self.getCache();
@@ -1105,7 +1124,10 @@ BelongsTo.prototype.related = function (refresh, params) {
   if (params instanceof ModelBaseClass) { // acts as setter
     modelTo = params.constructor;
     modelInstance[fk] = params[pk];
-    if (discriminator) modelInstance[discriminator] = params.constructor.modelName;
+    
+    if (discriminator) {
+      modelInstance[discriminator] = params.constructor.modelName;
+    }
     
     this.definition.applyProperties(modelInstance, params);
     
@@ -1245,10 +1267,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) {
   var relationName = params.as || i8n.camelize(modelTo.modelName, true);
 
   var fk = params.foreignKey || i8n.camelize(modelFrom.modelName + '_id', true);
-  var discriminator;
+  var discriminator, polymorphic;
   
   if (params.polymorphic) {
-    var polymorphic = polymorphicParams(params.polymorphic);
+    polymorphic = polymorphicParams(params.polymorphic);
     fk = polymorphic.foreignKey;
     discriminator = polymorphic.discriminator;
     if (!params.through) {
@@ -1262,10 +1284,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) {
     modelFrom: modelFrom,
     keyFrom: pk,
     keyTo: fk,
-    discriminator: discriminator,
     modelTo: modelTo,
     properties: params.properties,
-    options: params.options
+    options: params.options,
+    polymorphic: polymorphic
   });
 
   modelFrom.dataSource.defineForeignKey(modelTo.modelName, fk, modelFrom.modelName);

test/relations.test.js

@@ -1030,6 +1030,40 @@ describe('relations', function () {
         });
       });
     });
+    
+    it('should create polymorphic item through relation scope', function (done) {
+      Picture.findById(anotherPicture.id, function(err, p) {
+        p.authors.create({ name: 'Author 3' }, function(err, a) {
+          should.not.exist(err);
+          author = a;
+          author.name.should.equal('Author 3');
+          done();
+        });
+      });
+    });
+    
+    it('should create polymorphic through model - new author', function (done) {
+      PictureLink.findOne({ where: { 
+          pictureId: anotherPicture.id, imageableId: author.id, imageableType: 'Author'
+        } }, function(err, link) {
+        should.not.exist(err);
+        link.pictureId.should.eql(anotherPicture.id);
+        link.imageableId.should.eql(author.id);
+        link.imageableType.should.equal('Author');
+        done();
+      });
+    });
+    
+    it('should find polymorphic items - new author', function (done) {
+      Author.findById(author.id, function(err, author) {
+        author.pictures(function(err, pics) {
+          pics.should.have.length(1);
+          pics[0].id.should.eql(anotherPicture.id);
+          pics[0].name.should.equal('Example');
+          done();
+        });
+      });
+    });
   
   });