From 536fb4794c7b997ef8a03b40f4b700abff853e82 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 31 Oct 2026 13:52:40 -0400 Subject: [PATCH] =?UTF-8?q?fix(collections):=20the=20create=20modal's=20se?= =?UTF-8?q?ed=20POST=20400'd=20=E2=80=94=20echo=20cannot=20bind=20a=20form?= =?UTF-8?q?=20string=20into=20an=20any-typed=20field?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real-browser create of an Episodes-scope collection failed with "invalid request": the modal's hx-post submits application/x-www-form-urlencoded, and the form value for filter_tree had to bind into CreateCollectionRequest.FilterTree (any) — echo's form binder errors on any-typed fields instead of setting the string, so every form carrying a tree 400'd before validation even ran. JSON bodies (the save-filter bridge, the editor's Apply) were unaffected, which is why no API-level or Go test caught it: only a browser posting the real form shape exercised the transport. Fix: the form variant binds a dedicated string field (FilterTreeForm, form:"filter_tree") on both request structs while FilterTree keeps the JSON tag; resolveTreePayload picks whichever transport arrived and both ride the same parseRequestFilterTree validation. Pinned by TestEpisodeCollectionsFormSeedCreate (form-encoded seed → 201 → scope round-trips). --- cmd/server/tests/podcast_test.go | 29 ++++++++++++++++++++++++++++ internal/handlers/collections.go | 33 ++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/cmd/server/tests/podcast_test.go b/cmd/server/tests/podcast_test.go index c4657977..40ebee36 100644 --- a/cmd/server/tests/podcast_test.go +++ b/cmd/server/tests/podcast_test.go @@ -2150,3 +2150,32 @@ func TestEpisodeCollectionsEmptySeedMatchesNothing(t *testing.T) { "an empty-seed episodes collection matches nothing — a composer that degraded to the bare scope head would report the whole library here") assert.Empty(t, out.Books) } + +// The create modal posts the seed as a FORM (urlencoded hx-post), and +// echo's binder cannot set an any-typed struct field from a form +// string — the first in-browser create 400'd "invalid request" +// (2026-10-31). The form variant binds a dedicated string field and +// rides the same parse path; this pins the form transport. +func TestEpisodeCollectionsFormSeedCreate(t *testing.T) { + setup := setupTestServer(t) + regToken := setup.RegularToken + marker := fmt.Sprintf("formseed%d", time.Now().UnixNano()%100000) + + form := url.Values{} + form.Set("collection_type", "smart") + form.Set("name", "test FormSeed "+marker) + form.Set("filter_tree", `{"scope":"episodes","mode":"all","children":[]}`) + code, body := podcastAuthedForm(t, setup, regToken, "POST", "/api/collections", form) + require.Equal(t, http.StatusCreated, code, "the FORM-encoded seed must bind and parse: %s", string(body)) + var coll struct { + ID string `json:"id"` + } + require.NoError(t, json.Unmarshal(body, &coll)) + t.Cleanup(func() { + podcastAuthed(t, setup, regToken, "DELETE", "/api/collections/"+coll.ID, "") + }) + + code, body = podcastAuthed(t, setup, regToken, "GET", "/api/collections/"+coll.ID, "") + require.Equal(t, http.StatusOK, code) + assert.Contains(t, string(body), `"scope":"episodes"`, "the seeded scope round-trips") +} diff --git a/internal/handlers/collections.go b/internal/handlers/collections.go index d505c1ee..d8ce4137 100644 --- a/internal/handlers/collections.go +++ b/internal/handlers/collections.go @@ -46,8 +46,16 @@ type CreateCollectionRequest struct { // smart membership runs through the qf engine. INVALID trees are a // 400 (ParseFilterTree at the handler); a tree supersedes // auto_assign_rules. Persisted as-is (any). RawMessage for - // round-trip safety. - FilterTree any `form:"filter_tree" json:"filter_tree"` + // round-trip safety. JSON bodies bind FilterTree; FORM bodies + // (the create modal's urlencoded hx-post) bind FilterTreeForm — + // echo's binder cannot set an any-typed field from a form string + // (the create-modal seed POST 400'd "invalid request" on it, + // 2026-10-31). + FilterTree any `json:"filter_tree"` + // FilterTreeForm mirrors filter_tree for application/x-www-form- + // urlencoded submissions: the raw JSON as a string, validated by + // the same parseRequestFilterTree path. + FilterTreeForm string `form:"filter_tree"` } // treeFilterJSON returns the request's filter tree as JSON storage @@ -85,6 +93,16 @@ func parseRequestFilterTree(payload any) ([]byte, error) { return b, nil } +// resolveTreePayload — the tree payload from either transport: JSON +// bodies carry the object in FilterTree, form bodies (the create +// modal's urlencoded hx-post) the raw JSON string in FilterTreeForm. +func resolveTreePayload(jsonTree any, formTree string) any { + if formTree != "" { + return formTree + } + return jsonTree +} + type UpdateCollectionRequest struct { Name string `form:"name" json:"name" validate:"required"` // Smart-collection match semantics (item 2.4): "any" (OR) | "all" @@ -93,8 +111,11 @@ type UpdateCollectionRequest struct { // The unified filter tree (item 3.2): when present the collection's // smart membership runs through the qf engine. INVALID trees are a // 400 (ParseFilterTree at the handler); a tree supersedes - // auto_assign_rules. - FilterTree any `form:"filter_tree" json:"filter_tree"` + // auto_assign_rules. JSON bodies bind FilterTree; form bodies bind + // FilterTreeForm (echo's binder cannot set an any-typed field from + // a form string — see CreateCollectionRequest). + FilterTree any `json:"filter_tree"` + FilterTreeForm string `form:"filter_tree"` Description string `form:"description" json:"description"` Color string `form:"color" json:"color"` Icon string `form:"icon" json:"icon"` @@ -186,7 +207,7 @@ func (h *CollectionHandler) CreateCollection(c *echo.Context) error { return c.JSON(http.StatusBadRequest, map[string]string{"error": err.Error()}) } - treeJSON, treeErr := parseRequestFilterTree(req.FilterTree) + treeJSON, treeErr := parseRequestFilterTree(resolveTreePayload(req.FilterTree, req.FilterTreeForm)) if treeErr != nil { return c.JSON(http.StatusBadRequest, map[string]string{"error": treeErr.Error()}) } @@ -507,7 +528,7 @@ func (h *CollectionHandler) UpdateCollection(c *echo.Context) error { req.ViewSettings["show_up_next"] = *v } - treeJSON, treeErr := parseRequestFilterTree(req.FilterTree) + treeJSON, treeErr := parseRequestFilterTree(resolveTreePayload(req.FilterTree, req.FilterTreeForm)) if treeErr != nil { return c.JSON(http.StatusBadRequest, map[string]string{"error": treeErr.Error()}) }