From 3ab661b62809e6eecd7943e11a609452936c4209 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sun, 10 Jul 2022 17:31:01 -0400
Subject: [PATCH] Fix sorting by Item Type and feed item itemID

Regression from a8ed30ce803
---
 chrome/content/zotero/itemTree.jsx | 14 ++++++++------
 test/tests/itemTreeTest.js         | 11 ++++++++++-
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx
index db197b4b19..3a7cc38079 100644
--- a/chrome/content/zotero/itemTree.jsx
+++ b/chrome/content/zotero/itemTree.jsx
@@ -1825,6 +1825,10 @@ var ItemTree = class ItemTree extends LibraryTree {
 
 	getSortFields() {
 		var fields = [this.getSortField()];
+		if (!this._isValidSortField(fields[0])) {
+			Zotero.logError(`'${fields[0]}' is not a valid sort field -- skipping`);
+			fields.shift();
+		}
 		var secondaryField = this._getSecondarySortField();
 		if (secondaryField) {
 			fields.push(secondaryField);
@@ -1848,12 +1852,8 @@ var ItemTree = class ItemTree extends LibraryTree {
 			fallbackFields = Zotero.Prefs.get('fallbackSort').split(',');
 		}
 		fields = Zotero.Utilities.arrayUnique(fields.concat(fallbackFields));
-		var validFields = fields.filter(x => this._isValidSortField(x));
-		if (validFields.length) {
-			fields = validFields;
-		}
 		// If no valid fields, use default fallback
-		else {
+		if (!fields.length) {
 			Zotero.logError(`No valid fields in getSortFields() (${fields.join(',')}) `
 				+ '-- resetting');
 			Zotero.Prefs.clear('fallbackSort');
@@ -1873,7 +1873,9 @@ var ItemTree = class ItemTree extends LibraryTree {
 	}
 	
 	_isValidSortField(field) {
-		return field == 'year'
+		return field == 'itemType'
+			|| field == 'year'
+			|| field == 'id' // feeds
 			|| !!Zotero.ItemFields.getID(field)
 			|| Zotero.Items.primaryFields.includes(field);
 	}
diff --git a/test/tests/itemTreeTest.js b/test/tests/itemTreeTest.js
index a805c9201c..26ca09b2bf 100644
--- a/test/tests/itemTreeTest.js
+++ b/test/tests/itemTreeTest.js
@@ -152,7 +152,7 @@ describe("Zotero.ItemTree", function() {
 	})
 	
 	describe("#getSortFields()", function () {
-		before(async function () {
+		beforeEach(async function () {
 			itemsView = zp.itemsView;
 			
 			// Sort by title
@@ -183,6 +183,15 @@ describe("Zotero.ItemTree", function() {
 			// fallbackSort pref with title moved to beginning
 			assert.sameMembers(['title', 'firstCreator', 'date', 'dateAdded'], fields);
 		});
+		
+		it("should sort by item type", async function () {
+			// Sort by item type
+			const colIndex = itemsView.tree._getColumns().findIndex(column => column.dataKey == 'itemType');
+			await itemsView.tree._columns.toggleSort(colIndex);
+			
+			var fields = itemsView.getSortFields();
+			assert.sameMembers(['itemType', 'firstCreator', 'date', 'title', 'dateAdded'], fields);
+		});
 	});
 	
 	describe("#notify()", function () {