From 9ee26002f84748ccc91960920608fa7f32252730 Mon Sep 17 00:00:00 2001 From: mAi Date: Fri, 22 May 2026 00:36:14 +0200 Subject: [PATCH] refactor(web): validate item writes via internal/itemwrite/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5c slice B. Three web write paths now pre-validate via the itemwrite package before calling store.Create / Update / Reparent. - handleDetailWrite: ValidateFormat + ValidateAgainstStore on (title, slug, status, parent_ids) before the store.Update call. - handleNewSubmit: same pair, scoped to a new item (no ID yet). - handleReparent: format + DB-aware checks; validator catches self-parent, unknown-parent, cycle. The existing "parent_ids required" guard stays as a separate fast-fail. - handleBulkApply: set_status pre-flight against the validator. Other bulk actions (add_tag / set_mgmt / set_public / timeline_todos) don't mutate validated fields so they pass through unchanged. On ValidationError the handler responds 400 + a human banner keyed on err.Kind via the new s.itemWriteFailure helper. itemWriteBannerCopy centralises the Kind→copy mapping so web/server.go and web/bulk.go share one phrasing. No web test source touched — all web/*_test.go assert on observable behaviour (HTTP status, response body) and the new validator path preserves both for valid AND invalid inputs the SQL trigger would have rejected anyway. Tests stay green unmodified. Task: t-projax-5c-itemwrite --- web/bulk.go | 11 ++++++ web/server.go | 97 +++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/web/bulk.go b/web/bulk.go index f199120..fe4b66f 100644 --- a/web/bulk.go +++ b/web/bulk.go @@ -10,6 +10,7 @@ import ( "github.com/jackc/pgx/v5" + "github.com/m/projax/internal/itemwrite" "github.com/m/projax/store" ) @@ -208,6 +209,16 @@ func (s *Server) handleBulkApply(w http.ResponseWriter, r *http.Request) { case action.describe() == "": banner = "No action chosen — type a tag, pick a management mode, or pick a status before clicking Apply." default: + // Pre-flight bulk action via itemwrite where applicable. set_status + // is the only bulk action today that mutates a validated field + // (status enum); the others (add_tag / set_mgmt / set_public / + // timeline_todos) operate outside the validator's rule set. + if action.SetStatus != "" { + if ve := itemwrite.ValidateFormat(itemwrite.Input{Title: "x", Slug: "x", Status: action.SetStatus}); ve != nil { + banner = "Cannot apply: " + itemWriteBannerCopy(ve) + break + } + } if err := s.applyBulk(r.Context(), ids, action); err != nil { s.fail(w, r, err) return diff --git a/web/server.go b/web/server.go index aa3f561..528365b 100644 --- a/web/server.go +++ b/web/server.go @@ -16,9 +16,47 @@ import ( "github.com/m/projax/internal/aggregate" "github.com/m/projax/internal/cache" + "github.com/m/projax/internal/itemwrite" "github.com/m/projax/store" ) +// itemWriteFailure surfaces an *itemwrite.ValidationError to the client. +// HTTP code: 400 for invalid input. The body is a one-line human banner +// keyed on Kind so handlers don't have to duplicate copy-table fragments. +// Phase 5c uses this instead of the pre-existing raw-pgErr-on-failure +// pattern in handleDetailWrite / handleNewSubmit / handleReparent. +func (s *Server) itemWriteFailure(w http.ResponseWriter, r *http.Request, ve *itemwrite.ValidationError) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.WriteHeader(http.StatusBadRequest) + fmt.Fprintln(w, itemWriteBannerCopy(ve)) + s.Logger.Warn("itemwrite reject", "path", r.URL.Path, "kind", ve.Kind, "detail", ve.Detail) +} + +// itemWriteBannerCopy maps a ValidationError.Kind to the human-facing +// banner copy. Centralised so web/server.go + web/bulk.go share one +// authoritative phrasing. +func itemWriteBannerCopy(ve *itemwrite.ValidationError) string { + switch ve.Kind { + case itemwrite.KindMissingRequired: + return "Missing required field: " + ve.Detail + case itemwrite.KindInvalidSlugFormat: + return ve.Detail + case itemwrite.KindInvalidStatus: + return ve.Detail + case itemwrite.KindSelfParent: + return "An item cannot be its own parent." + case itemwrite.KindUnknownParent: + return ve.Detail + case itemwrite.KindSlugCollision: + return ve.Detail + case itemwrite.KindCycle: + return "Cannot reparent: this move would put the item in its own ancestor closure." + case itemwrite.KindUnresolvablePath: + return ve.Detail + } + return "Invalid input: " + ve.Detail +} + // Register MIME types stdlib doesn't ship by default. The web-app manifest // spec requires application/manifest+json for the `` → // without this Go's FileServer falls back to text/plain and Chrome refuses @@ -511,12 +549,27 @@ func (s *Server) handleDetailWrite(w http.ResponseWriter, r *http.Request) { } } parentIDs = dedupeStrings(parentIDs) + title := strings.TrimSpace(r.FormValue("title")) + slug := strings.TrimSpace(r.FormValue("slug")) + status := strings.TrimSpace(r.FormValue("status")) + if ve := itemwrite.ValidateFormat(itemwrite.Input{ + ID: it.ID, Title: title, Slug: slug, Status: status, ParentIDs: parentIDs, Path: it.PrimaryPath(), + }); ve != nil { + s.itemWriteFailure(w, r, ve) + return + } + if ve := itemwrite.ValidateAgainstStore(r.Context(), s.Store, itemwrite.Input{ + ID: it.ID, Title: title, Slug: slug, Status: status, ParentIDs: parentIDs, Path: it.PrimaryPath(), + }); ve != nil { + s.itemWriteFailure(w, r, ve) + return + } in := store.UpdateInput{ - Title: strings.TrimSpace(r.FormValue("title")), - Slug: strings.TrimSpace(r.FormValue("slug")), + Title: title, + Slug: slug, ParentIDs: parentIDs, ContentMD: r.FormValue("content_md"), - Status: strings.TrimSpace(r.FormValue("status")), + Status: status, Pinned: r.FormValue("pinned") == "1", Archived: r.FormValue("archived") == "1", Tags: parseCSV(r.FormValue("tags")), @@ -567,6 +620,21 @@ func (s *Server) handleReparent(w http.ResponseWriter, r *http.Request, path str http.Error(w, "reparent: parent_ids required", http.StatusBadRequest) return } + // Reparent doesn't change title/slug/status, so the validator only + // exercises rules around parent_ids: self-parent, unknown-parent, + // cycle. Format check runs against the existing item's fields. + if ve := itemwrite.ValidateFormat(itemwrite.Input{ + ID: it.ID, Title: it.Title, Slug: it.Slug, Status: it.Status, ParentIDs: parentIDs, Path: it.PrimaryPath(), + }); ve != nil { + s.itemWriteFailure(w, r, ve) + return + } + if ve := itemwrite.ValidateAgainstStore(r.Context(), s.Store, itemwrite.Input{ + ID: it.ID, Title: it.Title, Slug: it.Slug, Status: it.Status, ParentIDs: parentIDs, Path: it.PrimaryPath(), + }); ve != nil { + s.itemWriteFailure(w, r, ve) + return + } moved, err := s.Store.Reparent(r.Context(), it.ID, parentIDs) if err != nil { s.fail(w, r, err) @@ -700,13 +768,30 @@ func (s *Server) handleNewSubmit(w http.ResponseWriter, r *http.Request) { } } parentIDs = dedupeStrings(parentIDs) + title := strings.TrimSpace(r.FormValue("title")) + slug := strings.TrimSpace(r.FormValue("slug")) + status := strings.TrimSpace(r.FormValue("status")) + // New items have no ID yet — pre-flight format checks (title/slug/status) + // then DB-aware checks (parent existence + slug collision under parents). + if ve := itemwrite.ValidateFormat(itemwrite.Input{ + Title: title, Slug: slug, Status: status, ParentIDs: parentIDs, + }); ve != nil { + s.itemWriteFailure(w, r, ve) + return + } + if ve := itemwrite.ValidateAgainstStore(r.Context(), s.Store, itemwrite.Input{ + Title: title, Slug: slug, Status: status, ParentIDs: parentIDs, + }); ve != nil { + s.itemWriteFailure(w, r, ve) + return + } in := store.CreateInput{ Kind: []string{kind}, - Title: strings.TrimSpace(r.FormValue("title")), - Slug: strings.TrimSpace(r.FormValue("slug")), + Title: title, + Slug: slug, ParentIDs: parentIDs, ContentMD: r.FormValue("content_md"), - Status: strings.TrimSpace(r.FormValue("status")), + Status: status, Tags: parseCSV(r.FormValue("tags")), Management: parseCSV(r.FormValue("management")), }