Merge pull request #414 from strongloop/feature/fix-hasone-remoting

Fix hasOne remoting

lib/dao.js

@@ -1433,6 +1433,12 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb
     cb = function() {};
   }
 
+  // Convert the data to be plain object so that update won't be confused
+  if (data instanceof Model) {
+    data = data.toObject(false);
+  }
+  data = removeUndefined(data);
+
   var context = {
     Model: Model,
     where: byIdQuery(Model, getIdValue(Model, inst)).where,

lib/relation-definition.js

@@ -1490,12 +1490,26 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) {
   // FIXME: [rfeng] Wrap the property into a function for remoting
   // so that it can be accessed as /api/<model>/<id>/<hasOneRelationName>
   // For example, /api/orders/1/customer
-  var fn = function() {
+  modelFrom.prototype['__get__' + relationName] = function() {
     var f = this[relationName];
     f.apply(this, arguments);
   };
-  modelFrom.prototype['__get__' + relationName] = fn;
-  
+
+  modelFrom.prototype['__create__' + relationName] = function() {
+    var f = this[relationName].create;
+    f.apply(this, arguments);
+  };
+
+  modelFrom.prototype['__update__' + relationName] = function() {
+    var f = this[relationName].update;
+    f.apply(this, arguments);
+  };
+
+  modelFrom.prototype['__destroy__' + relationName] = function() {
+    var f = this[relationName].destroy;
+    f.apply(this, arguments);
+  };
+
   return definition;
 };
 
@@ -1545,10 +1559,12 @@ HasOne.prototype.create = function (targetModelData, cb) {
   });
 };
 
-HasOne.prototype.update = function (targetModelData, cb) {
+HasOne.prototype.update = function(targetModelData, cb) {
   var definition = this.definition;
+  var fk = this.definition.keyTo;
   this.fetch(function(err, targetModel) {
     if (targetModel instanceof ModelBaseClass) {
+      delete targetModelData[fk];
       targetModel.updateAttributes(targetModelData, cb);
     } else {
       cb(new Error('HasOne relation ' + definition.name

test/manipulation.test.js

@@ -244,7 +244,7 @@ describe('manipulation', function () {
 
     before(function (done) {
       Person.destroyAll(function () {
-        person = Person.create(done);
+        person = Person.create({name: 'Mary', age: 15}, done);
       });
     });
 
@@ -259,6 +259,33 @@ describe('manipulation', function () {
         });
       });
     });
+
+    it('should ignore undefined values on updateAttributes', function(done) {
+      person.updateAttributes({'name': 'John', age: undefined},
+        function(err, p) {
+          should.not.exist(err);
+          Person.findById(p.id, function(e, p) {
+            should.not.exist(err);
+            p.name.should.equal('John');
+            p.age.should.equal(15);
+            done();
+          });
+        });
+    });
+
+    it('should allows model instance on updateAttributes', function(done) {
+      person.updateAttributes(new Person({'name': 'John', age: undefined}),
+        function(err, p) {
+          should.not.exist(err);
+          Person.findById(p.id, function(e, p) {
+            should.not.exist(err);
+            p.name.should.equal('John');
+            p.age.should.equal(15);
+            done();
+          });
+        });
+    });
+
   });
 
   describe('destroy', function () {

test/relations.test.js

@@ -1809,6 +1809,24 @@ describe('relations', function () {
         });
       });
     });
+
+    it('should ignore the foreign key in the update', function(done) {
+      Supplier.create({name: 'Supplier 2'}, function (e, supplier) {
+        var sid = supplier.id;
+        Supplier.findById(supplierId, function(e, supplier) {
+          should.not.exist(e);
+          should.exist(supplier);
+          supplier.account.update({supplierName: 'Supplier A',
+              supplierId: sid},
+            function(err, act) {
+              should.not.exist(e);
+              act.supplierName.should.equal('Supplier A');
+              act.supplierId.should.equal(supplierId);
+              done();
+            });
+        });
+      });
+    });
     
     it('should get the related item on scope', function(done) {
       Supplier.findById(supplierId, function(e, supplier) {