Disable Delete/Restore menu items appropriately (#2340)

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) => {

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<items.length; i++) {
-				items[i].deleted = false;
-				yield items[i].save({
-					skipDateModifiedUpdate: true
-				});
+
+		let selectedIDs = new Set(items.map(item => 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));

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…

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);
+		});
 	});
 })