From ddb1daabbce09c3e32936b7bc8d06e86075b49be Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 9 Jan 2025 02:48:27 -0500
Subject: [PATCH] Fix `json[c.field] is undefined` sync error

Fix error if local note or attachment is added to a parent item while
the remote version was added to a collection. Now, the new local parent
item will be added to any collections the remote child item was added
to.

Fixes #2934
---
 chrome/content/zotero/xpcom/sync/syncLocal.js | 34 +++++++++++++++++++
 test/tests/syncLocalTest.js                   | 27 +++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js
index 25a1cc4566..66f8f2a6a9 100644
--- a/chrome/content/zotero/xpcom/sync/syncLocal.js
+++ b/chrome/content/zotero/xpcom/sync/syncLocal.js
@@ -933,6 +933,22 @@ Zotero.Sync.Data.Local = {
 									['mtime', 'md5', 'dateAdded', 'dateModified']
 								);
 								
+								// If local object became a child item and remote was added to any
+								// collections, we need to remove the 'collections' changes and add
+								// the parent item to those collections instead
+								if (objectType == 'item'
+										&& !obj.isTopLevelItem()
+										&& (obj.isNote() || obj.isAttachment())) {
+									let collections = result.changes
+										.filter(x => x.field == 'collections' && x.op == 'member-add')
+										.map(x => x.value);
+									if (collections.length) {
+										result.changes = result.changes
+											.filter(x => !(x.field == 'collections' && x.op == 'member-add'));
+										saveOptions.newParentItemCollections = collections;
+									}
+								}
+								
 								// If no changes, just update local version number and mark as synced
 								if (!result.changes.length && !result.conflicts.length) {
 									Zotero.debug("No remote changes to apply to local "
@@ -1542,6 +1558,24 @@ Zotero.Sync.Data.Local = {
 					Zotero.Libraries.get(obj.libraryID).storageDownloadNeeded = true;
 				}
 			}
+			
+			// See explanation in processObjectsFromJSON()
+			if (options.newParentItemCollections) {
+				let parentItem = obj.parentItem;
+				for (let c of options.newParentItemCollections) {
+					parentItem.addToCollection(c);
+				}
+				yield parentItem.save({
+					skipEditCheck: true,
+					skipDateModifiedUpdate: true,
+					skipSelect: true,
+					notifierQueue: options.notifierQueue,
+					// Errors are logged elsewhere, so skip in DataObject.save()
+					errorHandler: function (e) {
+						return;
+					}
+				});
+			}
 		}
 		catch (e) {
 			// For now, allow sync to proceed after all errors
diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js
index aa033d7d91..570beab367 100644
--- a/test/tests/syncLocalTest.js
+++ b/test/tests/syncLocalTest.js
@@ -718,6 +718,33 @@ describe("Zotero.Sync.Data.Local", function() {
 			}
 		});
 		
+		it("should handle local item becoming child and remote being added to a collection", async function () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			var collection = await createDataObject('collection');
+			var parentItem = await createDataObject('item');
+			var childItem = await createDataObject('item', { itemType: 'note' });
+			var data = childItem.toJSON();
+			await Zotero.Sync.Data.Local.saveCacheObjects('item', libraryID, [data]);
+			
+			// Add to parent locally
+			childItem.parentID = parentItem.id;
+			await childItem.saveTx();
+			
+			// Add to collection remotely
+			data.collections = [collection.key];
+			var json = {
+				key: childItem.key,
+				version: 10,
+				data
+			};
+			await Zotero.Sync.Data.Local.processObjectsFromJSON(
+				'item', libraryID, [json], { stopOnError: true }
+			);
+			
+			assert.isTrue(collection.hasItem(parentItem));
+		});
+		
 		it("should automatically resolve collection name conflict", async function () {
 			var libraryID = Zotero.Libraries.userLibraryID;