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 in6f0a318. ## 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 commit6f0a318. 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.
479 lines
15 KiB
Go
479 lines
15 KiB
Go
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)
|
||
}
|
||
}
|