From 882074e84777124fcc0fbd0dcbb5356cb5890352 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 14 Jul 2009 04:04:39 +0000 Subject: [PATCH] - Fix some collection sync issues --- chrome/content/zotero/xpcom/data/item.js | 41 +++++++++++++------ chrome/content/zotero/xpcom/sync.js | 50 +++++++++++++++++++++--- 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 315677670..9a6900723 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -293,9 +293,6 @@ Zotero.Item.prototype.loadPrimaryData = function(allowFail) { where.push(whereSQL); } } - else { - Zotero.debug("skipping " + field); - } } if (!columns.length) { @@ -1750,9 +1747,7 @@ Zotero.Item.prototype.save = function() { // TODO: clear caches throw ("Cannot set source to invalid item " + this._sourceItem); } - } - - if (newSourceItem) { + var newSourceItemNotifierData = {}; newSourceItemNotifierData[newSourceItem.id] = { old: newSourceItem.serialize() @@ -1788,6 +1783,10 @@ Zotero.Item.prototype.save = function() { + "WHERE itemID=?"; var changedCollections = Zotero.DB.columnQuery(sql, this.id); if (changedCollections) { + sql = "UPDATE collections SET dateModified=CURRENT_TIMESTAMP, clientDateModified=CURRENT_TIMESTAMP " + + "WHERE collectionID IN (SELECT collectionID FROM collectionItems WHERE itemID=?)"; + Zotero.DB.query(sql, this.id); + if (newSourceItem) { sql = "UPDATE OR REPLACE collectionItems " + "SET itemID=? WHERE itemID=?"; @@ -1990,10 +1989,13 @@ Zotero.Item.prototype.numChildren = function(includeTrashed) { /** - * Get the itemID of the source item for a note or file - * @return {Integer} + * @return {Integer|FALSE} itemID of the parent item for an attachment or note, or FALSE if none */ Zotero.Item.prototype.getSource = function() { + if (this._sourceItem === false) { + return false; + } + if (this._sourceItem !== null) { if (typeof this._sourceItem == 'number') { return this._sourceItem; @@ -2034,10 +2036,13 @@ Zotero.Item.prototype.getSource = function() { /** - * Get the key of the source item for a note or file - * @return {String} + * @return {String|FALSE} Key of the parent item for an attachment or note, or FALSE if none */ Zotero.Item.prototype.getSourceKey = function() { + if (this._sourceItem === false) { + return false; + } + if (this._sourceItem !== null) { if (typeof this._sourceItem == 'string') { return this._sourceItem; @@ -2094,7 +2099,7 @@ Zotero.Item.prototype.setSource = function(sourceItemID) { this._previousData = this.serialize(); } - this._sourceItem = sourceItemID ? parseInt(sourceItemID) : null; + this._sourceItem = sourceItemID ? parseInt(sourceItemID) : false; this._changedSource = true; return true; @@ -2120,7 +2125,7 @@ Zotero.Item.prototype.setSourceKey = function(sourceItemKey) { var oldSourceItemKey = sourceItem.key; } else { - var oldSourceItemKey = null; + var oldSourceItemKey = false; } if (oldSourceItemKey == sourceItemKey) { Zotero.debug("Source item has not changed in Zotero.Item.setSourceKey()"); @@ -2131,7 +2136,7 @@ Zotero.Item.prototype.setSourceKey = function(sourceItemKey) { this._previousData = this.serialize(); } - this._sourceItem = sourceItemKey ? sourceItemKey : null; + this._sourceItem = sourceItemKey ? sourceItemKey : false; this._changedSource = true; return true; @@ -3321,6 +3326,16 @@ Zotero.Item.prototype.diff = function (item, includeMatches, ignoreFields) { var changed = false; + changed = thisData.sourceItemKey != otherData.sourceItemKey; + if (includeMatches || changed) { + diff[0].sourceItemKey = thisData.sourceItemKey; + diff[1].sourceItemKey = otherData.sourceItemKey; + + if (changed) { + numDiffs++; + } + } + if (thisData.attachment) { for (var field in thisData.attachment) { changed = thisData.attachment[field] != otherData.attachment[field]; diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js index 373fe901b..cc97af6c3 100644 --- a/chrome/content/zotero/xpcom/sync.js +++ b/chrome/content/zotero/xpcom/sync.js @@ -1893,6 +1893,7 @@ Zotero.Sync.Server.Data = new function() { var remoteCreatorStore = {}; var relatedItemsStore = {}; var itemStorageModTimes = {}; + var childItemStore = []; if (xml.groups.length()) { Zotero.debug("Processing remotely changed groups"); @@ -1992,12 +1993,11 @@ Zotero.Sync.Server.Data = new function() { } var remoteObj = Zotero.Sync.Server.Data['xmlTo' + Type](xmlNode); - + // Some types we don't bother to reconcile if (_noMergeTypes.indexOf(type) != -1) { // If local is newer, send to server if (obj.dateModified > remoteObj.dateModified) { - Zotero.debug('c'); syncSession.addToUpdated(obj); continue; } @@ -2059,7 +2059,7 @@ Zotero.Sync.Server.Data = new function() { break; case 'collection': - var changed = _mergeCollection(obj, remoteObj); + var changed = _mergeCollection(obj, remoteObj, childItemStore); if (!changed) { syncSession.removeFromUpdated(obj); } @@ -2134,6 +2134,7 @@ Zotero.Sync.Server.Data = new function() { } } + // Temporarily remove and store related items that don't yet exist if (type == 'item') { var missing = Zotero.Sync.Server.Data.removeMissingRelatedItems(xmlNode); @@ -2303,6 +2304,13 @@ Zotero.Sync.Server.Data = new function() { Zotero.debug('Saving merged ' + types); if (type == 'collection') { + for each(var col in toSave) { + var changed = _removeChildItemsFromCollection(col, childItemStore); + if (changed) { + syncSession.addToUpdated(col); + } + } + // Save collections recursively from the top down _saveCollections(toSave); } @@ -2318,7 +2326,17 @@ Zotero.Sync.Server.Data = new function() { // Save the rest for each(var obj in toSave) { + // Keep list of all child items being saved + var store = false; + if (!obj.isTopLevelItem()) { + store = true; + } + obj.save(); + + if (store) { + childItemStore.push(obj.id) + } } // Add back related items (which now exist) @@ -2534,7 +2552,27 @@ Zotero.Sync.Server.Data = new function() { } - function _mergeCollection(localObj, remoteObj) { + // Remove any child items from collection, which might exist if an attachment in a collection was + // remotely changed from a top-level item to a child item + function _removeChildItemsFromCollection(collection, childItems) { + if (!childItems.length) { + return false; + } + var itemIDs = collection.getChildItems(true); + // TODO: fix to always return array + if (!itemIDs) { + return false; + } + var newItemIDs = Zotero.Utilities.prototype.arrayDiff(childItems, itemIDs); + if (itemIDs.length == newItemIDs.length) { + return false; + } + collection.childItems = newItemIDs; + return true; + } + + + function _mergeCollection(localObj, remoteObj, childItems) { var diff = localObj.diff(remoteObj, false, true); if (!diff) { return false; @@ -2594,6 +2632,8 @@ Zotero.Sync.Server.Data = new function() { alert(msg); } + _removeChildItemsFromCollection(targetObj, childItems); + targetObj.save(); return true; } @@ -3061,7 +3101,7 @@ Zotero.Sync.Server.Data = new function() { // Both notes and attachments might have parents and notes if (item.isNote() || item.isAttachment()) { var sourceItemKey = xmlItem.@sourceItem.toString(); - item.setSourceKey(sourceItemKey ? sourceItemKey : null); + item.setSourceKey(sourceItemKey ? sourceItemKey : false); item.setNote(xmlItem.note.toString()); }