diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index f52e02af84..5a9f8e9645 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -848,114 +848,116 @@ var CollectionTree = class CollectionTree extends LibraryTree { } else if (action == 'modify') { let row; - let id = ids[0]; - let rowID = "C" + id; - let selectedIndex = this.selection.focused; - let handleFocusDuringSearch = async (type) => { - let object = type == 'collection' ? Zotero.Collections.get(id) : Zotero.Searches.get(id); - // If collections/searches are being filtered, some rows - // need to be (un-)greyed out or removed, so reload. - if (!this._isFilterEmpty()) { - let offset = 0; - if (!this._includedInTree(object, true)) { - offset = this._calculateOffsetForRowSelection(type[0].toUpperCase() + id); - } - await this.reload(); - this._selectAfterRowRemoval(selectedIndex - offset); - } - }; - - switch (type) { - case 'collection': - let collection = Zotero.Collections.get(id); - row = this.getRowIndexByID(rowID); - // If collection is visible - if (row !== false) { - // TODO: Only move if name changed - let reopen = this.isContainerOpen(row); - if (reopen) { - this._closeContainer(row); - } - this._removeRow(row); - // Collection was moved to trash, so don't add it back - if (collection.deleted) { - this._refreshRowMap(); - // If collection was selected, select next row - if (selectedIndex == row) { - this._selectAfterRowRemoval(selectedIndex); + for (let id of ids) { + let rowID = "C" + id; + let selectedIndex = this.selection.focused; + + let handleFocusDuringSearch = async (type) => { + let object = type == 'collection' ? Zotero.Collections.get(id) : Zotero.Searches.get(id); + // If collections/searches are being filtered, some rows + // need to be (un-)greyed out or removed, so reload. + if (!this._isFilterEmpty()) { + let offset = 0; + if (!this._includedInTree(object, true)) { + offset = this._calculateOffsetForRowSelection(type[0].toUpperCase() + id); } + await this.reload(); + this._selectAfterRowRemoval(selectedIndex - offset); } - else { - await this._addSortedRow('collection', id); - await this.selectByID(currentTreeRow.id); - if (reopen) { - let newRow = this.getRowIndexByID(rowID); - if (!this.isContainerOpen(newRow)) { - await this.toggleOpenState(newRow); + }; + + switch (type) { + case 'collection': + let collection = Zotero.Collections.get(id); + row = this.getRowIndexByID(rowID); + // If collection is visible + if (row !== false) { + // TODO: Only move if name changed + let reopen = this.isContainerOpen(row); + if (reopen) { + this._closeContainer(row); + } + this._removeRow(row); + // Collection was moved to trash, so don't add it back + if (collection.deleted) { + this._refreshRowMap(); + // If collection was selected, select next row + if (selectedIndex == row) { + this._selectAfterRowRemoval(selectedIndex); + } + } + else { + await this._addSortedRow('collection', id); + await this.selectByID(currentTreeRow.id); + if (reopen) { + let newRow = this.getRowIndexByID(rowID); + if (!this.isContainerOpen(newRow)) { + await this.toggleOpenState(newRow); + } + } } } - } - } - // If collection isn't currently visible and it isn't in the trash (because it was - // undeleted), add it (if possible without opening any containers) - else if (!collection.deleted) { - await this._addSortedRow('collection', id); - await this.selectByID(currentTreeRow.id); - // Invalidate parent in case it's become non-empty - let parentRow = this.getRowIndexByID("C" + collection.parentID); - if (parentRow !== false) { - this.tree.invalidateRow(parentRow); - } - } - await handleFocusDuringSearch('collection'); - break; - - case 'search': - let search = Zotero.Searches.get(id); - row = this.getRowIndexByID("S" + id); - if (row !== false) { - // TODO: Only move if name changed - this._removeRow(row); + // If collection isn't currently visible and it isn't in the trash (because it was + // undeleted), add it (if possible without opening any containers) + else if (!collection.deleted) { + await this._addSortedRow('collection', id); + await this.selectByID(currentTreeRow.id); + // Invalidate parent in case it's become non-empty + let parentRow = this.getRowIndexByID("C" + collection.parentID); + if (parentRow !== false) { + this.tree.invalidateRow(parentRow); + } + } + await handleFocusDuringSearch('collection'); + break; - // Search was moved to trash - if (search.deleted) { - this._refreshRowMap(); - // If search was selected, select next row - if (selectedIndex == row) { - this._selectAfterRowRemoval(selectedIndex); + case 'search': + let search = Zotero.Searches.get(id); + row = this.getRowIndexByID("S" + id); + if (row !== false) { + // TODO: Only move if name changed + this._removeRow(row); + + // Search was moved to trash + if (search.deleted) { + this._refreshRowMap(); + // If search was selected, select next row + if (selectedIndex == row) { + this._selectAfterRowRemoval(selectedIndex); + } + } + // If search isn't in trash, add it back + else { + await this._addSortedRow('search', id); + await this.selectByID(currentTreeRow.id); + } } - } - // If search isn't in trash, add it back - else { - await this._addSortedRow('search', id); + // If search isn't currently visible and it isn't in the trash (because it was + // undeleted), add it + else if (!search.deleted) { + await this._addSortedRow('search', id); + await this.selectByID(currentTreeRow.id); + // Invalidate parent in case it's become non-empty + // NOTE: Not currently used, because searches can't yet have parents + if (search.parentID) { + let parentRow = this.getRowIndexByID("S" + search.parentID); + if (parentRow !== false) { + this.tree.invalidateRow(parentRow); + } + } + } + await handleFocusDuringSearch('search'); + break; + + case 'feed': + break; + + default: + await this.reload(); await this.selectByID(currentTreeRow.id); - } + break; } - // If search isn't currently visible and it isn't in the trash (because it was - // undeleted), add it - else if (!search.deleted) { - await this._addSortedRow('search', id); - await this.selectByID(currentTreeRow.id); - // Invalidate parent in case it's become non-empty - // NOTE: Not currently used, because searches can't yet have parents - if (search.parentID) { - let parentRow = this.getRowIndexByID("S" + search.parentID); - if (parentRow !== false) { - this.tree.invalidateRow(parentRow); - } - } - } - await handleFocusDuringSearch('search'); - break; - - case 'feed': - break; - - default: - await this.reload(); - await this.selectByID(currentTreeRow.id); - break; } } else if(action == 'add') @@ -1258,12 +1260,16 @@ var CollectionTree = class CollectionTree extends LibraryTree { */ async deleteSelection(deleteItems) { var treeRow = this.getRow(this.selection.focused); - if (treeRow.isCollection() || treeRow.isFeed()) { - await treeRow.ref.eraseTx({ deleteItems }); + if (treeRow.isFeed()) { + await treeRow.ref.eraseTx(); + return; } - else if (treeRow.isSearch()) { - await Zotero.Searches.erase(treeRow.ref.id); + treeRow.ref.deleted = true; + if (treeRow.isCollection()) { + await treeRow.ref.saveTx({ deleteItems }); + return; } + await treeRow.ref.saveTx(); } unregister() { @@ -2739,7 +2745,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { var collections = treeRow.getChildren(); if (isLibrary) { - var savedSearches = await Zotero.Searches.getAll(libraryID); + var savedSearches = await Zotero.Searches.getAll(libraryID).filter(s => !s.deleted); // Virtual collections default to showing if not explicitly hidden var showDuplicates = this.props.hideSources.indexOf('duplicates') == -1 && this._virtualCollectionLibraries.duplicates[libraryID] !== false; @@ -2859,7 +2865,10 @@ var CollectionTree = class CollectionTree extends LibraryTree { if (showTrash && this._isFilterEmpty()) { let deletedItems = await Zotero.Items.getDeleted(libraryID, true); - if (deletedItems.length || Zotero.Prefs.get("showTrashWhenEmpty")) { + let deletedCollections = await Zotero.Collections.getDeleted(libraryID, true); + let deletedSearches = await Zotero.Searches.getDeleted(libraryID, true); + let trashNotEmpty = deletedItems.length || deletedCollections.length || deletedSearches.length; + if (trashNotEmpty || Zotero.Prefs.get("showTrashWhenEmpty")) { var ref = { libraryID: libraryID }; @@ -2867,7 +2876,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { new Zotero.CollectionTreeRow(this, 'trash', ref, level + 1)); newRows++; } - this._trashNotEmpty[libraryID] = !!deletedItems.length; + this._trashNotEmpty[libraryID] = trashNotEmpty; } return newRows; diff --git a/chrome/content/zotero/elements/itemPane.js b/chrome/content/zotero/elements/itemPane.js index 219f5bbd4b..f0e47fd307 100644 --- a/chrome/content/zotero/elements/itemPane.js +++ b/chrome/content/zotero/elements/itemPane.js @@ -112,7 +112,11 @@ if (this.data.length == 1) { let item = this.data[0]; - if (item.isNote()) { + // If a collection or search is selected, it must be in the trash. + if (item.isCollection() || item.isSearch()) { + renderStatus = this.renderMessage(); + } + else if (item.isNote()) { hideSidenav = true; renderStatus = this.renderNoteEditor(item); } @@ -218,6 +222,18 @@ // Display label in the middle of the item pane else { if (count) { + if (count == 1) { + let item = this.data[0]; + // If a collection or search is selected, it must be in the trash. + if (item.isCollection()) { + let subcollectionsCount = item.getDescendents(false, 'collection', true).length; + msg = Zotero.getString('pane.collections.deletedCollection', subcollectionsCount); + } + else if (item.isSearch()) { + msg = Zotero.getString('pane.collections.deletedSearch'); + } + } + msg = Zotero.getString('pane.item.selected.multiple', count); } else { diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 92a600eb2e..c6a761df22 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -103,7 +103,7 @@ var ItemTree = class ItemTree extends LibraryTree { this._unregisterID = Zotero.Notifier.registerObserver( this, - ['item', 'collection-item', 'item-tag', 'share-items', 'bucket', 'feedItem', 'search', 'itemtree'], + ['item', 'collection-item', 'item-tag', 'share-items', 'bucket', 'feedItem', 'search', 'itemtree', 'collection'], 'itemTreeView', 50 ); @@ -213,15 +213,22 @@ var ItemTree = class ItemTree extends LibraryTree { Zotero.CollectionTreeCache.clear(); // Get the full set of items we want to show let newSearchItems = await this.collectionTreeRow.getItems(); + if (this.collectionTreeRow.isTrash()) { + // When in trash, also fetch trashed collections and searched + // So that they are displayed among deleted items + newSearchItems = newSearchItems + .concat(await this.collectionTreeRow.getTrashedCollections()) + .concat(await Zotero.Searches.getDeleted(this.collectionTreeRow.ref.libraryID)); + } // TEMP: Hide annotations newSearchItems = newSearchItems.filter(item => !item.isAnnotation()); // Remove notes and attachments if necessary if (this.regularOnly) { - newSearchItems = newSearchItems.filter(item => item.isRegularItem()); + newSearchItems = newSearchItems.filter(item => item.isCollection() || item.isRegularItem()); } let newSearchItemIDs = new Set(newSearchItems.map(item => item.id)); // Find the items that aren't yet in the tree - let itemsToAdd = newSearchItems.filter(item => this._rowMap[item.id] === undefined); + let itemsToAdd = newSearchItems.filter(item => this._rowMap[item.treeViewID] === undefined); // Find the parents of search matches let newSearchParentIDs = new Set( this.regularOnly @@ -251,7 +258,7 @@ var ItemTree = class ItemTree extends LibraryTree { if (row.ref.parentID) { continue; } - let isSearchParent = newSearchParentIDs.has(row.ref.id); + let isSearchParent = newSearchParentIDs.has(row.ref.treeViewID); // If not showing children or no children match the search, close if (this.regularOnly || !isSearchParent) { row.isOpen = false; @@ -276,13 +283,13 @@ var ItemTree = class ItemTree extends LibraryTree { continue; } newRows.push(row); - allItemIDs.add(row.ref.id); + allItemIDs.add(row.ref.treeViewID); } // Add new items for (let i = 0; i < itemsToAdd.length; i++) { let item = itemsToAdd[i]; - + // If child item matches search and parent hasn't yet been added, add parent let parentItemID = item.parentItemID; if (parentItemID) { @@ -292,15 +299,15 @@ var ItemTree = class ItemTree extends LibraryTree { item = Zotero.Items.get(parentItemID); } // Parent item may have already been added from child - else if (allItemIDs.has(item.id)) { + else if (allItemIDs.has(item.treeViewID)) { continue; } // Add new top-level items let row = new ItemTreeRow(item, 0, false); newRows.push(row); - allItemIDs.add(item.id); - addedItemIDs.add(item.id); + allItemIDs.add(item.treeViewID); + addedItemIDs.add(item.treeViewID); } this._rows = newRows; @@ -377,10 +384,24 @@ var ItemTree = class ItemTree extends LibraryTree { return; } - if (type == 'search' && action == 'modify') { - // TODO: Only refresh on condition change (not currently available in extraData) - await this.refresh(); - return; + // If a collection with subcollections is deleted/restored, ids will include subcollections + // though they are not showing in itemTree. + // Filter subcollections out to treat it as single selected row + if (type == 'collection' && action == "modify") { + let deletedParents = new Set(); + let collections = []; + for (let id of ids) { + let collection = Zotero.Collections.get(id); + deletedParents.add(collection.key); + collections.push(collection); + } + ids = collections.filter(c => !c.parentKey || !deletedParents.has(c.parentKey)).map(c => c.id); + } + + // Add C or S prefix to match .treeViewID + if (type == 'collection' || type == 'search') { + let prefix = type == 'collection' ? 'C' : 'S'; + ids = ids.map(id => prefix + id); } // Clear item type icon and tag colors when a tag is added to or removed from an item @@ -559,7 +580,7 @@ var ItemTree = class ItemTree extends LibraryTree { madeChanges = true; } } - else if (type == 'item' && action == 'modify') + else if (['item', 'collection', 'search'].includes(type) && action == 'modify') { // Clear row caches for (const id of ids) { @@ -1741,7 +1762,7 @@ var ItemTree = class ItemTree extends LibraryTree { this.tree.invalidate(); // Create an array of selected items - var ids = Array.from(this.selection.selected).map(index => this.getRow(index).id); + var ids = Array.from(this.selection.selected).filter(index => this.getRow(index).ref.isItem()).map(index => this.getRow(index).id); var collectionTreeRow = this.collectionTreeRow; @@ -1749,7 +1770,25 @@ var ItemTree = class ItemTree extends LibraryTree { collectionTreeRow.ref.deleteItems(ids); } if (collectionTreeRow.isTrash()) { - await Zotero.Items.erase(ids); + let selectedObjects = Array.from(this.selection.selected).map(index => this.getRow(index).ref); + let [trashedCollectionIDs, trashedSearches] = [[], []]; + for (let obj of selectedObjects) { + if (obj.isCollection()) { + trashedCollectionIDs.push(obj.id); + } + if (obj.isSearch()) { + trashedSearches.push(obj.id); + } + } + if (trashedCollectionIDs.length > 0) { + await Zotero.Collections.erase(trashedCollectionIDs); + } + if (trashedSearches.length > 0) { + await Zotero.Searches.erase(trashedSearches); + } + if (ids.length > 0) { + await Zotero.Items.erase(ids); + } } else if (collectionTreeRow.isLibrary(true) || collectionTreeRow.isSearch() @@ -1790,7 +1829,7 @@ var ItemTree = class ItemTree extends LibraryTree { var items = this.selection ? Array.from(this.selection.selected) : []; items = items.filter(index => index < this._rows.length); try { - if (asIDs) return items.map(index => this.getRow(index).ref.id); + if (asIDs) return items.map(index => this.getRow(index).ref.treeViewID); return items.map(index => this.getRow(index).ref); } catch (e) { Zotero.debug(items); @@ -2774,8 +2813,15 @@ var ItemTree = class ItemTree extends LibraryTree { let itemTypeAriaLabel; try { - var itemType = Zotero.ItemTypes.getName(item.itemTypeID); - itemTypeAriaLabel = Zotero.getString(`itemTypes.${itemType}`) + '.'; + if (item.isSearch() || item.isCollection()) { + // Special treatment for trashed collections or searches since they are not an actual + // item and do not have an item type + itemTypeAriaLabel = Zotero.getString(`pane.collections.deleted${item._ObjectType}Aria`) + '.'; + } + else { + var itemType = Zotero.ItemTypes.getName(item.itemTypeID); + itemTypeAriaLabel = Zotero.getString(`itemTypes.${itemType}`) + '.'; + } } catch (e) { Zotero.debug('Error attempting to get a localized item type label for ' + itemType, 1); @@ -3000,7 +3046,8 @@ var ItemTree = class ItemTree extends LibraryTree { } if (!oldDiv) { - if (this.props.dragAndDrop) { + // No drag-drop for collections or searches in the trash + if (this.props.dragAndDrop && rowData.isItem) { div.setAttribute('draggable', true); div.addEventListener('dragstart', e => this.onDragStart(e, index), { passive: true }); div.addEventListener('dragover', e => this.onDragOver(e, index)); @@ -3128,7 +3175,7 @@ var ItemTree = class ItemTree extends LibraryTree { let row = {}; // Mark items not matching search as context rows, displayed in gray - if (this._searchMode && !this._searchItemIDs.has(itemID)) { + if (this._searchMode && !this._searchItemIDs.has(itemID) && treeRow.ref.isItem()) { row.contextRow = true; } @@ -3145,8 +3192,9 @@ var ItemTree = class ItemTree extends LibraryTree { row.unread = true; } - - row.itemType = Zotero.ItemTypes.getLocalizedString(treeRow.ref.itemTypeID); + if (!(treeRow.ref.isCollection() || treeRow.ref.isSearch())) { + row.itemType = Zotero.ItemTypes.getLocalizedString(treeRow.ref.itemTypeID); + } // Year column is just date field truncated row.year = treeRow.getField('date', true).substr(0, 4); if (row.year) { @@ -3200,7 +3248,7 @@ var ItemTree = class ItemTree extends LibraryTree { } row[key] = val; } - + row.isItem = treeRow.ref.isItem(); return this._rowCache[itemID] = row; } @@ -3774,6 +3822,24 @@ var ItemTree = class ItemTree extends LibraryTree { _getIcon(index) { var item = this.getRow(index).ref; + + // TEMP + if (item.isCollection() || item.isSearch()) { + let iconClsName; + if (item.isCollection()) { + iconClsName = "IconTreeitemCollection"; + } + if (item.isSearch()) { + iconClsName = "IconTreeitemSearch"; + } + var icon = getDOMElement(iconClsName); + if (!icon) { + Zotero.debug('Could not find tree icon for "' + itemType + '"'); + return document.createElement('span'); + } + return icon; + } + var itemType = item.getItemTypeIconName(); return getCSSItemTypeIcon(itemType); } @@ -3818,7 +3884,7 @@ var ItemTreeRow = function(ref, level, isOpen) this.ref = ref; //the item associated with this this.level = level; this.isOpen = isOpen; - this.id = ref.id; + this.id = ref.treeViewID; } ItemTreeRow.prototype.getField = function(field, unformatted) diff --git a/chrome/content/zotero/xpcom/collectionTreeRow.js b/chrome/content/zotero/xpcom/collectionTreeRow.js index 4f32ebceb9..6d76d0003e 100644 --- a/chrome/content/zotero/xpcom/collectionTreeRow.js +++ b/chrome/content/zotero/xpcom/collectionTreeRow.js @@ -269,6 +269,22 @@ Zotero.CollectionTreeRow.prototype.getChildren = function () { } } +// Returns the list of deleted collections in the trash. +// Subcollections of deleted collections are filtered out. +Zotero.CollectionTreeRow.prototype.getTrashedCollections = async function () { + if (!this.isTrash()) { + return []; + } + let deleted = await Zotero.Collections.getDeleted(this.ref.libraryID); + + let deletedParents = new Set(); + for (let d of deleted) { + deletedParents.add(d.key); + } + return deleted.filter(d => !d.parentKey || !deletedParents.has(d.parentKey)); +}; + + Zotero.CollectionTreeRow.prototype.getItems = Zotero.Promise.coroutine(function* () { switch (this.type) { diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 042bc68144..ffb91cde87 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -106,6 +106,8 @@ Zotero.Collection.prototype.getName = function() { return this.name; } +// Properties for a collection to "pretend" to be an item for trash itemTree +Object.assign(Zotero.Collection.prototype, Zotero.DataObjectUtilities.itemTreeMockProperties); /* * Populate collection data from a database row @@ -334,12 +336,28 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env) if (this._changedData.deleted !== undefined) { if (this._changedData.deleted) { - sql = "REPLACE INTO deletedCollections (collectionID) VALUES (?)"; + yield this.trash({ ...env, isNew: isNew }); } else { - sql = "DELETE FROM deletedCollections WHERE collectionID=?"; + let sql = "DELETE FROM deletedCollections WHERE collectionID=?"; + + // Subcollection is restored from trash - add it back into the object cache + if (this.parentKey) { + let parent = Zotero.Collections.getIDFromLibraryAndKey(this.libraryID, this.parentKey); + Zotero.DB.addCurrentCallback("commit", function () { + this.ObjectsClass.registerChildCollection(parent, this.id); + }.bind(this)); + } + + // Add restored collection back into item's _collections cache + this.getChildItems(false, true).forEach((item) => { + const collectionNotCached = item._collections.filter(c => c != this.id).length == 0; + if (collectionNotCached) { + item._collections.push(this.id); + } + }); + yield Zotero.DB.queryAsync(sql, collectionID); } - yield Zotero.DB.queryAsync(sql, collectionID); this._clearChanged('deleted'); this._markForReload('primaryData'); @@ -581,15 +599,14 @@ Zotero.Collection.prototype.clone = function (libraryID) { /** -* Deletes collection and all descendent collections (and optionally items) +* Moves the collection and all descendent collections (and optionally items) to trash **/ -Zotero.Collection.prototype._eraseData = Zotero.Promise.coroutine(function* (env) { +Zotero.Collection.prototype.trash = Zotero.Promise.coroutine(function* (env) { Zotero.DB.requireTransaction(); var collections = [this.id]; - var descendents = this.getDescendents(false, null, true); - var items = []; + var descendents = env.isNew ? [] : this.getDescendents(false, null, false); var libraryHasTrash = Zotero.Libraries.hasTrash(this.libraryID); var del = []; @@ -641,7 +658,74 @@ Zotero.Collection.prototype._eraseData = Zotero.Promise.coroutine(function* (env } } } + + yield Zotero.Utilities.Internal.forEachChunkAsync( + collections, + Zotero.DB.MAX_BOUND_PARAMETERS, + async function (chunk) { + // Send collection to trash + var placeholders = chunk.map(() => '(?)').join(','); + await Zotero.DB.queryAsync('INSERT OR IGNORE INTO deletedCollections (collectionID) VALUES ' + placeholders, chunk); + } + ); + + if (env.isNew) { + return; + } + env.deletedObjectIDs = collections; + // Reload collection data to show/restore deleted collections from trash + for (let collectionID of collections) { + let collection = Zotero.Collections.get(collectionID); + yield collection.loadDataType('primaryData', true); + yield collection.loadDataType('childCollections', true); + } + // Update collection cache for descendant items + if (itemsToUpdate.length) { + let deletedCollections = new Set(env.deletedObjectIDs); + itemsToUpdate.forEach((itemID) => { + let item = Zotero.Items.get(itemID); + item._collections = item._collections.filter(c => !deletedCollections.has(c)); + }); + } +}); + +/** +* Completely erases the collection and it's descendants. +**/ +Zotero.Collection.prototype._eraseData = Zotero.Promise.coroutine(function* (env) { + Zotero.DB.requireTransaction(); + + if (!this.deleted) { + yield this.trash(env); + } + + var collections = [this.id]; + var descendents = this.getDescendents(false, null, true) + .filter(d => d.type == 'collection') + .map(c => c.id); + collections = collections.concat(descendents); + + yield Zotero.Utilities.Internal.forEachChunkAsync( + collections, + Zotero.DB.MAX_BOUND_PARAMETERS, + async function (chunk) { + var placeholders = chunk.map(() => '?').join(','); + + // Remove item associations for all descendent collections + await Zotero.DB.queryAsync('DELETE FROM collectionItems WHERE collectionID IN ' + + '(' + placeholders + ')', chunk); + + // Remove parent definitions first for FK check + await Zotero.DB.queryAsync('UPDATE collections SET parentCollectionID=NULL ' + + 'WHERE parentCollectionID IN (' + placeholders + ')', chunk); + + // And delete all descendent collections + await Zotero.DB.queryAsync('DELETE FROM collections WHERE collectionID IN ' + + '(' + placeholders + ')', chunk); + } + ); + // Update child collection cache of parent collection if (this.parentKey) { let parentCollectionID = this.ObjectsClass.getIDFromLibraryAndKey( @@ -651,37 +735,6 @@ Zotero.Collection.prototype._eraseData = Zotero.Promise.coroutine(function* (env this.ObjectsClass.unregisterChildCollection(parentCollectionID, this.id); }.bind(this)); } - - yield Zotero.Utilities.Internal.forEachChunkAsync( - collections, - Zotero.DB.MAX_BOUND_PARAMETERS, - async function (chunk) { - var placeholders = chunk.map(() => '?').join(); - - // Remove item associations for all descendent collections - await Zotero.DB.queryAsync('DELETE FROM collectionItems WHERE collectionID IN ' - + '(' + placeholders + ')', chunk); - - // Remove parent definitions first for FK check - await Zotero.DB.queryAsync('UPDATE collections SET parentCollectionID=NULL ' - + 'WHERE parentCollectionID IN (' + placeholders + ')', chunk); - - // And delete all descendent collections - await Zotero.DB.queryAsync('DELETE FROM collections WHERE collectionID IN ' - + '(' + placeholders + ')', chunk); - } - ); - - env.deletedObjectIDs = collections; - - // Update collection cache for descendant items - if (itemsToUpdate.length) { - let deletedCollections = new Set(env.deletedObjectIDs); - itemsToUpdate.forEach(itemID => { - let item = Zotero.Items.get(itemID); - item._collections = item._collections.filter(c => !deletedCollections.has(c)); - }); - } }); Zotero.Collection.prototype._finalizeErase = Zotero.Promise.coroutine(function* (env) { @@ -690,10 +743,6 @@ Zotero.Collection.prototype._finalizeErase = Zotero.Promise.coroutine(function* yield Zotero.Libraries.get(this.libraryID).updateCollections(); }); -Zotero.Collection.prototype.isCollection = function() { - return true; -} - Zotero.Collection.prototype.serialize = function(nested) { var childCollections = this.getChildCollections(true); diff --git a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js index fc7c2569c2..9edd1c20e8 100644 --- a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js +++ b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js @@ -726,5 +726,45 @@ Zotero.DataObjectUtilities = { throw new Error("Unexpected change operation '" + c.op + "'"); } } + }, + + /** + * Methods shared by Zotero.Item, Zotero.Search and Zotero.Collection to allow + * collections and saved searches to "pretend" to be items in itemTree of the trash. + * Most of these are overriden by Zotero.Item. + */ + itemTreeMockProperties: { + isCollection: function () { + return this._ObjectType == "Collection"; + }, + isAnnotation: () => false, + isNote: () => false, + numNotes: () => 0, + isAttachment: () => false, + numAttachments: () => false, + getColoredTags: () => false, + isRegularItem: () => false, // Should be false to prevent items dropped into deleted searches + isSearch: function () { + return this._ObjectType == "Search"; + }, + getNotes: () => [], + getAttachments: () => [], + isFileAttachment: () => false, + isTopLevelItem: () => false, + isItem: function () { + return this._ObjectType == "Item"; + }, + getField: function (field, _) { + return this['_' + field] || ""; + }, + getDisplayTitle: function () { + return this.name; + }, + getBestAttachment: async function () { + return false; + }, + getBestAttachments: async function () { + return false; + } } }; diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index a3f3d94859..267a700a40 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -211,6 +211,10 @@ Zotero.Item.prototype._setParentKey = function() { Zotero.Item._super.prototype._setParentKey.apply(this, arguments); } +// Shared properties with Zotero.Collection and Zotero.Search to display them in trash +// along actual items +Object.assign(Zotero.Item.prototype, Zotero.DataObjectUtilities.itemTreeMockProperties); + ////////////////////////////////////////////////////////////////////////////// // // Public Zotero.Item methods @@ -4980,7 +4984,6 @@ Zotero.Item.prototype.isCollection = function() { return false; } - /** * Populate the object's data from an API JSON data object * diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js index 40d3d20e86..4f774e9293 100644 --- a/chrome/content/zotero/xpcom/data/search.js +++ b/chrome/content/zotero/xpcom/data/search.js @@ -109,6 +109,9 @@ Zotero.defineProperty(Zotero.Search.prototype, 'treeViewImage', { } }); +// Properties for a search to "pretend" to be an item for trash itemTree +Object.assign(Zotero.Search.prototype, Zotero.DataObjectUtilities.itemTreeMockProperties); + Zotero.Search.prototype.loadFromRow = function (row) { var primaryFields = this._ObjectsClass.primaryFields; for (let i=0; i item.id)); - let isSelected = itemOrID => (itemOrID.id ? selectedIDs.has(itemOrID.id) : selectedIDs.has(itemOrID)); + let isSelected = itemOrID => (itemOrID.treeViewID ? selectedIDs.includes(itemOrID.treeViewID) : selectedIDs.includes(itemOrID)); await Zotero.DB.executeTransaction(async () => { for (let row = 0; row < this.itemsView.rowCount; row++) { @@ -2403,9 +2402,17 @@ var ZoteroPane = new function() let parent = this.itemsView.getRow(row).ref; let children = []; - if (!parent.isNote()) children.push(...parent.getNotes(true)); - if (!parent.isAttachment()) children.push(...parent.getAttachments(true)); - + let subcollections = []; + if (parent.isCollection()) { + // If the restored item is a collection, restore its subcollections too + if (isSelected(parent)) { + subcollections = parent.getDescendents(false, 'collection', true).map(col => col.id); + } + } + else { + if (!parent.isNote()) children.push(...parent.getNotes(true)); + if (!parent.isAttachment()) children.push(...parent.getAttachments(true)); + } if (isSelected(parent)) { if (parent.deleted) { parent.deleted = false; @@ -2413,7 +2420,8 @@ var ZoteroPane = new function() } let noneSelected = !children.some(isSelected); - for (let child of Zotero.Items.get(children)) { + let allChildren = Zotero.Items.get(children).concat(Zotero.Collections.get(subcollections)); + for (let child of allChildren) { if ((noneSelected || isSelected(child)) && child.deleted) { child.deleted = false; await child.save(); @@ -2448,6 +2456,10 @@ var ZoteroPane = new function() if (result) { Zotero.showZoteroPaneProgressMeter(null, true); try { + let deletedSearches = yield Zotero.Searches.getDeleted(libraryID, true); + yield Zotero.Searches.erase(deletedSearches); + let deletedCollections = yield Zotero.Collections.getDeleted(libraryID, true); + yield Zotero.Collections.erase(deletedCollections); let deleted = yield Zotero.Items.emptyTrash( libraryID, { @@ -3882,6 +3894,14 @@ var ZoteroPane = new function() else { menu.childNodes[m.showInLibrary].setAttribute('label', Zotero.getString('general.showInLibrary')); } + // For collections and search, only keep restore/delete options + if (items.some(item => item.isCollection() || item.isSearch())) { + for (let option of options) { + if (!['restoreToLibrary', 'deleteFromLibrary'].includes(option)) { + show.delete(m[option]); + } + } + } // Set labels, plural if necessary menu.childNodes[m.findPDF].setAttribute('label', Zotero.getString('pane.items.menu.findAvailablePDF' + multiple)); diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index 21628f3163..7fee47f9c5 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -276,6 +276,10 @@ pane.collections.retracted = Retracted Items pane.collections.duplicate = Duplicate Items pane.collections.removeLibrary = Remove Library pane.collections.removeLibrary.text = Are you sure you want to permanently remove “%S” from this computer? +pane.collections.deletedCollection = Deleted collection with %1$S subcollections +pane.collections.deletedCollectionAria = Deleted collection +pane.collections.deletedSearch = Deleted search +pane.collections.deletedSearchAria = Deleted search pane.collections.menu.duplicate.savedSearch = Duplicate Saved Search pane.collections.menu.remove.library = Remove Library… diff --git a/test/tests/collectionTest.js b/test/tests/collectionTest.js index c19dede231..3699e9671a 100644 --- a/test/tests/collectionTest.js +++ b/test/tests/collectionTest.js @@ -76,6 +76,58 @@ describe("Zotero.Collection", function() { assert.notInclude(deleted, collection2.key); assert.notInclude(deleted, collection3.key); }); + + it("should send deleted collections to trash", async function () { + var collection1 = await createDataObject('collection'); + var collection2 = await createDataObject('collection', { parentID: collection1.id }); + var collection3 = await createDataObject('collection', { parentID: collection2.id }); + + collection1.deleted = true; + await collection1.saveTx(); + + var deleted = await Zotero.Collections.getDeleted(collection1.libraryID, true); + + assert.include(deleted, collection1.id); + assert.include(deleted, collection2.id); + assert.include(deleted, collection3.id); + }); + + it("should restore deleted collection", async function () { + var collection1 = await createDataObject('collection'); + var item1 = await createDataObject('item', { collections: [collection1.id] }); + + assert.include(item1.getCollections(), collection1.id); + + collection1.deleted = true; + await collection1.saveTx(); + + // Deleted collection is gone from item's cache + assert.notInclude(item1.getCollections(), collection1.id); + + // Restore deleted collection + collection1.deleted = false; + await collection1.saveTx(); + + var deleted = await Zotero.Collections.getDeleted(collection1.libraryID, true); + + // Collection is restored from trash + assert.notInclude(deleted, collection1.id); + + // Collection is back in item's cache + assert.include(item1.getCollections(), collection1.id); + }); + + it("should permanently delete collections from trash", async function () { + var collection1 = await createDataObject('collection'); + var collection2 = await createDataObject('collection', { parentID: collection1.id }); + var collection3 = await createDataObject('collection', { parentID: collection2.id }); + + await collection1.eraseTx(); + + assert.equal(await Zotero.Collections.getAsync(collection1.id), false); + assert.equal(await Zotero.Collections.getAsync(collection2.id), false); + assert.equal(await Zotero.Collections.getAsync(collection3.id), false); + }); }) describe("#version", function () { diff --git a/test/tests/collectionTreeTest.js b/test/tests/collectionTreeTest.js index 92902198fd..ba64a7fc03 100644 --- a/test/tests/collectionTreeTest.js +++ b/test/tests/collectionTreeTest.js @@ -212,6 +212,101 @@ describe("Zotero.CollectionTree", function() { }) }) + describe("Trash for collections/searches", function () { + var one, two, three; + for (let objectType of ['collection', 'search']) { + it(`should remove deleted ${objectType} from collectionTree`, async function () { + var ran = Zotero.Utilities.randomString(); + one = await createDataObject(objectType, { name: ran + "_DELETE_ONE" }); + two = await createDataObject(objectType, { name: ran + "_DELETE_TWO" }); + three = await createDataObject(objectType, { name: ran + "_DELETE_THREE" }); + + // Move them to trash + one.deleted = true; + two.deleted = true; + three.deleted = true; + await one.saveTx(); + await two.saveTx(); + await three.saveTx(); + + // Make sure they're gone from collectionTree + assert.isFalse(cv.getRowIndexByID(one.treeViewID)); + assert.isFalse(cv.getRowIndexByID(two.treeViewID)); + assert.isFalse(cv.getRowIndexByID(three.treeViewID)); + }) + it(`should put restored ${objectType} back into collectionTree`, async function () { + await cv.selectByID("T" + userLibraryID); + await waitForItemsLoad(win); + + // Restore + await Zotero.DB.executeTransaction(async function () { + one.deleted = false; + two.deleted = false; + three.deleted = false; + await one.save({ skipSelect: true }); + await two.save({ skipSelect: true }); + await three.save({ skipSelect: true }); + }); + + // Check if trash is still selected + let trashRow = cv.getRowIndexByID("T" + userLibraryID); + assert.equal(cv.selection.focused, trashRow); + + // Check if restored entries are back in collectionTree + assert.isNumber(cv.getRowIndexByID(one.treeViewID)); + assert.isNumber(cv.getRowIndexByID(two.treeViewID)); + assert.isNumber(cv.getRowIndexByID(three.treeViewID)); + // Make sure it's all gone from trash + assert.isFalse(zp.itemsView.getRowIndexByID(one.treeViewID)); + assert.isFalse(zp.itemsView.getRowIndexByID(two.treeViewID)); + assert.isFalse(zp.itemsView.getRowIndexByID(three.treeViewID)); + }); + } + + it(`should delete subcollections when parent is deleted`, async function () { + var ran = Zotero.Utilities.randomString(); + one = await createDataObject('collection', { name: ran + "_DELETE_ONE" }); + two = await createDataObject('collection', { name: ran + "_DELETE_TWO", parentID: one.id }); + three = await createDataObject('collection', { name: ran + "_DELETE_THREE", parentID: two.id }); + + // Select top parent + cv.selection.select(cv.getRowIndexByID(one.treeViewID)); + // Move parent to trash + await cv.deleteSelection(); + + // Make sure they're gone from collectionTree + assert.isFalse(cv.getRowIndexByID(one.treeViewID)); + assert.isFalse(cv.getRowIndexByID(two.treeViewID)); + assert.isFalse(cv.getRowIndexByID(three.treeViewID)); + }) + + it(`should restore deleted subcollections with parent`, async function () { + await cv.selectByID("T" + userLibraryID); + + // Restore items + await waitForItemsLoad(win); + zp.itemsView.selectItem(one.treeViewID); + await zp.restoreSelectedItems(); + + // Check if trash is still selected + let trashRow = cv.getRowIndexByID("T" + userLibraryID); + assert.equal(cv.selection.focused, trashRow); + + // Check if restored collections are back in collectionTree + let parentRowIndex = cv.getRowIndexByID(one.treeViewID); + await cv.toggleOpenState(parentRowIndex); + assert.equal(cv.getRow(parentRowIndex).level, 1); + await Zotero.Promise.delay(5000); + let middleRowIndex = cv.getRowIndexByID(two.treeViewID); + assert.equal(cv.getRow(middleRowIndex).level, 2); + let bottomRowindex = cv.getRowIndexByID(three.treeViewID); + assert.equal(cv.getRow(bottomRowindex).level, 3); + + await waitForItemsLoad(win); + //Make sure they're gone from trash + assert.isFalse(zp.itemsView.getRowIndexByID(one.treeViewID)); + }); + }); describe("#notify()", function () { it("should select a new collection", function* () { // Create collection diff --git a/test/tests/itemTreeTest.js b/test/tests/itemTreeTest.js index f2e549c75e..5f0f1114a7 100644 --- a/test/tests/itemTreeTest.js +++ b/test/tests/itemTreeTest.js @@ -758,6 +758,7 @@ describe("Zotero.ItemTree", function() { }); describe("Trash", function () { + var one, two, three; it("should remove untrashed parent item when last trashed child is deleted", function* () { var userLibraryID = Zotero.Libraries.userLibraryID; var item = yield createDataObject('item'); @@ -770,8 +771,74 @@ describe("Zotero.ItemTree", function() { var promise = waitForDialog(); yield zp.emptyTrash(); yield promise; + // Small delay for modal to close and notifications to go through + // otherwise, next publications tab does not get opened + yield Zotero.Promise.delay(100); assert.equal(zp.itemsView.rowCount, 0); }); + + it("should show only top-most trashed collection", async function() { + var userLibraryID = Zotero.Libraries.userLibraryID; + var ran = Zotero.Utilities.randomString(); + var objectType = "collection"; + one = await createDataObject(objectType, { name: ran + "_DELETE_ONE" }); + two = await createDataObject(objectType, { name: ran + "_DELETE_TWO", parentID: one.id }); + three = await createDataObject(objectType, { name: ran + "_DELETE_THREE", parentID: two.id }); + + one.deleted = true; + await one.saveTx(); + + // Go to trash + await zp.collectionsView.selectByID("T" + userLibraryID); + await waitForItemsLoad(win); + + // Make sure only top-level collection shows + assert.isNumber(itemsView.getRowIndexByID(one.treeViewID)); + assert.isFalse(itemsView.getRowIndexByID(two.treeViewID)); + assert.isFalse(itemsView.getRowIndexByID(three.treeViewID)); + }) + + it("should restore all subcollections when parent is restored", async function() { + var userLibraryID = Zotero.Libraries.userLibraryID; + // Go to trash + await zp.collectionsView.selectByID("T" + userLibraryID); + await waitForItemsLoad(win); + + // Restore + await itemsView.selectItem(one.treeViewID); + await zp.restoreSelectedItems(); + + // Make sure it's gone from trash + assert.isFalse(zp.itemsView.getRowIndexByID(one.treeViewID)); + assert.isFalse(zp.itemsView.getRowIndexByID(two.treeViewID)); + assert.isFalse(zp.itemsView.getRowIndexByID(three.treeViewID)); + + // Make sure it shows up back in collectionTree + assert.isNumber(zp.collectionsView.getRowIndexByID(one.treeViewID)); + }) + + for (let objectType of ['collection', 'search']) { + it(`should remove ${objectType} from trash on delete`, async function (){ + var userLibraryID = Zotero.Libraries.userLibraryID; + var ran = Zotero.Utilities.randomString(); + one = await createDataObject(objectType, { name: ran + "_DELETE_ONE", deleted: true }); + two = await createDataObject(objectType, { name: ran + "_DELETE_TWO", deleted: true }); + three = await createDataObject(objectType, { name: ran + "_DELETE_THREE", deleted: true }); + + // Go to trash + await zp.collectionsView.selectByID("T" + userLibraryID); + await waitForItemsLoad(win); + + // Permanently delete + await itemsView.selectItems([one.treeViewID, two.treeViewID, three.treeViewID]); + await itemsView.deleteSelection(); + + // Make sure it's gone from trash + assert.isFalse(zp.itemsView.getRowIndexByID(one.treeViewID)); + assert.isFalse(zp.itemsView.getRowIndexByID(two.treeViewID)); + assert.isFalse(zp.itemsView.getRowIndexByID(three.treeViewID)); + }) + } }); describe("My Publications", function () { diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js index e6f3891239..2f554015c6 100644 --- a/test/tests/searchTest.js +++ b/test/tests/searchTest.js @@ -661,6 +661,13 @@ describe("Zotero.Search", function() { await search.saveTx(); assert.isFalse(search.deleted); }); + it("should permanently delete", async function () { + var search = await createDataObject('search'); + assert.isFalse(search.deleted); + await search.eraseTx(); + search = await Zotero.Searches.getAsync(search.id); + assert.isFalse(search); + }); }); describe("#toJSON()", function () { diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 6a57d05193..4f1a0d24b1 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -898,7 +898,7 @@ describe("ZoteroPane", function() { let iv = zp.itemsView; assert.ok(await iv.selectItem(item.id)); - await Zotero.Promise.delay(1); + await Zotero.Promise.delay(100); let promise = waitForDialog(); let modifyPromise = waitForItemEvent('modify'); @@ -916,22 +916,22 @@ describe("ZoteroPane", function() { }); describe("#deleteSelectedCollection()", function () { - it("should delete collection but not descendant items by default", function* () { + it("should move collection to trash but not descendant items by default", function* () { var collection = yield createDataObject('collection'); var item = yield createDataObject('item', { collections: [collection.id] }); var promise = waitForDialog(); yield zp.deleteSelectedCollection(); - assert.isFalse(Zotero.Collections.exists(collection.id)); + assert.isTrue(collection.deleted); assert.isTrue(Zotero.Items.exists(item.id)); assert.isFalse(item.deleted); }); - it("should delete collection and descendant items when deleteItems=true", function* () { + it("should move to trash collection and descendant items when deleteItems=true", function* () { var collection = yield createDataObject('collection'); var item = yield createDataObject('item', { collections: [collection.id] }); var promise = waitForDialog(); yield zp.deleteSelectedCollection(true); - assert.isFalse(Zotero.Collections.exists(collection.id)); + assert.isTrue(collection.deleted); assert.isTrue(Zotero.Items.exists(item.id)); assert.isTrue(item.deleted); });