Feat 4597 : Pass through values using options.through for N:M relationships (#6931)

* feat: set/add/create through model with options.through

* docs: options.through for set/add/create N:M

* [ci skip] changelog

changelog.md

 # Future
+- [FIXED] N:M `through` option naming collisions [#4597](https://github.com/sequelize/sequelize/issues/4597)
+ [#6444](https://github.com/sequelize/sequelize/issues/6444)
 - [CHANGED] Updated deprecated `node-uuid` package to `uuid` [#6919](https://github.com/sequelize/sequelize/pull/6919)
 - [ADDED] UPSERT Support for MSSQL [#6842](https://github.com/sequelize/sequelize/pull/6842)
 - [FIXED] Execute queries parallel in findAndCount [#6695](https://github.com/sequelize/sequelize/issues/6695)
@@ -20,6 +22,7 @@
 
 ## BC breaks:
 - `DATEONLY` now returns string in `YYYY-MM-DD` format rather than `Date` type
+- With `BelongsToMany` relationships `add/set/create` setters now set `through` attributes by passing them as `options.through` (previously second argument was used as `through` attributes, now its considered `options` with `through` being a sub option)
 
 # 4.0.0-2
 - [ADDED] include now supports string as an argument (on top of model/association), string will expand into an association matched literally from Model.associations

docs/api/associations/belongs-to-many.md

@@ -195,4 +195,4 @@ Count everything currently associated with this, using an optional where clause.
 
 ***
 
-_This document is automatically generated based on source code comments. Please do not edit it directly, as your changes will be ignored. Please write on <a href="irc://irc.freenode.net/#sequelizejs">IRC</a>, open an issue or a create a pull request if you feel something can be improved. For help on how to write source code documentation see [JSDoc](http://usejsdoc.org) and [dox](https://github.com/tj/dox)_
\ No newline at end of file
+_This document is automatically generated based on source code comments. Please do not edit it directly, as your changes will be ignored. Please write on <a href="irc://irc.freenode.net/#sequelizejs">IRC</a>, open an issue or a create a pull request if you feel something can be improved. For help on how to write source code documentation see [JSDoc](http://usejsdoc.org) and [dox](https://github.com/tj/dox)_

docs/docs/associations.md

@@ -240,10 +240,10 @@ User.belongsToMany(Project, { through: UserProjects })
 Project.belongsToMany(User, { through: UserProjects })
 ```
 
-To add a new project to a user and set its status, you pass an extra object to the setter, which contains the attributes for the join table
+To add a new project to a user and set its status, you pass extra `options.through` to the setter, which contains the attributes for the join table
 
 ```js
-user.addProject(project, { status: 'started' })
+user.addProject(project, { through: { status: 'started' }})
 ```
 
 By default the code above will add projectId and userId to the UserProjects table, and _remove any previously defined primary key attribute_ - the table will be uniquely identified by the combination of the keys of the two tables, and there is no reason to have other PK columns. To enforce a primary key on the `UserProjects` model you can add it manually.
@@ -542,8 +542,8 @@ project.UserProjects = {
 }
 u.addProject(project)
  
-// Or by providing a second argument when adding the association, containing the data that should go in the join table
-u.addProject(project, { status: 'active' })
+// Or by providing a second options.through argument when adding the association, containing the data that should go in the join table
+u.addProject(project, { through: { status: 'active' }})
  
  
 // When associating multiple objects, you can combine the two options above. In this case the second argument
@@ -552,7 +552,7 @@ project1.UserProjects = {
     status: 'inactive'
 }
  
-u.setProjects([project1, project2], { status: 'active' })
+u.setProjects([project1, project2], { through: { status: 'active' }})
 // The code above will record inactive for project one, and active for project two in the join table
 ```
 
@@ -770,9 +770,9 @@ return Product.create({
     }]
   }
 }, {
-  include: [{ 
-    association: Product.User, 
-    include: [ User.Addresses ] 
+  include: [{
+    association: Product.User,
+    include: [ User.Addresses ]
   }]
 });
 ```

lib/associations/belongs-to-many.js

@@ -21,7 +21,7 @@ const HasOne = require('./has-one');
  * Project.belongsToMany(User, { through: UserProject });
  * // through is required!
  *
- * user.addProject(project, { role: 'manager', transaction: t });
+ * user.addProject(project, { through: { role: 'manager', transaction: t }});
  * ```
  *
  * All methods allow you to pass either a persisted instance, its primary key, or a mixture:
@@ -217,8 +217,9 @@ class BelongsToMany extends Association {
        * Set the associated models by passing an array of instances or their primary keys. Everything that it not in the passed array will be un-associated.
        *
        * @param {Array<Model|String|Number>} [newAssociations] An array of persisted instances or primary key of instances to associate with this. Pass `null` or `undefined` to remove all associations.
-       * @param {Object} [options] Options passed to `through.findAll`, `bulkCreate`, `update` and `destroy`. Can also hold additional attributes for the join table
+       * @param {Object} [options] Options passed to `through.findAll`, `bulkCreate`, `update` and `destroy`
        * @param {Object} [options.validate] Run validation for the join model
+       * @param {Object} [options.through] Additional attributes for the join table.
        * @return {Promise}
        * @method setAssociations
        * @memberof Associations.BelongsToMany
@@ -229,8 +230,9 @@ class BelongsToMany extends Association {
        * Associate several persisted instances with this.
        *
        * @param {Array<Model|String|Number>} [newAssociations] An array of persisted instances or primary key of instances to associate with this.
-       * @param {Object} [options] Options passed to `through.findAll`, `bulkCreate` and `update`. Can also hold additional attributes for the join table.
+       * @param {Object} [options] Options passed to `through.findAll`, `bulkCreate` and `update`
        * @param {Object} [options.validate] Run validation for the join model.
+       * @param {Object} [options.through] Additional attributes for the join table.
        * @return {Promise}
        * @method addAssociations
        * @memberof Associations.BelongsToMany
@@ -241,8 +243,9 @@ class BelongsToMany extends Association {
        * Associate a persisted instance with this.
        *
        * @param {Model|String|Number} [newAssociation] A persisted instance or primary key of instance to associate with this.
-       * @param {Object} [options] Options passed to `through.findAll`, `bulkCreate` and `update`. Can also hold additional attributes for the join table.
+       * @param {Object} [options] Options passed to `through.findAll`, `bulkCreate` and `update`
        * @param {Object} [options.validate] Run validation for the join model.
+       * @param {Object} [options.through] Additional attributes for the join table.
        * @return {Promise}
        * @method addAssociation
        * @memberof Associations.BelongsToMany
@@ -253,7 +256,8 @@ class BelongsToMany extends Association {
        * Create a new instance of the associated model and associate it with this.
        *
        * @param {Object} [values]
-       * @param {Object} [options] Options passed to create and add. Can also hold additional attributes for the join table
+       * @param {Object} [options] Options passed to create and add
+       * @param {Object} [options.through] Additional attributes for the join table
        * @return {Promise}
        * @method createAssociation
        * @memberof Associations.BelongsToMany
@@ -585,7 +589,7 @@ class BelongsToMany extends Association {
     return association.through.model.findAll(_.defaults({where, raw: true}, options)).then(currentRows => {
       const obsoleteAssociations = [];
       const promises = [];
-      let defaultAttributes = options;
+      let defaultAttributes = options.through || {};
 
       // Don't try to insert the transaction as an attribute in the through table
       defaultAttributes = _.omit(defaultAttributes, ['transaction', 'hooks', 'individualHooks', 'ignoreDuplicates', 'validate', 'fields', 'logging']);
@@ -647,19 +651,18 @@ class BelongsToMany extends Association {
     });
   }
 
-  add(sourceInstance, newInstances, additionalAttributes) {
+  add(sourceInstance, newInstances, options) {
     // If newInstances is null or undefined, no-op
     if (!newInstances) return Utils.Promise.resolve();
 
-    additionalAttributes = _.clone(additionalAttributes) || {};
+    options = _.clone(options) || {};
 
     const association = this;
-    const defaultAttributes = _.omit(additionalAttributes, ['transaction', 'hooks', 'individualHooks', 'ignoreDuplicates', 'validate', 'fields', 'logging']);
     const sourceKey = association.source.primaryKeyAttribute;
     const targetKey = association.target.primaryKeyAttribute;
     const identifier = association.identifier;
     const foreignIdentifier = association.foreignIdentifier;
-    const options = additionalAttributes;
+    const defaultAttributes = _.omit(options.through || {}, ['transaction', 'hooks', 'individualHooks', 'ignoreDuplicates', 'validate', 'fields', 'logging']);
 
     newInstances = association.toInstanceArray(newInstances);
 

sscce_template.js

+'use strict';
+
 /*
  * Copy this file to ./sscce.js
  * Add code from issue
  * npm run sscce-{dialect}
  */
 
-var Sequelize = require('./index');
-var sequelize = require('./test/support').createSequelizeInstance();
+const Sequelize = require('./index');
+const sequelize = require('./test/support').createSequelizeInstance();

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

@@ -208,7 +208,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
         this.u = u;
         return AcmeProject.create();
       }).then(function(p) {
-        return this.u.addProject(p, { status: 'active', data: 42 });
+        return this.u.addProject(p, { through: { status: 'active', data: 42 }});
       }).then(function() {
         return this.u.getProjects();
       }).then(function(projects) {
@@ -489,12 +489,12 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
       return Promise.join(
         this.Task.create().then(function (task) {
           return user.addTask(task, {
-            started: true
+            through: { started: true }
           });
         }),
         this.Task.create().then(function (task) {
           return user.addTask(task, {
-            started: true
+            through: { started: true }
           });
         })
       ).then(function () {
@@ -687,8 +687,8 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
         return Group.create({});
       }).then(function(group) {
         return Promise.join(
-          group.createUser({ id: 1 }, { isAdmin: true }),
-          group.createUser({ id: 2 }, { isAdmin: false }),
+          group.createUser({ id: 1 }, { through: {isAdmin: true }}),
+          group.createUser({ id: 2 }, { through: {isAdmin: false }}),
           function() {
             return UserGroups.findAll();
           }
@@ -810,9 +810,9 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
           this.task = task;
           this.user = user;
           this.t = t;
-          return task.addUser(user, { status: 'pending' }); // Create without transaction, so the old value is accesible from outside the transaction
+          return task.addUser(user, { through: {status: 'pending'} }); // Create without transaction, so the old value is accesible from outside the transaction
         }).then(function() {
-          return this.task.addUser(this.user, { transaction: this.t, status: 'completed' }); // Add an already exisiting user in a transaction, updating a value in the join table
+          return this.task.addUser(this.user, { transaction: this.t, through: {status: 'completed'}}); // Add an already exisiting user in a transaction, updating a value in the join table
         }).then(function() {
           return Promise.all([
             this.user.getTasks(),
@@ -991,15 +991,15 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
     });
 
     it('runs on add', function () {
-      return expect(this.project.addParticipant(this.employee, { role: ''})).to.be.rejected;
+      return expect(this.project.addParticipant(this.employee, { through: {role: ''}})).to.be.rejected;
     });
 
     it('runs on set', function () {
-      return expect(this.project.setParticipants([this.employee], { role: ''})).to.be.rejected;
+      return expect(this.project.setParticipants([this.employee], { through: {role: ''}})).to.be.rejected;
     });
 
     it('runs on create', function () {
-      return expect(this.project.createParticipant({ name: 'employee 2'}, { role: ''})).to.be.rejected;
+      return expect(this.project.createParticipant({ name: 'employee 2'}, { through: {role: ''}})).to.be.rejected;
     });
   });
 
@@ -1453,7 +1453,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
           this.User.create(),
           this.Project.create()
         ]).spread(function(user, project) {
-          return user.addProject(project, { status: 'active', data: 42 }).return (user);
+          return user.addProject(project, { through: { status: 'active', data: 42 }}).return (user);
         }).then(function(user) {
           return user.getProjects();
         }).then(function(projects) {
@@ -1471,7 +1471,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
           this.User.create(),
           this.Project.create()
         ]).spread(function(user, project) {
-          return user.addProject(project, { status: 'active', data: 42 }).return (user);
+          return user.addProject(project, { through: { status: 'active', data: 42 }}).return (user);
         }).then(function(user) {
           return user.getProjects({ joinTableAttributes: ['status']});
         }).then(function(projects) {
@@ -1512,7 +1512,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
             this.u = u;
             this.p = p;
 
-            return u.addProject(p, { status: 'active' });
+            return u.addProject(p, { through: { status: 'active' }});
           }).then(function() {
             return this.UserProjects.findOne({ where: { UserId: this.u.id, ProjectId: this.p.id }});
           }).then(function(up) {
@@ -1599,7 +1599,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
 
             this.p1.UserProjects = { status: 'inactive' };
 
-            return user.setProjects([this.p1, this.p2], { status: 'active' });
+            return user.setProjects([this.p1, this.p2], { through: { status: 'active' }});
           }).then(function() {
             return Promise.all([
               self.UserProjects.findOne({ where: { UserId: this.user.id, ProjectId: this.p1.id }}),
@@ -1638,9 +1638,9 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
         it('should support query the through model', function () {
           return this.User.create().then(function (user) {
             return Promise.all([
-              user.createProject({}, { status: 'active', data: 1 }),
-              user.createProject({}, { status: 'inactive', data: 2 }),
-              user.createProject({}, { status: 'inactive', data: 3 })
+              user.createProject({}, { through: { status: 'active', data: 1 }}),
+              user.createProject({}, { through: { status: 'inactive', data: 2 }}),
+              user.createProject({}, { through: { status: 'inactive', data: 3 }})
             ]).then(function () {
               return Promise.all([
                 user.getProjects({ through: { where: { status: 'active' } } }),

test/integration/include/find.test.js

@@ -157,7 +157,7 @@ describe(Support.getTestDialectTeaser('Include'), function() {
           );
         })
         .spread(function(a, b) {
-          return a.addB(b, {name : 'Foobar'});
+          return a.addB(b, { through: {name : 'Foobar'}});
         })
         .then(function() {
           return A.find({

test/integration/include/findAll.test.js

@@ -770,12 +770,12 @@ describe(Support.getTestDialectTeaser('Include'), function() {
           })
         }).then(function (results) {
           return Promise.join(
-            results.products[0].addTag(results.tags[0], {priority: 1}),
-            results.products[0].addTag(results.tags[1], {priority: 2}),
-            results.products[1].addTag(results.tags[1], {priority: 1}),
-            results.products[2].addTag(results.tags[0], {priority: 3}),
-            results.products[2].addTag(results.tags[1], {priority: 1}),
-            results.products[2].addTag(results.tags[2], {priority: 2})
+            results.products[0].addTag(results.tags[0], { through: {priority: 1}}),
+            results.products[0].addTag(results.tags[1], { through: {priority: 2}}),
+            results.products[1].addTag(results.tags[1], { through: {priority: 1}}),
+            results.products[2].addTag(results.tags[0], { through: {priority: 3}}),
+            results.products[2].addTag(results.tags[1], { through: {priority: 1}}),
+            results.products[2].addTag(results.tags[2], { through: {priority: 2}})
           );
         }).then(function () {
           return Product.findAll({

test/integration/include/schema.test.js

@@ -518,12 +518,12 @@ describe(Support.getTestDialectTeaser('Includes with schemas'), function() {
           ]);
         }).spread(function(products, tags) {
           return Promise.all([
-            products[0].addTag(tags[0], {priority: 1}),
-            products[0].addTag(tags[1], {priority: 2}),
-            products[1].addTag(tags[1], {priority: 1}),
-            products[2].addTag(tags[0], {priority: 3}),
-            products[2].addTag(tags[1], {priority: 1}),
-            products[2].addTag(tags[2], {priority: 2})
+            products[0].addTag(tags[0], { through: {priority: 1}}),
+            products[0].addTag(tags[1], { through: {priority: 2}}),
+            products[1].addTag(tags[1], { through: {priority: 1}}),
+            products[2].addTag(tags[0], { through: {priority: 3}}),
+            products[2].addTag(tags[1], { through: {priority: 1}}),
+            products[2].addTag(tags[2], { through: {priority: 2}})
           ]);
         }).spread(function() {
           return Product.findAll({

test/integration/model/attributes.test.js

@@ -51,7 +51,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
             self.Student.create({no: 1, name: 'ryan'}),
             self.Course.create({no: 100, name: 'history'})
           ).spread(function(student, course) {
-            return student.addCourse(course, {score: 98, test_value: 1000});
+            return student.addCourse(course, { through: {score: 98, test_value: 1000}});
           }).then(function() {
             expect(self.callCount).to.equal(1);
             return self.Score.find({ where: { StudentId: 1, CourseId: 100 } }).then(function(score) {

test/integration/model/findAll.test.js

@@ -1121,10 +1121,10 @@ describe(Support.getTestDialectTeaser('Model'), function() {
               });
 
               return self.sequelize.Promise.all([
-                self.england.addIndustry(self.energy, {numYears: 20}),
-                self.england.addIndustry(self.media, {numYears: 40}),
-                self.france.addIndustry(self.media, {numYears: 80}),
-                self.korea.addIndustry(self.tech, {numYears: 30})
+                self.england.addIndustry(self.energy, { through: { numYears: 20 }}),
+                self.england.addIndustry(self.media, { through: { numYears: 40 }}),
+                self.france.addIndustry(self.media, { through: { numYears: 80 }}),
+                self.korea.addIndustry(self.tech, { through: { numYears: 30 }})
               ]);
             });
           });