From 2dd16b44d654669255463cb7342055708b92c593 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 20 Jul 2021 23:11:20 -0400 Subject: [PATCH] Ignore items in target library trash for cross-library drags Just don't consider items in the trash to be linked items in item.getLinkedItem(). I'm not totally sure why we didn't do this many years ago, since it's one of the biggest sources of confusion in Zotero. This addresses #1648 and closes #1610, though not by undeleting and overwriting the item in the trash. When deleting an item and then re-dragging it from another library, I think most people would expect the item in the trash to still exist (possibly with notes, etc.), rather than having been automatically restored and overwritten with new data. --- chrome/content/zotero/xpcom/collectionTreeView.js | 11 +---------- chrome/content/zotero/xpcom/data/dataObject.js | 8 ++++++++ test/tests/dataObjectTest.js | 13 +++++++++++++ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index e786224137..b299d7a758 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -1872,7 +1872,7 @@ Zotero.CollectionTreeView.prototype.canDropCheckAsync = Zotero.Promise.coroutine // Cross-library drag if (treeRow.ref.libraryID != item.libraryID) { let linkedItem = yield item.getLinkedItem(treeRow.ref.libraryID, true); - if (linkedItem && !linkedItem.deleted) { + if (linkedItem) { // For drag to root, skip if linked item exists if (treeRow.isLibrary(true)) { Zotero.debug("Linked item " + linkedItem.key + " already exists " @@ -1974,15 +1974,6 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r // Check if there's already a copy of this item in the library var linkedItem = yield item.getLinkedItem(targetLibraryID, true); if (linkedItem) { - // If linked item is in the trash, undelete it and remove it from collections - // (since it shouldn't be restored to previous collections) - if (linkedItem.deleted) { - linkedItem.setCollections(); - linkedItem.deleted = false; - yield linkedItem.save({ - skipSelect: true - }); - } return linkedItem.id; /* diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index ca87a58a3e..daf88df9fa 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -516,6 +516,10 @@ Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function + "in Zotero." + this._ObjectType + "::getLinked" + this._ObjectType + "()", 2); continue; } + // Ignore items in the trash + if (obj.objectType == 'item' && obj.deleted) { + continue; + } return obj; } } @@ -534,6 +538,10 @@ Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function continue; } if (obj.libraryID == libraryID) { + // Ignore items in the trash + if (obj.objectType == 'item' && obj.deleted) { + continue; + } return obj; } } diff --git a/test/tests/dataObjectTest.js b/test/tests/dataObjectTest.js index 460fd3f2de..feffd8a462 100644 --- a/test/tests/dataObjectTest.js +++ b/test/tests/dataObjectTest.js @@ -573,6 +573,19 @@ describe("Zotero.DataObject", function() { assert.equal(linkedItem.id, item2.id); }) + it("shouldn't return a linked item in the trash in another library", async function () { + var group = await getGroup(); + var item1 = await createDataObject('item'); + var item2 = await createDataObject('item', { libraryID: group.libraryID }); + var item2URI = Zotero.URI.getItemURI(item2); + + await item2.addLinkedItem(item1); + item2.deleted = true; + await item2.saveTx(); + var linkedItem = await item1.getLinkedItem(item2.libraryID); + assert.isFalse(linkedItem); + }) + it("shouldn't return reverse linked objects by default", function* () { var group = yield getGroup(); var item1 = yield createDataObject('item');