Make hasmanydouble add and addMultiple a single function that accepts both insta…

…nces and arrays. Closes #2138

changelog.md

@@ -6,6 +6,7 @@ Notice: All 1.7.x changes are present in 2.0.x aswell
 - [FEATURE] Added support for passing an `indexes` array in options to `sequelize.define`. [#1485](https://github.com/sequelize/sequelize/issues/1485). See API reference for details.
 - [FEATURE/INTERNALS] Standardized the output from `QueryInterface.showIndex`.
 - [FEATURE] Include deleted rows in find [#2083](https://github.com/sequelize/sequelize/pull/2083)
+- [FEATURE] Make addSingular and addPlural for n:m assocations (fx `addUser` and `addUsers` now both accept an array or an instance.
 - [BUG] Hid `dottie.transform` on raw queries behind a flag (`nest`) [#2064](https://github.com/sequelize/sequelize/pull/2064)
 - [BUG] Fixed problems with transcation parameter being removed / not passed on in associations [#1789](https://github.com/sequelize/sequelize/issues/1789) and [#1968](https://github.com/sequelize/sequelize/issues/1968)
 - [BUG] Fix problem with minConnections. [#2048](https://github.com/sequelize/sequelize/issues/2048)

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

@@ -69,6 +69,8 @@ module.exports = (function() {
   };
 
   HasManyDoubleLinked.prototype.injectSetter = function(oldAssociations, newAssociations, defaultAttributes) {
+    defaultAttributes = defaultAttributes || {};
+
     var self = this
       , targetAssociation = self.association.targetAssociation
       , foreignIdentifier = self.association.foreignIdentifier
@@ -76,15 +78,11 @@ module.exports = (function() {
       , targetKeys = Object.keys(self.association.target.primaryKeys)
       , obsoleteAssociations = []
       , changedAssociations = []
-      , options = {}
+      , options = defaultAttributes
       , promises = []
       , unassociatedObjects;
 
-    if ((defaultAttributes || {}).transaction instanceof Transaction) {
-      options.transaction = defaultAttributes.transaction;
-
-      defaultAttributes = Utils._.omit(defaultAttributes, 'transaction'); // Don't try to insert the transaction as an attribute in the through table
-    }
+    defaultAttributes = Utils._.omit(defaultAttributes, ['transaction', 'hooks', 'individualHooks', 'ignoreDuplicates', 'validate', 'fields']); // Don't try to insert the transaction as an attribute in the through table
 
     unassociatedObjects = newAssociations.filter(function(obj) {
       return !Utils._.find(oldAssociations, function(old) {
@@ -160,18 +158,17 @@ module.exports = (function() {
   };
 
   HasManyDoubleLinked.prototype.injectAdder = function(newAssociation, additionalAttributes, exists) {
+    additionalAttributes = additionalAttributes || {};
+
     var attributes = {}
       , targetAssociation = this.association.targetAssociation
       , foreignIdentifier = targetAssociation.identifier
-      , options = {};
+      , options = additionalAttributes;
 
     var sourceKeys = Object.keys(this.association.source.primaryKeys);
     var targetKeys = Object.keys(this.association.target.primaryKeys);
 
-    if ((additionalAttributes || {}).transaction instanceof Transaction) {
-      options.transaction = additionalAttributes.transaction;
-      delete additionalAttributes.transaction;
-    }
+    additionalAttributes = Utils._.omit(additionalAttributes, ['transaction', 'hooks', 'individualHooks', 'ignoreDuplicates', 'validate', 'fields']); // Don't try to insert the transaction as an attribute in the through table
 
     attributes[this.association.identifier] = ((sourceKeys.length === 1) ? this.instance[sourceKeys[0]] : this.instance.id);
     attributes[foreignIdentifier] = ((targetKeys.length === 1) ? newAssociation[targetKeys[0]] : newAssociation.id);

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

@@ -103,12 +103,7 @@ module.exports = (function() {
   HasManySingleLinked.prototype.injectAdder = function(newAssociation, additionalAttributes) {
     var primaryKeys = Object.keys(this.instance.Model.primaryKeys)
       , primaryKey = primaryKeys.length === 1 ? primaryKeys[0] : 'id'
-      , options = {};
-
-    if ((additionalAttributes || {}).transaction instanceof Transaction) {
-      options.transaction = additionalAttributes.transaction;
-      delete additionalAttributes.transaction;
-    }
+      , options = additionalAttributes;
 
     newAssociation[this.__factory.identifier] = this.instance[primaryKey];
 

lib/associations/has-many.js

@@ -374,7 +374,7 @@ module.exports = (function() {
 
       var instance = this;
 
-      return instance[association.accessors.get]({
+      return instance[association.accessors.get]({}, {
         transaction: (additionalAttributes || {}).transaction
       }).then(function(oldAssociatedObjects) {
         var Class = Object(association.through) === association.through ? HasManyDoubleLinked : HasManySingleLinked;
@@ -382,26 +382,39 @@ module.exports = (function() {
       });
     };
 
-    obj[this.accessors.add] = function(newInstance, additionalAttributes) {
+    obj[this.accessors.addMultiple] = obj[this.accessors.add] = function(newInstance, additionalAttributes) {
       // If newInstance is null or undefined, no-op
       if (!newInstance) return Utils.Promise.resolve();
 
       var instance = this
         , primaryKeyAttribute = association.target.primaryKeyAttribute;
 
-      if (!(newInstance instanceof association.target.Instance)) {
-        var tmpInstance = {};
-        tmpInstance[primaryKeyAttribute] = newInstance;
-        newInstance = association.target.build(tmpInstance, {
-          isNewRecord: false
+      if (Array.isArray(newInstance)) {
+        var newInstances = newInstance.map(function(newInstance) {
+          if (!(newInstance instanceof association.target.Instance)) {
+            var tmpInstance = {};
+            tmpInstance[primaryKeyAttribute] = newInstance;
+            return association.target.build(tmpInstance, {
+              isNewRecord: false
+            });
+          }
+          return newInstance;
         });
-      }
 
-      if (Array.isArray(newInstance)) {
-        return obj[association.accessors.addMultiple](newInstance, additionalAttributes);
+        var Class = Object(association.through) === association.through ? HasManyDoubleLinked : HasManySingleLinked;
+        return new Class(association, this).injectSetter([], newInstances, additionalAttributes);
       } else {
+        if (!(newInstance instanceof association.target.Instance)) {
+          var tmpInstance = {};
+          tmpInstance[primaryKeyAttribute] = newInstance;
+          newInstance = association.target.build(tmpInstance, {
+            isNewRecord: false
+          });
+        }
+
         return instance[association.accessors.get]({
-          where: newInstance.primaryKeyValues,
+          where: newInstance.primaryKeyValues
+        }, {
           transaction: (additionalAttributes || {}).transaction
         }).then(function(currentAssociatedObjects) {
           if (currentAssociatedObjects.length === 0 || Object(association.through) === association.through) {
@@ -414,30 +427,9 @@ module.exports = (function() {
       }
     };
 
-    obj[this.accessors.addMultiple] = function(newInstances, additionalAttributes) {
-      var primaryKeyAttribute = association.target.primaryKeyAttribute;
-
-      newInstances = newInstances.map(function(newInstance) {
-        if (!(newInstance instanceof association.target.Instance)) {
-          var tmpInstance = {};
-          tmpInstance[primaryKeyAttribute] = newInstance;
-          return association.target.build(tmpInstance, {
-            isNewRecord: false
-          });
-        }
-        return newInstance;
-      });
-
-      var Class = Object(association.through) === association.through ? HasManyDoubleLinked : HasManySingleLinked;
-      return new Class(association, this).injectSetter([], newInstances, additionalAttributes);
-    };
-
-
     obj[this.accessors.remove] = function(oldAssociatedObject, options) {
       var instance = this;
-      return instance[association.accessors.get]({
-        transaction: (options || {}).transaction
-      }).then(function(currentAssociatedObjects) {
+      return instance[association.accessors.get]({}, options).then(function(currentAssociatedObjects) {
         var newAssociations = [];
 
         currentAssociatedObjects.forEach(function(association) {
@@ -446,7 +438,7 @@ module.exports = (function() {
           }
         });
 
-        return instance[association.accessors.set](newAssociations);
+        return instance[association.accessors.set](newAssociations, options);
       });
     };
 

test/associations/has-many.test.js

@@ -1086,6 +1086,33 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
     });
 
     describe('addAssociations', function() {
+      it('supports both single instance and array', function () {
+         var User = this.sequelize.define('User', { username: DataTypes.STRING })
+          , Task = this.sequelize.define('Task', { title: DataTypes.STRING });
+
+        User.hasMany(Task);
+        Task.hasMany(User);
+
+        return this.sequelize.sync({ force: true }).then(function () {
+          return Promise.all([
+            User.create({ id: 12 }),
+            Task.create({ id: 50, title: 'get started' }),
+            Task.create({ id: 52, title: 'get done' }),
+          ]);
+        }).spread(function (user, task1, task2) {
+          return Promise.all([
+            user.addTask(task1),
+            user.addTask([task2]),
+          ]).return(user);
+        }).then(function (user) {
+          return user.getTasks();
+        }).then(function (tasks) {
+          expect(tasks).to.have.length(2);
+          expect(_.find(tasks, function (item) { return item.title === 'get started'; })).to.be.ok;
+          expect(_.find(tasks, function (item) { return item.title === 'get done'; })).to.be.ok;
+        });
+      });
+
       it('supports transactions', function() {
         return Support.prepareTransactionTest(this.sequelize).bind({}).then(function(sequelize) {
           this.User = sequelize.define('User', { username: DataTypes.STRING });
@@ -1181,6 +1208,33 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
     });
 
     describe('addMultipleAssociations', function () {
+      it('supports both single instance and array', function () {
+         var User = this.sequelize.define('User', { username: DataTypes.STRING })
+          , Task = this.sequelize.define('Task', { title: DataTypes.STRING });
+
+        User.hasMany(Task);
+        Task.hasMany(User);
+
+        return this.sequelize.sync({ force: true }).then(function () {
+          return Promise.all([
+            User.create({ id: 12 }),
+            Task.create({ id: 50, title: 'get started' }),
+            Task.create({ id: 52, title: 'get done' }),
+          ]);
+        }).spread(function (user, task1, task2) {
+          return Promise.all([
+            user.addTasks(task1),
+            user.addTasks([task2]),
+          ]).return(user);
+        }).then(function (user) {
+          return user.getTasks();
+        }).then(function (tasks) {
+          expect(tasks).to.have.length(2);
+          expect(_.find(tasks, function (item) { return item.title === 'get started'; })).to.be.ok;
+          expect(_.find(tasks, function (item) { return item.title === 'get done'; })).to.be.ok;
+        });
+      });
+
       it('adds associations without removing the current ones', function () {
         var User = this.sequelize.define('User', { username: DataTypes.STRING })
           , Task = this.sequelize.define('Task', { title: DataTypes.STRING });