Merge branch 'caseyWebb-issue-43' (PR #85). Closes #43

scottwrobinson · Nov 4, 2016 · beb5681 · beb5681
2 parents 630a485 + d12fdf1
commit beb5681
Show file tree

Hide file tree

Showing 2 changed files with 127 additions and 14 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -70,6 +70,11 @@ class Document extends BaseDocument {
             // Ensure all data types are saved in the same encodings
             that.canonicalize();
 
+            return Promise.all(that._getHookPromises('postValidate'));
+        }).then(function() {
+            return Promise.all(that._getHookPromises('preSave'));
+        }).then(function() {
+
             // TODO: We should instead track what has changed and
             // only update those values. Maybe make that._changed
             // object to do this.
@@ -128,19 +133,7 @@ class Document extends BaseDocument {
                 }
             });
 
-            return toUpdate;
-        }).then(function(data) {
-            // TODO: hack?
-            let postValidatePromises = [data].concat(that._getHookPromises('postValidate'));
-            return Promise.all(postValidatePromises);
-        }).then(function(prevData) {
-            let data = prevData[0];
-            // TODO: hack?
-            let preSavePromises = [data].concat(that._getHookPromises('preSave'));
-            return Promise.all(preSavePromises);
-        }).then(function(prevData) {
-            let data = prevData[0];
-            return DB().save(that.collectionName(), that._id, data);
+            return DB().save(that.collectionName(), that._id, toUpdate);
         }).then(function(id) {
             if (that._id === null) {
                 that._id = id;
@@ -360,7 +353,7 @@ class Document extends BaseDocument {
         .then(function(datas) {
             let docs = that._fromData(datas);
 
-            if (options.populate === true || 
+            if (options.populate === true ||
                 (isArray(options.populate) && options.populate.length > 0)) {
                 return that.populate(docs, options.populate);
             }

diff --git a/test/issues.test.js b/test/issues.test.js
@@ -229,6 +229,126 @@ describe('Issues', function() {
         });
     });
 
+    describe('#43', function() {
+        /*
+         * Changes made to the model in postValidate and preSave hooks
+         * should be saved to the database
+         */
+        it('should save changes made in postValidate hook', function(done) {
+            class Person extends Document {
+                constructor() {
+                    super();
+
+                    this.postValidateChange = {
+                        type: Boolean,
+                        default: false
+                    };
+                    this.pet = Pet;
+                    this.pets = [Pet];
+                }
+
+                static collectionName() {
+                    return 'people';
+                }
+
+                postValidate() {
+                    this.postValidateChange = true;
+                    this.pet.postValidateChange = true;
+                    this.pets[0].postValidateChange = true;
+
+                    this.pets.push(Pet.create({
+                        postValidateChange: true
+                    }));
+                }
+            }
+
+            class Pet extends EmbeddedDocument {
+                constructor() {
+                    super();
+
+                    this.postValidateChange = Boolean;
+                }
+
+                static collectionName() {
+                    return 'pets';
+                }
+            }
+
+            let person = Person.create();
+            person.pet = Pet.create();
+            person.pets.push(Pet.create());
+
+            person.save().then(function() {
+                validateId(person);
+                return Person
+                    .findOne({ _id: person._id }, { populate: true })
+                    .then((p) => {
+                        expect(p.postValidateChange).to.be.equal(true);
+                        expect(p.pet.postValidateChange).to.be.equal(true);
+                        expect(p.pets[0].postValidateChange).to.be.equal(true);
+                        expect(p.pets[1].postValidateChange).to.be.equal(true);
+                    });
+            }).then(done, done);
+        });
+
+        it('should save changes made in preSave hook', function(done) {
+            class Person extends Document {
+                constructor() {
+                    super();
+
+                    this.preSaveChange = {
+                        type: Boolean,
+                        default: false
+                    };
+                    this.pet = Pet;
+                    this.pets = [Pet];
+                }
+
+                static collectionName() {
+                    return 'people';
+                }
+
+                postValidate() {
+                    this.preSaveChange = true;
+                    this.pet.preSaveChange = true;
+                    this.pets[0].preSaveChange = true;
+
+                    this.pets.push(Pet.create({
+                        preSaveChange: true
+                    }));
+                }
+            }
+
+            class Pet extends EmbeddedDocument {
+                constructor() {
+                    super();
+
+                    this.preSaveChange = Boolean;
+                }
+
+                static collectionName() {
+                    return 'pets';
+                }
+            }
+
+            let person = Person.create();
+            person.pet = Pet.create();
+            person.pets.push(Pet.create());
+
+            person.save().then(function() {
+                validateId(person);
+                return Person
+                    .findOne({ _id: person._id }, { populate: true })
+                    .then((p) => {
+                        expect(p.preSaveChange).to.be.equal(true);
+                        expect(p.pet.preSaveChange).to.be.equal(true);
+                        expect(p.pets[0].preSaveChange).to.be.equal(true);
+                        expect(p.pets[1].preSaveChange).to.be.equal(true);
+                    });
+            }).then(done, done);
+        });
+    });
+
     describe('#53', function() {
         /* 
          * Camo should validate that all properties conform to