From 5df6846b786d15ab90cdbb6bffdeebc2132ea856 Mon Sep 17 00:00:00 2001
From: noviny <novinyarts@gmail.com>
Date: Fri, 24 Feb 2017 19:04:58 +1100
Subject: [PATCH] Update relationship field to use standard error handling

Previously, Relationship fields were cleared by calls to updateItem if
not explicitly passed in. Now this and other undefined values will
leave them untouched.

This is consistent with how other fields work, however is a breaking
change to updateItem() with relationships
---
 fields/types/relationship/Readme.md           |  2 ++
 fields/types/relationship/RelationshipType.js | 13 +++++++++----
 fields/types/relationship/test/type.js        | 10 ++++++----
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/fields/types/relationship/Readme.md b/fields/types/relationship/Readme.md
index 9c3a6961e67..0065ff83fc0 100644
--- a/fields/types/relationship/Readme.md
+++ b/fields/types/relationship/Readme.md
@@ -53,6 +53,8 @@ Will return unexpected results if the relationship path has been populated.
 
 Updates with the provided value if it is different from the stored value.
 
+`undefined` values are ignored.
+
 When `null` is passed, mongoose will remove the path from the stored document and the value will be `undefined` when the item is next retrieved. This behaviour is different in `many` mode, when an empty array will be stored.
 
 ### `validateInput`
diff --git a/fields/types/relationship/RelationshipType.js b/fields/types/relationship/RelationshipType.js
index c545462f8f3..a3b0006876d 100644
--- a/fields/types/relationship/RelationshipType.js
+++ b/fields/types/relationship/RelationshipType.js
@@ -219,10 +219,15 @@ relationship.prototype.updateItem = function (item, data, callback) {
 	if (item.populated(this.path)) {
 		throw new Error('fieldTypes.relationship.updateItem() Error - You cannot update populated relationships.');
 	}
+
+	var value = this.getValueFromData(data);
+	if (value === undefined) {
+		return process.nextTick(callback);
+	}
 	if (this.many) {
 		var arr = item.get(this.path);
 		var _old = arr.map(function (i) { return String(i); });
-		var _new = data[this.path];
+		var _new = value;
 		if (!utils.isArray(_new)) {
 			_new = String(_new || '').split(',');
 		}
@@ -232,9 +237,9 @@ relationship.prototype.updateItem = function (item, data, callback) {
 			item.set(this.path, _new);
 		}
 	} else {
-		if (data[this.path]) {
-			if (data[this.path] !== item.get(this.path)) {
-				item.set(this.path, data[this.path]);
+		if (value) {
+			if (value !== item.get(this.path)) {
+				item.set(this.path, value);
 			}
 		} else if (item.get(this.path)) {
 			item.set(this.path, null);
diff --git a/fields/types/relationship/test/type.js b/fields/types/relationship/test/type.js
index cd537a28950..88aca2db4a3 100644
--- a/fields/types/relationship/test/type.js
+++ b/fields/types/relationship/test/type.js
@@ -187,7 +187,7 @@ exports.testFieldType = function (List) {
 			});
 		});
 
-		it('should clear the current value when data object does not contain the field', function (done) {
+		it('should not clear the current value when data object does not contain the field', function (done) {
 			var testItem = new List.model({
 				single: relatedItem.id,
 			});
@@ -195,7 +195,7 @@ exports.testFieldType = function (List) {
 				List.fields.single.updateItem(testItem, {}, function () {
 					testItem.save(function (err, updatedItem) {
 						List.model.findById(updatedItem.id, function (err, persistedData) {
-							demand(persistedData.single).be.null();
+							demand(String(persistedData.single)).equal(String(relatedItem.id));
 							done();
 						});
 					});
@@ -247,7 +247,7 @@ exports.testFieldType = function (List) {
 			});
 		});
 
-		it('should clear the current values when data object does not contain the field', function (done) {
+		it('should not clear the current values when data object does not contain the field', function (done) {
 			var testItem = new List.model({
 				many: [relatedItem.id, relatedItem.id],
 			});
@@ -255,7 +255,9 @@ exports.testFieldType = function (List) {
 				List.fields.many.updateItem(testItem, {}, function () {
 					testItem.save(function (err, updatedItem) {
 						List.model.findById(updatedItem.id, function (err, persistedData) {
-							demand(persistedData.many).to.eql([]);
+							demand(persistedData.many.length).equal(2);
+							demand(String(persistedData.many[0])).equal(String(relatedItem.id));
+							demand(String(persistedData.many[1])).equal(String(relatedItem.id));
 							done();
 						});
 					});