feat(dashboard): custom-section builder saves again — the flagged tree migration
Archaeology, prompted by the user asking when it broke: the builder's save NEVER worked. The page was born (2026-02-19,a92bf99a) posting auto_assign_rules as a JSON-encoded STRING into an endpoint that has bound []Rule since its creation (2026-01-31,6495cc2c), plus fields (manual_book_ids, match_type, library_id, show_on_dashboard) the endpoint never accepted. The preview button hit a real endpoint, so the page looked alive while Save always failed at request binding. Verified live before this change: the page's exact payload 400'd "invalid request" against the unmodified server. The fix adapts the builder's payload at write time — the legacy rules vocabulary converts to a filter tree (RulesToFilterTree restored from the retirement commit as the single write path, unit test restored with it): - match_type seeds the converted tree's root mode (any default). - manual_book_ids attach to the created collection after create (unparsable ids skipped — the AddBooks leniency). - show_on_dashboard persists (new create field). - Converted trees validate under ParseFilterTree: an unhonorable rule is a 400 at write naming the problem, never a stored 500 at render. Dashboard sections: smart user sections now evaluate their tree LIVE — GetUserCollectionItems merges stored membership with the composed engine's matches (deduped, library-scoped, capped at the section limit). This exposed a REAL production wiring bug: every production NewDashboardService call omitted the search service, so tree evaluation through the dashboard could never have worked (the test harness wired it correctly — the gap was production-only and latent, because nothing in the dashboard evaluated trees before this migration). The dashboard handler and both inline sites now receive the search service, and evaluation failures log instead of silently emptying a section. Supersedes the same-day loud-400 stance for the builder's payload (contract_pins migration entry); custom-section-builder.md documents the save contract; the create_collection API doc gains the adapted fields. New TestCustomSectionBuilderSave/BadRule pin the payload → tree → dashboard-render path end to end. Full container suite green (155s).
This commit is contained in:
John O'Keefe committed
1 parent
a86bdac87e
commit
7c7e282e79
11 files changed
+445
-36
No files matched your search
@@ -40,12 +40,16 @@ type CreateCollectionRequest struct {
|
||||
Color string `form:"color" json:"color"`
|
||||
Icon string `form:"icon" json:"icon"`
|
||||
CollectionType string `form:"collection_type" json:"collection_type"` // "static" (default) or "smart"
|
||||
// Retired custom-section-builder payload (2026-10-31): the rules
|
||||
// ARRAY storage is gone — the filter tree is the only smart
|
||||
// membership. The field exists ONLY so the builder's legacy save
|
||||
// fails LOUDLY (400) instead of silently creating a hollow static
|
||||
// section. Remove it at the builder's tree migration.
|
||||
// Custom-section builder payload (adapted 2026-10-31): the page
|
||||
// posts the legacy rules vocabulary — rules as a JSON-encoded array
|
||||
// (a string), a match_type select, hand-picked book ids, and the
|
||||
// dashboard pin. The server converts rules → filter tree at write
|
||||
// time and attaches the manual picks; storage stays tree-only.
|
||||
AutoAssignRules json.RawMessage `json:"auto_assign_rules"`
|
||||
MatchType string `json:"match_type"`
|
||||
ManualBookIDs []string `json:"manual_book_ids"`
|
||||
LibraryID string `json:"library_id"` // selection context only — not stored
|
||||
ShowOnDashboard bool `json:"show_on_dashboard"`
|
||||
ViewSettings map[string]interface{} `form:"view_settings" json:"view_settings"`
|
||||
// The unified filter tree (item 3.2): when present the collection's
|
||||
// smart membership runs through the qf engine. INVALID trees are a
|
||||
@@ -63,6 +67,28 @@ type CreateCollectionRequest struct {
|
||||
FilterTreeForm string `form:"filter_tree"`
|
||||
}
|
||||
|
||||
// parseBuilderRules decodes the custom-section builder's rules payload:
|
||||
// the page posts JSON.stringify(rules) — a JSON string CONTAINING the
|
||||
// array (it has done so since the page was written, 2026-02-19) — while
|
||||
// JSON clients may post the array directly. Null/empty = no rules.
|
||||
func parseBuilderRules(raw json.RawMessage) ([]services.Rule, error) {
|
||||
if len(raw) == 0 || string(raw) == "null" {
|
||||
return nil, nil
|
||||
}
|
||||
var rules []services.Rule
|
||||
if raw[0] == '"' {
|
||||
var encoded string
|
||||
if err := json.Unmarshal(raw, &encoded); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if strings.TrimSpace(encoded) == "" {
|
||||
return nil, nil
|
||||
}
|
||||
return rules, json.Unmarshal([]byte(encoded), &rules)
|
||||
}
|
||||
return rules, json.Unmarshal(raw, &rules)
|
||||
}
|
||||
|
||||
// treeFilterJSON returns the request's filter tree as JSON storage
|
||||
// bytes (nil when none): object payloads marshal; strings (a legacy
|
||||
// double-encoded client, the custom-section builder's known bug class)
|
||||
@@ -206,18 +232,37 @@ func (h *CollectionHandler) CreateCollection(c *echo.Context) error {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": err.Error()})
|
||||
}
|
||||
|
||||
// The custom-section builder still posts the RETIRED rules payload
|
||||
// (its save was already failing to bind — verified 2026-10-31).
|
||||
// Reject loudly rather than silently creating a hollow section.
|
||||
if len(req.AutoAssignRules) > 0 {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{
|
||||
"error": "auto_assign_rules was retired — collections carry filter trees now (the custom-section builder needs its flagged tree migration)",
|
||||
})
|
||||
// Custom-section builder adapter: convert the posted rules to a
|
||||
// filter tree at WRITE time (the save-fix — the page's payload had
|
||||
// never once bound successfully before this).
|
||||
builderRules, rulesErr := parseBuilderRules(req.AutoAssignRules)
|
||||
if rulesErr != nil {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": "auto_assign_rules: " + rulesErr.Error()})
|
||||
}
|
||||
matchMode := req.MatchType
|
||||
if matchMode != services.RuleMatchAll && matchMode != services.RuleMatchAny {
|
||||
matchMode = services.RuleMatchAny
|
||||
}
|
||||
treeJSON, treeErr := parseRequestFilterTree(resolveTreePayload(req.FilterTree, req.FilterTreeForm))
|
||||
if treeErr != nil {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": treeErr.Error()})
|
||||
}
|
||||
if treeJSON == nil && len(builderRules) > 0 {
|
||||
tree, cerr := services.RulesToFilterTree(builderRules, matchMode)
|
||||
if cerr != nil {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": "section rule: " + cerr.Error()})
|
||||
}
|
||||
b, merr := json.Marshal(tree)
|
||||
if merr != nil {
|
||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": merr.Error()})
|
||||
}
|
||||
// The strict-parse guarantee holds for converted trees too: an
|
||||
// invalid tree is a 400 at write, never a stored 500 at render.
|
||||
if _, verr := services.ParseFilterTree(string(b)); verr != nil {
|
||||
return c.JSON(http.StatusBadRequest, map[string]string{"error": "section rule: " + verr.Error()})
|
||||
}
|
||||
treeJSON = b
|
||||
}
|
||||
collection, err := h.collectionService.CreateCollection(
|
||||
c.Request().Context(),
|
||||
userUUID,
|
||||
@@ -226,6 +271,7 @@ func (h *CollectionHandler) CreateCollection(c *echo.Context) error {
|
||||
req.Color,
|
||||
req.Icon,
|
||||
req.ViewSettings,
|
||||
req.ShowOnDashboard,
|
||||
treeJSON,
|
||||
)
|
||||
if err != nil {
|
||||
@@ -235,6 +281,17 @@ func (h *CollectionHandler) CreateCollection(c *echo.Context) error {
|
||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
}
|
||||
|
||||
// Manual picks: a section = rules + hand-picked books. Attach after
|
||||
// create; unparsable ids are skipped (same leniency as AddBooks).
|
||||
for _, bookIDStr := range req.ManualBookIDs {
|
||||
bookID, perr := uuid.Parse(bookIDStr)
|
||||
if perr != nil {
|
||||
continue
|
||||
}
|
||||
_ = h.collectionService.AddBookToCollection(c.Request().Context(),
|
||||
uuid.UUID(collection.ID.Bytes), bookID, userUUID)
|
||||
}
|
||||
|
||||
bookCount := int32(0)
|
||||
isSmart := req.CollectionType == "smart" || treeJSON != nil
|
||||
response := map[string]interface{}{
|
||||
@@ -383,7 +440,7 @@ func (h *CollectionHandler) GetCollection(c *echo.Context) error {
|
||||
} else if collection.IsSystemCollection.Bool && collection.QueryType.Valid && collection.QueryType.String != "" && collection.QueryType.String != "filter" {
|
||||
user := c.Get("user").(database.Users)
|
||||
userUUID := uuid.UUID(user.ID.Bytes)
|
||||
dashboardSvc := services.NewDashboardService(h.db)
|
||||
dashboardSvc := services.NewDashboardService(h.db, h.collectionService.Search())
|
||||
sections, secErr := dashboardSvc.GetDashboardSections(c.Request().Context(), userUUID, libUUID, 1000, []string{}, []string{})
|
||||
if secErr != nil {
|
||||
return c.JSON(http.StatusInternalServerError, map[string]string{"error": secErr.Error()})
|
||||
|
||||
Reference in new issue
Block a user