feat(filter): label endpoint names id values for client-held trees
The SSR embed (65f257f3) covers trees the server renders; one path
still fell through to the flawed fallback — applying a SAVED filter
from a cold bookshelf. That page was rendered before any filter
existed (nothing embedded), and the fallback reused the suggest facet,
whose rows are MEDIA-CONSTRAINED: "feed in [A,B,C] AND title contains
Chapter" labeled only the feeds with a matching episode (the exact
live repro from the original bug report).
- GET /api/filter/labels?field=X&ids=a,b,c resolves names straight
from the entity tables via the shared ResolveFieldLabels core
(extracted from FilterTreeLabels): no media join, no tree
constraint. RLS/visibility ride the request identity — a
zero-episode subscribed feed resolves its title (pinned; the facet
path structurally could not), an unsubscribed feed id stays unnamed
(pinned), non-id fields are 400 (pinned), ids cap at 100.
- Client: resolveIdLabels/resolveLabelsAsync use ONLY the endpoint;
the quiet suggest-fetch mode is deleted (its only caller was the
flawed fallback). The saved-filter apply flow resolves through the
endpoint and rebuilds the chips bar when labels land — chips render
raw ids only while the round trip is in flight, never stuck.
- 188 web tests green; Go endpoint pins + filter/episode slices green;
contract_pins documents the endpoint and the fallback removal.
This commit is contained in:
1 parent
a20741ccc9
commit
96a40f7d88
8 files changed
+236
-40
No files matched your search
@@ -2415,3 +2415,69 @@ func TestSmartCollectionBornReversed(t *testing.T) {
|
||||
"static collections get no chain_order seed")
|
||||
})
|
||||
}
|
||||
|
||||
// ---- label endpoint: names id-kind values for client-held trees ----
|
||||
|
||||
// TestFilterLabelsEndpoint pins GET /api/filter/labels (2026-11-01):
|
||||
// the client path that never touches a server render — applying a
|
||||
// SAVED filter from a cold bookshelf — resolves names through ENTITY
|
||||
// tables, not the media-constrained suggest facet. A zero-episode feed
|
||||
// resolves its title (the facet path structurally could not); an
|
||||
// unsubscribed feed id resolves to nothing (RLS); a non-id field is a
|
||||
// 400.
|
||||
func TestFilterLabelsEndpoint(t *testing.T) {
|
||||
setup := setupTestServer(t)
|
||||
regUUID := getRegularUserID(t, setup.DB)
|
||||
regToken := setup.RegularToken
|
||||
|
||||
lib, _ := createPodcastsLibrary(t, setup, "test LabelsEP Podcasts")
|
||||
marker := fmt.Sprintf("labelsep%d", time.Now().UnixNano()%100000)
|
||||
|
||||
feedA, err := setup.DB.CreatePodcastFeed(adminSeedCtx(t, setup), database.CreatePodcastFeedParams{
|
||||
LibraryID: lib.ID,
|
||||
FeedUrl: "https://feed.invalid/" + marker + "-a.xml",
|
||||
Title: "LabelsEP Feed A " + marker,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
feedB, err := setup.DB.CreatePodcastFeed(adminSeedCtx(t, setup), database.CreatePodcastFeedParams{
|
||||
LibraryID: lib.ID,
|
||||
FeedUrl: "https://feed.invalid/" + marker + "-b.xml",
|
||||
Title: "LabelsEP Feed B " + marker,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
// feedA is subscribed; feedB is NOT (RLS must hide it even by id).
|
||||
subscribeRaw(t, setup, regUUID, feedA.ID)
|
||||
|
||||
feedAID := uuid.UUID(feedA.ID.Bytes).String()
|
||||
feedBID := uuid.UUID(feedB.ID.Bytes).String()
|
||||
|
||||
t.Run("zero_episode_feed_resolves", func(t *testing.T) {
|
||||
code, body := podcastAuthed(t, setup, regToken, "GET",
|
||||
"/api/filter/labels?field=feed&ids="+feedAID, "")
|
||||
require.Equal(t, http.StatusOK, code, "%s", string(body))
|
||||
assert.Contains(t, string(body), `"labels"`)
|
||||
assert.Contains(t, string(body), "LabelsEP Feed A "+marker,
|
||||
"a subscribed feed with ZERO episodes resolves its title (no media join)")
|
||||
})
|
||||
|
||||
t.Run("comma_joined_ids_resolve_together", func(t *testing.T) {
|
||||
code, body := podcastAuthed(t, setup, regToken, "GET",
|
||||
"/api/filter/labels?field=feed&ids="+feedAID+","+feedBID, "")
|
||||
require.Equal(t, http.StatusOK, code)
|
||||
assert.Contains(t, string(body), "LabelsEP Feed A "+marker)
|
||||
})
|
||||
|
||||
t.Run("unsubscribed_feed_stays_unnamed", func(t *testing.T) {
|
||||
code, body := podcastAuthed(t, setup, regToken, "GET",
|
||||
"/api/filter/labels?field=feed&ids="+feedBID, "")
|
||||
require.Equal(t, http.StatusOK, code)
|
||||
assert.NotContains(t, string(body), "LabelsEP Feed B "+marker,
|
||||
"an unsubscribed feed id is invisible to resolution (RLS)")
|
||||
})
|
||||
|
||||
t.Run("non_id_field_is_400", func(t *testing.T) {
|
||||
code, body := podcastAuthed(t, setup, regToken, "GET",
|
||||
"/api/filter/labels?field=title&ids=whatever", "")
|
||||
assert.Equal(t, http.StatusBadRequest, code, "%s", string(body))
|
||||
})
|
||||
}
|
||||
@@ -685,3 +685,21 @@ born-reversed chain/page pins replace the former native expectations
|
||||
pin from 81e54f89 was corrected while flipping: the API envelope's
|
||||
match list is NOT chain-ordered and never was — order pins live on the
|
||||
SSR page and the reader chain.
|
||||
|
||||
## 2026-11-01 — label endpoint replaces the suggest-facet fallback
|
||||
|
||||
`GET /api/filter/labels?field=X&ids=a,b,c` names id-kind filter values
|
||||
from their source tables (entity rows only — no media join, no tree
|
||||
constraint; visibility/RLS ride the request identity). Non-id fields
|
||||
are a 400; ids truncate at the same 100 cap the SSR embed uses.
|
||||
|
||||
The client's label resolution (`resolveIdLabels`/`resolveLabelsAsync`)
|
||||
now uses ONLY this endpoint. The former fallback — the quiet suggest
|
||||
fetch through the MEDIA-CONSTRAINED facet — is deleted: it labeled only
|
||||
the ids whose media matched the tree's sibling rules (the live feed
|
||||
repro), which mattered exactly where the SSR embed can't reach:
|
||||
applying a SAVED filter from a cold bookshelf. The apply flow now
|
||||
resolves through the endpoint and rebuilds the chips bar when labels
|
||||
land. Pinned (TestFilterLabelsEndpoint): a zero-episode feed resolves
|
||||
its title, comma-joined ids resolve together, an unsubscribed feed id
|
||||
stays unnamed (RLS), non-id fields are 400.
|
||||
@@ -248,6 +248,31 @@ func NewMediaHandler(db *database.Queries, raw database.DBTX, libraryService *se
|
||||
// resolve id labels through it — 2026-11-01).
|
||||
func (h *MediaHandler) SearchService() *services.SearchService { return h.searchService }
|
||||
|
||||
// ResolveFilterLabels — GET /api/filter/labels?field=X&ids=a,b,c names
|
||||
// id-kind filter values straight from their source tables: the client
|
||||
// path that never touches a server render (applying a SAVED filter
|
||||
// from a cold bookshelf) would otherwise fall back to the suggest
|
||||
// facet, whose rows are media-constrained and label only the ids whose
|
||||
// media matches the tree's sibling rules (2026-11-01 feed repro).
|
||||
// Visibility/RLS ride the request identity, same as the SSR embed.
|
||||
func (h *MediaHandler) ResolveFilterLabels(c *echo.Context) error {
|
||||
user := c.Get("user").(database.Users)
|
||||
field := c.QueryParam("field")
|
||||
if !services.IsIDValuedField(field) {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": "field is not an id-valued filter field"})
|
||||
}
|
||||
ids := services.ParseUUIDValues(strings.Split(c.QueryParam("ids"), ","))
|
||||
if len(ids) == 0 {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": "ids required"})
|
||||
}
|
||||
if len(ids) > services.MaxResolveIDs {
|
||||
ids = ids[:services.MaxResolveIDs]
|
||||
}
|
||||
labels := h.SearchService().ResolveFieldLabels(
|
||||
c.Request().Context(), user.ID, field, ids)
|
||||
return c.JSON(http.StatusOK, map[string]interface{}{"labels": labels})
|
||||
}
|
||||
|
||||
func (h *MediaHandler) ExecuteSearch(ctx context.Context, params services.SearchParams) ([]database.SearchMediaItemsUnifiedRow, int, error) {
|
||||
return h.searchService.ExecuteSearch(ctx, params)
|
||||
}
|
||||
|
||||
@@ -16,6 +16,10 @@ func registerSearchRoutes(cfg *Config) {
|
||||
// JWT middleware for protected routes
|
||||
jwtMiddleware := createJWTMiddleware(cfg)
|
||||
protected := e.Group("/api", jwtMiddleware)
|
||||
// Filter label resolution (2026-11-01): names id-kind filter values
|
||||
// from their source tables for trees the server didn't render (the
|
||||
// cold saved-filter apply). Visibility/RLS ride the request identity.
|
||||
protected.GET("/filter/labels", cfg.MediaHandler.ResolveFilterLabels)
|
||||
// Search endpoint - returns JSON or HTML based on HX-Request header
|
||||
protected.GET("/media-items/search", func(c *echo.Context) error {
|
||||
// Check if HTMX request (expects HTML)
|
||||
|
||||
@@ -29,9 +29,25 @@ import (
|
||||
// a legal pick, the resolver only names it.
|
||||
// ════════════════════════════════════════════════════════════════════
|
||||
|
||||
// maxResolveIDs bounds one field's resolution query (picks accumulate
|
||||
// per rule; menus cap far below this).
|
||||
const maxResolveIDs = 100
|
||||
// MaxResolveIDs bounds one field's resolution query (picks accumulate
|
||||
// per rule; menus cap far below this). The label endpoint truncates to
|
||||
// the same cap.
|
||||
const MaxResolveIDs = 100
|
||||
|
||||
// IsIDValuedField — whether the field's values are entity ids (the
|
||||
// label endpoint's accepted universe: id-kind + a known source).
|
||||
func IsIDValuedField(field string) bool {
|
||||
f, ok := FilterFieldByKey(field)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return valueKindFor(f.Kind, field) == VKId && f.Suggest != ""
|
||||
}
|
||||
|
||||
// ParseUUIDValues splits comma-joined picks into uuids, skipping junk.
|
||||
func ParseUUIDValues(values []string) []uuid.UUID {
|
||||
return parseUUIDValues(values)
|
||||
}
|
||||
|
||||
// FilterTreeLabels resolves every id-kind rule value in the tree to its
|
||||
// display name. Best-effort by design: a resolve failure degrades to
|
||||
@@ -64,7 +80,7 @@ func (s *SearchService) FilterTreeLabels(ctx context.Context, userID pgtype.UUID
|
||||
continue
|
||||
}
|
||||
for _, id := range parseUUIDValues(c.Values) {
|
||||
if len(byField[c.Field]) >= maxResolveIDs {
|
||||
if len(byField[c.Field]) >= MaxResolveIDs {
|
||||
return
|
||||
}
|
||||
byField[c.Field] = append(byField[c.Field], id)
|
||||
@@ -74,7 +90,22 @@ func (s *SearchService) FilterTreeLabels(ctx context.Context, userID pgtype.UUID
|
||||
walk(tree.Children)
|
||||
|
||||
for field, ids := range byField {
|
||||
f, _ := FilterFieldByKey(field)
|
||||
for k, v := range s.ResolveFieldLabels(ctx, userID, field, ids) {
|
||||
labels[k] = v
|
||||
}
|
||||
}
|
||||
return labels
|
||||
}
|
||||
|
||||
// ResolveFieldLabels names one field's picked ids against its source
|
||||
// table — the shared core of the SSR embed and the label endpoint.
|
||||
// Unknown fields resolve to nothing (the caller validates).
|
||||
func (s *SearchService) ResolveFieldLabels(ctx context.Context, userID pgtype.UUID, field string, ids []uuid.UUID) map[string]string {
|
||||
labels := map[string]string{}
|
||||
f, ok := FilterFieldByKey(field)
|
||||
if !ok {
|
||||
return labels
|
||||
}
|
||||
idArgs := make([]uuid.UUID, len(ids))
|
||||
copy(idArgs, ids)
|
||||
var sqlStr string
|
||||
@@ -101,13 +132,14 @@ WHERE COALESCE(lv.is_visible, true) = true AND l.id = ANY($1)`
|
||||
sqlStr = `SELECT id::text, username FROM users WHERE id = ANY($1)`
|
||||
args = []any{idArgs}
|
||||
default:
|
||||
continue
|
||||
return labels
|
||||
}
|
||||
rows, err := s.raw.Query(ctx, sqlStr, args...)
|
||||
if err != nil {
|
||||
log.Printf("[filter-labels] resolve %q failed: %v", field, err)
|
||||
continue
|
||||
return labels
|
||||
}
|
||||
defer rows.Close()
|
||||
for rows.Next() {
|
||||
var v, display string
|
||||
if err := rows.Scan(&v, &display); err != nil {
|
||||
@@ -115,7 +147,5 @@ WHERE COALESCE(lv.is_visible, true) = true AND l.id = ANY($1)`
|
||||
}
|
||||
labels[v] = display
|
||||
}
|
||||
rows.Close()
|
||||
}
|
||||
return labels
|
||||
}
|
||||
@@ -287,6 +287,15 @@ Alpine.data("bookshelf", () => ({
|
||||
// Rehydrate the rule-builder tree from the refilled form
|
||||
builderData()?.hydrateFromForm();
|
||||
|
||||
// Cold-cache labels (2026-11-01): this page was rendered BEFORE
|
||||
// the filter existed, so the SSR embed has no names for the saved
|
||||
// filter's ids — resolve through the label endpoint, then rebuild
|
||||
// the chips (without the refresh they'd show raw ids until the
|
||||
// next grid event).
|
||||
void builderData()
|
||||
?.resolveLabelsAsync()
|
||||
.then(() => refreshFilterChips());
|
||||
|
||||
// Trigger HTMX to apply the filter by submitting the form
|
||||
window.htmx.trigger(filterForm, "submit");
|
||||
|
||||
|
||||
@@ -466,20 +466,44 @@ describe("id-kind value cells", () => {
|
||||
url = String(u);
|
||||
return {
|
||||
ok: true,
|
||||
json: async () => ({ results: [{ value: "lib-9", display: "Ebooks", count: 4 }] }),
|
||||
json: async () => ({ labels: { "lib-9": "Ebooks" } }),
|
||||
};
|
||||
}),
|
||||
);
|
||||
c.resolveIdLabels();
|
||||
await vi.waitFor(() => expect(c.idLabels["lib-9"]).toBe("Ebooks"));
|
||||
// the stored id rides as the typed lookup
|
||||
expect(url).toContain("library=lib-9");
|
||||
// resolution rides the LABEL ENDPOINT (entity tables, no media
|
||||
// constraints) — not the suggest facet
|
||||
expect(url).toContain("/api/filter/labels");
|
||||
expect(url).toContain("field=library");
|
||||
expect(url).toContain("ids=lib-9");
|
||||
expect(c.idChipsFor([0]).map((ch) => ch.label)).toEqual(["Ebooks"]);
|
||||
// label resolution never opens a menu
|
||||
expect(c.suggestionsOpen).toBe("");
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("unlabeled ids from a PARTIAL response stay raw, labeled ones resolve", async () => {
|
||||
// the server can only name what exists (invisible/unsubscribed ids
|
||||
// return nothing) — the rest must keep their raw ids, not break
|
||||
const c = filterBuilderData();
|
||||
c.filterTree = {
|
||||
mode: "all",
|
||||
children: [{ field: "library", op: "in", values: ["lib-1,lib-404"] }],
|
||||
};
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => ({
|
||||
ok: true,
|
||||
json: async () => ({ labels: { "lib-1": "Comics" } }),
|
||||
})),
|
||||
);
|
||||
c.resolveIdLabels();
|
||||
await vi.waitFor(() => expect(c.idLabels["lib-1"]).toBe("Comics"));
|
||||
expect(c.idChipsFor([0]).map((ch) => ch.label)).toEqual(["Comics", "lib-404"]);
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("opening the menu after a pick browses the WHOLE list (regression)", async () => {
|
||||
const c = filterBuilderData();
|
||||
c.filterTree = {
|
||||
|
||||
+48
-28
@@ -466,7 +466,7 @@ export function filterBuilderData() {
|
||||
// the tree's id-kind values to names at render time and embedded
|
||||
// the map here — ingest it BEFORE any tree lands, so the cells
|
||||
// show names from the very first paint and resolveIdLabels (the
|
||||
// quiet fetch below) early-returns instead of hitting the server.
|
||||
// label endpoint below) early-returns instead of hitting it.
|
||||
const labelsRaw = el?.dataset.idLabels;
|
||||
if (labelsRaw) {
|
||||
try {
|
||||
@@ -502,7 +502,7 @@ export function filterBuilderData() {
|
||||
this.hydrateFromForm();
|
||||
}
|
||||
// Anything still unlabeled (a tree hydrated from a form the SSR
|
||||
// didn't know about) resolves through the quiet fetch fallback.
|
||||
// didn't know about) resolves through the label endpoint.
|
||||
this.resolveIdLabels();
|
||||
},
|
||||
// treeJSON is the APPLY wire for collection mounts: the page's
|
||||
@@ -725,28 +725,58 @@ export function filterBuilderData() {
|
||||
this.openMenu = "";
|
||||
this._touch();
|
||||
},
|
||||
// resolveIdLabels — a freshly ingested tree (page load, the Apply
|
||||
// reload) carries id-kind values with no labels in sight; one QUIET
|
||||
// fetch per id-kind rule fills idLabels so the cells render names,
|
||||
// not uuids (2026-11-01: the library cell reverted to the id after
|
||||
// Apply because labels only ever came from suggestion picks).
|
||||
// resolveIdLabels — fire-and-forget wrapper (init/ingest paths).
|
||||
resolveIdLabels(): void {
|
||||
const visit = (node: FilterRuleState | FilterGroupState, path: number[]): void => {
|
||||
void this.resolveLabelsAsync();
|
||||
},
|
||||
|
||||
// resolveLabelsAsync — names every UNLABELED id-kind value in the
|
||||
// tree through the label endpoint (entity tables, no media
|
||||
// constraints). The SSR embed covers page renders; this covers
|
||||
// trees the client holds that the server never saw — applying a
|
||||
// SAVED filter from a cold bookshelf (2026-11-01: the old fallback
|
||||
// reused the suggest facet, which labels only the ids whose media
|
||||
// matches the tree's sibling rules). Resolves when labels have
|
||||
// landed so callers can refresh id-labeled UI (the chips bar).
|
||||
async resolveLabelsAsync(): Promise<void> {
|
||||
const byField: Record<string, Set<string>> = {};
|
||||
const visit = (node: FilterRuleState | FilterGroupState): void => {
|
||||
if (isRule(node)) {
|
||||
const meta = FIELD_BY_KEY.get(node.field);
|
||||
if (meta?.valueKind !== "id") return;
|
||||
if (FIELD_BY_KEY.get(node.field)?.valueKind !== "id") return;
|
||||
const ids = node.values
|
||||
.join(",")
|
||||
.split(",")
|
||||
.filter((v) => v.trim() !== "");
|
||||
if (ids.length === 0) return;
|
||||
if (ids.every((v) => this.idLabels[v] !== undefined)) return;
|
||||
void this.fetchSuggestions(path.join("-"), path, node, true);
|
||||
const unlabeled = ids.filter((v) => this.idLabels[v] === undefined);
|
||||
if (unlabeled.length === 0) return;
|
||||
const set = (byField[node.field] ??= new Set<string>());
|
||||
unlabeled.forEach((v) => set.add(v));
|
||||
return;
|
||||
}
|
||||
node.children.forEach((child, idx) => visit(child, [...path, idx]));
|
||||
node.children.forEach(visit);
|
||||
};
|
||||
this.filterTree.children.forEach((child, idx) => visit(child, [idx]));
|
||||
this.filterTree.children.forEach(visit);
|
||||
const fields = Object.keys(byField);
|
||||
if (fields.length === 0) return;
|
||||
await Promise.all(
|
||||
fields.map(async (field) => {
|
||||
const ids = [...byField[field]].join(",");
|
||||
try {
|
||||
const response = await fetch(
|
||||
`/api/filter/labels?field=${encodeURIComponent(field)}&ids=${encodeURIComponent(ids)}`,
|
||||
{ headers: authHeaders() },
|
||||
);
|
||||
if (!response.ok) return;
|
||||
const data = (await response.json()) as { labels?: Record<string, string> };
|
||||
for (const [id, label] of Object.entries(data.labels ?? {})) {
|
||||
this.idLabels[id] = label;
|
||||
}
|
||||
} catch {
|
||||
// offline/failed: cells keep the raw ids (the pre-embed floor)
|
||||
}
|
||||
}),
|
||||
);
|
||||
},
|
||||
|
||||
// valuePlaceholderFor — the value cell's hint. Id-kind cells are
|
||||
@@ -930,20 +960,13 @@ export function filterBuilderData() {
|
||||
void this.fetchSuggestions(rowKey, path, rule);
|
||||
}, 250);
|
||||
},
|
||||
async fetchSuggestions(
|
||||
rowKey: string,
|
||||
path: number[],
|
||||
rule: FilterRuleState,
|
||||
quiet = false,
|
||||
): Promise<void> {
|
||||
async fetchSuggestions(rowKey: string, path: number[], rule: FilterRuleState): Promise<void> {
|
||||
const meta = FIELD_BY_KEY.get(rule.field);
|
||||
if (!meta || !meta.suggest) return;
|
||||
// Fixed-option fields never hit the server.
|
||||
if (meta.valueKind === "fixed") return;
|
||||
const typed = (
|
||||
this.isIdValueRow(path)
|
||||
? (this.idDrafts[path.join("-")] ?? (quiet ? rule.values.join(",") : ""))
|
||||
: (rule.values[0] ?? "")
|
||||
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
|
||||
@@ -952,12 +975,9 @@ export function filterBuilderData() {
|
||||
if (typed.length === 1) return;
|
||||
// cookie auth carries the call; a null bearer must not strand it
|
||||
// Open the menu NOW (loading state) — never a blank flash.
|
||||
// QUIET mode (label resolution) touches no menu state.
|
||||
if (!quiet) {
|
||||
this.suggestions[rowKey] = [];
|
||||
this.suggestionsLoading = rowKey;
|
||||
this.suggestionsOpen = rowKey;
|
||||
}
|
||||
// The menu is computed under the suggest-constraint tree: AND
|
||||
// siblings keep narrowing, OR siblings drop (a same-field pick
|
||||
// negates in, taking its value off the list), and the edited
|
||||
@@ -986,10 +1006,10 @@ export function filterBuilderData() {
|
||||
for (const s of data.results ?? []) {
|
||||
if (s.display) this.idLabels[s.value] = s.display;
|
||||
}
|
||||
if (!quiet) this.suggestions[rowKey] = data.results ?? [];
|
||||
this.suggestions[rowKey] = data.results ?? [];
|
||||
} catch (error) {
|
||||
console.warn("suggestion fetch failed", error);
|
||||
if (!quiet) this.suggestions[rowKey] = [];
|
||||
this.suggestions[rowKey] = [];
|
||||
} finally {
|
||||
if (this.suggestionsLoading === rowKey) this.suggestionsLoading = "";
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user