From 48a3235a2ee91284bebbb7115612cd69b4fdf8cd Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Sun, 20 Feb 2022 08:45:53 -0800 Subject: [PATCH] Disable Delete/Restore menu items appropriately (#2340) --- chrome/content/zotero/itemTree.jsx | 11 +- chrome/content/zotero/zoteroPane.js | 121 ++++++++++--- chrome/locale/en-US/zotero/zotero.properties | 3 +- test/tests/zoteroPaneTest.js | 175 +++++++++++++++++++ 4 files changed, 280 insertions(+), 30 deletions(-) diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 3efa466069..e451391c71 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -1843,8 +1843,17 @@ var ItemTree = class ItemTree extends LibraryTree { */ isSelectable = (index, selectAll=false) => { if (!selectAll || !this._searchMode || this.collectionTreeRow.isPublications()) return true; + let row = this.getRow(index); - return row && this._searchItemIDs.has(row.id); + if (!row) { + return false; + } + if (this.collectionTreeRow.isTrash()) { + return row.ref.deleted; + } + else { + return this._searchItemIDs.has(row.id); + } } isContainer = (index) => { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index a2b529db42..8f0efe6ddb 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -1688,6 +1688,31 @@ var ZoteroPane = new function() return newItem; }); + + /** + * Return whether every selected item can be deleted from the current + * collection context (library, trash, collection, etc.). + * + * @return {Boolean} + */ + this.canDeleteSelectedItems = function () { + let collectionTreeRow = this.getCollectionTreeRow(); + if (collectionTreeRow.isTrash()) { + for (let index of this.itemsView.selection.selected) { + while (index != -1 && !this.itemsView.getRow(index).ref.deleted) { + index = this.itemsView.getParentIndex(index); + } + if (index == -1) { + return false; + } + } + } + else if (collectionTreeRow.isShare()) { + return false; + } + return true; + }; + this.deleteSelectedItem = function () { Zotero.debug("ZoteroPane_Local.deleteSelectedItem() is deprecated -- use ZoteroPane_Local.deleteSelectedItems()"); @@ -1730,6 +1755,10 @@ var ZoteroPane = new function() 'pane.items.remove' + (this.itemsView.selection.count > 1 ? '.multiple' : '') ) }; + + if (!this.canDeleteSelectedItems()) { + return; + } if (collectionTreeRow.isPublications()) { let toRemoveFromPublications = { @@ -1757,22 +1786,9 @@ var ZoteroPane = new function() var prompt = force ? toTrash : toRemove; } - // Do nothing in trash view if any non-deleted items are selected - else if (collectionTreeRow.isTrash()) { - for (const index of this.itemsView.selection.selected) { - if (!this.itemsView.getRow(index).ref.deleted) { - return; - } - } + else if (collectionTreeRow.isTrash() || collectionTreeRow.isBucket()) { var prompt = toDelete; } - else if (collectionTreeRow.isBucket()) { - var prompt = toDelete; - } - // Do nothing in share views - else if (collectionTreeRow.isShare()) { - return; - } var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] .getService(Components.interfaces.nsIPromptService); @@ -1893,26 +1909,71 @@ var ZoteroPane = new function() } this.selectItem(item.id).done(); } + + + /** + * Check whether every selected item can be restored from trash + * + * @return {Boolean} + */ + this.canRestoreSelectedItems = function () { + let collectionTreeRow = this.getCollectionTreeRow(); + if (!collectionTreeRow.isTrash()) { + return false; + } + + return this.getSelectedItems().some(item => item.deleted); + }; /** * @return {Promise} */ - this.restoreSelectedItems = Zotero.Promise.coroutine(function* () { - var items = this.getSelectedItems(); - if (!items) { + this.restoreSelectedItems = async function () { + let items = this.getSelectedItems(); + if (!items.length) { return; } - - yield Zotero.DB.executeTransaction(function* () { - for (let i=0; i item.id)); + let isSelected = itemOrID => (itemOrID.id ? selectedIDs.has(itemOrID.id) : selectedIDs.has(itemOrID)); + + await Zotero.DB.executeTransaction(async () => { + for (let row = 0; row < this.itemsView.rowCount; row++) { + // Only look at top-level items + if (this.itemsView.getLevel(row) !== 0) { + continue; + } + + let parent = this.itemsView.getRow(row).ref; + + if (isSelected(parent)) { + if (parent.deleted) { + parent.deleted = false; + await parent.save(); + } + + let children = [...parent.getNotes(true), ...parent.getAttachments(true)]; + let noneSelected = !children.some(isSelected); + for (let child of Zotero.Items.get(children)) { + if ((noneSelected || isSelected(child)) && child.deleted) { + child.deleted = false; + await child.save(); + } + } + } + else { + let children = [...parent.getNotes(true), ...parent.getAttachments(true)]; + for (let child of Zotero.Items.get(children)) { + if (isSelected(child) && child.deleted) { + child.deleted = false; + await child.save(); + } + } + } } - }.bind(this)); - }); + }); + }; /** @@ -2721,6 +2782,12 @@ var ZoteroPane = new function() if (isTrash) { show.add(m.deleteFromLibrary); show.add(m.restoreToLibrary); + if (!ZoteroPane_Local.canDeleteSelectedItems()) { + disable.add(m.deleteFromLibrary); + } + if (!ZoteroPane_Local.canRestoreSelectedItems()) { + disable.add(m.restoreToLibrary); + } } else if (!collectionTreeRow.isFeed()) { show.add(m.moveToTrash); @@ -3053,7 +3120,7 @@ var ZoteroPane = new function() menu.childNodes[m.createNoteFromAnnotationsMenu].setAttribute('label', Zotero.getString('pane.items.menu.addNoteFromAnnotations' + multiple)); menu.childNodes[m.findPDF].setAttribute('label', Zotero.getString('pane.items.menu.findAvailablePDF' + multiple)); menu.childNodes[m.moveToTrash].setAttribute('label', Zotero.getString('pane.items.menu.moveToTrash' + multiple)); - menu.childNodes[m.deleteFromLibrary].setAttribute('label', Zotero.getString('pane.items.menu.delete' + multiple)); + menu.childNodes[m.deleteFromLibrary].setAttribute('label', Zotero.getString('pane.items.menu.delete')); menu.childNodes[m.exportItems].setAttribute('label', Zotero.getString(`pane.items.menu.export${noteExport ? 'Note' : ''}` + multiple)); menu.childNodes[m.createBib].setAttribute('label', Zotero.getString('pane.items.menu.createBib' + multiple)); menu.childNodes[m.loadReport].setAttribute('label', Zotero.getString('pane.items.menu.generateReport' + multiple)); diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index 924978b208..76b9f5f575 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -334,8 +334,7 @@ pane.items.menu.removeFromPublications = Remove Item from My Publications… pane.items.menu.removeFromPublications.multiple = Remove Items from My Publications… pane.items.menu.moveToTrash = Move Item to Trash… pane.items.menu.moveToTrash.multiple = Move Items to Trash… -pane.items.menu.delete = Delete Item… -pane.items.menu.delete.multiple = Delete Items… +pane.items.menu.delete = Delete Permanently… pane.items.menu.export = Export Item… pane.items.menu.export.multiple = Export Items… pane.items.menu.exportNote = Export Note… diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index d4b775515e..6f552027a0 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -832,5 +832,180 @@ describe("ZoteroPane", function() { Zotero.getString('pane.items.menu.exportNote.multiple') ); }); + + it("should enable “Delete Item…” when selected item or an ancestor is in trash", async function () { + var item1 = await createDataObject('item', { deleted: true }); + var attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + + await zp.selectItems([attachment1.id]); + await zp.buildItemContextMenu(); + var menu = win.document.getElementById('zotero-itemmenu'); + var deleteMenuItem = menu.querySelector('.zotero-menuitem-delete-from-lib'); + assert.isFalse(deleteMenuItem.disabled); + + await zp.selectItems([item1.id, attachment1.id]); + await zp.buildItemContextMenu(); + assert.isFalse(deleteMenuItem.disabled); + + item1.deleted = false; + attachment1.deleted = true; + await item1.saveTx(); + await attachment1.saveTx(); + await zp.buildItemContextMenu(); + assert.isTrue(deleteMenuItem.disabled); + }); + + it("should enable “Restore to Library” when at least one selected item is in trash", async function () { + var item1 = await createDataObject('item', { deleted: true }); + var attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + + await zp.selectItems([item1.id]); + await zp.buildItemContextMenu(); + var menu = win.document.getElementById('zotero-itemmenu'); + var restoreMenuItem = menu.querySelector('.zotero-menuitem-restore-to-library'); + assert.isFalse(restoreMenuItem.disabled); + + await zp.selectItems([item1.id, attachment1.id]); + await zp.buildItemContextMenu(); + assert.isFalse(restoreMenuItem.disabled); + }); + + it("should disable “Restore to Library” when no selected items are in trash", async function () { + var item1 = await createDataObject('item'); + var attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + + await zp.selectItems([item1.id]); + await zp.buildItemContextMenu(); + var menu = win.document.getElementById('zotero-itemmenu'); + var restoreMenuItem = menu.querySelector('.zotero-menuitem-restore-to-library'); + assert.isTrue(restoreMenuItem.disabled); + }); + }); + + describe("#restoreSelectedItems()", function () { + it("should restore trashed parent and single trashed child when both are selected", async function () { + let item1 = await createDataObject('item', { deleted: true }); + let attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + await zp.selectItems([item1.id, attachment1.id]); + await zp.restoreSelectedItems(); + + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + }); + + it("should restore child when parent and trashed child are selected", async function () { + let item1 = await createDataObject('item', { deleted: false }); + let attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + await zp.selectItems([item1.id, attachment1.id]); + await zp.restoreSelectedItems(); + + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + }); + + it("should restore parent and selected children when parent and some trashed children are selected", async function () { + let item1 = await createDataObject('item', { deleted: false }); + let attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment2 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + attachment2.deleted = true; + await attachment2.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + await zp.selectItems([item1.id, attachment1.id]); + await zp.restoreSelectedItems(); + + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + assert.isTrue(attachment2.deleted); + }); + + it("should restore parent and all children when trashed parent and no children are selected", async function () { + let item1 = await createDataObject('item', { deleted: true }); + let attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment2 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment3 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + attachment2.deleted = true; + await attachment2.saveTx(); + attachment3.deleted = true; + await attachment3.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + await zp.selectItems([item1.id]); + await zp.restoreSelectedItems(); + + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + assert.isFalse(attachment2.deleted); + assert.isFalse(attachment3.deleted); + }); + + it("should restore parent and selected children when trashed parent and some trashed children are selected", async function () { + let item1 = await createDataObject('item', { deleted: true }); + let attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment2 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment3 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + attachment2.deleted = true; + await attachment2.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + await zp.selectItems([item1.id, attachment2.id, attachment3.id]); + await zp.restoreSelectedItems(); + + assert.isFalse(item1.deleted); + assert.isTrue(attachment1.deleted); + assert.isFalse(attachment2.deleted); + assert.isFalse(attachment3.deleted); + }); + + it("should restore selected children when trashed children and untrashed children are selected", async function () { + let item1 = await createDataObject('item', { deleted: false }); + let attachment1 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment2 = await importFileAttachment('test.png', { parentItemID: item1.id }); + let attachment3 = await importFileAttachment('test.png', { parentItemID: item1.id }); + attachment1.deleted = true; + await attachment1.saveTx(); + attachment2.deleted = true; + await attachment2.saveTx(); + + var userLibraryID = Zotero.Libraries.userLibraryID; + await zp.collectionsView.selectByID('T' + userLibraryID); + await zp.selectItems([attachment1.id, attachment2.id, attachment3.id]); + await zp.restoreSelectedItems(); + + assert.isFalse(item1.deleted); + assert.isFalse(attachment1.deleted); + assert.isFalse(attachment2.deleted); + assert.isFalse(attachment3.deleted); + }); }); })