From c822aad2b6fb159c5028d9795b2d51f3eec35e40 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 4 Sep 2008 20:27:41 +0000
Subject: [PATCH] - Fix "Creator must be a Zotero.Creator object in
 Zotero.Item.setCreator()" problem -- now looks for creator data within item
 block when it doesn't exist locally - Fix a couple other problems with
 creators that could cause changes not to be synced or to cause unnecessary
 conflicts

---
 chrome/content/zotero/xpcom/data/item.js |  6 +-
 chrome/content/zotero/xpcom/sync.js      | 77 ++++++++++++++++++------
 2 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 36d053c0d..2cdd6fec7 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -3538,8 +3538,12 @@ Zotero.Item.prototype._loadCreators = function() {
 	}
 	
 	for (var i=0; i<creators.length; i++) {
+		var creatorObj = Zotero.Creators.get(creators[i].creatorID);
+		if (!creatorObj) {
+			creatorObj = new Zotero.Creator(creators[i].creatorID);
+		}
 		this._creators[creators[i].orderIndex] = {
-			ref: new Zotero.Creator(creators[i].creatorID),
+			ref: creatorObj,
 			creatorTypeID: creators[i].creatorTypeID
 		};
 	}
diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js
index ddf35417f..3ccb4b286 100644
--- a/chrome/content/zotero/xpcom/sync.js
+++ b/chrome/content/zotero/xpcom/sync.js
@@ -1392,6 +1392,9 @@ Zotero.Sync.Server.Data = new function() {
 		var relatedItemsStore = {};
 		var itemStorageModTimes = {};
 		
+		//Zotero.debug("Updated IDs:");
+		//Zotero.debug(uploadIDs);
+		
 		Zotero.DB.beginTransaction();
 		
 		for each(var syncObject in Zotero.Sync.syncObjects) {
@@ -1480,7 +1483,9 @@ Zotero.Sync.Server.Data = new function() {
 										var creators = obj.getCreators();
 										creators = creators.concat(remoteObj.getCreators());
 										for each(var creator in creators) {
-											if (remoteCreatorStore[obj.id]) {
+											var r = remoteCreatorStore[creator.ref.id];
+											// Doesn't include dateModified
+											if (r && !r.equals(creator.ref)) {
 												creatorsChanged = true;
 												break;
 											}
@@ -1656,21 +1661,38 @@ Zotero.Sync.Server.Data = new function() {
 				// Don't use assigned-but-unsaved ids for new ids
 				Zotero.ID.skip(types, obj.id);
 				
-				if (type == 'item' && obj.isAttachment() &&
-						(obj.attachmentLinkMode ==
-							Zotero.Attachments.LINK_MODE_IMPORTED_FILE ||
-						 obj.attachmentLinkMode ==
-							Zotero.Attachments.LINK_MODE_IMPORTED_URL)) {
-					// Mark new attachments for download
-					if (isNewObject) {
-						obj.attachmentSyncState =
-							Zotero.Sync.Storage.SYNC_STATE_TO_DOWNLOAD;
+				if (type == 'item') {
+					// Make sure none of the item's creators are marked as
+					// deleted, which could happen if a creator was deleted
+					// locally but attached to a new/modified item remotely
+					// and added back in xmlToItem()
+					if (obj.isRegularItem()) {
+						var creators = obj.getCreators();
+						for each(var creator in creators) {
+							Zotero.Sync.removeFromDeleted(
+								uploadIDs.deleted.creators,
+								creator.ref.id,
+								creator.ref.key,
+								true
+							);
+						}
 					}
-					// Set existing attachments mtime update check
-					else {
-						var mtime = xmlNode.@storageModTime.toString();
-						if (mtime) {
-							itemStorageModTimes[obj.id] = parseInt(mtime);
+					else if (obj.isAttachment() &&
+							(obj.attachmentLinkMode ==
+								Zotero.Attachments.LINK_MODE_IMPORTED_FILE ||
+							 obj.attachmentLinkMode ==
+								Zotero.Attachments.LINK_MODE_IMPORTED_URL)) {
+						// Mark new attachments for download
+						if (isNewObject) {
+							obj.attachmentSyncState =
+								Zotero.Sync.Storage.SYNC_STATE_TO_DOWNLOAD;
+						}
+						// Set existing attachments mtime update check
+						else {
+							var mtime = xmlNode.@storageModTime.toString();
+							if (mtime) {
+								itemStorageModTimes[obj.id] = parseInt(mtime);
+							}
 						}
 					}
 				}
@@ -1867,6 +1889,9 @@ Zotero.Sync.Server.Data = new function() {
 		
 		var xmlstr = Zotero.Sync.Server.Data.buildUploadXML(uploadIDs);
 		
+		//Zotero.debug(xmlstr);
+		//throw ('break');
+		
 		Zotero.DB.commitTransaction();
 		
 		return xmlstr;
@@ -2127,8 +2152,20 @@ Zotero.Sync.Server.Data = new function() {
 			
 			var creatorID = parseInt(creator.@id);
 			var creatorObj = Zotero.Creators.get(creatorID);
+			// If creator doesn't exist locally (e.g., if it was deleted locally
+			// and appears in a new/modified item remotely), get it from within
+			// the item's creator block, where a copy should be provided
 			if (!creatorObj) {
-				throw ("Creator " + creatorID + " does not exist");
+				if (creator.creator.length() == 0) {
+					throw ("Data for missing local creator " + creatorID
+						+ " not provided in Zotero.Sync.Server.Data.xmlToItem()");
+				}
+				var creatorObj =
+					Zotero.Sync.Server.Data.xmlToCreator(creator.creator);
+				if (creatorObj.id != creatorID) {
+					throw ("Creator id " + creatorObj.id + " does not match "
+						+ "item creator in Zotero.Sync.Server.Data.xmlToItem()");
+				}
 			}
 			item.setCreator(
 				pos,
@@ -2328,10 +2365,14 @@ Zotero.Sync.Server.Data = new function() {
 	function xmlToCreator(xmlCreator, creator, skipPrimary) {
 		if (!creator) {
 			if (skipPrimary) {
-				creator = new Zotero.Creator(null);
+				creator = new Zotero.Creator;
 			}
 			else {
-				creator = new Zotero.Creator(parseInt(xmlCreator.@id));
+				var creatorID = parseInt(xmlCreator.@id);
+				creator = Zotero.Creators.get(creatorID);
+				if (!creator) {
+					creator = new Zotero.Creator(creatorID);
+				}
 				/*
 				if (creator.exists()) {
 					throw ("Creator specified in XML node already exists "