Files
projax/web/bulk.go
mAi 788479c6cb fix(filters): project dim narrows /admin/bulk + timeline multi-value kind
m reported /timeline filters don't narrow, then clarified that the
project-filter dim added in Phase 5i Slice A (kahn, 13923aa) "doesn't
work ANYWHERE." Systematic reproduction:

  /tree?project=admin         → narrows ✓
  /timeline?project=admin     → narrows ✓
  /calendar?project=admin     → narrows ✓
  /dashboard?project=admin    → narrows ✓
  /admin/bulk?project=admin   → SILENT NO-OP ✗

Plus a small parser bug on /timeline's ?kind=… handling that mirrors
the calendar bug fixed in 6f0a318.

## Root causes

(1) `bulkMatches` in web/bulk.go is a near-clone of `TreeFilter.Matches`
that the Phase 5i Slice A author updated only on Matches itself — the
clone never picked up the ProjectPath block. Filter parses fine, gets
threaded into filterFlat, and silently ignored. `/admin/bulk?project=…`
sees every item.

(2) Timeline's own `?kind=event,doc` parser used
`r.URL.Query().Get("kind")` + comma-split — same shape calendar carried
before commit 6f0a318. When the chip strip's `<select multiple>`
submits `?kind=event&kind=doc`, only the first value lands in q.Kinds.
The user picks two kinds, sees only one applied.

## Fix

bulkMatches gets the ProjectPath block copied verbatim from
TreeFilter.Matches — same predicate, same IncludeDescendants gate,
same multi-parent "ANY path qualifies" semantics.

timeline.parseTimelineQuery's ?kind handling drops the bespoke
Get+Split+dedup-map and uses `parseValues(r.URL.Query(), "kind")` —
the helper already added to web/server.go covers both URL shapes
transparently (`?kind=a,b` and `?kind=a&kind=b`).

## Tests

web/project_filter_test.go (new, 6 tests):
  - TestProjectFilterNarrowsTree
  - TestProjectFilterNarrowsTimeline
  - TestProjectFilterNarrowsCalendar
  - TestProjectFilterNarrowsDashboard
  - TestProjectFilterNarrowsBulk  ← was failing pre-fix
  - TestProjectFilterDescendantsToggle
  - TestTimelineKindMultiValueSurvives  ← was failing pre-fix

The fixture seeds a three-row subtree under dev/ (root + child +
outside sibling) and asserts each surface narrows to root + child
while excluding the outside sibling. The descendants toggle test
flips `?project_descendants=0` and confirms the child drops out.

web/timeline_filter_test.go (new, 3 tests): URL-driven tag narrowing,
multi-value kind parsing, and chip-strip HTMX form target wiring.
These are the immediate "reproduce first" probes athena's brief asked
for; they all PASSED on the pre-fix code (the filter narrowing was
fine on URL paths; the bug was elsewhere) — they stay as defence-in-
depth against future regressions.

## Surfaces double-checked (not broken)

- /graph?project=… dims non-matching nodes instead of narrowing per
  graph.go's explicit comment "the graph deliberately shows the full
  DAG; the filter dims non-matches via opacity unless isolate=1
  hides them." Working as documented.
- The chip strip + project-picker template + Views-page hidden inputs
  all preserve the project value across chip changes — verified by
  template rendering probes.

Full web suite green (76 tests). Pre-existing db/TestBackfillTagsFromArea
unchanged.

Net: +442 / -12.
2026-05-27 14:27:26 +02:00

479 lines
15 KiB
Go
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

package web
import (
"context"
"errors"
"fmt"
"net/http"
"sort"
"strings"
"github.com/jackc/pgx/v5"
"github.com/m/projax/internal/itemwrite"
"github.com/m/projax/store"
)
// handleBulk renders the bulk-edit admin page: a filter bar (reusing the
// tree-page TreeFilter so URL params translate 1:1), a checkbox list of every
// matching item, and an action bar that posts to /admin/bulk/apply. The page
// is intentionally desktop-only — m bulk-edits from a keyboard.
func (s *Server) handleBulk(w http.ResponseWriter, r *http.Request) {
items, err := s.Store.ListAll(r.Context())
if err != nil {
s.fail(w, r, err)
return
}
linkKinds, err := s.linkKindsByItem(r.Context())
if err != nil {
s.fail(w, r, err)
return
}
allTags, err := s.Store.AllTags(r.Context())
if err != nil {
s.fail(w, r, err)
return
}
filter := ParseTreeFilter(r.URL.Query())
// Bulk filters never default to status=active; the page is for editing,
// and m wants to see done/archived rows too. Override the implicit default.
if r.URL.Query().Get("status") == "" {
filter.Status = nil
}
rows := filterFlat(items, filter, linkKinds)
data := map[string]any{
"Title": "bulk edit",
"Rows": rows,
"AllTags": allTags,
"Filter": filter,
"Total": len(items),
"Matched": len(rows),
}
if r.Header.Get("HX-Request") == "true" {
s.render(w, r, "bulk_section", data)
return
}
s.render(w, r, "bulk", data)
}
// filterFlat returns every item matching the filter as a flat sorted list
// (no tree shape — bulk-edit treats items as a flat working set). Sort by
// primary path so siblings cluster together.
func filterFlat(items []*store.Item, f TreeFilter, linkKinds map[string]map[string]struct{}) []*store.Item {
out := []*store.Item{}
for _, it := range items {
// Bulk filter is "match this item directly" — no descendant credit.
// Override status default: if Status is nil treat as "any".
if !bulkMatches(f, it, linkKinds[it.ID]) {
continue
}
out = append(out, it)
}
sort.Slice(out, func(i, j int) bool { return out[i].PrimaryPath() < out[j].PrimaryPath() })
return out
}
// bulkMatches is a near-clone of TreeFilter.Matches but with Status treated
// as "any" when the slice is empty (the bulk-edit page deliberately surfaces
// every status when m has not picked one). Archived is still honoured
// through ShowArchived.
func bulkMatches(f TreeFilter, it *store.Item, itemLinkKinds map[string]struct{}) bool {
if len(f.Status) > 0 && !contains(f.Status, it.Status) {
return false
}
if it.Archived && !f.ShowArchived {
// "any status" mode still hides archived rows unless ShowArchived is
// explicit; otherwise a single archived item would dominate the list.
// When the user filters to status=archived the previous branch already
// accepted it, and ShowArchived stays default-off — which means we'd
// still hide it here. Honour status=archived as an implicit ShowArchived.
if !contains(f.Status, "archived") {
return false
}
}
for _, t := range f.Tags {
if !it.HasTag(t) {
return false
}
}
if len(f.Management) > 0 {
ok := false
for _, m := range f.Management {
if m == "unmanaged" && len(it.Management) == 0 {
ok = true
break
}
if it.HasManagement(m) {
ok = true
break
}
}
if !ok {
return false
}
}
for _, h := range f.HasLinks {
if _, ok := itemLinkKinds[h]; !ok {
return false
}
}
// Phase 5i Slice A: project scope. Same predicate as TreeFilter.Matches —
// at least one of the item's paths must equal ProjectPath, with the
// IncludeDescendants toggle gating the prefix-match for the subtree.
// bulkMatches was a near-clone of Matches() that wasn't updated when
// the project dim landed, so /admin/bulk silently ignored ?project=…
// (and the chip's hidden-input round-trip too).
if f.ProjectPath != "" {
prefix := f.ProjectPath + "."
hit := false
for _, p := range it.Paths {
if p == f.ProjectPath {
hit = true
break
}
if f.IncludeDescendants && strings.HasPrefix(p, prefix) {
hit = true
break
}
}
if !hit {
return false
}
}
if f.Q != "" {
q := strings.ToLower(f.Q)
hit := strings.Contains(strings.ToLower(it.Title), q) ||
strings.Contains(strings.ToLower(it.Slug), q) ||
strings.Contains(strings.ToLower(it.ContentMD), q)
if !hit {
for _, p := range it.Paths {
if strings.Contains(strings.ToLower(p), q) {
hit = true
break
}
}
}
if !hit {
return false
}
}
return true
}
// bulkAction parses the action descriptor sent by the form. Exactly one field
// is non-zero per request; the dispatch picks the matching code path.
type bulkAction struct {
AddTag string // free-text input
RemoveTag string
SetMgmt string // "mai" / "self" / "external" / "clear"
SetStatus string // "active" / "done" / "archived"
SetPublic string // "" / "make_public" / "make_private" — Phase 4d
TimelineTodos string // "" / "exclude" / "include" — Phase 4f
}
func parseBulkAction(r *http.Request) bulkAction {
get := func(k string) string { return strings.TrimSpace(r.FormValue(k)) }
return bulkAction{
AddTag: strings.ToLower(get("add_tag")),
RemoveTag: strings.ToLower(get("remove_tag")),
SetMgmt: get("set_mgmt"),
SetStatus: get("set_status"),
SetPublic: get("set_public"),
TimelineTodos: get("timeline_todos"),
}
}
func (a bulkAction) describe() string {
switch {
case a.AddTag != "":
return "add tag " + a.AddTag
case a.RemoveTag != "":
return "remove tag " + a.RemoveTag
case a.SetMgmt != "":
return "set management " + a.SetMgmt
case a.SetStatus != "":
return "set status " + a.SetStatus
case a.SetPublic == "make_public":
return "make public"
case a.SetPublic == "make_private":
return "make private"
case a.TimelineTodos == "exclude":
return "exclude todos from timeline"
case a.TimelineTodos == "include":
return "re-include todos on timeline"
}
return ""
}
// handleBulkApply applies one action to every selected item in a single
// transaction. The form posts:
// ids=<uuid>&ids=<uuid>&… plus one action field (add_tag / remove_tag /
// set_mgmt / set_status). The action is validated here, dispatched to the
// matching SQL, and returns the re-rendered list partial.
func (s *Server) handleBulkApply(w http.ResponseWriter, r *http.Request) {
if err := r.ParseForm(); err != nil {
s.fail(w, r, err)
return
}
ids := dedupeStrings(r.Form["ids"])
action := parseBulkAction(r)
// Validation banners: re-render the section instead of replacing it with
// a plain-text 400. The previous behaviour caused HTMX swaps to wipe out
// the page chrome when m clicked Apply without ticking a row or filling
// an action field. Two distinct messages so m can tell what he missed.
var banner string
switch {
case len(ids) == 0 && action.describe() == "":
banner = "Pick at least one row and choose an action before clicking Apply."
case len(ids) == 0:
banner = "No rows selected — tick the checkboxes for the rows you want to change."
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
}
}
// HTMX: re-render the list partial with the same filter context (and
// banner, if any). The form sends every filter input alongside the
// action via hx-include="#bulk-filter".
if r.Header.Get("HX-Request") == "true" {
s.renderBulkList(w, r, banner)
return
}
// Non-HTMX fallback: redirect back to the bulk page preserving filters.
dest := "/admin/bulk"
if q := r.FormValue("filter_query"); q != "" {
dest = "/admin/bulk?" + q
}
http.Redirect(w, r, dest, http.StatusSeeOther)
}
// applyBulk runs the action against every id in a single transaction so
// partial failures roll back. Each branch is its own UPDATE because Postgres
// array operators cannot be parameterised cleanly across different operations.
func (s *Server) applyBulk(ctx context.Context, ids []string, a bulkAction) error {
tx, err := s.Store.Pool.BeginTx(ctx, pgx.TxOptions{})
if err != nil {
return fmt.Errorf("begin bulk tx: %w", err)
}
defer tx.Rollback(ctx)
switch {
case a.AddTag != "":
// array_append guards against duplicate tag with a CASE: only append
// when the tag isn't already present.
_, err = tx.Exec(ctx, `
update projax.items
set tags = case when $2 = any(tags) then tags else array_append(tags, $2) end
where id = any($1::uuid[]) and deleted_at is null`,
ids, a.AddTag)
case a.RemoveTag != "":
_, err = tx.Exec(ctx, `
update projax.items
set tags = array_remove(tags, $2)
where id = any($1::uuid[]) and deleted_at is null`,
ids, a.RemoveTag)
case a.SetMgmt != "":
if a.SetMgmt == "clear" {
_, err = tx.Exec(ctx, `
update projax.items set management = '{}'::text[]
where id = any($1::uuid[]) and deleted_at is null`,
ids)
} else {
// Replace management entirely — single-mode semantics matches
// the chip group on detail.tmpl.
_, err = tx.Exec(ctx, `
update projax.items set management = ARRAY[$2]::text[]
where id = any($1::uuid[]) and deleted_at is null`,
ids, a.SetMgmt)
}
case a.SetStatus != "":
_, err = tx.Exec(ctx, `
update projax.items set status = $2
where id = any($1::uuid[]) and deleted_at is null`,
ids, a.SetStatus)
case a.SetPublic == "make_public":
_, err = tx.Exec(ctx, `
update projax.items set public = true
where id = any($1::uuid[]) and deleted_at is null`,
ids)
case a.SetPublic == "make_private":
_, err = tx.Exec(ctx, `
update projax.items set public = false
where id = any($1::uuid[]) and deleted_at is null`,
ids)
case a.TimelineTodos == "exclude":
// Idempotent: append 'todos' only when not already in the array.
_, err = tx.Exec(ctx, `
update projax.items
set timeline_exclude = case
when 'todos' = any(timeline_exclude) then timeline_exclude
else array_append(timeline_exclude, 'todos')
end
where id = any($1::uuid[]) and deleted_at is null`,
ids)
case a.TimelineTodos == "include":
_, err = tx.Exec(ctx, `
update projax.items
set timeline_exclude = array_remove(timeline_exclude, 'todos')
where id = any($1::uuid[]) and deleted_at is null`,
ids)
default:
return errors.New("bulk: empty action")
}
if err != nil {
return fmt.Errorf("bulk %s: %w", a.describe(), err)
}
return tx.Commit(ctx)
}
// normaliseFormStrings deduplicates, lowercases, and trims the slice of
// values a multi-value form key submitted. Mirrors what parseCSV would do
// to a single comma-joined string — but takes the already-split slice that
// r.Form[k] returns for multi-select inputs. Drops empties.
func normaliseFormStrings(in []string) []string {
if len(in) == 0 {
return []string{}
}
seen := map[string]struct{}{}
out := make([]string, 0, len(in))
for _, v := range in {
t := strings.ToLower(strings.TrimSpace(v))
if t == "" {
continue
}
if _, ok := seen[t]; ok {
continue
}
seen[t] = struct{}{}
out = append(out, t)
}
return out
}
// renderBulkList re-renders the bulk section after a bulk apply or a
// validation failure. The apply form ships every filter input alongside the
// action via hx-include, so this reads them straight off r.Form (which the
// caller has already ParseForm'd). banner, when non-empty, surfaces an
// inline note above the action bar.
//
// Multi-value selects (tag, mgmt, status, has) submit ONE name=value pair
// per selected option. r.Form[k] is []string{...}; r.FormValue returns only
// the first. Must use the slice form here or the second+ values silently
// drop on every Apply round-trip.
func (s *Server) renderBulkList(w http.ResponseWriter, r *http.Request, banner string) {
items, err := s.Store.ListAll(r.Context())
if err != nil {
s.fail(w, r, err)
return
}
linkKinds, err := s.linkKindsByItem(r.Context())
if err != nil {
s.fail(w, r, err)
return
}
allTags, err := s.Store.AllTags(r.Context())
if err != nil {
s.fail(w, r, err)
return
}
filter := TreeFilter{
Q: strings.TrimSpace(r.FormValue("q")),
Tags: normaliseFormStrings(r.Form["tag"]),
Management: normaliseFormStrings(r.Form["mgmt"]),
Status: normaliseFormStrings(r.Form["status"]),
HasLinks: normaliseFormStrings(r.Form["has"]),
ShowArchived: r.FormValue("show-archived") == "1",
}
rows := filterFlat(items, filter, linkKinds)
s.render(w, r, "bulk_section", map[string]any{
"Title": "bulk edit",
"Rows": rows,
"AllTags": allTags,
"Filter": filter,
"Banner": banner,
"Total": len(items),
"Matched": len(rows),
})
}
// handleBulkChip handles per-row inline chip edits — the small +/× buttons
// next to a tag chip on the bulk page. POST /admin/bulk/chip with id + op +
// kind + value. op ∈ {add, remove}; kind ∈ {tag, mgmt}. Returns the re-rendered
// chip cell.
func (s *Server) handleBulkChip(w http.ResponseWriter, r *http.Request) {
if err := r.ParseForm(); err != nil {
s.fail(w, r, err)
return
}
id := strings.TrimSpace(r.FormValue("id"))
op := strings.TrimSpace(r.FormValue("op"))
kind := strings.TrimSpace(r.FormValue("kind"))
val := strings.ToLower(strings.TrimSpace(r.FormValue("value")))
if id == "" || op == "" || kind == "" || val == "" {
http.Error(w, "id, op, kind, value all required", http.StatusBadRequest)
return
}
a := bulkAction{}
switch {
case kind == "tag" && op == "add":
a.AddTag = val
case kind == "tag" && op == "remove":
a.RemoveTag = val
case kind == "mgmt" && op == "set":
a.SetMgmt = val
default:
http.Error(w, "unsupported op/kind", http.StatusBadRequest)
return
}
if err := s.applyBulk(r.Context(), []string{id}, a); err != nil {
s.fail(w, r, err)
return
}
it, err := s.Store.GetByID(r.Context(), id)
if err != nil {
s.fail(w, r, err)
return
}
// Return just the chip cell's inner HTML (HTMX hx-swap=innerHTML on the <td>).
// The chip templates render against the Item directly, not a map — we pass
// it through .Item-keyed data so the same fragment-render path works.
switch kind {
case "tag":
s.renderChip(w, "bulk_chip_tags", it)
case "mgmt":
s.renderChip(w, "bulk_chip_mgmt", it)
default:
http.Error(w, "unknown chip kind", http.StatusBadRequest)
}
}
// renderChip renders a chip-cell template directly against an *Item.
func (s *Server) renderChip(w http.ResponseWriter, name string, it *store.Item) {
t, ok := s.pages[name]
if !ok {
http.Error(w, "unknown page: "+name, http.StatusInternalServerError)
return
}
entry := strings.ReplaceAll(name, "_", "-")
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if err := t.ExecuteTemplate(w, entry, it); err != nil {
s.Logger.Error("render chip", "page", name, "err", err)
}
}