fix(populate): make transform option work consistently for both virtu…

…al populate and conventional populate Fix #3775
Automattic · Mar 4, 2021 · a7ceb7e · a7ceb7e
1 parent e5e2937
commit a7ceb7e
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 38 deletions.
diff --git a/lib/helpers/populate/assignVals.js b/lib/helpers/populate/assignVals.js
@@ -209,10 +209,11 @@ function valueFilter(val, assignmentOpts, populateOptions, allIds) {
     const numValues = val.length;
     for (let i = 0; i < numValues; ++i) {
       let subdoc = val[i];
+      const _allIds = Array.isArray(allIds) ? allIds[i] : allIds;
       if (!isPopulatedObject(subdoc) && (!populateOptions.retainNullValues || subdoc != null) && !userSpecifiedTransform) {
         continue;
       } else if (userSpecifiedTransform) {
-        subdoc = transform(isPopulatedObject(subdoc) ? subdoc : null, allIds);
+        subdoc = transform(isPopulatedObject(subdoc) ? subdoc : null, _allIds);
       }
       maybeRemoveId(subdoc, assignmentOpts);
       ret.push(subdoc);

diff --git a/test/model.populate.test.js b/test/model.populate.test.js
@@ -10001,17 +10001,19 @@ describe('model: populate:', function() {
       });
 
       let called = [];
+      function transform(doc, id) {
+        called.push({
+          doc: doc,
+          id: id
+        });
+
+        return id;
+      }
 
+      // Populate array of ids
       p = yield Parent.findById(p).populate({
         path: 'children',
-        transform: function(doc, id) {
-          called.push({
-            doc: doc,
-            id: id
-          });
-
-          return id;
-        }
+        transform: transform
       });
 
       assert.equal(called.length, 2);
@@ -10021,56 +10023,50 @@ describe('model: populate:', function() {
       assert.equal(called[1].doc.name, 'Leia');
       assert.equal(called[1].id.toHexString(), children[1]._id.toHexString());
 
+      // Populate single id
       called = [];
       p = yield Parent.findById(p).populate({
         path: 'child',
-        transform: function(doc, id) {
-          called.push({
-            doc: doc,
-            id: id
-          });
-
-          return id;
-        }
+        transform: transform
       });
 
       assert.equal(called.length, 1);
       assert.equal(called[0].doc.name, 'Luke');
       assert.equal(called[0].id.toHexString(), children[0]._id.toHexString());
 
+      // Push a nonexistent id
       const newId = new mongoose.Types.ObjectId();
       yield Parent.updateOne({ _id: p._id }, { $push: { children: newId } });
 
       called = [];
       p = yield Parent.findById(p).populate({
         path: 'children',
-        transform: function(doc, id) {
-          called.push({
-            doc: doc,
-            id: id
-          });
-
-          return id;
-        }
+        transform: transform
       });
       assert.equal(called.length, 3);
       assert.strictEqual(called[2].doc, null);
       assert.equal(called[2].id.toHexString(), newId.toHexString());
 
       assert.equal(p.children[2].toHexString(), newId.toHexString());
 
+      // Populate 2 docs with same id
+      yield Parent.updateOne({ _id: p._id }, { $set: { children: [children[0], children[0]] } });
+      called = [];
+
+      p = yield Parent.findById(p).populate({
+        path: 'children',
+        transform: transform
+      });
+      assert.equal(called.length, 2);
+      assert.equal(called[0].id.toHexString(), children[0]._id.toHexString());
+      assert.equal(called[1].id.toHexString(), children[0]._id.toHexString());
+
+      // Populate single id that points to nonexistent doc
       yield Parent.updateOne({ _id: p._id }, { $set: { child: newId } });
       called = [];
       p = yield Parent.findById(p).populate({
         path: 'child',
-        transform: function(doc, id) {
-          called.push({
-            doc: doc,
-            id: id
-          });
-
-          return id;
-        }
+        transform: transform
       });
 
       assert.equal(called.length, 1);
@@ -10095,9 +10091,9 @@ describe('model: populate:', function() {
 
     return co(function*() {
       let p = yield Parent.create({ name: 'Anakin' });
-      const child = yield Child.create({ name: 'Luke', parentId: p._id });
+      yield Child.create({ name: 'Luke', parentId: p._id });
 
-      let called = [];
+      const called = [];
 
       p = yield Parent.findById(p).populate({
         path: 'child',
@@ -10117,7 +10113,7 @@ describe('model: populate:', function() {
     });
   });
 
-  it('transform with virtual populate, justOne = false (gh-3375) XYZ', function() {
+  it('transform with virtual populate, justOne = false (gh-3375)', function() {
     const parentSchema = new Schema({
       name: String
     });
@@ -10133,12 +10129,12 @@ describe('model: populate:', function() {
 
     return co(function*() {
       let p = yield Parent.create({ name: 'Anakin' });
-      const children = yield Child.create([
+      yield Child.create([
         { name: 'Luke', parentId: p._id },
         { name: 'Leia', parentId: p._id }
       ]);
 
-      let called = [];
+      const called = [];
 
       p = yield Parent.findById(p).populate({
         path: 'children',