Bookhoard/bookhoard
0
0 Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 68 Wiki Activity

refactor(collections): retire the legacy rules surface — tree-only storage

Browse Source 
The pre-tree rules machinery is deleted: no row ever carried a rules
array or a non-default match mode (verified in the dev DB), the web
never read the envelope keys, and the Android app consumes none of it.
Dead code gets no documentation.

Removed:
- PUT /api/collections/:id/match-mode — endpoint, route, and the
  UpdateCollectionMatchMode query. The tree's root mode owns any|all.
- auto_assign_rules / rule_match_mode from every envelope
  (create/list/detail/update), from the create/update request shapes,
  and from the schema: both columns DROPPED (schema.sql carries
  idempotent DROP COLUMN IF EXISTS heals for existing DBs; sqlc
  regenerated).
- The rules CRUD service layer (AddRule/SetRuleEnabled/RemoveRule +
  validateRule + UpdateCollectionRulesJSON), ParseRules /
  EnabledRules / IsSmartCollection, RulesToFilterTree (+ the SSR
  preload branch), the legacy branch of EvaluateSmartMembership, and
  SmartEvalOptions.MatchMode. IsCollectionSmart = tree presence;
  create-path isSmart = collection_type=smart OR a filter_tree.

Kept (live consumers): POST /api/collections/preview still evaluates
rules payloads in memory (the custom-section builder's preview), and
include_auto stays on the list (the Android app sends it).

The custom-section builder's SAVE was already broken — its
stringified auto_assign_rules payload failed request binding (probed
live: 400 invalid request). Create now rejects that payload loudly
with a 400 naming the retirement, instead of silently creating a
hollow static section; custom-section-builder.md carries the flag.
The builder's tree migration remains the flagged follow-up.

Test-side (asymmetry rule — deleted behavior, deleted tests):
TestPodcastSmartCollections rewritten onto trees (mode lives in the
tree root), match-mode subtests and tree_wins_over_legacy_rules
deleted, TestRulesToFilterTree deleted, contract-pin envelope keys
retracted per the contract_pins 2026-10-31 retirement entry. Mirrors
hand-synced: openapi.yaml, bruno create/update, docs/api/*.md,
docs/schema.go, api.d.ts. Full container suite green (155s).
This commit is contained in:
John O'Keefe committed 2026-10-31 19:06:48 -04:00
1 parent ab98015190
commit a86bdac87e
27 files changed
+239 -1093
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/handlers/collections.go
+79 -121
View File
@@ -35,12 +35,17 @@ func NewCollectionHandler(db *database.Queries, libraryService *services.Library
}
type CreateCollectionRequest struct {
Name string `form:"name" json:"name" validate:"required"`
Description string `form:"description" json:"description"`
Color string `form:"color" json:"color"`
Icon string `form:"icon" json:"icon"`
CollectionType string `form:"collection_type" json:"collection_type"` // "static" (default) or "smart" (rule-based)
AutoAssignRules []services.Rule `form:"auto_assign_rules" json:"auto_assign_rules"`
Name string `form:"name" json:"name" validate:"required"`
Description string `form:"description" json:"description"`
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.
AutoAssignRules json.RawMessage `json:"auto_assign_rules"`
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
@@ -105,22 +110,17 @@ func resolveTreePayload(jsonTree any, formTree string) any {
type UpdateCollectionRequest struct {
Name string `form:"name" json:"name" validate:"required"`
// Smart-collection match semantics (item 2.4): "any" (OR) | "all"
// (AND). Empty/invalid keeps the stored value.
RuleMatchMode string `form:"rule_match_mode" json:"rule_match_mode"`
// 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. JSON bodies bind FilterTree; form bodies bind
// 400 (ParseFilterTree at the handler). 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"`
AutoAssignRules []services.Rule `form:"auto_assign_rules" json:"auto_assign_rules"`
ViewSettings map[string]interface{} `form:"view_settings" json:"view_settings"`
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"`
ViewSettings map[string]interface{} `form:"view_settings" json:"view_settings"`
// A2c §3.3: playlist toggles — merged into view_settings server-side
// (nil = untouched). []string because the form posts a hidden "false"
// sibling next to the checkbox; the LAST value wins. JSON clients may
@@ -158,15 +158,14 @@ type DeviceMappingSummary struct {
// CollectionSummary is one entry in GET /api/collections. Package-level so
// the docs schema registry (internal/docs) can pin the response shape.
type CollectionSummary struct {
ID uuid.UUID `json:"id"`
Name string `json:"name"`
Description string `json:"description"`
Color string `json:"color"`
Icon string `json:"icon"`
AutoAssignRules json.RawMessage `json:"auto_assign_rules"`
CreatedAt string `json:"created_at"`
BookCount int `json:"book_count"`
IsSmart bool `json:"is_smart"`
ID uuid.UUID `json:"id"`
Name string `json:"name"`
Description string `json:"description"`
Color string `json:"color"`
Icon string `json:"icon"`
CreatedAt string `json:"created_at"`
BookCount int `json:"book_count"`
IsSmart bool `json:"is_smart"`
}
// BookCollectionSummary is one entry in GET /api/collections/:id/books —
@@ -207,6 +206,14 @@ 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)",
})
}
treeJSON, treeErr := parseRequestFilterTree(resolveTreePayload(req.FilterTree, req.FilterTreeForm))
if treeErr != nil {
return c.JSON(http.StatusBadRequest, map[string]string{"error": treeErr.Error()})
@@ -218,7 +225,6 @@ func (h *CollectionHandler) CreateCollection(c *echo.Context) error {
req.Description,
req.Color,
req.Icon,
req.AutoAssignRules,
req.ViewSettings,
treeJSON,
)
@@ -230,19 +236,18 @@ func (h *CollectionHandler) CreateCollection(c *echo.Context) error {
}
bookCount := int32(0)
isSmart := req.CollectionType == "smart" || len(req.AutoAssignRules) > 0
isSmart := req.CollectionType == "smart" || treeJSON != nil
response := map[string]interface{}{
"id": uuid.UUID(collection.ID.Bytes).String(),
"user_id": uuid.UUID(collection.UserID.Bytes).String(),
"name": collection.Name,
"description": utils.PgTextString(collection.Description),
"color": utils.PgTextString(collection.Color),
"icon": utils.PgTextString(collection.Icon),
"collection_type": map[bool]string{true: "smart", false: "static"}[isSmart],
"auto_assign_rules": collection.AutoAssignRules,
"view_settings": collection.ViewSettings,
"book_count": bookCount,
"created_at": collection.CreatedAt.Time.String(),
"id": uuid.UUID(collection.ID.Bytes).String(),
"user_id": uuid.UUID(collection.UserID.Bytes).String(),
"name": collection.Name,
"description": utils.PgTextString(collection.Description),
"color": utils.PgTextString(collection.Color),
"icon": utils.PgTextString(collection.Icon),
"collection_type": map[bool]string{true: "smart", false: "static"}[isSmart],
"view_settings": collection.ViewSettings,
"book_count": bookCount,
"created_at": collection.CreatedAt.Time.String(),
}
// Add redirect header for HTMX requests
@@ -292,7 +297,6 @@ func (h *CollectionHandler) GetCollections(c *echo.Context) error {
matches, evalErr := h.collectionService.EvaluateSmartMembership(c.Request().Context(), col, services.SmartEvalOptions{
LibraryID: libUUID,
VisibleIDs: visibleIDs,
MatchMode: col.RuleMatchMode,
ViewerUserID: MustGetAuthenticatedUser(c).ID,
})
if evalErr == nil {
@@ -310,15 +314,14 @@ func (h *CollectionHandler) GetCollections(c *echo.Context) error {
}
response = append(response, CollectionSummary{
ID: col.ID.Bytes,
Name: col.Name,
Description: utils.PgTextString(col.Description),
Color: utils.PgTextString(col.Color),
Icon: utils.PgTextString(col.Icon),
AutoAssignRules: col.AutoAssignRules,
CreatedAt: col.CreatedAt.Time.String(),
BookCount: bookCount,
IsSmart: isSmart,
ID: col.ID.Bytes,
Name: col.Name,
Description: utils.PgTextString(col.Description),
Color: utils.PgTextString(col.Color),
Icon: utils.PgTextString(col.Icon),
CreatedAt: col.CreatedAt.Time.String(),
BookCount: bookCount,
IsSmart: isSmart,
})
}
@@ -371,7 +374,6 @@ func (h *CollectionHandler) GetCollection(c *echo.Context) error {
matches, evalErr := h.collectionService.EvaluateSmartMembership(c.Request().Context(), collection, services.SmartEvalOptions{
LibraryID: libUUID,
VisibleIDs: visibleIDs,
MatchMode: collection.RuleMatchMode,
ViewerUserID: MustGetAuthenticatedUser(c).ID,
})
if evalErr != nil {
@@ -479,21 +481,19 @@ func (h *CollectionHandler) GetCollection(c *echo.Context) error {
}
return c.JSON(http.StatusOK, map[string]interface{}{
"id": uuid.UUID(collection.ID.Bytes).String(),
"user_id": uuid.UUID(collection.UserID.Bytes).String(),
"name": collection.Name,
"description": utils.PgTextString(collection.Description),
"color": utils.PgTextString(collection.Color),
"icon": utils.PgTextString(collection.Icon),
"auto_assign_rules": json.RawMessage(collection.AutoAssignRules),
"view_settings": viewSettings,
"rule_match_mode": collection.RuleMatchMode,
"filter_tree": treeEnvelope(collection.FilterTree),
"books": bookList,
"book_count": len(bookList),
"is_smart": isSmart,
"up_next": upNext,
"created_at": collection.CreatedAt.Time.String(),
"id": uuid.UUID(collection.ID.Bytes).String(),
"user_id": uuid.UUID(collection.UserID.Bytes).String(),
"name": collection.Name,
"description": utils.PgTextString(collection.Description),
"color": utils.PgTextString(collection.Color),
"icon": utils.PgTextString(collection.Icon),
"view_settings": viewSettings,
"filter_tree": treeEnvelope(collection.FilterTree),
"books": bookList,
"book_count": len(bookList),
"is_smart": isSmart,
"up_next": upNext,
"created_at": collection.CreatedAt.Time.String(),
})
}
@@ -539,9 +539,7 @@ func (h *CollectionHandler) UpdateCollection(c *echo.Context) error {
req.Description,
req.Color,
req.Icon,
req.AutoAssignRules,
req.ViewSettings,
req.RuleMatchMode,
treeJSON,
)
if err != nil {
@@ -555,15 +553,14 @@ func (h *CollectionHandler) UpdateCollection(c *echo.Context) error {
}
response := map[string]interface{}{
"id": uuid.UUID(collection.ID.Bytes).String(),
"user_id": uuid.UUID(collection.UserID.Bytes).String(),
"name": collection.Name,
"description": utils.PgTextString(collection.Description),
"color": utils.PgTextString(collection.Color),
"icon": utils.PgTextString(collection.Icon),
"auto_assign_rules": json.RawMessage(collection.AutoAssignRules),
"view_settings": json.RawMessage(collection.ViewSettings),
"created_at": collection.CreatedAt.Time.String(),
"id": uuid.UUID(collection.ID.Bytes).String(),
"user_id": uuid.UUID(collection.UserID.Bytes).String(),
"name": collection.Name,
"description": utils.PgTextString(collection.Description),
"color": utils.PgTextString(collection.Color),
"icon": utils.PgTextString(collection.Icon),
"view_settings": json.RawMessage(collection.ViewSettings),
"created_at": collection.CreatedAt.Time.String(),
}
// Add redirect header for HTMX requests
@@ -624,9 +621,9 @@ func (h *CollectionHandler) SetCollectionChainOrder(c *echo.Context) error {
if err != nil {
return c.JSON(http.StatusNotFound, map[string]string{"error": "collection not found"})
}
// Same owner gate as the sibling config mutations (SetMatchMode,
// ReorderItems) — my endpoint writes the row, so it must not let a
// non-owner flip someone else's chain order.
// Same owner gate as the sibling config mutation (ReorderItems) —
// my endpoint writes the row, so it must not let a non-owner flip
// someone else's chain order.
if u, ok := c.Get("user").(database.Users); ok {
owns, err := h.collectionService.UserOwnsCollection(c.Request().Context(), collectionID, uuid.UUID(u.ID.Bytes))
if err != nil || !owns {
@@ -1035,15 +1032,13 @@ func (h *CollectionHandler) rejectSmartMutation(c *echo.Context, collectionID uu
return echo.NewHTTPError(http.StatusNotFound, "collection not found")
}
if services.IsCollectionSmart(collection) {
return echo.NewHTTPError(http.StatusBadRequest, "smart collections are rule-based — membership is managed by the collection's rules, not by hand")
return echo.NewHTTPError(http.StatusBadRequest, "smart collections are tree-based — membership is managed by the collection's filter tree, not by hand")
}
return nil
}
// AddRule handles POST /api/collections/:id/rules — appends one rule to
// the ruleset. The first rule converts a static collection to smart.
// treeEnvelope renders the stored filter tree for the envelope: nil
// (legacy/static collections) serializes as null, trees as the object.
// (static collections) serializes as null, trees as the object.
func treeEnvelope(t []byte) any {
if len(t) == 0 {
return nil
@@ -1255,43 +1250,6 @@ func (h *CollectionHandler) PreviewCollection(c *echo.Context) error {
// (A2c §3.2). Owner-only; the payload is the COMPLETE ordered id list —
// ids omitted keep their relative order appended after (the service never
// deletes membership).
// SetMatchMode handles PUT /api/collections/:id/match-mode (plan item
// 2.4): flips the smart ruleset's semantics any|all on its own channel
// — the rules page toggle writes exactly this column, nothing else.
func (h *CollectionHandler) SetMatchMode(c *echo.Context) error {
collectionID, err := uuid.Parse(c.Param("id"))
if err != nil {
return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid collection id"})
}
user, ok := c.Get("user").(database.Users)
if !ok {
return c.JSON(http.StatusUnauthorized, map[string]string{"error": "unauthorized"})
}
var req struct {
Mode string `json:"mode" form:"mode" validate:"required,oneof=any all"`
}
if err := c.Bind(&req); err != nil {
return c.JSON(http.StatusBadRequest, map[string]string{"error": "mode must be any or all"})
}
if err := c.Validate(&req); err != nil {
return c.JSON(http.StatusBadRequest, map[string]string{"error": err.Error()})
}
owns, err := h.collectionService.UserOwnsCollection(c.Request().Context(), collectionID, uuid.UUID(user.ID.Bytes))
if err != nil || !owns {
return c.JSON(http.StatusForbidden, map[string]string{"error": "only the collection owner can change it"})
}
if err := h.db.UpdateCollectionMatchMode(c.Request().Context(), database.UpdateCollectionMatchModeParams{
ID: pgtype.UUID{Bytes: collectionID, Valid: true},
RuleMatchMode: req.Mode,
}); err != nil {
return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
}
return c.JSON(http.StatusOK, map[string]string{"mode": req.Mode})
}
func (h *CollectionHandler) ReorderItems(c *echo.Context) error {
collectionID, err := uuid.Parse(c.Param("id"))
if err != nil {