mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Default to user library when assigning unsaved item to collection

Browse source 
And fix some issues setting the libraryID property on unsaved objects

Also return .deleted as false, not an empty string
This commit is contained in:
Dan Stillman 2015-05-30 19:07:12 -04:00
parent ed1c0a4637
commit 420985661b
4 changed files with 168 additions and 137 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
chrome/content/zotero/xpcom/data/dataObject.js
View file

@ -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;

117
chrome/content/zotero/xpcom/data/item.js
View file

@ -315,86 +315,6 @@ 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<primaryFields.length; i++) {
if (primaryFields[i] === undefined) {
Zotero.debug('Skipping missing field ' + primaryFields[i]);
continue;
}
}
if (row.itemID !== undefined) {
this._id = parseInt(row.itemID);
}
if (row.itemTypeID !== undefined) {
this._id = parseInt(row.itemTypeID);
}
if (row.libraryID !== undefined) {
this._libraryID = parseInt(row.libraryID);
}
if (row.key !== undefined) {
this._key = row.key;
}
if (row.dateAdded !== undefined) {
this._dateAdded = row.dateAdded;
}
if (row.dateModified !== undefined) {
this._dateModified = row.dateModified;
}
if (row.version !== undefined) {
this._version = parseInt(row.version);
}
if (row.numNotes !== undefined) {
this._numNotes = parseInt(row.numNotes);
}
if (row.numNotesTrashed !== undefined) {
this._numNotesTrashed = parseInt(row.numNotesTrashed);
}
if (row.numNotesEmbedded !== undefined) {
this._numNotesEmbedded = parseInt(row.numNotesEmbedded);
}
if (row.numNotesEmbeddedTrashed !== undefined) {
this._numNotesEmbeddedTrashed = parseInt(row.numNotesEmbeddedTrashed);
}
if (row.numAttachments !== undefined) {
this._numAttachments = parseInt(row.numAttachments);
}
if (row.numAttachmentsTrashed !== undefined) {
this._numAttachmentsTrashed = parseInt(row.numAttachmentsTrashed);
}
if (row.parentKey !== undefined) {
this._parentKey = row.parentKey || false;
}
if (row.parentID !== undefined) {
this._parentID = row.parentID ? parseInt(row.parentID) : false;
}
if (row.synced !== undefined) {
this._synced = !!row.synced;
}
if (row.firstCreator !== undefined) {
this._firstCreator = row.firstCreator ? row.firstCreator : '';
}
if (row.sortCreator !== undefined) {
this._sortCreator = row.sortCreator ? row.sortCreator : '';
}
if (row.attachmentCharset !== undefined) {
this._attachmentCharset = row.attachmentCharset ? row.attachmentCharset : null;
}
if (row.attachmentLinkMode !== undefined) {
this._attachmentLinkMode = parseInt(row.attachmentLinkMode);
}
if (row.attachmentContentType !== undefined) {
this._attachmentContentType = row.attachmentContentType ? row.attachmentContentType : '';
}
if (row.attachmentPath !== undefined) {
this._attachmentPath = row.attachmentPath ? row.attachmentPath : '';
}
if (row.attachmentSyncState !== undefined) {
this._attachmentSyncState = parseInt(row.attachmentSyncState);
}
}
else {
var primaryFields = this.ObjectsClass.primaryFields;
for (let i=0; i<primaryFields.length; i++) {
let col = primaryFields[i];
@ -407,15 +327,17 @@ Zotero.Item.prototype._parseRowData = function(row) {
let val = row[col];
//Zotero.debug("Setting field '" + col + "' to '" + val + "' for item " + this.id);
switch (col) {
// Unchanged
case 'itemID':
this._id = val;
col = 'id';
break;
case 'libraryID':
this['_' + col] = val;
break;
// Integer or 0
case 'version':
case 'numNotes':
case 'numNotesTrashed':
@ -423,29 +345,34 @@ Zotero.Item.prototype._parseRowData = function(row) {
case 'numNotesEmbeddedTrashed':
case 'numAttachments':
case 'numAttachmentsTrashed':
this['_' + col] = val ? parseInt(val) : 0;
val = val ? parseInt(val) : 0;
break;
// Value or false
case 'parentKey':
this['_parentKey'] = val || false;
val = val || false;
break;
// Integer or false if falsy
case 'parentID':
this['_' + col] = val ? parseInt(val) : false;
val = val ? parseInt(val) : false;
break;
case 'attachmentLinkMode':
this['_' + col] = val !== null ? parseInt(val) : false;
val = val !== null ? parseInt(val) : false;
break;
// Boolean
case 'synced':
this['_synced'] = !!val;
case 'deleted':
val = !!val;
break;
default:
this['_' + col] = val ? val : '';
}
val = val ? val : '';
}
this['_' + col] = val;
}
}
@ -3436,6 +3363,10 @@ Zotero.Item.prototype.getCollections = function () {
* @param {Array<String|Integer>} 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)

66
test/tests/dataObjectsTest.js
View file

@ -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");
})
})
})

20
test/tests/itemTest.js
View file

@ -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');