commit 424cae173be31c442d1e0e4ecc2fdfc4fb223634
parent ffff044ce6c4dfd2bfa127965c956ba7aba949b7
Author: Dan Stillman <dstillman@zotero.org>
Date: Tue, 2 Jun 2015 19:08:52 -0400
Fix getField/setField behavior/tests with regard to falsy values
In particular, 0 is kept as a value, and passing undefined to setField
now throws an error.
I'm not sure if we actually want to return an empty string in all cases
for missing/invalid fields, but that's what we do currently.
Diffstat:
2 files changed, 73 insertions(+), 19 deletions(-)
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
@@ -269,7 +269,7 @@ Zotero.Item.prototype.getField = function(field, unformatted, includeBaseMapped)
);
}
- value = value ? value : '';
+ value = (value !== null && value !== false) ? value : '';
if (!unformatted) {
// Multipart date fields
@@ -619,6 +619,10 @@ Zotero.Item.prototype.getFieldsNotInType = function (itemTypeID, allowBaseConver
Zotero.Item.prototype.setField = function(field, value, loadIn) {
this._disabledCheck();
+ if (value === undefined) {
+ throw new Error("Value cannot be undefined");
+ }
+
if (typeof value == 'string') {
value = value.trim().normalize();
}
diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js
@@ -1,64 +1,114 @@
describe("Zotero.Item", function () {
describe("#getField()", function () {
- it("should return false for valid unset fields on unsaved items", function () {
+ it("should return an empty string for valid unset fields on unsaved items", function () {
var item = new Zotero.Item('book');
- assert.equal(item.getField('rights'), false);
+ assert.strictEqual(item.getField('rights'), "");
});
- it("should return false for valid unset fields on unsaved items after setting on another field", function () {
+ it("should return an empty string for valid unset fields on unsaved items after setting on another field", function () {
var item = new Zotero.Item('book');
item.setField('title', 'foo');
- assert.equal(item.getField('rights'), false);
+ assert.strictEqual(item.getField('rights'), "");
});
- it("should return false for invalid unset fields on unsaved items after setting on another field", function () {
+ it("should return an empty string for invalid unset fields on unsaved items after setting on another field", function () {
var item = new Zotero.Item('book');
item.setField('title', 'foo');
- assert.equal(item.getField('invalid'), false);
+ assert.strictEqual(item.getField('invalid'), "");
});
});
describe("#setField", function () {
it("should throw an error if item type isn't set", function () {
var item = new Zotero.Item;
- assert.throws(item.setField.bind(item, 'title', 'test'), "Item type must be set before setting field data");
+ assert.throws(() => item.setField('title', 'test'), "Item type must be set before setting field data");
})
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());
+ assert.isTrue(item._changed.itemData[Zotero.ItemFields.getID('title')]);
+ assert.isTrue(item.hasChanged());
})
- it("should clear an existing field set to a falsy value", function* () {
+ it('should clear an existing field when ""/null/false is passed', function* () {
var field = 'title';
+ var val = 'foo';
var fieldID = Zotero.ItemFields.getID(field);
var item = new Zotero.Item('book');
- item.setField(field, 'Foo');
- id = yield item.saveTx();
- item = yield Zotero.Items.getAsync(id);
+ item.setField(field, val);
+ yield item.saveTx();
item.setField(field, "");
assert.ok(item._changed.itemData[fieldID]);
- assert.ok(item.hasChanged());
- yield item.reload();
+ assert.isTrue(item.hasChanged());
+ // Reset to original value
+ yield item.reload();
assert.isFalse(item.hasChanged());
+ assert.equal(item.getField(field), val);
+ // false
item.setField(field, false);
assert.ok(item._changed.itemData[fieldID]);
- assert.ok(item.hasChanged());
+ assert.isTrue(item.hasChanged());
+
+ // Reset to original value
+ yield item.reload();
+ assert.isFalse(item.hasChanged());
+ assert.equal(item.getField(field), val);
+
+ // null
+ item.setField(field, null);
+ assert.ok(item._changed.itemData[fieldID]);
+ assert.isTrue(item.hasChanged());
+
+ yield item.saveTx();
+ assert.equal(item.getField(field), "");
+ })
+
+ it('should clear a field set to 0 when a ""/null/false is passed', function* () {
+ var field = 'title';
+ var val = 0;
+ var fieldID = Zotero.ItemFields.getID(field);
+ var item = new Zotero.Item('book');
+ item.setField(field, val);
+ yield item.saveTx();
+
+ assert.strictEqual(item.getField(field), val);
+
+ // ""
+ item.setField(field, "");
+ assert.ok(item._changed.itemData[fieldID]);
+ assert.isTrue(item.hasChanged());
+
+ // Reset to original value
yield item.reload();
+ assert.isFalse(item.hasChanged());
+ assert.strictEqual(item.getField(field), val);
+
+ // False
+ item.setField(field, false);
+ assert.ok(item._changed.itemData[fieldID]);
+ assert.isTrue(item.hasChanged());
+ // Reset to original value
+ yield item.reload();
assert.isFalse(item.hasChanged());
+ assert.strictEqual(item.getField(field), val);
+ // null
item.setField(field, null);
assert.ok(item._changed.itemData[fieldID]);
- assert.ok(item.hasChanged());
+ assert.isTrue(item.hasChanged());
yield item.saveTx();
- assert.isFalse(item.getField(fieldID));
+ assert.strictEqual(item.getField(field), "");
+ })
+
+ it("should throw if value is undefined", function () {
+ var item = new Zotero.Item('book');
+ assert.throws(() => item.setField('title'), "Value cannot be undefined");
})
it("should not mark an empty field set to an empty string as changed", function () {
