From 20c6fe6737d1b4b31e1b32d635ef7785d5f177d3 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 16 Aug 2022 20:49:07 -0400
Subject: [PATCH] Match parent attachments for annotation tags

Expose annotation tags in tag selector and match parent attachments when
filtering/searching

This also fixes searching for annotation text or comments when using
Everything quick search.

This is temporary until we display annotations in the items list
directly.
---
 chrome/content/zotero/xpcom/data/search.js | 58 +++++++++++++++-------
 chrome/content/zotero/xpcom/data/tags.js   |  9 ++--
 test/tests/searchTest.js                   | 57 ++++++++++++++++++++-
 test/tests/tagSelectorTest.js              | 15 ++++++
 4 files changed, 115 insertions(+), 24 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js
index b060a26df9..4d68ff98e9 100644
--- a/chrome/content/zotero/xpcom/data/search.js
+++ b/chrome/content/zotero/xpcom/data/search.js
@@ -948,7 +948,11 @@ Zotero.Search.idsToTempTable = Zotero.Promise.coroutine(function* (ids) {
 Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 	this._requireData('conditions');
 	
-	var sql = 'SELECT itemID FROM items';
+	// TEMP: Match parent attachment for annotation matches
+	// var sql = 'SELECT itemID FROM items';
+	var sql = "SELECT COALESCE(IA.parentItemID, itemID) AS itemID FROM items "
+		+ "LEFT JOIN itemAnnotations IA USING (itemID)";
+	
 	var sqlParams = [];
 	// Separate ANY conditions for 'required' condition support
 	var anySQL = '';
@@ -962,7 +966,8 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 		let conditionData = Zotero.SearchConditions.get(name);
 		
 		// Has a table (or 'savedSearch', which doesn't have a table but isn't special)
-		if (conditionData.table || name == 'savedSearch' || name == 'tempTable') {
+		// TEMP: Or 'tag', which needs to match annotation parents
+		if (conditionData.table || name == 'savedSearch' || name == 'tempTable' || name == 'tag') {
 			// For conditions with an inline filter using 'is'/'isNot', combine with last condition
 			// if the same
 			if (lastCondition
@@ -1119,22 +1124,35 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 				//
 				// Special table handling
 				//
-				if (condition['table']){
-					switch (condition['table']){
-						default:
-							condSelectSQL += 'itemID '
-							switch (condition['operator']){
-								case 'isNot':
-								case 'doesNotContain':
-									condSelectSQL += 'NOT ';
-									break;
-							}
-							condSelectSQL += 'IN (';
-							selectOpenParens = 1;
-							condSQL += 'SELECT itemID FROM ' +
-								condition['table'] + ' WHERE (';
-							openParens = 1;
+				if (condition.table) {
+					condSelectSQL += 'itemID '
+					switch (condition.operator) {
+						case 'isNot':
+						case 'doesNotContain':
+							condSelectSQL += 'NOT ';
+							break;
 					}
+					condSelectSQL += 'IN (';
+					selectOpenParens = 1;
+					
+					switch (condition.name) {
+						// TEMP: Match parent attachments of matching annotations
+						case 'tag':
+							condSQL += "SELECT COALESCE(IAnT.parentItemID, itemID) FROM itemTags "
+								+ "LEFT JOIN itemAnnotations IAnT USING (itemID) WHERE (";
+							break;
+						
+						// TEMP: Match parent attachments of matching annotations
+						case 'annotationText':
+						case 'annotationComment':
+							condSQL += `SELECT parentItemID FROM ${condition.table} WHERE (`
+							break;
+							
+						default:
+							condSQL += `SELECT itemID FROM ${condition.table} WHERE (`;
+					}
+					
+					openParens = 1;
 				}
 				
 				//
@@ -1709,7 +1727,11 @@ Zotero.Search.prototype._buildQuery = Zotero.Promise.coroutine(function* () {
 		
 		// Add on quicksearch conditions
 		if (quicksearchSQLSet) {
-			sql = "SELECT itemID FROM items WHERE itemID IN (" + sql + ") "
+			// TEMP: Match parent attachments for annotations
+			//sql = "SELECT itemID FROM items WHERE itemID IN (" + sql + ") "
+			sql = "SELECT COALESCE(IAn.parentItemID, itemID) AS itemID FROM items "
+				+ "LEFT JOIN itemAnnotations IAn USING (itemID) "
+				+ "WHERE itemID IN (" + sql + ") "
 				+ "AND ((" + quicksearchSQLSet.join(') AND (') + "))";
 			
 			for (var k=0; k<quicksearchParamsSet.length; k++) {
diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js
index 36a90bf1bd..6ac722a0be 100644
--- a/chrome/content/zotero/xpcom/data/tags.js
+++ b/chrome/content/zotero/xpcom/data/tags.js
@@ -163,9 +163,6 @@ Zotero.Tags = new function() {
 		if (libraryID) {
 			sql += "JOIN items USING (itemID) WHERE libraryID = ? ";
 			params.push(libraryID);
-			// TEMP: Don't show annotation tags in tag selector
-			sql += "AND itemTypeID != ? ";
-			params.push(Zotero.ItemTypes.getID('annotation'));
 		}
 		else {
 			sql += "WHERE 1 ";
@@ -174,7 +171,11 @@ Zotero.Tags = new function() {
 			if (libraryID) {
 				throw new Error("tmpTable and libraryID are mutually exclusive");
 			}
-			sql += "AND itemID IN (SELECT itemID FROM " + tmpTable + ") ";
+			sql += "AND itemID IN (SELECT itemID FROM " + tmpTable;
+			// TEMP: Match parent attachments for annotation tags
+			sql += " UNION SELECT itemID FROM itemAnnotations WHERE parentItemID IN "
+				+ "(SELECT itemID FROM " + tmpTable + ")";
+			sql += ") ";
 		}
 		if (types && types.length) {
 			sql += "AND type IN (" + new Array(types.length).fill('?').join(', ') + ") ";
diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js
index ea15597623..dcb69b414d 100644
--- a/test/tests/searchTest.js
+++ b/test/tests/searchTest.js
@@ -218,6 +218,23 @@ describe("Zotero.Search", function() {
 				});
 			});
 			
+			describe("tag", function () {
+				// TEMP
+				it("should match parent attachments for annotation tags", async function () {
+					var attachment = await importPDFAttachment();
+					var annotation = await createAnnotation('highlight', attachment);
+					var tag = Zotero.Utilities.randomString();
+					annotation.addTag(tag);
+					await annotation.saveTx();
+					
+					var s = new Zotero.Search();
+					s.libraryID = userLibraryID;
+					s.addCondition('tag', 'is', tag);
+					var matches = await s.search();
+					assert.sameMembers(matches, [attachment.id]);
+				});
+			});
+			
 			describe("dateAdded", function () {
 				it("should handle 'today'", async function () {
 					var item = await createDataObject('item');
@@ -371,7 +388,8 @@ describe("Zotero.Search", function() {
 					s.addCondition('joinMode', 'any');
 					s.addCondition('annotationText', 'contains', str);
 					var matches = await s.search();
-					assert.sameMembers(matches, [annotation.id]);
+					// TEMP: Match parent attachment
+					assert.sameMembers(matches, [attachment.id]);
 				});
 			});
 			
@@ -386,7 +404,8 @@ describe("Zotero.Search", function() {
 					s.addCondition('joinMode', 'any');
 					s.addCondition('annotationComment', 'contains', str);
 					var matches = await s.search();
-					assert.sameMembers(matches, [annotation.id]);
+					// TEMP: Match parent attachment
+					assert.sameMembers(matches, [attachment.id]);
 				});
 			});
 			
@@ -525,6 +544,40 @@ describe("Zotero.Search", function() {
 					assert.notInclude(matches, item2.id);
 				});
 			});
+			
+			describe("Quick search", function () {
+				describe("All Fields & Tags", function () {
+					it("should match parent attachment for annotation tag", async function () {
+						var attachment = await importPDFAttachment();
+						var annotation = await createAnnotation('highlight', attachment);
+						var tag = Zotero.Utilities.randomString();
+						annotation.addTag(tag);
+						await annotation.saveTx();
+						
+						var s = new Zotero.Search();
+						s.libraryID = userLibraryID;
+						s.addCondition('quicksearch-fields', 'contains', tag);
+						var matches = await s.search();
+						// TEMP: Match parent attachment
+						assert.sameMembers(matches, [attachment.id]);
+					});
+				})
+				
+				describe("Everything", function () {
+					it("should match parent attachment for annotation comment", async function () {
+						var attachment = await importPDFAttachment();
+						var annotation = await createAnnotation('highlight', attachment);
+						var comment = annotation.annotationComment;
+						
+						var s = new Zotero.Search();
+						s.libraryID = userLibraryID;
+						s.addCondition('quicksearch-everything', 'contains', comment);
+						var matches = await s.search();
+						// TEMP: Match parent attachment
+						assert.sameMembers(matches, [attachment.id]);
+					});
+				});
+			});
 		});
 	});
 	
diff --git a/test/tests/tagSelectorTest.js b/test/tests/tagSelectorTest.js
index 2b3c38bc94..f2aa7e5ecd 100644
--- a/test/tests/tagSelectorTest.js
+++ b/test/tests/tagSelectorTest.js
@@ -95,6 +95,21 @@ describe("Tag Selector", function () {
 		assert.sameMembers(tags, ['A', 'B']);
 	});
 	
+	it("should show tags from annotations for attachments in scope", async function () {
+		var collection = await createDataObject('collection');
+		var item = await createDataObject('item', { collections: [collection.id] });
+		var attachment = await importPDFAttachment(item);
+		var annotation = await createAnnotation('highlight', attachment);
+		var tag = Zotero.Utilities.randomString();
+		annotation.addTag(tag);
+		var promise = waitForTagSelector(win)
+		await annotation.saveTx();
+		await promise;
+		
+		var tags = getRegularTags();
+		assert.sameMembers(tags, [tag]);
+	});
+	
 	describe("#handleSearch()", function () {
 		it("should filter to tags matching the search", function* () {
 			var collection = yield createDataObject('collection');