- Fix some collection sync issues

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|FALSE}	 itemID of the parent item for an attachment or note, or FALSE if none
- * @return	{Integer}
  */
 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|FALSE}	 Key of the parent item for an attachment or note, or FALSE if none
- * @return	{String}
  */
 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];

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());
 		}