diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index fbea2806e0..08b335789a 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -142,22 +142,12 @@ Zotero.DataObject.prototype._set = function (field, value) { Zotero.DataObject.prototype._setIdentifier = function (field, value) { - // If primary data is loaded, the only allowed identifier change is libraryID - // (allowed mainly for the library switcher in the advanced search window), - // and then only for unsaved objects (i.e., those without an id or key) - if (this._loaded.primaryData) { - if (field != 'libraryID' || this._id || this._key) { - throw new Error("Cannot set " + field + " after object is already loaded"); - } - } - switch (field) { case 'id': if (this._key) { throw new Error("Cannot set id if key is already set"); } value = Zotero.DataObjectUtilities.checkDataID(value); - this._identified = true; break; case 'libraryID': @@ -172,13 +162,29 @@ Zotero.DataObject.prototype._setIdentifier = function (field, value) { throw new Error("Cannot set key if id is already set"); } value = Zotero.DataObjectUtilities.checkKey(value); - this._identified = true; } if (value === this['_' + field]) { return; } + // If primary data is loaded, the only allowed identifier change is libraryID, and then only + // for unidentified objects, and then only either if a libraryID isn't yet set (because + // primary data gets marked as loaded when fields are set for new items, but some methods + // (setCollections(), save()) automatically set the user library ID after that if none is + // specified), or for searches (for the sake of the library switcher in the advanced search + // window, though that could probably be rewritten) + if (this._loaded.primaryData) { + if (!(!this._identified && field == 'libraryID' + && (!this._libraryID || this._objectType == 'search'))) { + throw new Error("Cannot change " + field + " after object is already loaded"); + } + } + + if (field == 'id' || field == 'key') { + this._identified = true; + } + this['_' + field] = value; } @@ -624,7 +630,7 @@ Zotero.DataObject.prototype.save = Zotero.Promise.coroutine(function* (options) try { if (Zotero.DataObject.prototype._finalizeSave == this._finalizeSave) { - throw new Error("_finalizeSave not implement for Zotero." + this._ObjectType); + throw new Error("_finalizeSave not implemented for Zotero." + this._ObjectType); } env.notifierData = {}; @@ -687,7 +693,7 @@ Zotero.DataObject.prototype.hasChanged = function() { Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env) { // Default to user library if not specified if (this.libraryID === null) { - this.libraryID = Zotero.Libraries.userLibraryID; + this._libraryID = Zotero.Libraries.userLibraryID; } env.isNew = !this.id; diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index e9e261118e..c3f2aced9a 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -315,137 +315,64 @@ Zotero.Item.prototype.loadFromRow = function(row, reload) { } Zotero.Item.prototype._parseRowData = function(row) { - if (false) { - var primaryFields = this.ObjectsClass.primaryFields; - for (let i=0; i} collectionIDsOrKeys Collection ids or keys */ Zotero.Item.prototype.setCollections = function (collectionIDsOrKeys) { + if (!this.libraryID) { + this.libraryID = Zotero.Libraries.userLibraryID; + } + this._requireData('collections'); if (!collectionIDsOrKeys) { @@ -3469,6 +3400,10 @@ Zotero.Item.prototype.setCollections = function (collectionIDsOrKeys) { * @param {Number} collectionID */ Zotero.Item.prototype.addToCollection = function (collectionIDOrKey) { + if (!this.libraryID) { + this.libraryID = Zotero.Libraries.userLibraryID; + } + var collectionID = parseInt(collectionIDOrKey) == collectionIDOrKey ? parseInt(collectionIDOrKey) : this.ContainerObjectsClass.getIDFromLibraryAndKey(this.libraryID, collectionIDOrKey) @@ -3494,6 +3429,10 @@ Zotero.Item.prototype.addToCollection = function (collectionIDOrKey) { * @param {Number} collectionID */ Zotero.Item.prototype.removeFromCollection = function (collectionIDOrKey) { + if (!this.libraryID) { + this.libraryID = Zotero.Libraries.userLibraryID; + } + var collectionID = parseInt(collectionIDOrKey) == collectionIDOrKey ? parseInt(collectionIDOrKey) : this.ContainerObjectsClass.getIDFromLibraryAndKey(this.libraryID, collectionIDOrKey) diff --git a/test/tests/dataObjectsTest.js b/test/tests/dataObjectsTest.js index ca917d73bb..fe0dfa00f2 100644 --- a/test/tests/dataObjectsTest.js +++ b/test/tests/dataObjectsTest.js @@ -43,4 +43,70 @@ describe("Zotero.DataObjects", function () { } }); }); + + describe("#_setIdentifier", function () { + it("should not allow an id change", function* () { + var item = yield createDataObject('item'); + try { + item.id = item.id + 1; + } + catch (e) { + assert.equal(e.message, "ID cannot be changed"); + return; + } + assert.fail("ID change allowed"); + }) + + it("should not allow a key change", function* () { + var item = yield createDataObject('item'); + try { + item.key = Zotero.DataObjectUtilities.generateKey(); + } + catch (e) { + assert.equal(e.message, "Key cannot be changed"); + return; + } + assert.fail("Key change allowed"); + }) + + it("should not allow key to be set if id is set", function* () { + var item = createUnsavedDataObject('item'); + item.id = Zotero.Utilities.rand(100000, 1000000); + try { + item.libraryID = Zotero.Libraries.userLibraryID; + item.key = Zotero.DataObjectUtilities.generateKey(); + } + catch (e) { + assert.equal(e.message, "Cannot set key if id is already set"); + return; + } + assert.fail("ID change allowed"); + }) + + it("should not allow id to be set if key is set", function* () { + var item = createUnsavedDataObject('item'); + item.libraryID = Zotero.Libraries.userLibraryID; + item.key = Zotero.DataObjectUtilities.generateKey(); + try { + item.id = Zotero.Utilities.rand(100000, 1000000); + } + catch (e) { + assert.equal(e.message, "Cannot set id if key is already set"); + return; + } + assert.fail("Key change allowed"); + }) + + it("should not allow key to be set if library isn't set", function* () { + var item = createUnsavedDataObject('item'); + try { + item.key = Zotero.DataObjectUtilities.generateKey(); + } + catch (e) { + assert.equal(e.message, "libraryID must be set before key"); + return; + } + assert.fail("libraryID change allowed"); + }) + }) }) diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index c77ef9da4f..bee454cc9d 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -183,6 +183,26 @@ describe("Zotero.Item", function () { }) }) + describe("#deleted", function () { + it("should be set to true after save", function* () { + var item = yield createDataObject('item'); + item.deleted = true; + yield item.saveTx(); + assert.ok(item.deleted); + }) + + it("should be set to false after save", function* () { + var collection = yield createDataObject('collection'); + var item = createUnsavedDataObject('item'); + item.deleted = true; + yield item.saveTx(); + + item.deleted = false; + yield item.saveTx(); + assert.isFalse(item.deleted); + }) + }) + describe("#parentID", function () { it("should create a child note", function* () { var item = new Zotero.Item('book');