www

commit 93a6a4ffdb8087306c07ef336b97fcabbdc0dd0d
parent 0cefa038275d2d47eac277c24600d612a3d60b9f
Author: Dan Stillman <dstillman@zotero.org>
Date:   Tue,  5 May 2015 04:45:48 -0400

Don't mark an empty field set to an empty string as changed

And some tests

Diffstat:
M
chrome/content/zotero/xpcom/data/item.js
 | 
9
++++-----
M
test/tests/itemTest.js
 | 
43
+++++++++++++++++++++++++++++++++++++++++++

2 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
@@ -789,7 +789,7 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) {
 		return true;
 	}
 	
-	if (value === "") {
+	if (value === "" || value === null || value === false) {
 		value = false;
 	}
 	
@@ -850,9 +850,8 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) {
 		}
 		
 		// If existing value, make sure it's actually changing
-		if ((typeof this._itemData[fieldID] == 'undefined' && value === false)
-				|| (typeof this._itemData[fieldID] != 'undefined'
-					&& this._itemData[fieldID] === value)) {
+		if ((this._itemData[fieldID] === null && value === false)
+				|| (this._itemData[fieldID] !== null && this._itemData[fieldID] === value)) {
 			return false;
 		}
 		
@@ -1264,7 +1263,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			let value = this.getField(fieldID, true);
 			
 			// If field changed and is empty, mark row for deletion
-			if (!value) {
+			if (value === '') {
 				del.push(fieldID);
 				continue;
 			}
diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js
@@ -19,6 +19,49 @@ describe("Zotero.Item", function () {
 	});
 	
 	describe("#setField", function () {
+		it("should mark a field as changed", function () {
+			var item = new Zotero.Item('book');
+			item.setField('title', 'Foo');
+			assert.ok(item._changed.itemData[Zotero.ItemFields.getID('title')]);
+			assert.ok(item.hasChanged());
+		})
+		
+		it("should clear an existing field set to a falsy value", function () {
+			var field = 'title';
+			var fieldID = Zotero.ItemFields.getID(field);
+			var item = new Zotero.Item('book');
+			item.setField(field, 'Foo');
+			id = yield item.save();
+			item = yield Zotero.Items.getAsync(id);
+			
+			item.setField(field, "");
+			assert.ok(item._changed.itemData[fieldID]);
+			assert.ok(item.hasChanged());
+			yield item.reload();
+			
+			assert.isFalse(item.hasChanged());
+			
+			item.setField(field, false);
+			assert.ok(item._changed.itemData[fieldID]);
+			assert.ok(item.hasChanged());
+			yield item.reload();
+			
+			assert.isFalse(item.hasChanged());
+			
+			item.setField(field, null);
+			assert.ok(item._changed.itemData[fieldID]);
+			assert.ok(item.hasChanged());
+			
+			yield item.save();
+			assert.isFalse(item.getField(fieldID));
+		})
+		
+		it("should not mark an empty field set to an empty string as changed", function () {
+			var item = new Zotero.Item('book');
+			item.setField('url', '');
+			assert.isUndefined(item._changed.itemData);
+		})
+		
 		it("should save version as object version", function* () {
 			var item = new Zotero.Item('book');
 			item.setField("version", 1);