Merge pull request #476 from strongloop/feature/fix-lb-1076

Make sure inclusion filter is applied to the target model

lib/dao.js

@@ -1140,54 +1140,61 @@ DataAccessObject.find = function find(query, options, cb) {
     }
   }
 
-  var allCb = function (err, data) {
+  var allCb = function(err, data) {
-    if (data && data.forEach) {
+    var results = [];
-      data.forEach(function (d, i) {
+    if (Array.isArray(data)) {
+      for (var i = 0, n = data.length; i < n; i++) {
+        var d = data[i];
         var Model = self.lookupModel(d);
         var obj = new Model(d, {fields: query.fields, applySetters: false, persisted: true});
 
         if (query && query.include) {
           if (query.collect) {
             // The collect property indicates that the query is to return the
-            // standlone items for a related model, not as child of the parent object
+            // standalone items for a related model, not as child of the parent object
             // For example, article.tags
             obj = obj.__cachedRelations[query.collect];
+            if (obj === null) {
+              obj = undefined;
+            }
           } else {
             // This handles the case to return parent items including the related
             // models. For example, Article.find({include: 'tags'}, ...);
             // Try to normalize the include
             var includes = Inclusion.normalizeInclude(query.include || []);
-            includes.forEach(function (inc) {
+            includes.forEach(function(inc) {
               var relationName = inc;
               if (utils.isPlainObject(inc)) {
                 relationName = Object.keys(inc)[0];
               }
 
               // Promote the included model as a direct property
-              var data = obj.__cachedRelations[relationName];
+              var included = obj.__cachedRelations[relationName];
-              if(Array.isArray(data)) {
+              if (Array.isArray(included)) {
-                data = new List(data, null, obj);
+                included = new List(included, null, obj);
               }
-              if (data) obj.__data[relationName] = data;
+              if (included) obj.__data[relationName] = included;
             });
             delete obj.__data.__cachedRelations;
           }
         }
-        data[i] = obj;
+        if (obj !== undefined) {
-      });
+          results.push(obj);
+        }
+      }
 
       if (data && data.countBeforeLimit) {
-        data.countBeforeLimit = data.countBeforeLimit;
+        results.countBeforeLimit = data.countBeforeLimit;
       }
       if (!supportsGeo && near) {
-        data = geo.filter(data, near);
+        results = geo.filter(results, near);
       }
 
-      cb(err, data);
+      cb(err, results);
     }
     else
       cb(err, []);
-  }
+  };
 
   if (options.notify === false) {
     self.getDataSource().connector.all(self.modelName, query, allCb);

lib/scope.js

@@ -156,10 +156,24 @@ function defineScope(cls, targetClass, name, params, methods, options) {
           } else if (self.__cachedRelations) {
             return self.__cachedRelations[name];
           }
-        } else if (arguments.length === 1) {
-          return definition.related(self, f._scope, condOrRefresh);
         } else {
-          return definition.related(self, f._scope, condOrRefresh, cb);
+          // Check if there is a through model
+          // see https://github.com/strongloop/loopback/issues/1076
+          if (f._scope.collect &&
+            condOrRefresh !== null && typeof condOrRefresh === 'object') {
+            // Adjust the include so that the condition will be applied to
+            // the target model
+            f._scope.include = {
+              relation: f._scope.collect,
+              scope: condOrRefresh
+            };
+            condOrRefresh = {};
+          }
+          if (arguments.length === 1) {
+            return definition.related(self, f._scope, condOrRefresh);
+          } else {
+            return definition.related(self, f._scope, condOrRefresh, cb);
+          }
         }
       };
       

test/relations.test.js

@@ -2066,7 +2066,7 @@ describe('relations', function () {
     var Article, TagName, ArticleTag;
     it('can be declared', function (done) {
       Article = db.define('Article', {title: String});
-      TagName = db.define('TagName', {name: String});
+      TagName = db.define('TagName', {name: String, flag: String});
       Article.hasAndBelongsToMany('tagNames');
       ArticleTag = db.models.ArticleTagName;
       db.automigrate(function () {
@@ -2139,6 +2139,58 @@ describe('relations', function () {
     it('should set targetClass on scope property', function() {
       should.equal(Article.prototype.tagNames._targetClass, 'TagName');
     });
+
+    it('should apply inclusion fields to the target model', function(done) {
+      Article.create({title: 'a1'}, function (e, article) {
+        should.not.exist(e);
+        article.tagNames.create({name: 't1', flag: '1'}, function(e, t) {
+          should.not.exist(e);
+          Article.find({
+              where: {id: article.id},
+              include: {relation: 'tagNames', scope: {fields: ['name']}}},
+            function(e, articles) {
+              should.not.exist(e);
+              articles.should.have.property('length', 1);
+              var a = articles[0].toJSON();
+              a.should.have.property('title', 'a1');
+              a.should.have.property('tagNames');
+              a.tagNames.should.have.property('length', 1);
+              var n = a.tagNames[0];
+              n.should.have.property('name', 't1');
+              n.should.have.property('flag', undefined);
+              n.id.should.eql(t.id);
+              done();
+            });
+        });
+      });
+    });
+
+    it('should apply inclusion where to the target model', function(done) {
+      Article.create({title: 'a2'}, function (e, article) {
+        should.not.exist(e);
+        article.tagNames.create({name: 't2', flag: '2'}, function(e, t2) {
+          should.not.exist(e);
+          article.tagNames.create({name: 't3', flag: '3'}, function(e, t3) {
+            Article.find({
+                where: {id: article.id},
+                include: {relation: 'tagNames', scope: {where: {flag: '2'}}}},
+              function(e, articles) {
+                should.not.exist(e);
+                articles.should.have.property('length', 1);
+                var a = articles[0].toJSON();
+                a.should.have.property('title', 'a2');
+                a.should.have.property('tagNames');
+                a.tagNames.should.have.property('length', 1);
+                var n = a.tagNames[0];
+                n.should.have.property('name', 't2');
+                n.should.have.property('flag', '2');
+                n.id.should.eql(t2.id);
+                done();
+              });
+          });
+        });
+      });
+    });
   });
 
   describe('embedsOne', function () {