diff --git a/chrome/content/zotero/bindings/merge.xml b/chrome/content/zotero/bindings/merge.xml index bc41134c0..5747ff7af 100644 --- a/chrome/content/zotero/bindings/merge.xml +++ b/chrome/content/zotero/bindings/merge.xml @@ -380,10 +380,15 @@ // Store JSON this._data = val; + // Create a copy of the JSON that we can clean for display, since the remote object + // might reference things that don't exist locally + var displayJSON = Object.assign({}, val); + displayJSON.collections = []; + // Create item from JSON for metadata box var item = new Zotero.Item(val.itemType); item.libraryID = this.libraryID; - item.fromJSON(val); + item.fromJSON(displayJSON); objbox.item = item; ]]> diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 5c574bcf8..0c1058ffc 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3505,7 +3505,7 @@ Zotero.Item.prototype.setCollections = function (collectionIDsOrKeys) { var id = this.ContainerObjectsClass.getIDFromLibraryAndKey(this.libraryID, val); if (!id) { let e = new Error("Collection " + val + " not found for item " + this.libraryKey); - e.name = "ZoteroObjectNotFoundError"; + e.name = "ZoteroMissingObjectError"; throw e; } return id; diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index a62079d58..de8fe36a6 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -694,9 +694,9 @@ Zotero.Sync.Data.Engine.prototype._downloadObjects = async function (objectType, let results = await Zotero.Sync.Data.Local.processConflicts( objectType, this.libraryID, conflicts, this._getOptions() ); - // Keys can be unprocessed if conflict resolution is cancelled let keys = results.filter(x => x.processed).map(x => x.key); - if (!keys.length) { + // If all keys are unprocessed and didn't fail from an error, conflict resolution was cancelled + if (results.every(x => !x.processed && !x.error)) { throw new Zotero.Sync.UserCancelledException(); } await Zotero.Sync.Data.Local.removeObjectsFromSyncQueue(objectType, this.libraryID, keys); diff --git a/chrome/content/zotero/xpcom/sync/syncExceptions.js b/chrome/content/zotero/xpcom/sync/syncExceptions.js index 473e017fe..06f04be2e 100644 --- a/chrome/content/zotero/xpcom/sync/syncExceptions.js +++ b/chrome/content/zotero/xpcom/sync/syncExceptions.js @@ -30,7 +30,7 @@ * sync completely */ Zotero.Sync.UserCancelledException = function (advanceToNextLibrary) { - this.message = "Sync cancelled"; + this.message = "User cancelled sync"; this.advanceToNextLibrary = advanceToNextLibrary; } diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 4fbeec79a..e5f7d019f 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -2102,6 +2102,130 @@ describe("Zotero.Sync.Data.Engine", function () { assert.isTrue(Zotero.Items.exists(itemID2)); }) + + it("should handle new remote item referencing locally deleted collection", async function () { + var lastLibraryVersion = 5; + var newLibraryVersion = 6; + var library = Zotero.Libraries.userLibrary; + library.libraryVersion = lastLibraryVersion; + await library.saveTx(); + ({ engine, client, caller } = await setup({ + stopOnError: false + })); + + // Create local deleted collection + var collection = await createDataObject('collection'); + var collectionKey = collection.key; + await collection.eraseTx(); + // Create item JSON + var item = await createDataObject('item'); + var itemResponseJSON = item.toResponseJSON(); + // Add collection to remote item + itemResponseJSON.data.collections = [collectionKey]; + var itemKey = item.key; + await item.eraseTx({ + skipDeleteLog: true + }); + + var headers = { + "Last-Modified-Version": newLibraryVersion + }; + setDefaultResponses({ + lastLibraryVersion, + libraryVersion: newLibraryVersion + }); + setResponse({ + method: "GET", + url: `users/1/items?format=versions&since=${lastLibraryVersion}&includeTrashed=1`, + status: 200, + headers, + json: { + [itemKey]: newLibraryVersion + } + }); + setResponse({ + method: "GET", + url: `users/1/items?format=json&itemKey=${itemKey}&includeTrashed=1`, + status: 200, + headers, + json: [itemResponseJSON] + }); + + await engine._startDownload(); + + // Item should be skipped and added to queue, which will allow collection deletion to upload + var keys = await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', library.id); + assert.sameMembers(keys, [itemKey]); + }); + + + it("should handle conflict with remote item referencing deleted local collection", async function () { + var lastLibraryVersion = 5; + var newLibraryVersion = 6; + var library = Zotero.Libraries.userLibrary; + library.libraryVersion = lastLibraryVersion; + await library.saveTx(); + ({ engine, client, caller } = await setup({ + stopOnError: false + })); + + // Create local deleted collection and item + var collection = await createDataObject('collection'); + var collectionKey = collection.key; + await collection.eraseTx(); + var item = await createDataObject('item'); + var itemResponseJSON = item.toResponseJSON(); + // Add collection to remote item + itemResponseJSON.data.collections = [collectionKey]; + var itemKey = item.key; + await item.eraseTx(); + + var headers = { + "Last-Modified-Version": newLibraryVersion + }; + setDefaultResponses({ + lastLibraryVersion, + libraryVersion: newLibraryVersion + }); + setResponse({ + method: "GET", + url: `users/1/items?format=versions&since=${lastLibraryVersion}&includeTrashed=1`, + status: 200, + headers, + json: { + [itemKey]: newLibraryVersion + } + }); + setResponse({ + method: "GET", + url: `users/1/items?format=json&itemKey=${itemKey}&includeTrashed=1`, + status: 200, + headers, + json: [itemResponseJSON] + }); + + // This will trigger a conflict for a remote item that references a collection that doesn't + // exist. The collection should be removed for display in the CR window, and then the save + // should fail and the item should be added to the queue so that the collection deletion + // can upload. + waitForWindow('chrome://zotero/content/merge.xul', function (dialog) { + var doc = dialog.document; + var wizard = doc.documentElement; + var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0]; + + // 1 (accept remote) + //assert.equal(mergeGroup.leftpane.getAttribute('selected'), 'true'); + mergeGroup.rightpane.click(); + wizard.getButton('finish').click(); + }); + await engine._startDownload(); + + // Item should be skipped and added to queue, which will allow collection deletion to upload + var keys = await Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', library.id); + assert.sameMembers(keys, [itemKey]); + }); + + it("should handle cancellation of conflict resolution window", function* () { var library = Zotero.Libraries.userLibrary; library.libraryVersion = 5;