Fixes som setter/adder bugs introduced with assumed through models

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

@@ -129,8 +129,10 @@ module.exports = (function() {
         changedAssociation.where[self.association.identifier] = self.instance[self.association.identifier] || self.instance.id
         changedAssociation.where[self.association.identifier] = self.instance[self.association.identifier] || self.instance.id
         changedAssociation.where[foreignIdentifier] = newObj[foreignIdentifier] || newObj.id
         changedAssociation.where[foreignIdentifier] = newObj[foreignIdentifier] || newObj.id
 
 
+        if (Object.keys(changedAssociation.attributes).length) {
           changedAssociations.push(changedAssociation)
           changedAssociations.push(changedAssociation)
         }
         }
+      }
     })
     })
 
 
     if (obsoleteAssociations.length > 0) {
     if (obsoleteAssociations.length > 0) {
@@ -187,12 +189,16 @@ module.exports = (function() {
     attributes[this.association.identifier] = ((sourceKeys.length === 1) ? this.instance[sourceKeys[0]] : this.instance.id)
     attributes[this.association.identifier] = ((sourceKeys.length === 1) ? this.instance[sourceKeys[0]] : this.instance.id)
     attributes[foreignIdentifier] = ((targetKeys.length === 1) ? newAssociation[targetKeys[0]] : newAssociation.id)
     attributes[foreignIdentifier] = ((targetKeys.length === 1) ? newAssociation[targetKeys[0]] : newAssociation.id)
 
 
-    if (exists) { // implies hasJoinTableModel === true
+    if (exists) {
       var where = attributes
       var where = attributes
-      attributes = Utils._.defaults({}, newAssociation[association.through.name], additionalAttributes)
+      attributes = Utils._.defaults({}, newAssociation[targetAssociation.through.name], additionalAttributes)
 
 
+      if (Object.keys(attributes).length) {
         targetAssociation.through.update(attributes, where).proxy(emitterProxy)
         targetAssociation.through.update(attributes, where).proxy(emitterProxy)
       } else {
       } else {
+        emitterProxy.emit('success')
+      }
+    } else {
       attributes = Utils._.defaults(attributes, newAssociation[targetAssociation.through.name], additionalAttributes)
       attributes = Utils._.defaults(attributes, newAssociation[targetAssociation.through.name], additionalAttributes)
 
 
       this.association.through.create(attributes)
       this.association.through.create(attributes)

lib/associations/has-many.js

@@ -241,12 +241,7 @@ module.exports = (function() {
             var Class = self.doubleLinked ? HasManyMultiLinked : HasManySingleLinked
             var Class = self.doubleLinked ? HasManyMultiLinked : HasManySingleLinked
             new Class(self, instance).injectSetter(emitter, oldAssociatedObjects, newAssociatedObjects, defaultAttributes)
             new Class(self, instance).injectSetter(emitter, oldAssociatedObjects, newAssociatedObjects, defaultAttributes)
           })
           })
-          .error(function(err) {
-            emitter.emit('error', err)
-          })
-          .on('sql', function(sql) {
-            emitter.emit('sql', sql)
-          })
+          .proxy(emitter, {events: ['error', 'sql']})
       }).run()
       }).run()
     }
     }
 
 
@@ -260,9 +255,9 @@ module.exports = (function() {
       where[newAssociatedObject.daoFactory.tableName+'.'+primaryKey] = newAssociatedObject[primaryKey]
       where[newAssociatedObject.daoFactory.tableName+'.'+primaryKey] = newAssociatedObject[primaryKey]
       return new Utils.CustomEventEmitter(function(emitter) {
       return new Utils.CustomEventEmitter(function(emitter) {
         instance[self.accessors.get]({ where: where })
         instance[self.accessors.get]({ where: where })
-          .error(function(err){ emitter.emit('error', err)})
+          .proxy(emitter, {events: ['error', 'sql']})
           .success(function(currentAssociatedObjects) {
           .success(function(currentAssociatedObjects) {
-            if (currentAssociatedObjects.length === 0 || self.hasJoinTableModel === true) {
+            if (currentAssociatedObjects.length === 0 || Object(self.through) === self.through) {
               var Class = self.doubleLinked ? HasManyMultiLinked : HasManySingleLinked
               var Class = self.doubleLinked ? HasManyMultiLinked : HasManySingleLinked
               new Class(self, instance).injectAdder(emitter, newAssociatedObject, additionalAttributes, !!currentAssociatedObjects.length)
               new Class(self, instance).injectAdder(emitter, newAssociatedObject, additionalAttributes, !!currentAssociatedObjects.length)
             } else {
             } else {
@@ -274,7 +269,7 @@ module.exports = (function() {
 
 
     obj[this.accessors.remove] = function(oldAssociatedObject) {
     obj[this.accessors.remove] = function(oldAssociatedObject) {
       var instance = this
       var instance = this
-      var customEventEmitter = new Utils.CustomEventEmitter(function() {
+      return new Utils.CustomEventEmitter(function(emitter) {
         instance[self.accessors.get]().success(function(currentAssociatedObjects) {
         instance[self.accessors.get]().success(function(currentAssociatedObjects) {
           var newAssociations = []
           var newAssociations = []
             , oldAssociations = []
             , oldAssociations = []
@@ -302,12 +297,10 @@ module.exports = (function() {
 
 
           var run = function(err) {
           var run = function(err) {
             if (!!err) {
             if (!!err) {
-              return customEventEmitter.emit('error', err)
+              return emitter.emit('error', err)
             }
             }
 
 
-            instance[self.accessors.set](newAssociations)
-              .success(function() { customEventEmitter.emit('success', null) })
-              .error(function(err) { customEventEmitter.emit('error', err) })
+            instance[self.accessors.set](newAssociations).proxy(emitter)
           }
           }
 
 
           if (oldAssociations.length > 0) {
           if (oldAssociations.length > 0) {
@@ -317,7 +310,6 @@ module.exports = (function() {
           }
           }
         })
         })
       })
       })
-      return customEventEmitter.run()
     }
     }
 
 
     return this
     return this

test/associations/has-many.test.js

@@ -766,8 +766,6 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
       })
       })
 
 
       describe('inserting in join table', function () {
       describe('inserting in join table', function () {
-
-
         describe('add', function () {
         describe('add', function () {
           it('should insert data provided on the object into the join table', function (done) {
           it('should insert data provided on the object into the join table', function (done) {
             var self = this
             var self = this
@@ -800,6 +798,32 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
               })
               })
             })
             })
           })
           })
+
+          it('should be able to add twice (second call result in UPDATE call) without any attributes (and timestamps off) on the through model', function (done) {
+            var Worker = this.sequelize.define('Worker', {}, {timestamps: false})
+              , Task = this.sequelize.define('Task', {}, {timestamps: false})
+              , WorkerTasks = this.sequelize.define('WorkerTasks', {}, {timestamps: false})
+
+            Worker.hasMany(Task, { through: WorkerTasks })
+            Task.hasMany(Worker, { through: WorkerTasks })
+
+            this.sequelize.sync().done(function(err) { 
+              expect(err).not.to.be.ok
+              Worker.create().done(function (err, worker) {
+                expect(err).not.to.be.ok
+                Task.create().done(function (err, task) {
+                  expect(err).not.to.be.ok
+                  worker.addTask(task).done(function (err) {
+                    expect(err).not.to.be.ok
+                    worker.addTask(task).done(function (err) {
+                      expect(err).not.to.be.ok
+                      done()
+                    })
+                  })
+                })
+              })
+            })
+          })
         })
         })
 
 
         describe('set', function () {
         describe('set', function () {
@@ -832,6 +856,34 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
               })
               })
             })
             })
           })
           })
+
+          it('should be able to set twice (second call result in UPDATE calls) without any attributes (and timestamps off) on the through model', function (done) {
+            var Worker = this.sequelize.define('Worker', {}, {timestamps: false})
+              , Task = this.sequelize.define('Task', {}, {timestamps: false})
+              , WorkerTasks = this.sequelize.define('WorkerTasks', {}, {timestamps: false})
+
+            Worker.hasMany(Task, { through: WorkerTasks })
+            Task.hasMany(Worker, { through: WorkerTasks })
+
+            this.sequelize.sync().done(function(err) { 
+              expect(err).not.to.be.ok
+              Worker.create().done(function (err, worker) {
+                expect(err).not.to.be.ok
+                Task.bulkCreate([{}, {}]).done(function (err) {
+                  expect(err).not.to.be.ok
+                  Task.findAll().done(function (err, tasks) {
+                    expect(err).not.to.be.ok
+                    worker.setTasks(tasks).done(function (err) {
+                      worker.setTasks(tasks).done(function (err) {
+                        expect(err).not.to.be.ok
+                        done()
+                      })
+                    })
+                  })
+                })
+              })
+            })
+          })
         })
         })
       })
       })
     })
     })