Restore foreignKey and as behaviour - reverts PR #5957 (#6109)

sequelize · Jun 20, 2016 · 47b3f61 · 47b3f61
1 parent 4b2bc4a
commit 47b3f61
Show file tree

Hide file tree

Showing 13 changed files with 40 additions and 97 deletions.
diff --git a/changelog.md b/changelog.md
@@ -10,9 +10,11 @@
 - [ADDED] `Model.count` now allow specifying column to count on, use `options.col` [#4442](https://github.com/sequelize/sequelize/issues/4442)
 - [ADDED] `DEBUG` support [#2852](https://github.com/sequelize/sequelize/issues/2852)
 - [ADDED] Intensive connection logging [#851](https://github.com/sequelize/sequelize/issues/851)
+- [FIXED] Only `belongsTo` uses `as` to construct foreign key - revert of [#5957](https://github.com/sequelize/sequelize/pull/5957) introduced in 4.0.0-0
 
 ## BC breaks:
 - Range type bounds now default to [postgres default](https://www.postgresql.org/docs/9.5/static/rangetypes.html#RANGETYPES-CONSTRUCT) `[)` (inclusive, exclusive), previously was `()` (exclusive, exclusive)
+- Only `belongsTo` uses `as` to construct foreign key - revert of [#5957](https://github.com/sequelize/sequelize/pull/5957) introduced in 4.0.0-0
 
 # 4.0.0-0
 - [FIXED] Pass ResourceLock instead of raw connection in MSSQL disconnect handling

diff --git a/docs/docs/associations.md b/docs/docs/associations.md
@@ -189,7 +189,7 @@ var Project = sequelize.define('project', {/* ... */})
 Project.hasMany(User, {as: 'Workers'})
 ```
 
-This will add the attribute `WorkersId` or `Workers_id` to User. Instances of Project will get the accessors `getWorkers` and `setWorkers`. We could just leave it the way it is and let it be a one-way association.
+This will add the attribute `projectId` or `project_id` to User. Instances of Project will get the accessors `getWorkers` and `setWorkers`. We could just leave it the way it is and let it be a one-way association.
 But we want more! Let's define it the other way around by creating a many to many association in the next section:
 
 ## Belongs-To-Many associations

diff --git a/lib/associations/belongs-to-many.js b/lib/associations/belongs-to-many.js
@@ -112,7 +112,7 @@ class BelongsToMany extends Association {
       this.foreignKeyAttribute = {};
       this.foreignKey = this.options.foreignKey || Utils.camelizeIf(
         [
-          Utils.underscoredIf(this.options.as || this.source.options.name.singular, this.source.options.underscored),
+          Utils.underscoredIf(this.source.options.name.singular, this.source.options.underscored),
           this.source.primaryKeyAttribute
         ].join('_'),
         !this.source.options.underscored

diff --git a/lib/associations/belongs-to.js b/lib/associations/belongs-to.js
@@ -47,7 +47,7 @@ class BelongsTo extends Association {
     if (!this.foreignKey) {
       this.foreignKey = Utils.camelizeIf(
         [
-          Utils.underscoredIf(this.options.as || this.as, this.source.options.underscored),
+          Utils.underscoredIf(this.as, this.source.options.underscored),
           this.target.primaryKeyAttribute
         ].join('_'),
         !this.source.options.underscored

diff --git a/lib/associations/has-many.js b/lib/associations/has-many.js
@@ -71,7 +71,7 @@ class HasMany extends Association {
     if (!this.foreignKey) {
       this.foreignKey = Utils.camelizeIf(
         [
-          Utils.underscoredIf(this.options.as || this.source.options.name.singular, this.source.options.underscored),
+          Utils.underscoredIf(this.source.options.name.singular, this.source.options.underscored),
           this.source.primaryKeyAttribute
         ].join('_'),
         !this.source.options.underscored

diff --git a/test/integration/associations/belongs-to-many.test.js b/test/integration/associations/belongs-to-many.test.js
@@ -470,14 +470,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
         }
       });
 
-      var self = this;
-      return this.UserTask.sync({ force: true })
-      .then(function() {
-        return self.user.setActiveTasks(self.tasks);
-      })
-      .then(function() {
-        return expect(self.user.countActiveTasks({})).to.eventually.equal(1);
-      });
+      return expect(this.user.countActiveTasks({})).to.eventually.equal(1);
     });
 
     it('should count scoped through associations', function () {
@@ -493,23 +486,18 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
         }
       });
 
-      return this.UserTask.sync({ force: true})
-      .bind(this)
-      .then(function() {
-        return Promise.join(
-          this.Task.create().then(function (task) {
-            return user.addStartedTask(task, {
-              started: true
-            });
-          }),
-          this.Task.create().then(function (task) {
-            return user.addStartedTask(task, {
-              started: true
-            });
-          })
-        );
-      })
-      .then(function () {
+      return Promise.join(
+        this.Task.create().then(function (task) {
+          return user.addTask(task, {
+            started: true
+          });
+        }),
+        this.Task.create().then(function (task) {
+          return user.addTask(task, {
+            started: true
+          });
+        })
+      ).then(function () {
         return expect(user.countStartedTasks({})).to.eventually.equal(2);
       });
     });
@@ -1837,8 +1825,8 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
       Group.belongsToMany(User, { as: 'MyUsers', through: 'group_user'});
 
       expect(Group.associations.MyUsers.through.model === User.associations.MyGroups.through.model);
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyGroupsId).to.exist;
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyUsersId).to.exist;
+      expect(Group.associations.MyUsers.through.model.rawAttributes.UserId).to.exist;
+      expect(Group.associations.MyUsers.through.model.rawAttributes.GroupId).to.exist;
     });
 
     it('correctly identifies its counterpart when through is a model', function() {
@@ -1851,8 +1839,8 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
 
       expect(Group.associations.MyUsers.through.model === User.associations.MyGroups.through.model);
 
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyGroupsId).to.exist;
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyUsersId).to.exist;
+      expect(Group.associations.MyUsers.through.model.rawAttributes.UserId).to.exist;
+      expect(Group.associations.MyUsers.through.model.rawAttributes.GroupId).to.exist;
     });
   });
 
@@ -2158,7 +2146,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
 
       Children = Person.belongsToMany(Person, { as: 'Children', through: PersonChildren});
 
-      expect(Children.foreignKey).to.equal('ChildrenId');
+      expect(Children.foreignKey).to.equal('PersonId');
       expect(Children.otherKey).to.equal('ChildId');
       expect(PersonChildren.rawAttributes[Children.foreignKey]).to.be.ok;
       expect(PersonChildren.rawAttributes[Children.otherKey]).to.be.ok;
@@ -2168,7 +2156,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
       PersonChildren = this.sequelize.define('PersonChildren', {}, {underscored: true});
       Children = Person.belongsToMany(Person, { as: 'Children', through: PersonChildren});
 
-      expect(Children.foreignKey).to.equal('children_id');
+      expect(Children.foreignKey).to.equal('person_id');
       expect(Children.otherKey).to.equal('child_id');
       expect(PersonChildren.rawAttributes[Children.foreignKey]).to.be.ok;
       expect(PersonChildren.rawAttributes[Children.otherKey]).to.be.ok;

diff --git a/test/integration/associations/scope.test.js b/test/integration/associations/scope.test.js
@@ -261,8 +261,8 @@ describe(Support.getTestDialectTeaser('associations'), function() {
             this.PostTag = this.sequelize.define('post_tag');
 
             this.Tag.belongsToMany(this.Post, {through: this.PostTag});
-            this.Post.belongsToMany(this.Tag, {as: 'categories', through: this.PostTag, foreignKey: 'PostId', scope: { type: 'category' }});
-            this.Post.belongsToMany(this.Tag, {as: 'tags', through: this.PostTag, foreignKey: 'PostId', scope: { type: 'tag' }});
+            this.Post.belongsToMany(this.Tag, {as: 'categories', through: this.PostTag, scope: { type: 'category' }});
+            this.Post.belongsToMany(this.Tag, {as: 'tags', through: this.PostTag, scope: { type: 'tag' }});
           });
 
           it('should create, find and include associations with scope values', function() {

diff --git a/test/integration/include/findAll.test.js b/test/integration/include/findAll.test.js
@@ -405,8 +405,8 @@ describe(Support.getTestDialectTeaser('Include'), function() {
             ]).spread(function(user, products) {
               return Promise.all([
                 GroupMember.bulkCreate([
-                  {MembershipsId: user.id, GroupId: groups[0].id, RankId: ranks[0].id},
-                  {MembershipsId: user.id, GroupId: groups[1].id, RankId: ranks[1].id}
+                  {UserId: user.id, GroupId: groups[0].id, RankId: ranks[0].id},
+                  {UserId: user.id, GroupId: groups[1].id, RankId: ranks[1].id}
                 ]),
                 user.setProducts([
                   products[(i * 2) + 0],

diff --git a/test/integration/include/schema.test.js b/test/integration/include/schema.test.js
@@ -275,8 +275,8 @@ describe(Support.getTestDialectTeaser('Includes with schemas'), function() {
                 ]).spread(function(user, products) {
                   return Promise.all([
                     GroupMember.bulkCreate([
-                      {MembershipsId: user.id, GroupId: groups[0].id, RankId: ranks[0].id},
-                      {MembershipsId: user.id, GroupId: groups[1].id, RankId: ranks[1].id}
+                      {AccUserId: user.id, GroupId: groups[0].id, RankId: ranks[0].id},
+                      {AccUserId: user.id, GroupId: groups[1].id, RankId: ranks[1].id}
                     ]),
                     user.setProducts([
                       products[(i * 2) + 0],

diff --git a/test/integration/model/scope/associations.test.js b/test/integration/model/scope/associations.test.js
@@ -96,7 +96,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         this.ScopeMe.hasOne(this.Profile, { foreignKey: 'userId' });
 
         this.ScopeMe.belongsTo(this.Company);
-        this.UserAssociation = this.Company.hasMany(this.ScopeMe, { as: 'users', foreignKey: 'companyId'});
+        this.UserAssociation = this.Company.hasMany(this.ScopeMe, { as: 'users' });
 
         return this.sequelize.sync({force: true}).bind(this).then(function() {
           return Promise.all([

diff --git a/test/unit/associations/belongs-to-many.test.js b/test/unit/associations/belongs-to-many.test.js
@@ -512,10 +512,10 @@ describe(Support.getTestDialectTeaser('belongsToMany'), function() {
       Group.belongsToMany(User, { as: 'MyUsers', through: 'group_user', onUpdate: 'SET NULL', onDelete: 'RESTRICT' });
 
       expect(Group.associations.MyUsers.through.model === User.associations.MyGroups.through.model);
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyGroupsId.onUpdate).to.equal('RESTRICT');
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyGroupsId.onDelete).to.equal('SET NULL');
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyUsersId.onUpdate).to.equal('SET NULL');
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyUsersId.onDelete).to.equal('RESTRICT');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.UserId.onUpdate).to.equal('RESTRICT');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.UserId.onDelete).to.equal('SET NULL');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.GroupId.onUpdate).to.equal('SET NULL');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.GroupId.onDelete).to.equal('RESTRICT');
     });
 
     it('work properly when through is a model', function() {
@@ -527,25 +527,10 @@ describe(Support.getTestDialectTeaser('belongsToMany'), function() {
       Group.belongsToMany(User, { as: 'MyUsers', through: UserGroup, onUpdate: 'SET NULL', onDelete: 'RESTRICT' });
 
       expect(Group.associations.MyUsers.through.model === User.associations.MyGroups.through.model);
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyGroupsId.onUpdate).to.equal('RESTRICT');
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyGroupsId.onDelete).to.equal('SET NULL');
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyUsersId.onUpdate).to.equal('SET NULL');
-      expect(Group.associations.MyUsers.through.model.rawAttributes.MyUsersId.onDelete).to.equal('RESTRICT');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.UserId.onUpdate).to.equal('RESTRICT');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.UserId.onDelete).to.equal('SET NULL');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.GroupId.onUpdate).to.equal('SET NULL');
+      expect(Group.associations.MyUsers.through.model.rawAttributes.GroupId.onDelete).to.equal('RESTRICT');
     });
   });
-
-  it('properly use the `as` key to generate foreign key name', function(){
-    var City = current.define('City', { cityname: DataTypes.STRING })
-      , State = current.define('State', { statename: DataTypes.STRING })
-      , CityStateMap = current.define('CityStateMap', { rating: DataTypes.STRING });
-
-    State.belongsToMany(City, { through: CityStateMap });
-    expect(CityStateMap.attributes.StateId).not.to.be.empty;
-    expect(CityStateMap.attributes.CityId).not.to.be.empty;
-
-    State.belongsToMany(City, { through: CityStateMap, as: 'Test'});
-    expect(CityStateMap.attributes.TestId).not.to.be.empty;
-    expect(CityStateMap.attributes.CityId).not.to.be.empty;
-
-  });
 });
diff --git a/test/unit/associations/belongs-to.test.js b/test/unit/associations/belongs-to.test.js
diff --git a/test/unit/associations/has-many.test.js b/test/unit/associations/has-many.test.js
@@ -162,15 +162,4 @@ describe(Support.getTestDialectTeaser('hasMany'), function() {
       });
     });
   });
-
-  it('properly use the `as` key to generate foreign key name', function(){
-    var City = current.define('City', { cityname: DataTypes.STRING })
-      , State = current.define('State', { statename: DataTypes.STRING });
-
-    State.hasMany(City);
-    expect(City.attributes.StateId).not.to.be.empty;
-
-    State.hasMany(City, {as : 'Capital'});
-    expect(City.attributes.CapitalId).not.to.be.empty;
-  });
 });