www

commit beb17436f8b301c5f599c0ee432c188c460a8430
parent b0f52a0f0737e0a9f89e7b40afdb56021bc710d8
Author: Dan Stillman <dstillman@zotero.org>
Date:   Tue,  5 May 2015 14:08:28 -0400

Don't mark parentKey as changed when set to false for an unsaved item

And return undefined for search objects

Diffstat:
M
chrome/content/zotero/xpcom/data/dataObject.js
 | 
21
+++++++++++++++++----
M
test/tests/collectionTest.js
 | 
21
++++++++++++++++++++-
M
test/tests/itemTest.js
 | 
54
+++++++++++++++++++++++++++++++++++++++++-------------

3 files changed, 78 insertions(+), 18 deletions(-)
diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js
@@ -26,8 +26,10 @@
 /**
  * @property {String} (readOnly) objectType
  * @property {String} (readOnly) libraryKey
- * @property {String|null} parentKey Null if no parent
- * @property {Integer|false|undefined} parentID False if no parent. Undefined if not applicable (e.g. search objects)
+ * @property {String|false|undefined} parentKey - False if no parent, or undefined if not
+ *                                                applicable (e.g. search objects)
+ * @property {Integer|false|undefined} parentID - False if no parent, or undefined if not
+ *                                                applicable (e.g. search objects)
  */
 
 Zotero.DataObject = function () {
@@ -73,7 +75,7 @@ Zotero.defineProperty(Zotero.DataObject.prototype, 'libraryKey', {
 	get: function() this._libraryID + "/" + this._key
 });
 Zotero.defineProperty(Zotero.DataObject.prototype, 'parentKey', {
-	get: function() this._parentKey,
+	get: function () this._getParentKey(),
 	set: function(v) this._setParentKey(v)
 });
 Zotero.defineProperty(Zotero.DataObject.prototype, 'parentID', {
@@ -148,6 +150,9 @@ Zotero.DataObject.prototype._getParentID = function () {
 		return this._parentID;
 	}
 	if (!this._parentKey) {
+		if (this._objectType == 'search') {
+			return undefined;
+		}
 		return false;
 	}
 	return this._parentID = this.ObjectsClass.getIDFromLibraryAndKey(this._libraryID, this._parentKey);
@@ -168,6 +173,14 @@ Zotero.DataObject.prototype._setParentID = function (id) {
 	);
 }
 
+
+Zotero.DataObject.prototype._getParentKey = function () {
+	if (this._objectType == 'search') {
+		return undefined;
+	}
+	return this._parentKey ? this._parentKey : false
+}
+
 /**
  * Set the key of the parent object
  *
@@ -181,7 +194,7 @@ Zotero.DataObject.prototype._setParentKey = function(key) {
 	
 	key = Zotero.DataObjectUtilities.checkKey(key) || false;
 	
-	if (this._parentKey == key) {
+	if (key === this._parentKey || (!this._parentKey && !key)) {
 		return false;
 	}
 	this._markFieldChange('parentKey', this._parentKey);
diff --git a/test/tests/collectionTest.js b/test/tests/collectionTest.js
@@ -24,7 +24,7 @@ describe("Zotero.Collection", function() {
 		});
 	})
 	
-	describe("#parent", function () {
+	describe("#parentKey", function () {
 		it("should set parent collection for new collections", function* () {
 			var parentCol = new Zotero.Collection
 			parentCol.name = "Parent";
@@ -66,6 +66,25 @@ describe("Zotero.Collection", function() {
 			assert.equal(col.parentKey, newParentKey);
 		});
 		
+		it("should not mark collection as unchanged if set to existing value", function* () {
+			// Create initial parent collection
+			var parentCol = new Zotero.Collection
+			parentCol.name = "Parent";
+			var parentID = yield parentCol.save();
+			var {libraryID, key: parentKey} = Zotero.Collections.getLibraryAndKeyFromID(parentID);
+			
+			// Create subcollection
+			var col = new Zotero.Collection
+			col.name = "Child";
+			col.parentKey = parentKey;
+			var id = yield col.save();
+			col = yield Zotero.Collections.getAsync(id);
+			
+			// Set to existing parent
+			col.parentKey = parentKey;
+			assert.isFalse(col.hasChanged());
+		});
+		
 		it("should not resave a collection with no parent if set to false", function* () {
 			var col = new Zotero.Collection
 			col.name = "Test";
diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js
@@ -80,19 +80,47 @@ describe("Zotero.Item", function () {
 	})
 	
 	describe("#parentID", function () {
-		it("should create a child note", function () {
-			return Zotero.DB.executeTransaction(function* () {
-				var item = new Zotero.Item('book');
-				var parentItemID = yield item.save();
-				
-				item = new Zotero.Item('note');
-				item.parentID = parentItemID;
-				var childItemID = yield item.save();
-				
-				item = yield Zotero.Items.getAsync(childItemID);
-				assert.ok(item.parentID);
-				assert.equal(item.parentID, parentItemID);
-			}.bind(this));
+		it("should create a child note", function* () {
+			var item = new Zotero.Item('book');
+			var parentItemID = yield item.save();
+			
+			item = new Zotero.Item('note');
+			item.parentID = parentItemID;
+			var childItemID = yield item.save();
+			
+			item = yield Zotero.Items.getAsync(childItemID);
+			assert.ok(item.parentID);
+			assert.equal(item.parentID, parentItemID);
+		});
+	});
+	
+	describe("#parentKey", function () {
+		it("should be false for an unsaved attachment", function () {
+			var item = new Zotero.Item('attachment');
+			assert.isFalse(item.parentKey);
+		});
+		
+		it("should be false on an unsaved non-attachment item", function () {
+			var item = new Zotero.Item('book');
+			assert.isFalse(item.parentKey);
+		});
+		
+		it("should not be marked as changed setting to false on an unsaved item", function () {
+			var item = new Zotero.Item('attachment');
+			item.attachmentLinkMode = 'linked_url';
+			item.parentKey = false;
+			assert.isUndefined(item._changed.parentKey);
+		});
+		
+		it("should not mark item as changed if false and no existing parent", function* () {
+			var item = new Zotero.Item('attachment');
+			item.attachmentLinkMode = 'linked_url';
+			item.url = "https://www.zotero.org/";
+			var id = yield item.save();
+			item = yield Zotero.Items.getAsync(id);
+			
+			item.parentKey = false;
+			assert.isFalse(item.hasChanged());
 		});
 	});