From 7811d78cb027337414fc82b6d65d634c94f41c4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adomas=20Ven=C4=8Dkauskas?= Date: Fri, 12 Jan 2024 10:31:00 +0200 Subject: [PATCH] Fix quick search item tree issues when moving attachments into/out of parents Closes #3561 --- chrome/content/zotero/itemTree.jsx | 26 +++--- test/tests/itemTreeTest.js | 136 ++++++++++++++++++++++------- 2 files changed, 116 insertions(+), 46 deletions(-) diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 737eaa7b4e..985d806886 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -246,6 +246,11 @@ var ItemTree = class ItemTree extends LibraryTree { let row = this._rows[i]; // Top-level items if (row.level == 0) { + // A top-level attachment moved into a parent. Don't copy, it will be added + // via this loop for the parent item. + if (row.ref.parentID) { + continue; + } let isSearchParent = newSearchParentIDs.has(row.ref.id); // If not showing children or no children match the search, close if (this.regularOnly || !isSearchParent) { @@ -260,6 +265,12 @@ var ItemTree = class ItemTree extends LibraryTree { continue; } } + else if (row.level == 1 && !row.ref.parentID) { + // A child attachment moved into top-level. It needs to be added anew in a different + // location. + itemsToAdd.push(row.ref); + continue; + } // Child items else if (skipChildren) { continue; @@ -2391,21 +2402,6 @@ var ItemTree = class ItemTree extends LibraryTree { }); } } - - // If moving, remove items from source collection - if (dropEffect == 'move' && toMove.length) { - if (!sameLibrary) { - throw new Error("Cannot move items between libraries"); - } - if (!sourceCollectionTreeRow || !sourceCollectionTreeRow.isCollection()) { - throw new Error("Drag source must be a collection"); - } - if (collectionTreeRow.id != sourceCollectionTreeRow.id) { - await Zotero.DB.executeTransaction(async function () { - await collectionTreeRow.ref.removeItems(toMove); - }.bind(this)); - } - } } else if (dataType == 'text/x-moz-url' || dataType == 'application/x-moz-file') { // Disallow drop into read-only libraries diff --git a/test/tests/itemTreeTest.js b/test/tests/itemTreeTest.js index 0e3179de23..6d4072deb7 100644 --- a/test/tests/itemTreeTest.js +++ b/test/tests/itemTreeTest.js @@ -34,42 +34,116 @@ describe("Zotero.ItemTree", function() { }); describe("when performing a quick search", function () { - let parentItem, match, nonMatch; - let selectAllEvent = {key: 'a'}; - before(async function () { - parentItem = await createDataObject('item'); - match = await importFileAttachment('test.png', { title: 'find-me', parentItemID: parentItem.id }); - nonMatch = await importFileAttachment('test.png', { title: 'not-a-result', parentItemID: parentItem.id }); - if (Zotero.isMac) { - selectAllEvent.metaKey = true; - } else { - selectAllEvent.ctrlKey = true; - } + let quicksearch; + + before(() => { + quicksearch = win.document.getElementById('zotero-tb-search'); }); - - it("should not select non-matching children when issuing a Select All command", async function () { - var quicksearch = win.document.getElementById('zotero-tb-search'); - quicksearch.value = match.getField('title'); + after(async () => { + quicksearch.value = ""; quicksearch.doCommand(); await itemsView._refreshPromise; - itemsView.tree._onKeyDown(selectAllEvent); - - var selected = itemsView.getSelectedItems(true); - assert.lengthOf(selected, 1); - assert.equal(selected[0], match.id); }); - - it("should expand collapsed parents with matching children when issuing a Select All command", async function () { - itemsView.collapseAllRows(); - var selected = itemsView.getSelectedItems(true); - // After collapse the parent item is selected - assert.lengthOf(selected, 1); - assert.equal(selected[0], parentItem.id); + + describe("when issuing a Select All command", function () { + let parentItem, match; + let selectAllEvent = { key: 'a' }; - itemsView.tree._onKeyDown(selectAllEvent); - selected = itemsView.getSelectedItems(true); - assert.lengthOf(selected, 1); - assert.equal(selected[0], match.id); + before(async function () { + parentItem = await createDataObject('item'); + match = await importFileAttachment('test.png', { title: 'find-me', parentItemID: parentItem.id }); + await importFileAttachment('test.png', { title: 'not-a-result', parentItemID: parentItem.id }); + if (Zotero.isMac) { + selectAllEvent.metaKey = true; + } + else { + selectAllEvent.ctrlKey = true; + } + }); + + after(async function() { + await parentItem.erase(); + }); + + it("should not select non-matching children", async function () { + quicksearch.value = match.getField('title'); + quicksearch.doCommand(); + await itemsView._refreshPromise; + itemsView.tree._onKeyDown(selectAllEvent); + + var selected = itemsView.getSelectedItems(true); + assert.lengthOf(selected, 1); + assert.equal(selected[0], match.id); + }); + + it("should expand collapsed parents with matching children", async function () { + itemsView.collapseAllRows(); + var selected = itemsView.getSelectedItems(true); + // After collapse the parent item is selected + assert.lengthOf(selected, 1); + assert.equal(selected[0], parentItem.id); + + itemsView.tree._onKeyDown(selectAllEvent); + selected = itemsView.getSelectedItems(true); + assert.lengthOf(selected, 1); + assert.equal(selected[0], match.id); + }); + }); + + describe("when dragging attachments", function () { + let parentItem, childItem; + before(async () => { + parentItem = await createDataObject('item', { title: "match-parent" }); + childItem = await importFileAttachment('test.png', { title: 'match-child', parentItemID: parentItem.id }); + }); + + it("should display a child attachment when it is dragged into top level if it matches the search", async function () { + childItem.parentID = parentItem.id; + await childItem.save(); + + quicksearch.value = "match"; + quicksearch.doCommand(); + + await itemsView._refreshPromise; + assert.lengthOf(itemsView._rows, 2); + assert.equal(itemsView.getRow(0).id, parentItem.id); + assert.equal(itemsView.getRow(1).id, childItem.id); + assert.equal(itemsView.getRow(1).level, 1); + + // The drop effectively does this + childItem.parentID = false; + await childItem.save(); + await itemsView._refreshPromise; + + assert.lengthOf(itemsView._rows, 2); + assert.equal(itemsView.getRow(0).id, childItem.id); + assert.equal(itemsView.getRow(0).level, 0); + assert.equal(itemsView.getRow(1).id, parentItem.id); + }); + + it("should display a child attachment when it is dragged onto a parent item if it matches the search", async function () { + childItem.parentID = false; + await childItem.save(); + + quicksearch.value = "match"; + quicksearch.doCommand(); + + await itemsView._refreshPromise; + assert.lengthOf(itemsView._rows, 2); + assert.equal(itemsView.getRow(0).id, childItem.id); + assert.equal(itemsView.getRow(0).level, 0); + assert.equal(itemsView.getRow(1).id, parentItem.id); + + // The drop effectively does this + childItem.parentID = parentItem.id; + await childItem.save(); + await itemsView._refreshPromise; + + assert.lengthOf(itemsView._rows, 2); + assert.equal(itemsView.getRow(0).id, parentItem.id); + assert.equal(itemsView.getRow(1).id, childItem.id); + assert.equal(itemsView.getRow(1).level, 1); + }); }); });