m/projax
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(phase 3n bulk): un-nest chip-add form, inline banner for empty Apply, multi-value filter preserved

Browse Source 
Three structural bugs from Phase 3d caught by m's "doesn't work" report:

1. The chip-add <form class="chip-add" ...> was rendered INSIDE the outer
   <form id="bulk-actions" ...> in bulk_section.tmpl. HTML forbids
   nested forms — browsers silently flatten them, so the chip-add's
   hx-trigger="submit" never fired and pressing Enter in any chip-add
   input dispatched the outer Apply form instead. Replaced the inner
   <form> with a <span class="chip-add"> wrapping an input that fires
   hx-post directly on Enter (hx-trigger="keyup[key=='Enter']") plus an
   explicit + button. No more nested forms. New TestBulkPageHasNoNested
   Forms regression-guards via a substring check on the rendered HTML.

2. handleBulkApply 400'd on empty ids OR empty action via http.Error,
   which HTMX swapped into #bulk-section as a plain-text error page —
   the page chrome vanished and the user saw "no action chosen". Now
   the handler validates inputs, sets a banner string, and falls through
   to renderBulkList (the section re-renders with the banner inline).
   Banner copy is task-specific so m can tell what he missed.

3. renderBulkList read filter values with r.FormValue, which returns
   ONLY the first value for multi-value names. Multi-select tag/mgmt/
   status filters dropped their 2nd+ values on every Apply round-trip.
   Switched to r.Form["..."] + a new normaliseFormStrings helper that
   dedupes / lowercases / trims the slice. TestBulkApplyRendersWithFilter
   Preserved regression-guards.

All 3 bugs caught by tests written first (TestBulkPageHasNoNestedForms,
TestBulkApplyEmpty{Action,Ids}RendersInlineBanner, TestBulkApplyRenders
WithFilterPreserved). Existing 4 bulk tests still green; full test suite
green.
This commit is contained in:
mAi
2026-05-16 01:25:48 +02:00
parent 2825078486
commit 838793ee69
4 changed files with 298 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

89
web/bulk.go
View File

@@ -181,27 +181,32 @@ func (s *Server) handleBulkApply(w http.ResponseWriter, r *http.Request) {
return
}
ids := dedupeStrings(r.Form["ids"])
if len(ids) == 0 {
http.Error(w, "no rows selected", http.StatusBadRequest)
return
}
action := parseBulkAction(r)
if action.describe() == "" {
http.Error(w, "no action chosen", http.StatusBadRequest)
return
// 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:
if err := s.applyBulk(r.Context(), ids, action); err != nil {
s.fail(w, r, err)
return
}
}
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.
// 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" {
// Re-derive filter from referrer query so the result list matches the
// view the user is on. The HTMX form sends hx-include=closest #bulk-filter
// so filter inputs are in the form body — read those.
s.renderBulkList(w, r)
s.renderBulkList(w, r, banner)
return
}
// Non-HTMX fallback: redirect back to the bulk page preserving filters.
@@ -265,10 +270,41 @@ func (s *Server) applyBulk(ctx context.Context, ids []string, a bulkAction) erro
return tx.Commit(ctx)
}
// renderBulkList re-renders only the rows list + summary line after a bulk
// apply. Builds a fresh r-equivalent that re-uses the filter posted alongside
// the form (the page includes filter inputs in the apply submission).
func (s *Server) renderBulkList(w http.ResponseWriter, r *http.Request) {
// 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)
@@ -284,14 +320,12 @@ func (s *Server) renderBulkList(w http.ResponseWriter, r *http.Request) {
s.fail(w, r, err)
return
}
// The apply form ships the same filter fields as the bulk page itself, so
// ParseTreeFilter against r.Form gives us the right view.
filter := TreeFilter{
Q: strings.TrimSpace(r.FormValue("q")),
Tags: parseCSV(r.FormValue("tag")),
Management: parseCSV(r.FormValue("mgmt")),
Status: parseCSV(r.FormValue("status")),
HasLinks: parseCSV(r.FormValue("has")),
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)
@@ -300,6 +334,7 @@ func (s *Server) renderBulkList(w http.ResponseWriter, r *http.Request) {
"Rows": rows,
"AllTags": allTags,
"Filter": filter,
"Banner": banner,
"Total": len(items),
"Matched": len(rows),
})

207
web/bulk_repro_test.go Normal file
View File

@@ -0,0 +1,207 @@
package web_test
import (
"context"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"
)
// TestBulkPageHasNoNestedForms guards against the chip-add <form> being
// nested inside the bulk-actions <form>. HTML forbids nested forms and
// browsers silently flatten them — the chip-add's hx-trigger="submit"
// stops firing and the chip-add inputs leak into the outer Apply form.
// Phase 3n regression.
func TestBulkPageHasNoNestedForms(t *testing.T) {
srv, pool := mustServer(t)
defer pool.Close()
h := srv.Routes()
_, body := get(t, h, "/admin/bulk")
// Locate bulk-actions form and verify NO inner <form> opens before its
// matching </form>. A naive but robust check: between the opening
// <form id="bulk-actions" ...> and the next </form>, there must be no
// other <form opening.
startTag := `<form id="bulk-actions"`
open := strings.Index(body, startTag)
if open < 0 {
t.Fatalf("bulk page lacks <form id=\"bulk-actions\">; body sample:\n%s", body[:min3(800, len(body))])
}
tail := body[open:]
end := strings.Index(tail, "</form>")
if end < 0 {
t.Fatalf("bulk-actions form never closes")
}
inner := tail[len(startTag):end]
if strings.Contains(inner, "<form") {
t.Errorf("bulk-actions form contains a nested <form ...>: HTML forbids this and browsers flatten it. Slice:\n%s",
inner[:min3(1200, len(inner))])
}
}
// TestBulkApplyEmptyActionRendersInlineBanner: clicking Apply without
// filling an action field should not 400 + plain-text error page (which
// HTMX swaps as the whole #bulk-section). It should render the section
// with an inline banner. Phase 3n regression.
func TestBulkApplyEmptyActionRendersInlineBanner(t *testing.T) {
srv, pool := mustServer(t)
defer pool.Close()
h := srv.Routes()
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
// Seed one item we can tick a checkbox for.
stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000000"), ".", "")
slug := "bulk-empty-act-" + stamp
var dev, id string
if err := pool.QueryRow(ctx, `select id from projax.items where slug='dev' and cardinality(parent_ids)=0`).Scan(&dev); err != nil {
t.Fatalf("dev: %v", err)
}
if err := pool.QueryRow(ctx,
`insert into projax.items (kind, title, slug, parent_ids)
values (array['project']::text[], 'bulk-empty', $1, ARRAY[$2]::uuid[])
returning id`,
slug, dev,
).Scan(&id); err != nil {
t.Fatalf("seed: %v", err)
}
defer pool.Exec(context.Background(), `delete from projax.items where id=$1`, id)
form := url.Values{}
form.Set("ids", id)
// Intentionally NO add_tag / remove_tag / set_mgmt / set_status set.
req := httptest.NewRequest(http.MethodPost, "/admin/bulk/apply", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("HX-Request", "true")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
code := w.Result().StatusCode
body := readBodyString(t, w.Result().Body)
if code != 200 {
t.Fatalf("expected 200 with inline banner for empty Apply, got %d body=%s", code, body)
}
if !strings.Contains(body, `id="bulk-section"`) {
t.Errorf("response should re-render the bulk section, not replace it with an error message")
}
if !strings.Contains(strings.ToLower(body), "action") {
t.Errorf("response should contain a 'no action chosen' style banner; body:\n%s", body[:min3(600, len(body))])
}
}
// TestBulkApplyEmptyIdsRendersInlineBanner: clicking Apply with no rows
// ticked should also render the section with a banner, not 400.
func TestBulkApplyEmptyIdsRendersInlineBanner(t *testing.T) {
srv, pool := mustServer(t)
defer pool.Close()
h := srv.Routes()
form := url.Values{}
form.Set("add_tag", "x") // action chosen but no ids
req := httptest.NewRequest(http.MethodPost, "/admin/bulk/apply", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("HX-Request", "true")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
code := w.Result().StatusCode
body := readBodyString(t, w.Result().Body)
if code != 200 {
t.Fatalf("expected 200 with inline banner for empty ids, got %d body=%s", code, body)
}
if !strings.Contains(body, `id="bulk-section"`) {
t.Errorf("response should re-render the bulk section, not replace it with an error message")
}
}
// TestBulkApplyRendersWithFilterPreserved: after Apply, the re-rendered
// section should keep multi-value filter selections (tag=A AND tag=B, the
// browser sends two name=tag entries). Pre-fix renderBulkList used
// r.FormValue which only returns the FIRST value, dropping B.
func TestBulkApplyRendersWithFilterPreserved(t *testing.T) {
srv, pool := mustServer(t)
defer pool.Close()
h := srv.Routes()
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
// Seed two items in different areas, distinct tags so we can verify the
// filter narrows.
stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000000"), ".", "")
var dev, home string
if err := pool.QueryRow(ctx, `select id from projax.items where slug='dev' and cardinality(parent_ids)=0`).Scan(&dev); err != nil {
t.Fatalf("dev: %v", err)
}
if err := pool.QueryRow(ctx, `select id from projax.items where slug='home' and cardinality(parent_ids)=0`).Scan(&home); err != nil {
t.Fatalf("home: %v", err)
}
mk := func(parent, slug string, tags []string) string {
var id string
if err := pool.QueryRow(ctx,
`insert into projax.items (kind, title, slug, parent_ids, tags)
values (array['project']::text[], $1, $2, ARRAY[$3]::uuid[], $4)
returning id`,
"f", slug, parent, tags,
).Scan(&id); err != nil {
t.Fatalf("seed %s: %v", slug, err)
}
return id
}
tagA := "bulkfilt-a-" + stamp
tagB := "bulkfilt-b-" + stamp
devSlug := "bulkfilt-dev-" + stamp
homeSlug := "bulkfilt-home-" + stamp
devID := mk(dev, devSlug, []string{tagA, tagB})
homeID := mk(home, homeSlug, []string{tagA})
defer func() {
_, _ = pool.Exec(context.Background(), `delete from projax.items where id in ($1, $2)`, devID, homeID)
}()
// Apply: ids=devID, add_tag=newtag, and tag filter has BOTH tagA AND tagB
// (multi-select scenario).
form := url.Values{}
form.Add("ids", devID)
form.Set("add_tag", "newtag-"+stamp)
form.Add("tag", tagA)
form.Add("tag", tagB)
req := httptest.NewRequest(http.MethodPost, "/admin/bulk/apply", strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("HX-Request", "true")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
if w.Result().StatusCode != 200 {
body := readBodyString(t, w.Result().Body)
t.Fatalf("apply → %d body=%s", w.Result().StatusCode, body)
}
body := readBodyString(t, w.Result().Body)
// dev-item carries both tags → matches the AND filter → should appear in the rendered list.
if !strings.Contains(body, fmt.Sprintf("/i/dev.%s", devSlug)) {
t.Errorf("expected dev-row to remain visible after Apply (filter AND-matches both tags)")
}
// home-item carries only tagA → should NOT appear (filter requires BOTH).
if strings.Contains(body, fmt.Sprintf("/i/home.%s", homeSlug)) {
t.Errorf("home-row should be filtered out — has only tagA, filter required tagA+tagB")
}
}
// readBodyString reads an HTTP response body as a string. Helper used by
// the regression tests since httptest.NewRecorder().Result().Body is a
// ReadCloser.
func readBodyString(t *testing.T, r io.ReadCloser) string {
t.Helper()
defer r.Close()
b, _ := io.ReadAll(r)
return string(b)
}
func min3(a, b int) int {
if a < b {
return a
}
return b
}

7
web/static/style.css
View File

@@ -180,8 +180,13 @@ table.bulk .chip-x {
background: transparent; border: none; color: var(--muted); padding: 0 4px; cursor: pointer; font-size: 1em;
}
table.bulk .chip-x:hover { color: var(--bad); }
table.bulk .chip-add { display: inline-block; margin-left: 4px; }
table.bulk .chip-add { display: inline-flex; align-items: center; gap: 2px; margin-left: 4px; }
table.bulk .chip-add input { padding: 1px 4px; font-size: 0.85em; width: 7em; }
table.bulk .chip-add-btn {
background: transparent; border: 1px solid var(--border); color: var(--accent);
padding: 0 6px; font-size: 0.9em; cursor: pointer; min-height: 0; border-radius: 3px;
}
table.bulk .chip-add-btn:hover { background: var(--accent); color: #fff; }
/* --- /dashboard --- */
.dashboard .dash-grid { display: grid; grid-template-columns: 1fr; gap: 16px; margin-top: 12px; }

33
web/templates/bulk_section.tmpl
View File

@@ -45,6 +45,8 @@
<p class="counts"><strong>{{.Matched}}</strong> / <strong>{{.Total}}</strong> items match</p>
</section>
{{with .Banner}}<p class="banner warn">{{.}}</p>{{end}}
<form id="bulk-actions"
method="post"
action="/admin/bulk/apply"
@@ -125,16 +127,27 @@
title="remove tag">×</button>
</span>
{{end}}
<form class="chip-add" onsubmit="return false"
hx-post="/admin/bulk/chip"
hx-target="#cell-tags-{{.ID}}"
hx-swap="innerHTML"
hx-trigger="submit">
<input type="hidden" name="id" value="{{.ID}}">
<input type="hidden" name="op" value="add">
<input type="hidden" name="kind" value="tag">
<input name="value" placeholder="+tag" size="6">
</form>
{{/* No nested <form> — HTML forbids it. The input fires hx-post on
Enter; the +-button is a click trigger on the same endpoint. Both
carry id/op/kind via hx-vals; the input's own value field rides
along as name="value". */}}
<span class="chip-add">
<input name="value" placeholder="+tag" size="6"
id="chip-add-value-{{.ID}}"
hx-post="/admin/bulk/chip"
hx-target="#cell-tags-{{.ID}}"
hx-swap="innerHTML"
hx-trigger="keyup[key=='Enter']"
hx-vals='{"id":"{{.ID}}","op":"add","kind":"tag"}'>
<button type="button"
class="chip-add-btn"
hx-post="/admin/bulk/chip"
hx-target="#cell-tags-{{.ID}}"
hx-swap="innerHTML"
hx-include="#chip-add-value-{{.ID}}"
hx-vals='{"id":"{{.ID}}","op":"add","kind":"tag"}'
title="add tag">+</button>
</span>
{{end}}
{{define "bulk-chip-mgmt"}}