From 28efaf3fe90aae60dc570061380795353aeca2c4 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 31 Oct 2026 23:27:21 -0400 Subject: [PATCH] fix(filter-builder): id cells showed raw ids; OR mode starved the value menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two user-reported builder defects: 1. Id-kind value cells (library/collection/feed/added_by) rendered the stored UUIDs in the input box. The menu offered names (s.display) and picking stored the id correctly — but the cell bound rule.values.join(',') raw. The cell now renders through valueCellText: idLabels give the NAMES; typing drafts a name filter in idDrafts (the stored values stay ids — the clause matches ids — and are updated only by picking). Text/number/days cells are unchanged. 2. The value menu ignored the connective: fetchSuggestions always sent the sibling rules as the qf constraint, so with "Author contains Lewis Carroll" present, flipping to OR still starved the author menu down to Lewis Carroll. Under OR the siblings are independent alternatives — a new rule's candidates must not be narrowed by them. pathUnderAll now checks the edited rule's group path: under AND all the way up, siblings constrain as before (helpful narrowing); under any OR on the path, NO tree rides and the menu browses the whole scope. Server contract unchanged — the suggest endpoint already treats a missing qf as an unconstrained browse; the fix is client-side mode-awareness. Pins: 6 new vitest cases (AND sends siblings, OR sends none, id label display, draft typing never touches stored ids, pick toggles + relabels, text passthrough). --- templates/filter_builder.templ | 2 +- templates/filter_builder_templ.go | 4 +- web/src/filter_builder.test.ts | 138 +++++++++++++++++++++++++++++- web/src/filter_builder.ts | 78 ++++++++++++++++- 4 files changed, 213 insertions(+), 9 deletions(-) diff --git a/templates/filter_builder.templ b/templates/filter_builder.templ index 55bb0da1..71681815 100644 --- a/templates/filter_builder.templ +++ b/templates/filter_builder.templ @@ -60,7 +60,7 @@ templ FilterBuilderRows(countNoun string) {
{ expect(opsForField("nonexistent_field")).toEqual([]); }); }); + +// ── 2026-10-31 user reports ───────────────────────────────────────── +// 1. id-kind value cells (library/collection/…) showed the raw ids in +// the input box; 2. the value menu stayed narrowed by sibling rules +// after flipping the connective to OR. Both fixed in the component; +// these pins hold them. + +const builderFields = [ + { + key: "library", + label: "Library", + group: "Holding", + valueKind: "id" as const, + defaultOp: "in", + suggest: true, + ops: [{ key: "in", label: "in", arity: "many" }], + }, + { + key: "author", + label: "Author", + group: "People", + valueKind: "text" as const, + defaultOp: "contains", + suggest: true, + ops: [{ key: "contains", label: "contains", arity: "many" }], + }, +]; + +describe("mode-aware value suggestions", () => { + // ingest REPLACES the registry — scoped to this describe so the + // earlier contract pins keep their vocabulary. + beforeEach(() => { + ingestFilterFields(builderFields); + }); + + let captured = ""; + beforeEach(() => { + captured = ""; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string | URL) => { + captured = String(url); + return { ok: true, json: async () => ({ results: [] }) }; + }), + ); + }); + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("constrains the menu by siblings under AND", async () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [ + { field: "author", op: "contains", values: ["Lewis Carroll"] }, + { field: "library", op: "in", values: [""] }, + ], + }; + await c.fetchSuggestions("1", [1], { field: "library", op: "in", values: [""] }); + expect(captured).toContain("qf="); + expect(decodeURIComponent(captured.replace(/\+/g, " "))).toContain("Lewis Carroll"); + }); + + it("sends NO sibling tree under OR — the menu browses the scope", async () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "any", + children: [ + { field: "author", op: "contains", values: ["Lewis Carroll"] }, + { field: "library", op: "in", values: [""] }, + ], + }; + await c.fetchSuggestions("1", [1], { field: "library", op: "in", values: [""] }); + expect(captured).not.toContain("qf="); + }); +}); + +describe("id-kind value cells", () => { + beforeEach(() => { + ingestFilterFields(builderFields); + }); + + it("displays labels while the rule stores ids", () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [{ field: "library", op: "in", values: ["lib-1"] }], + }; + c.idLabels["lib-1"] = "Books"; + expect(c.valueCellText([0])).toBe("Books"); + }); + + it("typing drafts a NAME filter and never writes into the rule's ids", () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [{ field: "library", op: "in", values: ["lib-1"] }], + }; + c.idLabels["lib-1"] = "Books"; + c.onValueCellInput([0], "boo"); + expect(c.filterTree.children[0] as FilterRuleState).toEqual({ + field: "library", + op: "in", + values: ["lib-1"], + }); + expect(c.valueCellText([0])).toBe("boo"); + }); + + it("picking toggles the id and flips the cell back to labels", () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [{ field: "library", op: "in", values: ["lib-1"] }], + }; + c.idLabels["lib-1"] = "Books"; + c.onValueCellInput([0], "com"); + c.pickSuggestion("0", [0], { value: "lib-2", display: "Comics", count: 3 }); + expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lib-1,lib-2"]); + expect(c.valueCellText([0])).toBe("Books, Comics"); + }); + + it("text cells keep writing straight through", () => { + const c = filterBuilderData(); + c.filterTree = { + mode: "all", + children: [{ field: "author", op: "contains", values: [""] }], + }; + c.onValueCellInput([0], "lewis"); + expect((c.filterTree.children[0] as FilterRuleState).values).toEqual(["lewis"]); + }); +}); diff --git a/web/src/filter_builder.ts b/web/src/filter_builder.ts index f4f61200..8fe33da2 100644 --- a/web/src/filter_builder.ts +++ b/web/src/filter_builder.ts @@ -371,7 +371,7 @@ interface Suggestion { display?: string; } -function filterBuilderData() { +export function filterBuilderData() { return { // Lifecycle: the Go registry renders this page's vocabulary (the // data-filter-fields payload); ingest once so the menus are born @@ -434,6 +434,7 @@ function filterBuilderData() { suggestionsOpen: "" as string, // rowKey whose value menu is open suggestionsLoading: "" as string, // rowKey currently fetching idLabels: {} as Record, // id → display, from suggestion fetches + idDrafts: {} as Record, // id-kind rowKey → the NAME text being typed (display draft; stored values stay ids) _suggestTimer: null as number | null, _countTimer: null as number | null, @@ -625,6 +626,66 @@ function filterBuilderData() { this.openMenu = ""; this._touch(); }, + // isIdValueRow — the value cell stores IDS (library/collection/ + // feed/added_by): display needs the label map, typing filters NAMES. + isIdValueRow(path: number[]): boolean { + const node = this.nodeAt(path); + if (!node || !isRule(node)) return false; + return FIELD_BY_KEY.get(node.field)?.valueKind === "id"; + }, + + // pathUnderAll — is every group from the root down to the edited + // rule's parent an AND ("all") group? Under any OR on the path the + // siblings cannot constrain the rule being edited. + pathUnderAll(path: number[]): boolean { + if (this.filterTree.mode !== "all") return false; + let node: FilterRuleState | FilterGroupState = this.filterTree; + for (let i = 0; i < path.length - 1; i++) { + if (isRule(node)) return false; + const child = (node as FilterGroupState).children[path[i]]; + if (!child || isRule(child)) return false; + node = child; + if (node.mode !== "all") return false; + } + return true; + }, + + // valueCellText — what the value INPUT displays. Id-kind rules keep + // the ids in rule.values (the clause matches ids); the cell shows + // their LABELS, or the name text being typed. + valueCellText(path: number[]): string { + const node = this.nodeAt(path); + if (!node || !isRule(node)) return ""; + if (this.isIdValueRow(path)) { + const draft = this.idDrafts[path.join("-")]; + if (draft !== undefined) return draft; + return node.values + .join(",") + .split(",") + .filter(Boolean) + .map((v) => this.idLabels[v] ?? v) + .join(", "); + } + return node.values.join(","); + }, + + // onValueCellInput — id-kind cells type a NAME filter (the draft); + // the stored values stay ids, updated only by picking from the + // menu. Every other kind writes straight through to the rule. + onValueCellInput(path: number[], value: string): void { + const node = this.nodeAt(path); + if (!node || !isRule(node)) return; + if (this.isIdValueRow(path)) { + const key = path.join("-"); + if (value === "") delete this.idDrafts[key]; + else this.idDrafts[key] = value; + this.scheduleSuggestions(key, path, node); + this._touch(); + return; + } + this.setRuleValue(path, 0, value); + }, + setRuleValue(path: number[], idx: number, value: string): void { const node = this.nodeAt(path); if (!node || !isRule(node)) return; @@ -639,6 +700,7 @@ function filterBuilderData() { if (!node || !isRule(node)) return; const meta = FIELD_BY_KEY.get(node.field); this.idLabels[s.value] = s.display ?? s.value; + delete this.idDrafts[path.join("-")]; // the cell shows labels again if (meta?.valueKind === "id") { // id-valued: toggle the id in the comma list. const ids = node.values.join(",").split(",").filter(Boolean); @@ -715,7 +777,9 @@ function filterBuilderData() { if (!meta || !meta.suggest) return; // Fixed-option fields never hit the server. if (meta.valueKind === "fixed") return; - const typed = (rule.values[0] ?? "").trim(); + const typed = ( + this.isIdValueRow(path) ? (this.idDrafts[path.join("-")] ?? "") : (rule.values[0] ?? "") + ).trim(); // 1 char is too little to filter on; EMPTY means BROWSE — the // user clicked the box to see the field's top values under the // other rules (the server answers the blank query with the @@ -728,8 +792,14 @@ function filterBuilderData() { this.suggestionsOpen = rowKey; // Symmetry: the tree sent EXCLUDES the rule being edited — the // menu is computed under every OTHER COMPLETED rule (incomplete - // rules are not filters; sending them would be a 400). - const others = this.completedTree(path); + // rules are not filters; sending them would be a 400). Under an + // OR connective on the edited rule's path (root mode "any" or an + // "any" ancestor group), siblings are INDEPENDENT alternatives — + // a new rule's candidates must not be narrowed by them — so no + // tree rides at all (2026-10-31 user report: with "Author + // contains Lewis Carroll" present, flipping to OR still starved + // the author menu down to Lewis Carroll). + const others = this.pathUnderAll(path) ? this.completedTree(path) : null; const params = new URLSearchParams(); params.set(rule.field, typed || " "); if (others) params.set("qf", JSON.stringify(others));