From dd6e7e916e80c8fb4ca49ab98d0029078bdd6103 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 ba9f7fe3e5..0e05241b7b 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 8b2c5a3a83..d6b473a645 100644 --- a/test/tests/dataObjectTest.js +++ b/test/tests/dataObjectTest.js @@ -564,6 +564,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');