From d5b2f67afa184f68d7edd466deed24154b0b2fd7 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sat, 30 Jul 2016 23:03:30 -0400
Subject: [PATCH] Automatically resolve item deletion/trash conflicts

If the item was deleted on one side and moved to the trash on the other,
just delete the item on the trash side. Since trash emptying happens
automatically, this would otherwise result in a conflict even if the
user carefully avoided making changes before a manual sync.
---
 .../content/zotero/xpcom/sync/syncEngine.js   |  8 ++
 chrome/content/zotero/xpcom/sync/syncLocal.js | 10 +++
 test/tests/syncEngineTest.js                  | 84 ++++++++++++++++++-
 3 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
index 94ef612ebf..c0f26e046c 100644
--- a/chrome/content/zotero/xpcom/sync/syncEngine.js
+++ b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -655,6 +655,14 @@ Zotero.Sync.Data.Engine.prototype._downloadDeletions = Zotero.Promise.coroutine(
 			}
 			// Conflict resolution
 			else if (objectType == 'item') {
+				// If item is already in trash locally, just delete it
+				if (obj.deleted) {
+					Zotero.debug("Local item is in trash -- applying remote deletion");
+					obj.eraseTx({
+						skipDeleteLog: true
+					});
+					continue;
+				}
 				conflicts.push({
 					libraryID: this.libraryID,
 					left: obj.toJSON(),
diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js
index 5e369a02e7..ed7663fab5 100644
--- a/chrome/content/zotero/xpcom/sync/syncLocal.js
+++ b/chrome/content/zotero/xpcom/sync/syncLocal.js
@@ -852,6 +852,16 @@ Zotero.Sync.Data.Local = {
 								
 								switch (objectType) {
 								case 'item':
+									if (jsonData.deleted) {
+										Zotero.debug("Remote item is in trash -- allowing local deletion to propagate");
+										results.push({
+											libraryID,
+											key: objectKey,
+											processed: true
+										});
+										return;
+									}
+									
 									results.push({
 										libraryID,
 										key: objectKey,
diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js
index bbd21189f7..790a2a17e0 100644
--- a/test/tests/syncEngineTest.js
+++ b/test/tests/syncEngineTest.js
@@ -2480,7 +2480,89 @@ describe("Zotero.Sync.Data.Engine", function () {
 			
 			var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID);
 			assert.lengthOf(keys, 0);
-		})
+		});
+		
+		it("should handle local deletion and remote move to trash", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			({ engine, client, caller } = yield setup());
+			var type = 'item';
+			var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
+			var responseJSON = [];
+			
+			// Create object, generate JSON, and delete
+			var obj = yield createDataObject(type, { version: 10 });
+			var jsonData = obj.toJSON();
+			var key = jsonData.key = obj.key;
+			jsonData.version = 10;
+			let json = {
+				key: obj.key,
+				version: jsonData.version,
+				data: jsonData
+			};
+			yield obj.eraseTx();
+			
+			json.version = jsonData.version = 15;
+			jsonData.deleted = true;
+			responseJSON.push(json);
+			
+			setResponse({
+				method: "GET",
+				url: `users/1/items?format=json&itemKey=${key}&includeTrashed=1`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 15
+				},
+				json: responseJSON
+			});
+			
+			yield engine._downloadObjects('item', [key]);
+			
+			assert.isFalse(objectsClass.exists(libraryID, key));
+			
+			var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID);
+			assert.lengthOf(keys, 0);
+			
+			// Deletion should still be in sync delete log for uploading
+			assert.ok(yield Zotero.Sync.Data.Local.getDateDeleted('item', libraryID, key));
+		});
+		
+		it("should handle remote move to trash and local deletion", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			({ engine, client, caller } = yield setup());
+			var type = 'item';
+			var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
+			var responseJSON = [];
+			
+			// Create trashed object
+			var obj = createUnsavedDataObject(type);
+			obj.deleted = true;
+			yield obj.saveTx();
+			
+			setResponse({
+				method: "GET",
+				url: `users/1/deleted?since=10`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 15
+				},
+				json: {
+					collections: [],
+					searches: [],
+					items: [obj.key],
+				}
+			});
+			
+			yield engine._downloadDeletions(10, 15);
+			
+			// Local object should have been deleted
+			assert.isFalse(objectsClass.exists(libraryID, obj.key));
+			
+			var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID);
+			assert.lengthOf(keys, 0);
+			
+			// Deletion shouldn't be in sync delete log
+			assert.isFalse(yield Zotero.Sync.Data.Local.getDateDeleted('item', libraryID, obj.key));
+		});
 		
 		it("should handle note conflict", function* () {
 			var libraryID = Zotero.Libraries.userLibraryID;