Merge pull request #9895 from Automattic/gh9885

Gh9885
Automattic · Feb 9, 2021 · a82b3ee · a82b3ee
2 parents fe8b97b + 6f1af75
commit a82b3ee
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 10 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -2260,7 +2260,6 @@ function _getPathsToValidate(doc) {
   const skipSchemaValidators = {};
 
   _evaluateRequiredFunctions(doc);
-
   // only validate required fields when necessary
   let paths = new Set(Object.keys(doc.$__.activePaths.states.require).filter(function(path) {
     if (!doc.$__isSelected(path) && !doc.isModified(path)) {
@@ -2344,7 +2343,6 @@ function _getPathsToValidate(doc) {
     }
   }
 
-
   for (const path of paths) {
     // Single nested paths (paths embedded under single nested subdocs) will
     // be validated on their own when we call `validate()` on the subdoc itself.
@@ -2438,11 +2436,9 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
     pathDetails[0].filter((path) => this.isModified(path)) :
     pathDetails[0];
   const skipSchemaValidators = pathDetails[1];
-
   if (Array.isArray(pathsToValidate)) {
     paths = _handlePathsToValidate(paths, pathsToValidate);
   }
-
   if (paths.length === 0) {
     return process.nextTick(function() {
       const error = _complete();
@@ -2610,7 +2606,6 @@ Document.prototype.validateSync = function(pathsToValidate, options) {
   if (Array.isArray(pathsToValidate)) {
     paths = _handlePathsToValidate(paths, pathsToValidate);
   }
-
   const validating = {};
 
   paths.forEach(function(path) {

diff --git a/lib/types/embedded.js b/lib/types/embedded.js
@@ -201,6 +201,9 @@ function registerRemoveListener(sub) {
  */
 
 EmbeddedDocument.prototype.$__remove = function(cb) {
+  if (cb == null) {
+    return;
+  }
   return cb(null, this);
 };
 
@@ -218,7 +221,7 @@ EmbeddedDocument.prototype.remove = function(options, fn) {
     options = undefined;
   }
   if (!this.__parentArray || (options && options.noop)) {
-    fn && fn(null);
+    this.$__remove(fn);
     return this;
   }
 
@@ -234,9 +237,7 @@ EmbeddedDocument.prototype.remove = function(options, fn) {
     registerRemoveListener(this);
   }
 
-  if (fn) {
-    fn(null);
-  }
+  this.$__remove(fn);
 
   return this;
 };

diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js
@@ -255,7 +255,6 @@ Subdocument.prototype.remove = function(options, callback) {
     callback = options;
     options = null;
   }
-
   registerRemoveListener(this);
 
   // If removing entire doc, no need to remove subdoc

diff --git a/test/document.test.js b/test/document.test.js
@@ -9932,6 +9932,45 @@ describe('document', function() {
     });
   });
 
+  it('Makes sure pre remove hook is executed gh-9885', function() {
+    const SubSchema = new Schema({
+      myValue: {
+        type: String
+      }
+    }, {});
+    let count = 0;
+    SubSchema.pre('remove', function(next) {
+      count++;
+      next();
+    });
+    const thisSchema = new Schema({
+      foo: {
+        type: String,
+        required: true
+      },
+      mySubdoc: {
+        type: [SubSchema],
+        required: true
+      }
+    }, { minimize: false, collection: 'test' });
+
+    const Model = db.model('TestModel', thisSchema);
+
+    return co(function*() {
+      yield Model.deleteMany({}); // remove all existing documents
+      const newModel = {
+        foo: 'bar',
+        mySubdoc: [{ myValue: 'some value' }]
+      };
+      const document = yield Model.create(newModel);
+      document.mySubdoc[0].remove();
+      yield document.save().catch((error) => {
+        console.error(error);
+      });
+      assert.equal(count, 1);
+    });
+  });  
+
   it('gh9880', function(done) {
     const testSchema = new Schema({
       prop: String,