diff --git a/lib/abstract-class.js b/lib/abstract-class.js index 3904a8ef..a32b1430 100644 --- a/lib/abstract-class.js +++ b/lib/abstract-class.js @@ -67,13 +67,15 @@ function AbstractClass(data) { enumerable: true }); - // Getter for initial property - Object.defineProperty(this, attr_was, { - writable: true, - value: data[attr], - configurable: true, - enumerable: false - }); + if (data.hasOwnProperty(attr)) { + // Getter for initial property + Object.defineProperty(this, attr_was, { + writable: true, + value: data[attr], + configurable: true, + enumerable: false + }); + } }.bind(this)); @@ -130,6 +132,8 @@ AbstractClass.create = function (data, callback) { if (data instanceof AbstractClass && !data.id) { obj = data; data = obj.toObject(true); + // recall constructor to update _was property states (maybe bad idea) + this.call(obj, data); create(); } else { obj = new this(data); @@ -204,19 +208,15 @@ AbstractClass.all = function all(params, cb) { if (data && data.map) { collection = data.map(function (d) { var obj = null; - // really questionable stuff - // goal is obvious: to not create different instances for the same object - // but the way it implemented... - // we can lost some dirty state of object, for example - // TODO: think about better implementation, test keeping dirty state - if (constr.cache[d.id]) { + // do not create different instances for the same object + if (d.id && constr.cache[d.id]) { obj = constr.cache[d.id]; // keep dirty attributes untouthed (remove from dataset) substractDirtyAttributes(obj, d); constr.call(obj, d); } else { obj = new constr(d); - constr.cache[d.id] = obj; + if (d.id) constr.cache[d.id] = obj; } return obj; }); @@ -227,7 +227,7 @@ AbstractClass.all = function all(params, cb) { function substractDirtyAttributes(object, data) { Object.keys(object.toObject()).forEach(function (attr) { - if (attr in data && object.propertyChanged(attr)) { + if (data.hasOwnProperty(attr) && object.propertyChanged(attr)) { delete data[attr]; } }); @@ -370,30 +370,34 @@ AbstractClass.prototype.updateAttribute = function (name, value, cb) { AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) { var inst = this; var model = this.constructor.modelName; - Object.keys(data).forEach(function (key) { - this[key] = data[key]; - }.bind(this)); - this.isValid(function (valid) { + // update instance's properties + Object.keys(data).forEach(function (key) { + inst[key] = data[key]; + }); + + inst.isValid(function (valid) { if (!valid) { if (cb) { cb(new Error('Validation error')); } } else { - update.call(this); + update(); } - }.bind(this)); + }); function update() { - this.trigger('save', function (saveDone) { - this.trigger('update', function (done) { + inst.trigger('save', function (saveDone) { + inst.trigger('update', function (done) { Object.keys(data).forEach(function (key) { - data[key] = this[key]; - }.bind(this)); + data[key] = inst[key]; + }); - this._adapter().updateAttributes(model, this.id, data, function (err) { + inst._adapter().updateAttributes(model, inst.id, data, function (err) { if (!err) { + inst.constructor.call(inst, data); + /* Object.keys(data).forEach(function (key) { inst[key] = data[key]; Object.defineProperty(inst, key + '_was', { @@ -403,13 +407,14 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) { value: data[key] }); }); + */ } done.call(inst, function () { saveDone.call(inst, function () { cb(err); }); }); - }.bind(this)); + }); }); }); } diff --git a/test/common_test.js b/test/common_test.js index 22f3a2ed..b0dd34e0 100644 --- a/test/common_test.js +++ b/test/common_test.js @@ -188,7 +188,20 @@ function testOrm(schema) { obj.save(function (err, obj) { test.equal(obj.title, title2); test.ok(!obj.propertyChanged('title')); - test.done(); + + var p = new Post({title: 1}); + p.title = 2; + p.save(function (err, obj) { + test.ok(!p.propertyChanged('title')); + p.title = 3; + test.ok(p.propertyChanged('title')); + test.equal(p.title_was, 2); + p.save(function () { + test.equal(p.title_was, 3); + test.ok(!p.propertyChanged('title')); + test.done(); + }); + }); }); }); }); diff --git a/test/validations_test.coffee b/test/validations_test.coffee index c33e66b6..bc1d962c 100644 --- a/test/validations_test.coffee +++ b/test/validations_test.coffee @@ -16,6 +16,7 @@ User = schema.define 'User', domain: String pendingPeriod: Number createdByAdmin: Boolean + updatedAt: Date validAttributes = name: 'Anatoliy' @@ -247,7 +248,7 @@ it 'should validate asynchronously', (test) -> setTimeout => err 'async' if @name == 'bad name' done() - , 100 + , 10 User.validateAsync 'name', validator, message: async: 'hello' @@ -272,14 +273,21 @@ it 'should validate uniqueness', (test) -> user.email = 'unique@email.tld' user.isValid (valid) -> test.ok valid, 'valid with unique email' - user.save -> - test.done() + user.save (err) -> + test.ok not user.propertyChanged('email'), 'Email changed' + user.updateAttributes { updatedAt: new Date, createdByAdmin: false }, (err) -> + User.all where: email: 'unique@email.tld', (err, users) -> + test.ok users[0] + test.ok users[0].email == 'unique@email.tld' + test.ok !err, 'Updated' + test.done() it 'should save dirty state when validating uniqueness', (test) -> - User.all where: email: 'unique@email.tld' , (err, users) -> + User.all where: email: 'unique@email.tld', (err, users) -> u = users[0] u.name = 'Hulk' u.isValid (valid) -> - test.ok valid + test.ok valid, 'Invalid user' test.equal u.name, 'Hulk' test.done() +