diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index bd249dbae5..555bec4b68 100644 --- 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 index 9991a09295..1e1cf717f9 100644 --- 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 index decef15664..2919d1e92a 100644 --- 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()); }); });