Fix a potential sync error with child attachments
If a standalone attachment existed in a collection and then was added to a parent (e.g., via Create Parent Item), and attachment metadata was also changed at the same time (e.g., due to file syncing), the 'collection item must be top level' trigger could throw on another syncing computer. To work around this, remove collections first, then make changes to the parentItemID columns, and then add new collections.
This commit is contained in:
Dan Stillman
parent
3272387afe
commit
e683b2be07
2 changed files with 115 additions and 93 deletions
|
|
@ -1382,7 +1382,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
}
|
||||
}
|
||||
|
||||
// Parent item
|
||||
// Parent item (DB update is done below after collection removals)
|
||||
var parentItemKey = this.parentKey;
|
||||
var parentItemID = this.ObjectsClass.getIDFromLibraryAndKey(this.libraryID, parentItemKey) || null;
|
||||
if (this._changed.parentKey) {
|
||||
|
|
@ -1411,9 +1411,6 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
}
|
||||
}
|
||||
else {
|
||||
let type = Zotero.ItemTypes.getName(itemTypeID);
|
||||
let Type = type[0].toUpperCase() + type.substr(1);
|
||||
|
||||
if (parentItemKey) {
|
||||
if (!parentItemID) {
|
||||
// TODO: clear caches
|
||||
|
|
@ -1494,15 +1491,6 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
}
|
||||
}
|
||||
|
||||
// Update DB, if not a note or attachment we're changing below
|
||||
if (!this._changed.attachmentData &&
|
||||
(!this._changed.note || !this.isNote())) {
|
||||
let sql = "UPDATE item" + Type + "s SET parentItemID=? "
|
||||
+ "WHERE itemID=?";
|
||||
let bindParams = [parentItemID, this.id];
|
||||
yield Zotero.DB.queryAsync(sql, bindParams);
|
||||
}
|
||||
|
||||
// Update the counts of the previous and new sources
|
||||
if (oldParentItemID) {
|
||||
reloadParentChildItems[oldParentItemID] = true;
|
||||
|
|
@ -1579,6 +1567,67 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
yield Zotero.DB.queryAsync(sql, itemID);
|
||||
}
|
||||
|
||||
// Collections
|
||||
//
|
||||
// Only diffing and removal are done here. Additions have to be done below after parentItemID has
|
||||
// been updated in itemAttachments/itemNotes, since a child item that was made a standalone item and
|
||||
// added to a collection can't be added to the collection while it still has a parent, and vice
|
||||
// versa, due to the trigger checks on collectionItems/itemAttachments/itemNotes.
|
||||
if (this._changed.collections) {
|
||||
if (libraryType == 'publications') {
|
||||
throw new Error("Items in My Publications cannot be added to collections");
|
||||
}
|
||||
|
||||
let oldCollections = this._previousData.collections || [];
|
||||
let newCollections = this._collections;
|
||||
|
||||
let toAdd = Zotero.Utilities.arrayDiff(newCollections, oldCollections);
|
||||
let toRemove = Zotero.Utilities.arrayDiff(oldCollections, newCollections);
|
||||
|
||||
env.collectionsAdded = toAdd;
|
||||
env.collectionsRemoved = toRemove;
|
||||
|
||||
if (toRemove.length) {
|
||||
let sql = "DELETE FROM collectionItems WHERE itemID=? AND collectionID IN ("
|
||||
+ toRemove.join(',')
|
||||
+ ")";
|
||||
yield Zotero.DB.queryAsync(sql, this.id);
|
||||
|
||||
for (let i=0; i<toRemove.length; i++) {
|
||||
let collectionID = toRemove[i];
|
||||
|
||||
if (!env.options.skipNotifier) {
|
||||
Zotero.Notifier.queue(
|
||||
'remove',
|
||||
'collection-item',
|
||||
collectionID + '-' + this.id,
|
||||
{},
|
||||
env.options.notifierQueue
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Remove this item from any loaded collections' cached item lists after commit
|
||||
Zotero.DB.addCurrentCallback("commit", function () {
|
||||
for (let i = 0; i < toRemove.length; i++) {
|
||||
this.ContainerObjectsClass.unregisterChildItem(toRemove[i], this.id);
|
||||
}
|
||||
}.bind(this));
|
||||
}
|
||||
}
|
||||
|
||||
// Add parent item for existing item, if note or attachment data isn't going to be updated below
|
||||
//
|
||||
// Technically this doesn't have to go below collection removals, but only because the
|
||||
// 'collectionitem must be top level' trigger check applies only to INSERTs, not UPDATEs, which was
|
||||
// probably done in an earlier attempt to solve this problem.
|
||||
if (!isNew && this._changed.parentKey && !this._changed.note && !this._changed.attachmentData) {
|
||||
let type = Zotero.ItemTypes.getName(itemTypeID);
|
||||
let Type = type[0].toUpperCase() + type.substr(1);
|
||||
let sql = "UPDATE item" + Type + "s SET parentItemID=? WHERE itemID=?";
|
||||
yield Zotero.DB.queryAsync(sql, [parentItemID, this.id]);
|
||||
}
|
||||
|
||||
// Note
|
||||
if ((isNew && this.isNote()) || this._changed.note) {
|
||||
if (!isNew) {
|
||||
|
|
@ -1627,7 +1676,6 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
reloadParentChildItems[parentItemID] = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (this._changed.attachmentData) {
|
||||
let sql = "REPLACE INTO itemAttachments "
|
||||
+ "(itemID, parentItemID, linkMode, contentType, charsetID, path, "
|
||||
|
|
@ -1666,6 +1714,39 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
}
|
||||
}
|
||||
|
||||
// Add to new collections
|
||||
if (env.collectionsAdded) {
|
||||
let toAdd = env.collectionsAdded;
|
||||
for (let i=0; i<toAdd.length; i++) {
|
||||
let collectionID = toAdd[i];
|
||||
|
||||
let sql = "SELECT IFNULL(MAX(orderIndex)+1, 0) FROM collectionItems "
|
||||
+ "WHERE collectionID=?";
|
||||
let orderIndex = yield Zotero.DB.valueQueryAsync(sql, collectionID);
|
||||
|
||||
sql = "INSERT OR IGNORE INTO collectionItems "
|
||||
+ "(collectionID, itemID, orderIndex) VALUES (?, ?, ?)";
|
||||
yield Zotero.DB.queryAsync(sql, [collectionID, this.id, orderIndex]);
|
||||
|
||||
if (!env.options.skipNotifier) {
|
||||
Zotero.Notifier.queue(
|
||||
'add',
|
||||
'collection-item',
|
||||
collectionID + '-' + this.id,
|
||||
{},
|
||||
env.options.notifierQueue
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Add this item to any loaded collections' cached item lists after commit
|
||||
Zotero.DB.addCurrentCallback("commit", function () {
|
||||
for (let i = 0; i < toAdd.length; i++) {
|
||||
this.ContainerObjectsClass.registerChildItem(toAdd[i], this.id);
|
||||
}
|
||||
}.bind(this));
|
||||
}
|
||||
|
||||
// Tags
|
||||
if (this._changed.tags) {
|
||||
let oldTags = this._previousData.tags || [];
|
||||
|
|
@ -1720,81 +1801,6 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
|
|||
}
|
||||
}
|
||||
|
||||
// Collections
|
||||
if (this._changed.collections) {
|
||||
if (libraryType == 'publications') {
|
||||
throw new Error("Items in My Publications cannot be added to collections");
|
||||
}
|
||||
|
||||
let oldCollections = this._previousData.collections || [];
|
||||
let newCollections = this._collections;
|
||||
|
||||
let toAdd = Zotero.Utilities.arrayDiff(newCollections, oldCollections);
|
||||
let toRemove = Zotero.Utilities.arrayDiff(oldCollections, newCollections);
|
||||
|
||||
env.collectionsAdded = toAdd;
|
||||
env.collectionsRemoved = toRemove;
|
||||
|
||||
if (toAdd.length) {
|
||||
for (let i=0; i<toAdd.length; i++) {
|
||||
let collectionID = toAdd[i];
|
||||
|
||||
let sql = "SELECT IFNULL(MAX(orderIndex)+1, 0) FROM collectionItems "
|
||||
+ "WHERE collectionID=?";
|
||||
let orderIndex = yield Zotero.DB.valueQueryAsync(sql, collectionID);
|
||||
|
||||
sql = "INSERT OR IGNORE INTO collectionItems "
|
||||
+ "(collectionID, itemID, orderIndex) VALUES (?, ?, ?)";
|
||||
yield Zotero.DB.queryAsync(sql, [collectionID, this.id, orderIndex]);
|
||||
|
||||
if (!env.options.skipNotifier) {
|
||||
Zotero.Notifier.queue(
|
||||
'add',
|
||||
'collection-item',
|
||||
collectionID + '-' + this.id,
|
||||
{},
|
||||
env.options.notifierQueue
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Add this item to any loaded collections' cached item lists after commit
|
||||
Zotero.DB.addCurrentCallback("commit", function () {
|
||||
for (let i = 0; i < toAdd.length; i++) {
|
||||
this.ContainerObjectsClass.registerChildItem(toAdd[i], this.id);
|
||||
}
|
||||
}.bind(this));
|
||||
}
|
||||
|
||||
if (toRemove.length) {
|
||||
let sql = "DELETE FROM collectionItems WHERE itemID=? AND collectionID IN ("
|
||||
+ toRemove.join(',')
|
||||
+ ")";
|
||||
yield Zotero.DB.queryAsync(sql, this.id);
|
||||
|
||||
for (let i=0; i<toRemove.length; i++) {
|
||||
let collectionID = toRemove[i];
|
||||
|
||||
if (!env.options.skipNotifier) {
|
||||
Zotero.Notifier.queue(
|
||||
'remove',
|
||||
'collection-item',
|
||||
collectionID + '-' + this.id,
|
||||
{},
|
||||
env.options.notifierQueue
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Remove this item from any loaded collections' cached item lists after commit
|
||||
Zotero.DB.addCurrentCallback("commit", function () {
|
||||
for (let i = 0; i < toRemove.length; i++) {
|
||||
this.ContainerObjectsClass.unregisterChildItem(toRemove[i], this.id);
|
||||
}
|
||||
}.bind(this));
|
||||
}
|
||||
}
|
||||
|
||||
// Update child item counts and contents
|
||||
if (reloadParentChildItems) {
|
||||
for (let parentItemID in reloadParentChildItems) {
|
||||
|
|
@ -4157,10 +4163,6 @@ Zotero.Item.prototype.fromJSON = function (json) {
|
|||
this.setTags(json.tags);
|
||||
break;
|
||||
|
||||
case 'collections':
|
||||
this.setCollections(json.collections);
|
||||
break;
|
||||
|
||||
case 'relations':
|
||||
this.setRelations(json.relations);
|
||||
break;
|
||||
|
|
@ -4220,6 +4222,10 @@ Zotero.Item.prototype.fromJSON = function (json) {
|
|||
}
|
||||
}
|
||||
|
||||
if (json.collections || this._collections.length) {
|
||||
this.setCollections(json.collections);
|
||||
}
|
||||
|
||||
// Clear existing fields not specified
|
||||
var previousFields = this.getUsedFields(true);
|
||||
for (let field of previousFields) {
|
||||
|
|
|
|||
|
|
@ -1448,6 +1448,22 @@ describe("Zotero.Item", function () {
|
|||
assert.strictEqual(item.getField('accessDate'), '');
|
||||
});
|
||||
|
||||
it("should remove child item from collection if 'collections' property not provided", function* () {
|
||||
var collection = yield createDataObject('collection');
|
||||
// Create standalone attachment in collection
|
||||
var attachment = yield importFileAttachment('test.png', { collections: [collection.id] });
|
||||
var item = yield createDataObject('item', { collections: [collection.id] });
|
||||
|
||||
var json = attachment.toJSON();
|
||||
json.path = 'storage:test2.png';
|
||||
// Add to parent, which implicitly removes from collection
|
||||
json.parentItem = item.key;
|
||||
delete json.collections;
|
||||
Zotero.debug(json);
|
||||
attachment.fromJSON(json);
|
||||
yield attachment.save();
|
||||
});
|
||||
|
||||
it("should ignore unknown fields", function* () {
|
||||
var json = {
|
||||
itemType: "journalArticle",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue