Search: Allow binding a group whose conditions match at any level

Binding is meaningful for a condition that matches at every level -- a
tag bound to an attachment means the tag is on the attachment -- but a
group carrying one lost the binding as soon as the search was
serialized, so "items with an attachment tagged foo" couldn't be built.

(cherry picked from commit 9da57a9fe3)
This commit is contained in:
Dan Stillman 2026-08-03 15:45:30 -04:00
parent 49055c7c63
commit ec5419f80e
2 changed files with 73 additions and 9 deletions

View file

@ -603,6 +603,8 @@
// Zotero.Search.combineConditions), so treat it as 'item' here.
let bindableBelow = resultLevel == 'any' ? 'item' : resultLevel;
let counts = {};
// Conditions that match at every level (a tag, say) can be bound to any of them
let anyLevel = 0;
for (let row of this.conditionsContainer.children) {
// Skip a row just added via "+" until the user engages with it, so adding a row
// doesn't immediately suggest grouping it
@ -610,19 +612,29 @@
continue;
}
let level = row.conditionLevel;
if (Zotero.Search._isAncestorLevel(bindableBelow, level)) {
if (level == 'any') {
anyLevel++;
}
else if (Zotero.Search._isAncestorLevel(bindableBelow, level)) {
counts[level] = (counts[level] || 0) + 1;
}
}
let levels = Object.keys(counts);
// Drop a stored binding once no condition at its level remains
if (this._resultLevel != 'any' && !levels.includes(this._resultLevel)) {
// The levels this group can bind to at all: one of its conditions matches there, or
// -- for a condition that matches anywhere -- any level below the result level
let optionLevels = ['attachment', 'note', 'annotation'].filter(l => counts[l]
|| (anyLevel && Zotero.Search._isAncestorLevel(bindableBelow, l)));
// Drop a stored binding once it isn't one of them, as when the result level moves
// down to the bound level and binding there stops meaning anything
if (this._resultLevel != 'any' && !optionLevels.includes(this._resultLevel)) {
this._resultLevel = 'any';
}
// Binding is offered once some level has 2+ conditions, and an existing binding
// stays visible (and clearable) even when its group no longer qualifies, so it
// can't invisibly constrain the group from a hidden menu
if (this._resultLevel == 'any' && !levels.some(l => counts[l] >= 2)) {
// Binding is offered once it would mean something: a level shared by 2+ conditions
// ties them to one entity, and a condition that matches at any level is narrowed to
// the bound one. An existing binding stays visible (and clearable) even when its
// group no longer qualifies, so it can't invisibly constrain the group from a
// hidden menu.
if (this._resultLevel == 'any' && !anyLevel
&& !optionLevels.some(l => counts[l] >= 2)) {
this.bindingMenu.hidden = true;
// Plain group: "Match [all] of the following:" (the suffix carries the colon)
this.querySelector('.join-mode-suffix').hidden = false;
@ -634,7 +646,6 @@
// Rebuild the popup only when its option set changes. Rebuilding it on every refresh
// would replace the menuitems mid-selection -- when the change came from this menu
// itself -- and wedge the drop-down.
let optionLevels = ['attachment', 'note', 'annotation'].filter(l => levels.includes(l));
let key = optionLevels.join(',');
if (key !== this._bindingMenuKey) {
this._bindingMenuKey = key;

View file

@ -1538,6 +1538,59 @@ describe("Advanced Search", function () {
['title', 'groupStart', 'joinMode', 'tag', 'tag', 'groupEnd']);
});
it("should keep a binding that scopes a condition matching at any level", async function () {
// An item whose attachment has the tag, and one where the tag is on a
// note instead
var hit = await createDataObject('item');
var attachment = await importFileAttachment('test.png', { parentID: hit.id });
attachment.setTags([{ tag: 'zbound' }]);
await attachment.saveTx();
var miss = await createDataObject('item');
var note = new Zotero.Item('note');
note.parentID = miss.id;
note.setNote('note');
note.setTags([{ tag: 'zbound' }]);
await note.saveTx();
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('groupStart', 'true', '');
s.addCondition('resultLevel', 'attachment');
s.addCondition('tag', 'is', 'zbound');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
searchBox.updateSearch();
var group = conditions.querySelector('search-condition-group');
assert.equal(group.resultLevel, 'attachment');
// The tag has to be on the attachment, not anywhere in the item
var ids = await searchBox.search.search();
assert.include(ids, hit.id);
assert.notInclude(ids, miss.id);
});
it("should drop a binding that the result level makes meaningless", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'attachment');
s.addCondition('groupStart', 'true', '');
s.addCondition('resultLevel', 'attachment');
s.addCondition('tag', 'is', 'zbound');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
searchBox.updateSearch();
// Results are attachments, so binding the group to the attachment says
// nothing the result level doesn't
var group = conditions.querySelector('search-condition-group');
assert.equal(group.resultLevel, 'any');
assert.deepEqual(
Object.values(searchBox.search.getConditions()).map(c => c.condition),
['resultLevel', 'groupStart', 'tag', 'groupEnd']
);
});
it("should wrap a condition in a new group in its place", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;