Skip to content

Commit

Permalink
Update relationship field to use standard error handling
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Noviny committed Feb 24, 2017
1 parent 9d235c2 commit 5df6846
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
2 changes: 2 additions & 0 deletions fields/types/relationship/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
13 changes: 9 additions & 4 deletions fields/types/relationship/RelationshipType.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(',');
}
Expand All @@ -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);
Expand Down
10 changes: 6 additions & 4 deletions fields/types/relationship/test/type.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,15 @@ 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,
});
testItem.save(function (err) {
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();
});
});
Expand Down Expand Up @@ -247,15 +247,17 @@ 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],
});
testItem.save(function (err) {
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();
});
});
Expand Down

0 comments on commit 5df6846

Please sign in to comment.