From f7dc6fab90a849eda71154f3c4dd10201bbf3b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 23 Mar 2015 08:34:21 +0100 Subject: [PATCH 1/3] Fix regression in prototype.save ffcaa4e7 added a "data" argument to the callback function, which shadowed the original data with the data returned by a connector. Not all connectors are returning a data object though, in which case the model instance ("this" object) is updated with wrong values. This commit fixes the problem by renaming the second callback argument to "unusedData". --- lib/dao.js | 2 +- test/manipulation.test.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dao.js b/lib/dao.js index 26bec26c0..ce3cad709 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1587,7 +1587,7 @@ DataAccessObject.prototype.save = function (options, cb) { inst.trigger('save', function (saveDone) { inst.trigger('update', function (updateDone) { data = removeUndefined(data); - inst._adapter().save(modelName, inst.constructor._forDB(data), function (err, data, result) { + inst._adapter().save(modelName, inst.constructor._forDB(data), function (err, unusedData, result) { if (err) { return cb(err, inst); } diff --git a/test/manipulation.test.js b/test/manipulation.test.js index b0c9165e8..f4486b61e 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -340,6 +340,7 @@ describe('manipulation', function () { p.name = 'Hans'; p.save(function (err) { should.not.exist(err); + p.name.should.equal('Hans'); Person.findOne(function (err, p) { should.not.exist(err); p.name.should.equal('Hans'); From 1fbaf4e3828c3c61c6eb237d2b7d0f3bd6949b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 23 Mar 2015 08:44:53 +0100 Subject: [PATCH 2/3] Improve test failure messages Replace foo.count.should.equal(X) Uncaught TypeError: Cannot read property 'should' of undefined with foo.should.have.property('count', X)` Uncaught AssertionError: expected 0 to have property count --- test/manipulation.test.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/manipulation.test.js b/test/manipulation.test.js index f4486b61e..df6cfdc95 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -769,9 +769,9 @@ describe('manipulation', function () { }); it('should only delete instances that satisfy the where condition', function (done) { - Person.deleteAll({name: 'John'}, function (err, data) { + Person.deleteAll({name: 'John'}, function (err, result) { if (err) return done(err); - data.count.should.equal(1); + result.should.have.property('count', 1); Person.find({where: {name: 'John'}}, function (err, data) { if (err) return done(err); data.should.have.length(0); @@ -785,9 +785,9 @@ describe('manipulation', function () { }); it('should report zero deleted instances', function (done) { - Person.deleteAll({name: 'does-not-match'}, function (err, data) { + Person.deleteAll({name: 'does-not-match'}, function (err, result) { if (err) return done(err); - data.count.should.equal(0); + result.should.have.property('count', 0); Person.count(function(err, count) { if (err) return done(err); count.should.equal(2); @@ -797,9 +797,9 @@ describe('manipulation', function () { }); it('should delete all instances when "where" is not provided', function(done) { - Person.deleteAll(function (err, data) { + Person.deleteAll(function (err, result) { if (err) return done(err); - data.count.should.equal(2); + result.should.have.property('count', 2); Person.count(function(err, count) { if (err) return done(err); count.should.equal(0); @@ -1039,7 +1039,7 @@ describe('manipulation', function () { Person.update({name: 'Harry Hoe'}, {name: 'Marta Moe'}, function(err, results) { should.not.exist(err); - results.count.should.equal(0); + results.should.have.property('count', 0); Person.find({where: {name: 'Harry Hoe'}}, function(err, people) { should.not.exist(err); people.should.be.empty; @@ -1053,7 +1053,7 @@ describe('manipulation', function () { Person.update({name: 'Brett Boe'}, {name: 'Harry Hoe'}, function(err, results) { should.not.exist(err); - results.count.should.equal(1); + results.should.have.property('count', 1); Person.find({where: {age: 19}}, function(err, people) { should.not.exist(err); people.should.have.length(1); @@ -1067,7 +1067,7 @@ describe('manipulation', function () { function(done) { Person.update({name: 'Harry Hoe'}, function(err, results) { should.not.exist(err); - results.count.should.equal(5); + results.should.have.property('count', 5); Person.find({where: {name: 'Brett Boe'}}, function(err, people) { should.not.exist(err); people.should.be.empty; @@ -1085,7 +1085,7 @@ describe('manipulation', function () { Person.update({name: 'Brett Boe'}, {name: undefined, gender: 'male'}, function(err, results) { should.not.exist(err); - results.count.should.equal(1); + results.should.have.property('count', 1) Person.find({where: {name: 'Brett Boe'}}, function(err, people) { should.not.exist(err); people.should.have.length(1); From 35b549543d5e274dae19affffc0c7643c067d903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 23 Mar 2015 09:18:46 +0100 Subject: [PATCH 3/3] test: fix test failure in MySQL connector Fix a test using a string Person.id value to use a numeric value instead, in order to support connectors that use numeric ids by default. --- test/manipulation.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/manipulation.test.js b/test/manipulation.test.js index df6cfdc95..1ca4fa0c1 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -617,7 +617,7 @@ describe('manipulation', function () { it('should allow save() of the created instance', function(done) { Person.updateOrCreate( - { id: 'new-id', name: 'a-name' }, + { id: 999 /* a new id */, name: 'a-name' }, function(err, inst) { if (err) return done(err); inst.save(done);