Merge pull request #1361 from sequelize/include-and-or-tests

Include: support .and()/.or() - Fixes #1341 & #1291

Merge pull request #1361 from sequelize/include-and-or-tests
Include: support .and()/.or() - Fixes #1341 & #1291
Mick Hansen
Commit bad4f0b3 authored Feb 07, 2014 by Mick Hansen
Showing with 122 additions and 19 deletions
changelog.md
lib/dialects/abstract/query-generator.js
lib/utils.js
test/include.test.js
test/include/findAll.test.js
--- a/changelog.md
+++ b/changelog.md
@@ -5,6 +5,7 @@ Notice: All 1.7.x changes are present in 2.0.x aswell
 - [BUG] sync() no longer fails with foreign key constraints references own table (toposort self-dependency error)
 - [FEATURE] makes it possible to specify exactly what timestamp attributes you want to utilize [#1334](https://github.com/sequelize/sequelize/pull/1334)
 - [FEATURE] Support coffee script files in migrations. [#1357](https://github.com/sequelize/sequelize/pull/1357)
+- [FEATURE] include.where now supports Sequelize.and()/.or(). [#1361](https://github.com/sequelize/sequelize/pull/1361)
 # v1.7.0-rc4
 - [BUG] fixes issue with postgres sync and enums [#1020](https://github.com/sequelize/sequelize/issues/1020)

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -604,19 +604,12 @@ module.exports = (function() {
            , includeWhere  = {}
            , whereOptions  = Utils._.clone(options)
+          whereOptions.keysEscaped = true
          if (tableName !== parentTable) {
            as = parentTable+'.'+include.as
          }
-          if (include.where) {
-            for (var key in include.where) {
-              if (include.where.hasOwnProperty(key)) {
-                includeWhere[self.quoteIdentifier(as)+'.'+self.quoteIdentifiers(key)] = include.where[key]
-              }
-            }
-            whereOptions.keysEscaped = true
-          }
          // includeIgnoreAttributes is used by aggregate functions
          if (options.includeIgnoreAttributes !== false) {
            attributes  = include.attributes.map(function(attr) {
@@ -677,7 +670,7 @@ module.exports = (function() {
            if (include.where) {
-              targetWhere = self.hashToWhereConditions(includeWhere, include.daoFactory, whereOptions)
+              targetWhere = self.getWhereConditions(include.where, self.sequelize.literal(self.quoteIdentifier(as)), include.daoFactory, whereOptions)
              joinQueryItem += " AND "+ targetWhere
              if (subQuery) {
                if (!options.where) options.where = {}
@@ -711,7 +704,7 @@ module.exports = (function() {
              joinQueryItem += where
            if (include.where) {
-              joinQueryItem += " AND "+self.hashToWhereConditions(includeWhere, include.daoFactory, whereOptions)
+              joinQueryItem += " AND "+self.getWhereConditions(include.where, self.sequelize.literal(self.quoteIdentifier(as)), include.daoFactory, whereOptions)
              // If its a multi association we need to add a where query to the main where (executed in the subquery)
              if (subQuery && association.isMultiAssociation) {
@@ -890,8 +883,9 @@ module.exports = (function() {
        , where  = {}
        , self   = this
-      if (typeof prepend === 'undefined')
+      if (typeof prepend === 'undefined') {
        prepend = true
+      }
      if ((smth instanceof Utils.and) || (smth instanceof Utils.or)) {
        var connector = (smth instanceof Utils.and) ? ' AND ' : ' OR '
@@ -903,8 +897,8 @@ module.exports = (function() {
        result = "(" + result + ")"
      } else if (Utils.isHash(smth)) {
        if (prepend) {
-          smth = Utils.prependTableNameToHash(tableName, smth)
+          smth = Utils.prependTableNameToHash(tableName, smth, self.quoteIdentifier.bind(self))
-        }        
+        }
        result = this.hashToWhereConditions(smth, factory, options)
      } else if (typeof smth === 'number') {
        var primaryKeys = !!factory ? Object.keys(factory.primaryKeys) : []

--- a/lib/utils.js
+++ b/lib/utils.js
@@ -445,7 +445,7 @@ var Utils = module.exports = {
    return result
  },
-  prependTableNameToHash: function(tableName, hash) {
+  prependTableNameToHash: function(tableName, hash, quote) {
    if (tableName) {
      var _hash = {}
@@ -453,7 +453,11 @@ var Utils = module.exports = {
        if (key instanceof Utils.literal) {
          _hash[key] = hash[key]
        } else if (key.indexOf('.') === -1) {
-          _hash[tableName + '.' + key] = hash[key]
+          if (tableName instanceof Utils.literal) {
+            _hash[tableName + '.' + quote(key)] = hash[key]
+          } else {
+            _hash[tableName + '.' + key] = hash[key]
+          }
        } else {
          _hash[key] = hash[key]
        }

--- a/test/include.test.js
+++ b/test/include.test.js
@@ -514,6 +514,110 @@ describe(Support.getTestDialectTeaser("Include"), function () {
    })
  })
+  describe('where', function () {
+    it('should support Sequelize.and()', function (done) {
+      var User = this.sequelize.define('User', {})
+        , Item = this.sequelize.define('Item', {'test': DataTypes.STRING})
+      User.hasOne(Item);
+      Item.belongsTo(User);
+      this.sequelize.sync().done(function() {
+        async.auto({
+          users: function(callback) {
+            User.bulkCreate([{}, {}, {}]).done(function() {
+              User.findAll().done(callback)
+            })
+          },
+          items: function(callback) {
+            Item.bulkCreate([
+              {'test': 'abc'},
+              {'test': 'def'},
+              {'test': 'ghi'}
+            ]).done(function() {
+              Item.findAll().done(callback)
+            })
+          },
+          associate: ['users', 'items', function(callback, results) {
+            var chainer = new Sequelize.Utils.QueryChainer()
+            var users = results.users
+            var items = results.items
+            chainer.add(users[0].setItem(items[0]))
+            chainer.add(users[1].setItem(items[1]))
+            chainer.add(users[2].setItem(items[2]))
+            chainer.run().done(callback)
+          }]
+        }, function() {
+          User.findAll({include: [
+            {model: Item, where: Sequelize.and({
+              test: 'def'
+            })}
+          ]}).done(function(err, result) {
+            expect(err).not.to.be.ok
+            expect(result.length).to.eql(1)
+            expect(result[0].item.test).to.eql('def')
+            done()
+          })
+        })
+      }) 
+    })
+    it('should support Sequelize.or()', function (done) {
+      var User = this.sequelize.define('User', {})
+        , Item = this.sequelize.define('Item', {'test': DataTypes.STRING})
+      User.hasOne(Item);
+      Item.belongsTo(User);
+      this.sequelize.sync().done(function() {
+        async.auto({
+          users: function(callback) {
+            User.bulkCreate([{}, {}, {}]).done(function() {
+              User.findAll().done(callback)
+            })
+          },
+          items: function(callback) {
+            Item.bulkCreate([
+              {'test': 'abc'},
+              {'test': 'def'},
+              {'test': 'ghi'}
+            ]).done(function() {
+              Item.findAll().done(callback)
+            })
+          },
+          associate: ['users', 'items', function(callback, results) {
+            var chainer = new Sequelize.Utils.QueryChainer()
+            var users = results.users
+            var items = results.items
+            chainer.add(users[0].setItem(items[0]))
+            chainer.add(users[1].setItem(items[1]))
+            chainer.add(users[2].setItem(items[2]))
+            chainer.run().done(callback)
+          }]
+        }, function() {
+          User.findAll({include: [
+            {model: Item, where: Sequelize.or({
+              test: 'def'
+            }, {
+              test: 'abc'
+            })}
+          ]}).done(function(err, result) {
+            expect(err).not.to.be.ok
+            expect(result.length).to.eql(2)
+            done()
+          })
+        })
+      }) 
+    })
+  })
  describe('findAndCountAll', function () {
    it('should include associations to findAndCountAll', function(done) {
      var User = this.sequelize.define('User', {})

--- a/test/include/findAll.test.js
+++ b/test/include/findAll.test.js
@@ -1073,7 +1073,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
      })
    })
-    it('should be possible use limit and a where with a belongsTo include', function (done) {
+    it('should be possible to use limit and a where with a belongsTo include', function (done) {
      var User = this.sequelize.define('User', {})
        , Group = this.sequelize.define('Group', {
            name: DataTypes.STRING
@@ -1153,7 +1153,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
      })
    })
-    it('should be possible use limit and a where on a hasMany with additional includes', function (done) {
+    it('should be possible to use limit and a where on a hasMany with additional includes', function (done) {
      var self = this
      this.fixtureA(function () {
        self.models.Product.findAll({
@@ -1184,7 +1184,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
      })
    })
-    it('should be possible use limit and a where on a hasMany with a through model with additional includes', function (done) {
+    it('should be possible to use limit and a where on a hasMany with a through model with additional includes', function (done) {
      var self = this
      this.fixtureA(function () {
        self.models.Product.findAll({