Show saved searches under "Collection" search condition

With icons to identify collections and searches

Also:

- `savedSearch` search condition in general
- Clean up some search window code
- Reorganize search tests

chrome/content/zotero/bindings/zoterosearch.xml

@@ -425,68 +425,63 @@
 							var rows = [];
 							
 							var libraryID = this.parent.search.libraryID;
-							var cols = Zotero.Collections.getByLibrary(libraryID, true);
+							
-							for (var i in cols) {
+							// Add collections
+							let cols = Zotero.Collections.getByLibrary(libraryID, true);
+							for (let col of cols) {
 								// Indent subcollections
 								var indent = '';
-								if (cols[i].level) {
+								if (col.level) {
-									for (var j=1; j<cols[i].level; j++) {
+									for (let j = 1; j < col.level; j++) {
 										indent += '    ';
 									}
 									indent += '- ';
 								}
-								rows.push([indent + cols[i].name, 'C' + cols[i].key]);
+								rows.push({
+									name: indent + col.name,
+									value: 'C' + col.key,
+									image: Zotero.Collection.prototype.treeViewImage
+								});
 							}
-							this.createValueMenu(rows);
-							break;
 							
-						case 'savedSearch':
+							// Add saved searches
-							var rows = [];
+							let searches = Zotero.Searches.getByLibrary(libraryID);
-							var libraryID = this.parent.search.libraryID;
+							for (let search of searches) {
-							var searches = Zotero.Searches.getAll(libraryID);
+								if (search.id != this.parent.search.id) {
-							for (var i in searches) {
+									rows.push({
-								if (searches[i].id != this.parent.search.id) {
+										name: search.name,
-									rows.push([searches[i].name, 'S' + searches[i].key]);
+										value: 'S' + search.key,
+										image: Zotero.Search.prototype.treeViewImage
+									});
 								}
 							}
 							this.createValueMenu(rows);
 							break;
 						
 						case 'itemType':
-							var t = Zotero.ItemTypes.getTypes();
+							var rows = Zotero.ItemTypes.getTypes().map(type => ({
+								name: Zotero.ItemTypes.getLocalizedString(type.id),
+								value: type.name
+							}));
 							
 							// Sort by localized name
-							var types = [];
-							for (var i=0; i<t.length; i++) {
-								types.push({
-									id: t[i].id,
-									name: t[i].name,
-									localized: Zotero.ItemTypes.getLocalizedString(t[i].id)
-								});
-							}
 							var collation = Zotero.getLocaleCollation();
-							types.sort(function(a, b) {
+							rows.sort((a, b) => collation.compareString(1, a.name, b.name));
-								return collation.compareString(1, a.localized, b.localized);
-							});
 							
-							for (var i in types) {
+							this.createValueMenu(rows);
-								types[i][0] = types[i].localized;
-								types[i][1] = types[i].name;
-								delete types[i]['name'];
-								delete types[i]['id'];
-							}
-							this.createValueMenu(types);
 							break;
 						
 						case 'fileTypeID':
-							var types = Zotero.FileTypes.getTypes();
+							var rows = Zotero.FileTypes.getTypes().map(type => ({
-							for (var i in types) {
+								name: Zotero.getString('fileTypes.' + type.name),
-								types[i][0] = Zotero.getString('fileTypes.' + types[i]['name']);
+								value: type.id
-								types[i][1] = types[i]['id'];
+							}));
-								delete types[i]['name'];
+							
-								delete types[i]['id'];
+							// Sort by localized name
-							}
+							var collation = Zotero.getLocaleCollation();
-							this.createValueMenu(types);
+							rows.sort((a, b) => collation.compareString(1, a.name, b.name));
+							
+							this.createValueMenu(rows);
 							break;
 						
 						default:
@@ -522,7 +517,6 @@
 					
 					// Drop-down menu
 					if (conditionsMenu.value == 'collection'
-							|| conditionsMenu.value == 'savedSearch'
 							|| conditionsMenu.value == 'itemType'
 							|| conditionsMenu.value == 'fileTypeID') {
 						this.id('valuefield').hidden = true;
@@ -557,19 +551,21 @@
 				</body>
 			</method>
 			<method name="createValueMenu">
-				<parameter name="values"/>
+				<parameter name="rows"/>
 				<body>
 					<![CDATA[
 						while (this.id('valuemenu').hasChildNodes()){
 							this.id('valuemenu').removeChild(this.id('valuemenu').firstChild);
 						}
 						
-						if (values.length){
+						for (let row of rows) {
-							for (var i in values){
+							let menuitem = this.id('valuemenu').appendItem(row.name, row.value);
-								this.id('valuemenu').appendItem(values[i][0], values[i][1]);
+							if (row.image) {
+								menuitem.className = 'menuitem-iconic';
+								menuitem.setAttribute('image', row.image);
+							}
 						}
 						this.id('valuemenu').selectedIndex = 0;
-						}
 						
 						if (this.value)
 						{
@@ -601,7 +597,15 @@
 								break;
 						}
 						
-						this.id('conditionsmenu').value = condition.condition;
+						// Map certain conditions to other menu items
+						var uiCondition = condition.condition;
+						switch (condition.condition) {
+							case 'savedSearch':
+								uiCondition = 'collection';
+								break;
+						}
+						
+						this.id('conditionsmenu').value = uiCondition;
 						
 						// Convert datetimes from UTC to localtime
 						if ((condition['condition']=='accessDate' ||
@@ -664,8 +668,7 @@
 							
 							// Handle special C1234 and S5678 form for
 							// collections and searches
-							else if (this.id('conditionsmenu').value == 'collection' || this.id('conditionsmenu').value == 'savedSearch')
+							else if (condition == 'collection') {
-							{
 								var letter = this.id('valuemenu').value.substr(0,1);
 								if (letter=='C')
 								{

chrome/content/zotero/xpcom/collectionTreeView.js

@@ -766,6 +766,7 @@ Zotero.CollectionTreeView.prototype.getImageSrc = function(row, col)
 		
 		case 'collection':
 		case 'search':
+			// Keep in sync with Zotero.(Collection|Search).prototype.treeViewImage
 			if (Zotero.isMac) {
 				return "chrome://zotero-platform/content/treesource-" + collectionType + ".png";
 			}

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

@@ -91,6 +91,9 @@ Zotero.defineProperty(Zotero.Collection.prototype, 'treeViewID', {
 
 Zotero.defineProperty(Zotero.Collection.prototype, 'treeViewImage', {
 	get: function () {
+		if (Zotero.isMac) {
+			return "chrome://zotero-platform/content/treesource-collection.png";
+		}
 		return "chrome://zotero/skin/treesource-collection" + Zotero.hiDPISuffix + ".png";
 	}
 });

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

@@ -91,6 +91,7 @@ Zotero.Collections = function() {
 			for (let id in this._objectCache) {
 				let c = this._objectCache[id];
 				if (c.libraryID == libraryID && !c.parentKey) {
+					c.level = 0;
 					children.push(c);
 				}
 			}
@@ -112,11 +113,11 @@ Zotero.Collections = function() {
 			var obj = children[i];
 			toReturn.push(obj);
 			
-			var desc = obj.getDescendents(false, 'collection');
+			var descendants = obj.getDescendents(false, 'collection');
-			for (var j in desc) {
+			for (let d of descendants) {
-				var obj2 = this.get(desc[j].id);
+				var obj2 = this.get(d.id);
 				if (!obj2) {
-					throw new Error('Collection ' + desc[j] + ' not found');
+					throw new Error('Collection ' + d.id + ' not found');
 				}
 				
 				// TODO: This is a quick hack so that we can indent subcollections
@@ -125,7 +126,7 @@ Zotero.Collections = function() {
 				// of calculating that without either storing it in the DB or
 				// changing the schema to Modified Preorder Tree Traversal,
 				// and I don't know if we'll actually need it anywhere else.
-				obj2.level = desc[j].level;
+				obj2.level = d.level;
 				
 				toReturn.push(obj2);
 			}

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

@@ -92,6 +92,20 @@ Zotero.defineProperty(Zotero.Search.prototype, '_canHaveParent', {
 	value: false
 });
 
+Zotero.defineProperty(Zotero.Search.prototype, 'treeViewID', {
+	get: function () {
+		return "S" + this.id
+	}
+});
+
+Zotero.defineProperty(Zotero.Search.prototype, 'treeViewImage', {
+	get: function () {
+		if (Zotero.isMac) {
+			return "chrome://zotero-platform/content/treesource-search.png";
+		}
+		return "chrome://zotero/skin/treesource-search" + Zotero.hiDPISuffix + ".png";
+	}
+});
 
 Zotero.Search.prototype.loadFromRow = function (row) {
 	var primaryFields = this._ObjectsClass.primaryFields;
@@ -1184,9 +1198,10 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 							condSQL += "collectionID IN (" + q.join() + ")";
 							condSQLParams = condSQLParams.concat(p);
 						}
+						// Saved search
 						else {
 							// Check if there are any post-search filters
-							var hasFilter = search.hasPostSearchFilter();
+							var hasFilter = obj.hasPostSearchFilter();
 							
 							// This is an ugly and inefficient way of doing a
 							// subsearch, but it's necessary if there are any
@@ -1197,13 +1212,18 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 							// or that this slows things down with large libraries
 							// -- should probably use a temporary table instead
 							if (hasFilter){
-								let subids = yield search.search();
+								let subids = yield obj.search();
 								condSQL += subids.join();
 							}
 							// Otherwise just put the SQL in a subquery
 							else {
-								condSQL += yield search.getSQL();
+								condSQL += "itemID ";
-								let subpar = yield search.getSQLParams();
+								if (condition.operator == 'isNot') {
+									condSQL += "NOT ";
+								}
+								condSQL += "IN (";
+								condSQL += yield obj.getSQL();
+								let subpar = yield obj.getSQLParams();
 								for (let k in subpar){
 									condSQLParams.push(subpar[k]);
 								}

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

@@ -239,7 +239,7 @@ Zotero.SearchConditions = new function(){
 					is: true,
 					isNot: true
 				},
-				special: false
+				special: true
 			},
 			
 			{

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

@@ -47,6 +47,24 @@ Zotero.Searches = function() {
 	});
 	
 	
+	this.getByLibrary = function (libraryID) {
+		var searches = [];
+		for (let id in this._objectCache) {
+			let s = this._objectCache[id];
+			if (s.libraryID == libraryID) {
+				searches.push(s);
+			}
+		}
+		
+		// Do proper collation sort
+		var collation = Zotero.getLocaleCollation();
+		searches.sort(function (a, b) {
+			return collation.compareString(1, a.name, b.name);
+		});
+		return searches;
+	};
+	
+	
 	/**
 	 * Returns an array of Zotero.Search objects, ordered by name
 	 *

test/tests/advancedSearchTest.js

@@ -4,6 +4,10 @@ describe("Advanced Search", function () {
 	var win, zp;
 	
 	before(function* () {
+		yield resetDB({
+			thisArg: this,
+			skipBundledFiles: true
+		});
 		win = yield loadZoteroPane();
 		zp = win.ZoteroPane;
 	});
@@ -40,5 +44,141 @@ describe("Advanced Search", function () {
 		assert.isNumber(index);
 		
 		searchWin.close();
+		
+		item.eraseTx();
+	});
+	
+	describe("Conditions", function () {
+		var searchWin, searchBox, conditions;
+		
+		before(function* () {
+			var promise = waitForWindow('chrome://zotero/content/advancedSearch.xul');
+			zp.openAdvancedSearchWindow();
+			searchWin = yield promise;
+			searchBox = searchWin.document.getElementById('zotero-search-box');
+			conditions = searchBox.id('conditions');
+		});
+		
+		after(function () {
+			searchWin.close();
+		});
+		
+		describe("Collection", function () {
+			it("should show collections and saved searches", function* () {
+				var col1 = yield createDataObject('collection', { name: "A" });
+				var col2 = yield createDataObject('collection', { name: "C", parentID: col1.id });
+				var col3 = yield createDataObject('collection', { name: "D", parentID: col2.id });
+				var col4 = yield createDataObject('collection', { name: "B" });
+				var search1 = yield createDataObject('search', { name: "A" });
+				var search2 = yield createDataObject('search', { name: "B" });
+				
+				// Add condition
+				var s = new Zotero.Search();
+				s.addCondition('title', 'is', '');
+				searchBox.search = s;
+				
+				var searchCondition = conditions.firstChild;
+				var conditionsMenu = searchCondition.id('conditionsmenu');
+				var valueMenu = searchCondition.id('valuemenu');
+				
+				assert.isTrue(valueMenu.hidden);
+				// Select 'Collection' condition
+				for (let i = 0; i < conditionsMenu.itemCount; i++) {
+					let menuitem = conditionsMenu.getItemAtIndex(i);
+					if (menuitem.value == 'collection') {
+						menuitem.click();
+						break;
+					}
+				}
+				
+				assert.isFalse(valueMenu.hidden);
+				assert.equal(valueMenu.itemCount, 6);
+				var valueMenuItem = valueMenu.getItemAtIndex(1);
+				assert.equal(valueMenuItem.getAttribute('label'), "- " + col2.name);
+				assert.equal(valueMenuItem.getAttribute('value'), "C" + col2.key);
+				valueMenuItem = valueMenu.getItemAtIndex(2);
+				assert.equal(valueMenuItem.getAttribute('label'), "    - " + col3.name);
+				assert.equal(valueMenuItem.getAttribute('value'), "C" + col3.key);
+				valueMenuItem = valueMenu.getItemAtIndex(4);
+				assert.equal(valueMenuItem.getAttribute('label'), search1.name);
+				assert.equal(valueMenuItem.getAttribute('value'), "S" + search1.key);
+				valueMenuItem = valueMenu.getItemAtIndex(5);
+				assert.equal(valueMenuItem.getAttribute('label'), search2.name);
+				assert.equal(valueMenuItem.getAttribute('value'), "S" + search2.key);
+				
+				yield Zotero.Collections.erase([col1.id, col2.id, col3.id, col4.id]);
+				yield Zotero.Searches.erase([search1.id, search2.id]);
+			});
+			
+			it("should be selected for 'savedSearch' condition", function* () {
+				var search = yield createDataObject('search', { name: "A" });
+				
+				var s = new Zotero.Search();
+				s.addCondition('savedSearch', 'is', search.key);
+				searchBox.search = s;
+				
+				var searchCondition = conditions.firstChild;
+				var conditionsMenu = searchCondition.id('conditionsmenu');
+				var valueMenu = searchCondition.id('valuemenu');
+				
+				assert.equal(conditionsMenu.selectedItem.value, 'collection');
+				assert.isFalse(valueMenu.hidden);
+				assert.equal(valueMenu.selectedItem.value, "S" + search.key);
+				
+				yield search.eraseTx();
+			});
+			
+			it("should set 'savedSearch' condition when a search is selected", function* () {
+				var collection = yield createDataObject('collection', { name: "A" });
+				var search = yield createDataObject('search', { name: "B" });
+				
+				var s = new Zotero.Search();
+				s.addCondition('title', 'is', '');
+				searchBox.search = s;
+				
+				var searchCondition = conditions.firstChild;
+				var conditionsMenu = searchCondition.id('conditionsmenu');
+				var valueMenu = searchCondition.id('valuemenu');
+				
+				// Select 'Collection' condition
+				for (let i = 0; i < conditionsMenu.itemCount; i++) {
+					let menuitem = conditionsMenu.getItemAtIndex(i);
+					if (menuitem.value == 'collection') {
+						menuitem.click();
+						break;
+					}
+				}
+				for (let i = 0; i < valueMenu.itemCount; i++) {
+					let menuitem = valueMenu.getItemAtIndex(i);
+					if (menuitem.getAttribute('value') == "S" + search.key) {
+						menuitem.click();
+						break;
+					}
+				}
+				
+				searchBox.updateSearch();
+				var condition = searchBox.search.getConditions()[0];
+				assert.equal(condition.condition, 'savedSearch');
+				assert.equal(condition.value, search.key);
+				
+				yield collection.eraseTx();
+				yield search.eraseTx();
+			});
+		});
+		
+		describe("Saved Search", function () {
+			it("shouldn't appear", function* () {
+				var searchCondition = conditions.firstChild;
+				var conditionsMenu = searchCondition.id('conditionsmenu');
+				
+				// Make sure "Saved Search" isn't present
+				for (let i = 0; i < conditionsMenu.itemCount; i++) {
+					let menuitem = conditionsMenu.getItemAtIndex(i);
+					if (menuitem.value == 'savedSearch') {
+						assert.fail();
+					}
+				}
+			});
+		});
 	});
 });

test/tests/collectionsTest.js

@@ -38,6 +38,15 @@ describe("Zotero.Collections", function () {
 			assert.isBelow(ids.indexOf(col7.id), ids.indexOf(col6.id), "F before G");
 			assert.isBelow(ids.indexOf(col6.id), ids.indexOf(col5.id), "G before D sibling E");
 			assert.isBelow(ids.indexOf(col5.id), ids.indexOf(col1.id), "E before A sibling C");
+			
+			// 'level' property, which is a hack for indenting in the advanced search window
+			assert.equal(cols[0].level, 0);
+			assert.equal(cols[1].level, 1);
+			assert.equal(cols[2].level, 1);
+			assert.equal(cols[3].level, 2);
+			assert.equal(cols[4].level, 2);
+			assert.equal(cols[5].level, 1);
+			assert.equal(cols[6].level, 0);
 		})
 	})
 	

test/tests/searchTest.js

@@ -105,6 +105,8 @@ describe("Zotero.Search", function() {
 			yield foobarItem.eraseTx();
 		});
 		
+		describe("Conditions", function () {
+			describe("collection", function () {
 				it("should find item in collection", function* () {
 					var col = yield createDataObject('collection');
 					var item = yield createDataObject('item', { collections: [col.id] });
@@ -116,6 +118,17 @@ describe("Zotero.Search", function() {
 					assert.sameMembers(matches, [item.id]);
 				});
 				
+				it("should find items not in collection", function* () {
+					var col = yield createDataObject('collection');
+					var item = yield createDataObject('item', { collections: [col.id] });
+					
+					var s = new Zotero.Search();
+					s.libraryID = item.libraryID;
+					s.addCondition('collection', 'isNot', col.key);
+					var matches = yield s.search();
+					assert.notInclude(matches, item.id);
+				});
+				
 				it("shouldn't find item in collection with no items", function* () {
 					var col = yield createDataObject('collection');
 					var item = yield createDataObject('item');
@@ -139,7 +152,18 @@ describe("Zotero.Search", function() {
 					var matches = yield s.search();
 					assert.sameMembers(matches, [item.id]);
 				});
+			});
 			
+			describe("fileTypeID", function () {
+				it("should search by attachment file type", function* () {
+					let s = new Zotero.Search();
+					s.addCondition('fileTypeID', 'is', Zotero.FileTypes.getID('webpage'));
+					let matches = yield s.search();
+					assert.sameMembers(matches, [fooItem.id, foobarItem.id]);
+				});
+			});
+			
+			describe("fulltextWord", function () {
 				it("should return matches with full-text conditions", function* () {
 					let s = new Zotero.Search();
 					s.addCondition('fulltextWord', 'contains', 'foo');
@@ -181,12 +205,33 @@ describe("Zotero.Search", function() {
 					let matches = yield s.search();
 					assert.deepEqual(matches, [foobarItem.id]);
 				});
+			});
 			
-		it("should search by attachment file type", function* () {
+			describe("savedSearch", function () {
-			let s = new Zotero.Search();
+				it("should return items in the saved search", function* () {
-			s.addCondition('fileTypeID', 'is', Zotero.FileTypes.getID('webpage'));
+					var search = yield createDataObject('search');
-			let matches = yield s.search();
+					var itemTitle = search.getConditions()[0].value;
-			assert.sameMembers(matches, [fooItem.id, foobarItem.id]);
+					var item = yield createDataObject('item', { title: itemTitle })
+					
+					var s = new Zotero.Search;
+					s.libraryID = Zotero.Libraries.userLibraryID;
+					s.addCondition('savedSearch', 'is', search.key);
+					var matches = yield s.search();
+					assert.deepEqual(matches, [item.id]);
+				});
+				
+				it("should return items not in the saved search for isNot operator", function* () {
+					var search = yield createDataObject('search');
+					var itemTitle = search.getConditions()[0].value;
+					var item = yield createDataObject('item', { title: itemTitle })
+					
+					var s = new Zotero.Search;
+					s.libraryID = Zotero.Libraries.userLibraryID;
+					s.addCondition('savedSearch', 'isNot', search.key);
+					var matches = yield s.search();
+					assert.notInclude(matches, item.id);
+				});
+			});
 		});
 	});