From 7782e7d4d2ffac9a9159cdf73777e6e278182e77 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Fri, 20 Mar 2026 13:52:42 -0700 Subject: [PATCH] new annotation search conditions - added annotationType, annotationColor, and annotationAuthor search conditions - updated logic in search.js to only include annotation on negative operators. Otherwise, 'annotationTitle' 'isNot' 'X' would always match every attachment and top-level item in the library Fixes: zotero#5837 --- .../content/zotero/elements/zoteroSearch.js | 46 ++++++- chrome/content/zotero/xpcom/annotations.js | 16 +++ chrome/content/zotero/xpcom/data/search.js | 27 ++-- .../zotero/xpcom/data/searchConditions.js | 33 +++++ chrome/locale/en-US/zotero/zotero.ftl | 3 + test/content/support.js | 2 +- test/tests/advancedSearchTest.js | 75 +++++++++++ test/tests/searchTest.js | 122 +++++++++++++++++- 8 files changed, 307 insertions(+), 17 deletions(-) diff --git a/chrome/content/zotero/elements/zoteroSearch.js b/chrome/content/zotero/elements/zoteroSearch.js index a73b56e1b7..fb53defe8d 100644 --- a/chrome/content/zotero/elements/zoteroSearch.js +++ b/chrome/content/zotero/elements/zoteroSearch.js @@ -374,7 +374,7 @@ return false; } - onConditionSelected(conditionName, reload) { + async onConditionSelected(conditionName, reload) { var conditionsMenu = this.querySelector('#conditionsmenu'); var operatorsList = this.querySelector('#operatorsmenu'); @@ -490,6 +490,44 @@ this.createValueMenu(rows); break; } + case 'annotationColor': + { + let rows = [ + { name: Zotero.getString('general.yellow'), value: '#ffd400' }, + { name: Zotero.getString('general.red'), value: '#ff6666' }, + { name: Zotero.getString('general.green'), value: '#5fb236' }, + { name: Zotero.getString('general.blue'), value: '#2ea8e5' }, + { name: Zotero.getString('general.purple'), value: '#a28ae5' }, + { name: Zotero.getString('general.magenta'), value: '#e56eee' }, + { name: Zotero.getString('general.orange'), value: '#f19837' }, + { name: Zotero.getString('general.gray'), value: '#aaaaaa' }, + ]; + this.createValueMenu(rows); + break; + } + case 'annotationType': + { + let rows = [ + { name: 'Highlight', value: Zotero.Annotations.ANNOTATION_TYPE_HIGHLIGHT }, + { name: 'Underline', value: Zotero.Annotations.ANNOTATION_TYPE_UNDERLINE }, + { name: 'Text', value: Zotero.Annotations.ANNOTATION_TYPE_TEXT }, + { name: 'Note', value: Zotero.Annotations.ANNOTATION_TYPE_NOTE }, + { name: 'Ink', value: Zotero.Annotations.ANNOTATION_TYPE_INK }, + { name: 'Image', value: Zotero.Annotations.ANNOTATION_TYPE_IMAGE }, + ]; + this.createValueMenu(rows); + break; + } + case 'annotationAuthor': + { + let libraryID = this.parent.search.libraryID; + let authors = await Zotero.Annotations.getAllAuthors(libraryID); + let collation = Zotero.getLocaleCollation(); + let rows = authors.map(a => ({ name: a.name, value: a.userID })); + rows.sort((a, b) => collation.compareString(1, a.name, b.name)); + this.createValueMenu(rows); + break; + } default: { if (operatorsList.value == 'isInTheLast') { @@ -522,7 +560,10 @@ // Drop-down menu if (this.selectedCondition == 'collection' || this.selectedCondition == 'itemType' - || this.selectedCondition == 'fileTypeID') { + || this.selectedCondition == 'fileTypeID' + || this.selectedCondition == 'annotationColor' + || this.selectedCondition == 'annotationType' + || this.selectedCondition == 'annotationAuthor') { this.querySelector('#valuefield').hidden = true; this.querySelector('#valuemenu').hidden = false; this.querySelector('#value-date-age').hidden = true; @@ -731,6 +772,7 @@ onLibraryChange() { switch (this.selectedCondition) { case 'collection': + case 'annotationAuthor': this.onConditionSelected(this.selectedCondition, true); break; } diff --git a/chrome/content/zotero/xpcom/annotations.js b/chrome/content/zotero/xpcom/annotations.js index 330c8f74aa..ccc2fd8a1e 100644 --- a/chrome/content/zotero/xpcom/annotations.js +++ b/chrome/content/zotero/xpcom/annotations.js @@ -334,6 +334,22 @@ Zotero.Annotations = new function () { * @param {Zotero.Item[]} items * @returns {Promise} */ + /** + * Return all annotation authors in a library + * + * @param {Integer} libraryID + * @return {Object[]} - Array of { userID, name } + */ + this.getAllAuthors = async function (libraryID) { + let sql = `SELECT DISTINCT u.userID, u.name FROM users u + JOIN groupItems gi ON (gi.createdByUserID = u.userID) + JOIN itemAnnotations ia ON (ia.itemID = gi.itemID) + JOIN items i ON (i.itemID = ia.itemID) + WHERE i.libraryID = ?`; + let rows = await Zotero.DB.queryAsync(sql, libraryID); + return rows.map(row => ({ userID: row.userID, name: row.name })); + }; + this.splitAnnotations = async function (items) { if (!Array.isArray(items)) { items = [items]; diff --git a/chrome/content/zotero/xpcom/data/search.js b/chrome/content/zotero/xpcom/data/search.js index 594d1cbede..25ea4adddd 100644 --- a/chrome/content/zotero/xpcom/data/search.js +++ b/chrome/content/zotero/xpcom/data/search.js @@ -1207,7 +1207,8 @@ Zotero.Search.prototype._buildQuery = async function () { // if (condition.table) { let negationOperators = ['isNot', 'doesNotContain']; - let isNegationOperator = negationOperators.includes(condition.operator); + let isNegationOperator = negationOperators.includes(condition.operator); + let isAnnotationCondition = condition.name.includes('annotation'); condSelectSQL += 'itemID ' if (isNegationOperator) { @@ -1215,23 +1216,23 @@ Zotero.Search.prototype._buildQuery = async function () { } condSelectSQL += 'IN ('; selectOpenParens = 1; - - // TEMP: Don't match annotations for negation operators, since it would result in - // all parent attachments being returned + + // For negative operators, add all items of the "wrong" type to the + // NOT IN set so they get excluded from results: + // - Annotation conditions: exclude non-annotations (so only + // annotations can match) + // - Non-annotation conditions: exclude annotations (so annotations + // don't trivially match all negative queries) if (isNegationOperator) { - condSelectSQL += "SELECT itemID FROM items WHERE itemTypeID=" + condSelectSQL += "SELECT itemID FROM items WHERE itemTypeID" + + (isAnnotationCondition ? "!=" : "=") + Zotero.ItemTypes.getID('annotation') + " UNION "; } switch (condition.name) { - case 'tag': - condSQL += "SELECT itemID FROM itemTags " - + "LEFT JOIN itemAnnotations IAnT USING (itemID) WHERE ("; - break; - - case 'annotationText': - case 'annotationComment': - condSQL += `SELECT itemID FROM ${condition.table} WHERE (` + case 'annotationAuthor': + condSQL += "SELECT itemID FROM itemAnnotations " + + "LEFT JOIN groupItems USING (itemID) WHERE ("; break; default: diff --git a/chrome/content/zotero/xpcom/data/searchConditions.js b/chrome/content/zotero/xpcom/data/searchConditions.js index abab69709a..07b04dea92 100644 --- a/chrome/content/zotero/xpcom/data/searchConditions.js +++ b/chrome/content/zotero/xpcom/data/searchConditions.js @@ -599,6 +599,39 @@ Zotero.SearchConditions = new function () { special: false, }, + { + name: 'annotationColor', + operators: { + is: true, + isNot: true + }, + table: 'itemAnnotations', + field: 'color', + special: false, + }, + + { + name: 'annotationType', + operators: { + is: true, + isNot: true + }, + table: 'itemAnnotations', + field: 'type', + special: false, + }, + + { + name: 'annotationAuthor', + operators: { + is: true, + isNot: true, + }, + table: 'groupItems', + field: 'createdByUserID', + special: false, + }, + { name: 'fulltextWord', operators: { diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index d828200f4b..3a84995138 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -830,6 +830,9 @@ search-conditions-lastRead = Attachment Last Read search-conditions-annotationText = Annotation Text search-conditions-annotationComment = Annotation Comment search-conditions-anyField = Any Field +search-conditions-annotationColor = Annotation Color +search-conditions-annotationAuthor = Annotation Author +search-conditions-annotationType = Annotation Type find-pdf-files-added = { $count -> [one] { $count } file added diff --git a/test/content/support.js b/test/content/support.js index e2386dcb70..28102de933 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -1186,7 +1186,7 @@ async function createAnnotation(type, parentItem, options = {}) { else { annotation.annotationComment = Zotero.Utilities.randomString(); } - annotation.annotationColor = '#ffd400'; + annotation.annotationColor = options.annotationColor || '#ffd400'; var page = Zotero.Utilities.rand(1, 100); annotation.annotationPageLabel = `${page}`; page = page.toString().padStart(5, '0'); diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js index 455409d4a1..0c414f2c8f 100644 --- a/test/tests/advancedSearchTest.js +++ b/test/tests/advancedSearchTest.js @@ -225,6 +225,81 @@ describe("Advanced Search", function () { }); }); + describe("Annotation Author", function () { + it("should show annotation authors for a group library", async function () { + var group = await getGroup(); + var groupLibraryID = group.libraryID; + + await Zotero.Users.setName(11111, 'Alice'); + await Zotero.Users.setName(22222, 'Bob'); + + var groupAttachment = await importFileAttachment('test.pdf', { + libraryID: groupLibraryID, + contentType: 'application/pdf', + }); + + var annotation1 = await createAnnotation('highlight', groupAttachment); + annotation1.createdByUserID = 11111; + annotation1.lastModifiedByUserID = 11111; + await annotation1.saveTx({ skipEditCheck: true }); + + var annotation2 = await createAnnotation('highlight', groupAttachment); + annotation2.createdByUserID = 22222; + annotation2.lastModifiedByUserID = 22222; + await annotation2.saveTx({ skipEditCheck: true }); + + // Switch to group library + var libraryMenu = searchWin.document.getElementById('libraryMenu'); + for (let i = 0; i < libraryMenu.itemCount; i++) { + let menuitem = libraryMenu.getItemAtIndex(i); + if (menuitem.value == groupLibraryID) { + menuitem.click(); + break; + } + } + + var s = new Zotero.Search(); + s.libraryID = groupLibraryID; + s.addCondition('title', 'is', ''); + searchBox.search = s; + + var searchCondition = conditions.firstChild; + var valueMenu = searchCondition.querySelector('#valuemenu'); + + // Select 'Annotation Author' condition from the "More" submenu + var moreMenu = searchCondition.querySelector('#more-conditions-menu menupopup'); + for (let menuitem of moreMenu.children) { + if (menuitem.value == 'annotationAuthor') { + menuitem.click(); + break; + } + } + + // Wait for async dropdown population + await Zotero.Promise.delay(9000); + + assert.isFalse(valueMenu.hidden); + assert.equal(valueMenu.itemCount, 2); + + var values = []; + var labels = []; + for (let i = 0; i < valueMenu.itemCount; i++) { + let menuitem = valueMenu.getItemAtIndex(i); + values.push(menuitem.getAttribute('value')); + labels.push(menuitem.getAttribute('label')); + } + + assert.include(labels, 'Alice'); + assert.include(labels, 'Bob'); + assert.include(values, '11111'); + assert.include(values, '22222'); + + await annotation1.eraseTx(); + await annotation2.eraseTx(); + await groupAttachment.eraseTx(); + }); + }); + describe("Saved Search", function () { it("shouldn't appear", async function () { var searchCondition = conditions.firstChild; diff --git a/test/tests/searchTest.js b/test/tests/searchTest.js index cc296eb5b5..7dcc5f44c9 100644 --- a/test/tests/searchTest.js +++ b/test/tests/searchTest.js @@ -404,8 +404,128 @@ describe("Zotero.Search", function () { var matches = await s.search(); assert.sameMembers(matches, [annotation.id]); }); + + it("should not return parent item on 'doesNotContain' condition", async function () { + var item = await createDataObject('item'); + var attachment = await importPDFAttachment(item); + var annotation = await createAnnotation('highlight', attachment, { annotationComment: "some comment" }); + + var s = new Zotero.Search(); + s.libraryID = userLibraryID; + s.addCondition('annotationText', 'doesNotContain', 'nonexistent'); + var matches = await s.search(); + assert.include(matches, annotation.id); + assert.notInclude(matches, item.id); + assert.notInclude(matches, attachment.id); + }); }); - + + describe("title", function () { + it("should not return annotations for a 'doesNotContain' condition", async function () { + var item = await createDataObject('item', { title: 'Test Title' }); + var attachment = await importPDFAttachment(item); + var annotation = await createAnnotation('highlight', attachment); + + var s = new Zotero.Search(); + s.libraryID = userLibraryID; + s.addCondition('title', 'doesNotContain', 'nonexistent'); + var matches = await s.search(); + assert.include(matches, item.id); + assert.notInclude(matches, annotation.id); + }); + }); + + describe("annotationColor", function () { + it("should return annotation matching color", async function () { + var attachment = await importPDFAttachment(); + var annotationOne = await createAnnotation('highlight', attachment, { annotationColor: '#ffd400' }); + var annotationTwo = await createAnnotation('highlight', attachment, { annotationColor: '#ff6666' }); + + var s = new Zotero.Search(); + s.libraryID = userLibraryID; + s.addCondition('annotationColor', 'is', '#ffd400'); + var matches = await s.search(); + assert.include(matches, annotationOne.id); + assert.notInclude(matches, annotationTwo.id); + }); + }); + + describe("annotationType", function () { + it("should return annotation matching type", async function () { + var attachment = await importPDFAttachment(); + var annotationOne = await createAnnotation('highlight', attachment); + var annotationTwo = await createAnnotation('underline', attachment); + + var s = new Zotero.Search(); + s.libraryID = userLibraryID; + s.addCondition('annotationType', 'is', Zotero.Annotations.ANNOTATION_TYPE_HIGHLIGHT); + var matches = await s.search(); + assert.include(matches, annotationOne.id); + assert.notInclude(matches, annotationTwo.id); + }); + }); + + describe("annotationAuthor", function () { + it("should return annotation by matching author and not by another", async function () { + var group = await getGroup(); + + await Zotero.Users.setName(12345, 'User One'); + await Zotero.Users.setName(67890, 'User Two'); + + var groupAttachment = await importFileAttachment('test.pdf', { + libraryID: group.libraryID, + contentType: 'application/pdf', + }); + + var annotation1 = await createAnnotation('highlight', groupAttachment); + annotation1.createdByUserID = 12345; + annotation1.lastModifiedByUserID = 12345; + await annotation1.saveTx({ skipEditCheck: true }); + + var annotation2 = await createAnnotation('highlight', groupAttachment); + annotation2.createdByUserID = 67890; + annotation2.lastModifiedByUserID = 67890; + await annotation2.saveTx({ skipEditCheck: true }); + + var s = new Zotero.Search(); + s.libraryID = group.libraryID; + s.addCondition('annotationAuthor', 'is', 12345); + var matches = await s.search(); + assert.include(matches, annotation1.id); + assert.notInclude(matches, annotation2.id); + }); + + it("should not return annotation by matching author with 'isNot'", async function () { + var group = await getGroup(); + + await Zotero.Users.setName(12345, 'User One'); + await Zotero.Users.setName(67890, 'User Two'); + + var groupAttachment = await importFileAttachment('test.pdf', { + libraryID: group.libraryID, + contentType: 'application/pdf', + }); + + var annotation1 = await createAnnotation('highlight', groupAttachment); + annotation1.createdByUserID = 12345; + annotation1.lastModifiedByUserID = 12345; + await annotation1.saveTx({ skipEditCheck: true }); + + var annotation2 = await createAnnotation('highlight', groupAttachment); + annotation2.createdByUserID = 67890; + annotation2.lastModifiedByUserID = 67890; + await annotation2.saveTx({ skipEditCheck: true }); + + var s = new Zotero.Search(); + s.libraryID = group.libraryID; + s.addCondition('annotationAuthor', 'isNot', 12345); + var matches = await s.search(); + assert.notInclude(matches, annotation1.id); + assert.include(matches, annotation2.id); + assert.notInclude(matches, groupAttachment.id); + }); + }); + describe("fulltextWord", function () { it("should return matches with full-text conditions", async function () { let s = new Zotero.Search();