Merge pull request #281 from clarkorz/fix/hasManyThrough-custom-relation-name

Allows the belongsTo relations in through model to have custom name

lib/relation-definition.js

@@ -540,13 +540,17 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
   
   var relationName = params.as || i8n.camelize(modelTo.pluralModelName, true);
   var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true);
+  var keyThrough = params.keyThrough || i8n.camelize(modelTo.modelName + '_id', true);
 
   var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id';
   var discriminator, polymorphic;
   
   if (params.polymorphic) {
     polymorphic = polymorphicParams(params.polymorphic);
-    if (params.invert) polymorphic.invert = true;
+    if (params.invert) {
+      polymorphic.invert = true;
+      keyThrough = polymorphic.foreignKey;
+    }
     discriminator = polymorphic.discriminator;
     if (!params.invert) {
       fk = polymorphic.foreignKey;
@@ -567,14 +571,12 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
     properties: params.properties,
     scope: params.scope,
     options: params.options,
+    keyThrough: keyThrough,
     polymorphic: polymorphic
   });
   
   definition.modelThrough = params.through;
   
-  var keyThrough = params.keyThrough || i8n.camelize(modelTo.modelName + '_id', true);
-  definition.keyThrough = keyThrough;
-  
   modelFrom.relations[relationName] = definition;
 
   if (!params.through) {
@@ -636,14 +638,32 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
     
     definition.applyScope(this, filter);
 
-    if (params.through && params.polymorphic && params.invert) {
+    if (definition.modelThrough) {
-      filter.where[discriminator] = modelTo.modelName; // overwrite
+      var throughRelationName;
-      filter.collect = params.polymorphic;
+
+      // find corresponding belongsTo relations from through model as collect
+      for(var r in definition.modelThrough.relations) {
+        var relation = definition.modelThrough.relations[r];
+
+        // should be a belongsTo and match modelTo and keyThrough
+        // if relation is polymorphic then check keyThrough only
+        if (relation.type === RelationTypes.belongsTo &&
+          (relation.polymorphic && !relation.modelTo || relation.modelTo === definition.modelTo) &&
+          (relation.keyFrom === definition.keyThrough)
+          ) {
+          throughRelationName = relation.name;
+          break;
+        }
+      }
+
+      if (definition.polymorphic && definition.polymorphic.invert) {
+        filter.collect = definition.polymorphic.as;
         filter.include = filter.collect;
-    } else if (params.through) {
+      } else {
-      filter.collect = i8n.camelize(modelTo.modelName, true);
+        filter.collect = throughRelationName || i8n.camelize(modelTo.modelName, true);
         filter.include = filter.collect;
       }
+    }
 
     return filter;
   }, scopeMethods, definition.options);

test/relations.test.js

@@ -481,6 +481,158 @@ describe('relations', function () {
 
   });
 
+  describe('hasMany through - collect', function () {
+    var Physician, Patient, Appointment, Address;
+
+    beforeEach(function (done) {
+      db = getSchema();
+      Physician = db.define('Physician', {name: String});
+      Patient = db.define('Patient', {name: String});
+      Appointment = db.define('Appointment', {date: {type: Date,
+        default: function () {
+          return new Date();
+        }}});
+      Address = db.define('Address', {name: String});
+
+      db.automigrate(['Physician', 'Patient', 'Appointment', 'Address'], function (err) {
+        done(err);
+      });
+    });
+
+    describe('with default options', function () {
+      it('can determine the collect by modelTo\'s name as default', function () {
+        Physician.hasMany(Patient, {through: Appointment});
+        Patient.hasMany(Physician, {through: Appointment, as: 'yyy'});
+        Patient.belongsTo(Address);
+        Appointment.belongsTo(Physician);
+        Appointment.belongsTo(Patient);
+        var physician = new Physician({id: 1});
+        var scope1 = physician.patients._scope;
+        scope1.should.have.property('collect', 'patient');
+        scope1.should.have.property('include', 'patient');
+        var patient = new Patient({id: 1});
+        var scope2 = patient.yyy._scope;
+        scope2.should.have.property('collect', 'physician');
+        scope2.should.have.property('include', 'physician');
+      });
+    });
+
+    describe('when custom reverse belongsTo names for both sides', function () {
+      it('can determine the collect via keyThrough', function () {
+        Physician.hasMany(Patient, {through: Appointment, foreignKey: 'fooId', keyThrough: 'barId'});
+        Patient.hasMany(Physician, {through: Appointment, foreignKey: 'barId', keyThrough: 'fooId', as: 'yyy'});
+        Appointment.belongsTo(Physician, {as: 'foo'});
+        Appointment.belongsTo(Patient, {as: 'bar'});
+        Patient.belongsTo(Address); // jam.
+        Appointment.belongsTo(Patient, {as: 'car'}); // jam. Should we complain in this case???
+
+        var physician = new Physician({id: 1});
+        var scope1 = physician.patients._scope;
+        scope1.should.have.property('collect', 'bar');
+        scope1.should.have.property('include', 'bar');
+        var patient = new Patient({id: 1});
+        var scope2 = patient.yyy._scope;
+        scope2.should.have.property('collect', 'foo');
+        scope2.should.have.property('include', 'foo');
+      });
+
+      it('can determine the collect via modelTo name', function () {
+        Physician.hasMany(Patient, {through: Appointment});
+        Patient.hasMany(Physician, {through: Appointment, as: 'yyy'});
+        Appointment.belongsTo(Physician, {as: 'foo', foreignKey: 'physicianId'});
+        Appointment.belongsTo(Patient, {as: 'bar', foreignKey: 'patientId'});
+        Patient.belongsTo(Address); // jam.
+
+        var physician = new Physician({id: 1});
+        var scope1 = physician.patients._scope;
+        scope1.should.have.property('collect', 'bar');
+        scope1.should.have.property('include', 'bar');
+        var patient = new Patient({id: 1});
+        var scope2 = patient.yyy._scope;
+        scope2.should.have.property('collect', 'foo');
+        scope2.should.have.property('include', 'foo');
+      });
+
+      it('can determine the collect via modelTo name (with jams)', function () {
+        Physician.hasMany(Patient, {through: Appointment});
+        Patient.hasMany(Physician, {through: Appointment, as: 'yyy'});
+        Appointment.belongsTo(Physician, {as: 'foo', foreignKey: 'physicianId'});
+        Appointment.belongsTo(Patient, {as: 'bar', foreignKey: 'patientId'});
+        Patient.belongsTo(Address); // jam.
+        Appointment.belongsTo(Physician, {as: 'goo', foreignKey: 'physicianId'}); // jam. Should we complain in this case???
+        Appointment.belongsTo(Patient, {as: 'car', foreignKey: 'patientId'}); // jam. Should we complain in this case???
+
+        var physician = new Physician({id: 1});
+        var scope1 = physician.patients._scope;
+        scope1.should.have.property('collect', 'bar');
+        scope1.should.have.property('include', 'bar');
+        var patient = new Patient({id: 1});
+        var scope2 = patient.yyy._scope;
+        scope2.should.have.property('collect', 'foo'); // first matched relation
+        scope2.should.have.property('include', 'foo'); // first matched relation
+      });
+    });
+
+    describe('when custom reverse belongsTo name for one side only', function () {
+
+      beforeEach(function () {
+        Physician.hasMany(Patient, {as: 'xxx', through: Appointment, foreignKey: 'fooId'});
+        Patient.hasMany(Physician, {as: 'yyy', through: Appointment, keyThrough: 'fooId'});
+        Appointment.belongsTo(Physician, {as: 'foo'});
+        Appointment.belongsTo(Patient);
+        Patient.belongsTo(Address); // jam.
+        Appointment.belongsTo(Physician, {as: 'bar'}); // jam. Should we complain in this case???
+      });
+
+      it('can determine the collect via model name', function () {
+        var physician = new Physician({id: 1});
+        var scope1 = physician.xxx._scope;
+        scope1.should.have.property('collect', 'patient');
+        scope1.should.have.property('include', 'patient');
+      });
+
+      it('can determine the collect via keyThrough', function () {
+        var patient = new Patient({id: 1});
+        var scope2 = patient.yyy._scope;
+        scope2.should.have.property('collect', 'foo');
+        scope2.should.have.property('include', 'foo');
+      });
+    });
+  });
+
+  describe('hasMany through - between same model', function () {
+    var User, Follow, Address;
+
+    before(function (done) {
+      db = getSchema();
+      User = db.define('User', {name: String});
+      Follow = db.define('Follow', {date: {type: Date,
+        default: function () {
+          return new Date();
+        }}});
+      Address = db.define('Address', {name: String});
+
+      User.hasMany(User, {as: 'followers', foreignKey: 'followeeId', keyThrough: 'followerId', through: Follow});
+      User.hasMany(User, {as: 'following', foreignKey: 'followerId', keyThrough: 'followeeId', through: Follow});
+      User.belongsTo(Address);
+      Follow.belongsTo(User, {as: 'follower'});
+      Follow.belongsTo(User, {as: 'followee'});
+      db.automigrate(['User', 'Follow', 'Address'], function (err) {
+        done(err);
+      });
+    });
+
+    it('can determine the collect via keyThrough for each side', function () {
+      var user = new User({id: 1});
+      var scope1 = user.followers._scope;
+      scope1.should.have.property('collect', 'follower');
+      scope1.should.have.property('include', 'follower');
+      var scope2 = user.following._scope;
+      scope2.should.have.property('collect', 'followee');
+      scope2.should.have.property('include', 'followee');
+    });
+  });
+
   describe('hasMany with properties', function () {
     it('can be declared with properties', function (done) {
       Book.hasMany(Chapter, { properties: { type: 'bookType' } });
@@ -933,6 +1085,29 @@ describe('relations', function () {
       db.automigrate(done);
     });
 
+    it('can determine the collect via modelTo name', function () {
+      Author.hasAndBelongsToMany(Picture, { through: PictureLink, polymorphic: 'imageable' });
+      Reader.hasAndBelongsToMany(Picture, { through: PictureLink, polymorphic: 'imageable' });
+      // Optionally, define inverse relations:
+      Picture.hasMany(Author, { through: PictureLink, polymorphic: 'imageable', invert: true });
+      Picture.hasMany(Reader, { through: PictureLink, polymorphic: 'imageable', invert: true });
+      var author = new Author({id: 1});
+      var scope1 = author.pictures._scope;
+      scope1.should.have.property('collect', 'picture');
+      scope1.should.have.property('include', 'picture');
+      var reader = new Reader({id: 1});
+      var scope2 = reader.pictures._scope;
+      scope2.should.have.property('collect', 'picture');
+      scope2.should.have.property('include', 'picture');
+      var picture = new Picture({id: 1});
+      var scope3 = picture.authors._scope;
+      scope3.should.have.property('collect', 'imageable');
+      scope3.should.have.property('include', 'imageable');
+      var scope4 = picture.readers._scope;
+      scope4.should.have.property('collect', 'imageable');
+      scope4.should.have.property('include', 'imageable');
+    });
+
     var author, reader, pictures = [];
     it('should create polymorphic relation - author', function (done) {
       Author.create({ name: 'Author 1' }, function (err, a) {