Fix collections in trash showing up in menus

New Collection menu and Add to Collection menu

https://forums.zotero.org/discussion/115447/zotero-7-beta-deleted-collections-still-appear-in-the-ui

chrome/content/zotero/xpcom/data/collection.js

@@ -194,18 +194,24 @@ Zotero.Collection.prototype.hasChildItems = function() {
  * Returns subcollections of this collection
  *
  * @param {Boolean} [asIDs=false] Return as collectionIDs
+ * @param {Boolean} [includeTrashed=false] - Include collections in the trash
  * @return {Zotero.Collection[]|Integer[]}
  */
-Zotero.Collection.prototype.getChildCollections = function (asIDs) {
+Zotero.Collection.prototype.getChildCollections = function (asIDs, includeTrashed) {
 	this._requireData('childCollections');
 	
+	var collections = [...this._childCollections].map(id => this.ObjectsClass.get(id));
+	if (!includeTrashed) {
+		collections = collections.filter(c => !c.deleted);
+	}
+	
 	// Return collectionIDs
 	if (asIDs) {
-		return [...this._childCollections.values()];
+		return collections.map(c => c.id);
 	}
 	
 	// Return Zotero.Collection objects
-	return Array.from(this._childCollections).map(id => this.ObjectsClass.get(id));
+	return collections;
 }
 
 
@@ -216,7 +222,7 @@ Zotero.Collection.prototype.getChildCollections = function (asIDs) {
  * @param	{Boolean}	includeDeleted	Include items in Trash
  * @return {Zotero.Item[]|Integer[]} - Array of Zotero.Item instances or itemIDs
  */
-Zotero.Collection.prototype.getChildItems = function (asIDs, includeDeleted) {
+Zotero.Collection.prototype.getChildItems = function (asIDs, includeTrashed) {
 	this._requireData('childItems');
 	
 	if (this._childItems.size == 0) {
@@ -227,7 +233,7 @@ Zotero.Collection.prototype.getChildItems = function (asIDs, includeDeleted) {
 	var childItems = [];
 	for (let itemID of this._childItems) {
 		let item = this.ChildObjects.get(itemID);
-		if (includeDeleted || !item.deleted) {
+		if (includeTrashed || !item.deleted) {
 			childItems.push(item);
 		}
 	}
@@ -840,11 +846,11 @@ Zotero.Collection.prototype.toJSON = function (options = {}) {
  * @param	{Boolean}	[nested=false]		Return multidimensional array with 'children'
  *											nodes instead of flat array
  * @param	{String}	[type]				'item', 'collection', or NULL for both
- * @param	{Boolean}	[includeDeletedItems=false]		Include items in Trash
+ * @param	{Boolean}	[includeTrashed=false]		Include collections and items in Trash
  * @return	{Object[]} - An array of objects with 'id', 'key', 'type' ('item' or 'collection'),
  *     'parent', and, if collection, 'name' and the nesting 'level'
  */
-Zotero.Collection.prototype.getDescendents = function (nested, type, includeDeletedItems, level) {
+Zotero.Collection.prototype.getDescendents = function (nested, type, includeTrashed, level) {
 	if (!this.id) {
 		throw new Error('Cannot be called on an unsaved item');
 	}
@@ -863,7 +869,7 @@ Zotero.Collection.prototype.getDescendents = function (nested, type, includeDele
 		}
 	}
 	
-	var collections = Zotero.Collections.getByParent(this.id);
+	var collections = Zotero.Collections.getByParent(this.id, false, includeTrashed);
 	var children = collections.map(c => ({
 		id: c.id,
 		name: c.name,
@@ -871,7 +877,7 @@ Zotero.Collection.prototype.getDescendents = function (nested, type, includeDele
 		key: c.key
 	}));
 	if (!type || type == 'item') {
-		let items = this.getChildItems(false, includeDeletedItems);
+		let items = this.getChildItems(false, includeTrashed);
 		children = children.concat(items.map(i => ({
 			id: i.id,
 			name: null,
@@ -902,7 +908,7 @@ Zotero.Collection.prototype.getDescendents = function (nested, type, includeDele
 				
 				let child = this.ObjectsClass.get(children[i].id);
 				let descendents = child.getDescendents(
-					nested, type, includeDeletedItems, level + 1
+					nested, type, includeTrashed, level + 1
 				);
 				
 				if (nested) {

chrome/content/zotero/xpcom/data/collections.js

@@ -65,10 +65,11 @@ Zotero.Collections = function() {
 	 *
 	 * @param {Integer} libraryID
 	 * @param {Boolean} [recursive=false]
+	 * @param {Boolean} [includeTrashed=false]
 	 * @return {Zotero.Collection[]}
 	 */
-	this.getByLibrary = function (libraryID, recursive) {
+	this.getByLibrary = function (libraryID, recursive, includeTrashed) {
-		return _getByContainer(libraryID, null, recursive);
+		return _getByContainer(libraryID, null, recursive, includeTrashed);
 	}
 	
 	
@@ -77,23 +78,24 @@ Zotero.Collections = function() {
 	 *
 	 * @param {Integer} parentCollectionID
 	 * @param {Boolean} [recursive=false]
+	 * @param {Boolean} [includeTrashed=false]
 	 * @return {Zotero.Collection[]}
 	 */
-	this.getByParent = function (parentCollectionID, recursive) {
+	this.getByParent = function (parentCollectionID, recursive, includeTrashed) {
-		return _getByContainer(null, parentCollectionID, recursive);
+		return _getByContainer(null, parentCollectionID, recursive, includeTrashed);
 	}
 	
 	
-	var _getByContainer = function (libraryID, parentID, recursive) {
+	var _getByContainer = function (libraryID, parentID, recursive, includeTrashed) {
 		let children = [];
 		
 		if (parentID) {
 			let parent = Zotero.Collections.get(parentID);
-			children = parent.getChildCollections();
+			children = parent.getChildCollections(false, includeTrashed);
 		} else if (libraryID) {
 			for (let id in this._objectCache) {
 				let c = this._objectCache[id];
-				if (c.libraryID == libraryID && !c.parentKey) {
+				if (c.libraryID == libraryID && !c.parentKey && (includeTrashed || !c.deleted)) {
 					c.level = 0;
 					children.push(c);
 				}
@@ -116,7 +118,7 @@ Zotero.Collections = function() {
 			var obj = children[i];
 			toReturn.push(obj);
 			
-			var descendants = obj.getDescendents(false, 'collection');
+			var descendants = obj.getDescendents(false, 'collection', includeTrashed);
 			for (let d of descendants) {
 				var obj2 = this.get(d.id);
 				if (!obj2) {

test/tests/collectionTest.js

@@ -285,6 +285,22 @@ describe("Zotero.Collection", function() {
 			assert.lengthOf(childCollections, 0);
 		})
 		
+		it("should not include collections in trash by default", async function () {
+			var collection1 = await createDataObject('collection');
+			var collection2 = await createDataObject('collection', { parentID: collection1.id, deleted: true });
+			
+			var childCollections = collection1.getChildCollections();
+			assert.lengthOf(childCollections, 0);
+		});
+		
+		it("should include collections in trash if includeTrashed=true", async function () {
+			var collection1 = await createDataObject('collection');
+			var collection2 = await createDataObject('collection', { parentID: collection1.id, deleted: true });
+			
+			var childCollections = collection1.getChildCollections(false, true);
+			assert.lengthOf(childCollections, 1);
+		});
+		
 		it("should not include collections that have been deleted", function* () {
 			var collection1 = yield createDataObject('collection');
 			var collection2 = yield createDataObject('collection', { parentID: collection1.id });

test/tests/collectionsTest.js

@@ -48,6 +48,24 @@ describe("Zotero.Collections", function () {
 			assert.equal(cols[5].level, 1);
 			assert.equal(cols[6].level, 0);
 		})
+		
+		it("should not include collections in trash", async function () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			var col = await createDataObject('collection', { deleted: true });
+			var cols = Zotero.Collections.getByLibrary(libraryID);
+			assert.notInclude(cols.map(c => c.id), col.id);
+		});
+		
+		it("should not include collections in trash in recursive mode", async function () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			var col1 = await createDataObject('collection');
+			var col2 = await createDataObject('collection', { parentID: col1.id, deleted: true });
+			var col3 = await createDataObject('collection', { parentID: col2.id });
+			var col4 = await createDataObject('collection', { parentID: col1.id });
+			var col5 = await createDataObject('collection', { parentID: col4.id, deleted: true });
+			var cols = Zotero.Collections.getByLibrary(libraryID, true);
+			assert.notIncludeMembers(cols.map(c => c.id), [col2.id, col3.id, col5.id]);
+		});
 	})
 	
 	describe("#getByParent()", function () {