www

commit d5b2f67afa184f68d7edd466deed24154b0b2fd7
parent 9077b2f4951c337b34b70fbd08657a97f0e91e0a
Author: Dan Stillman <dstillman@zotero.org>
Date:   Sat, 30 Jul 2016 23:03:30 -0400

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.

Diffstat:
M
chrome/content/zotero/xpcom/sync/syncEngine.js
 | 
8
++++++++
M
chrome/content/zotero/xpcom/sync/syncLocal.js
 | 
10
++++++++++
M
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
@@ -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
@@ -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
@@ -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;