Merge branch 'mai/knuth/fix-calendar-filters' (fix: <select multiple> filter strips drop values past first)
This commit is contained in:
@@ -128,20 +128,18 @@ func parseCalendarQuery(r *http.Request, now time.Time) calendarQuery {
|
||||
q.Month = startOfMonth(t.In(now.Location()))
|
||||
}
|
||||
}
|
||||
if v := strings.TrimSpace(r.URL.Query().Get("kind")); v != "" {
|
||||
seen := map[string]bool{}
|
||||
for _, k := range strings.Split(v, ",") {
|
||||
k = strings.TrimSpace(strings.ToLower(k))
|
||||
switch k {
|
||||
case calendarKindEvent, calendarKindTodo, calendarKindDoc:
|
||||
if !seen[k] {
|
||||
seen[k] = true
|
||||
q.Kinds = append(q.Kinds, k)
|
||||
}
|
||||
}
|
||||
// Accept both `?kind=event,doc` (single param, comma-joined) and
|
||||
// `?kind=event&kind=doc` (repeated param, HTMX multi-select form
|
||||
// submission). The latter is what the calendar_section.tmpl form
|
||||
// emits when the user clicks more than one option in the kind chip;
|
||||
// the prior q.Get call dropped everything past the first value.
|
||||
for _, k := range parseValues(r.URL.Query(), "kind") {
|
||||
switch k {
|
||||
case calendarKindEvent, calendarKindTodo, calendarKindDoc:
|
||||
q.Kinds = append(q.Kinds, k)
|
||||
}
|
||||
sort.Strings(q.Kinds)
|
||||
}
|
||||
sort.Strings(q.Kinds)
|
||||
return q
|
||||
}
|
||||
|
||||
|
||||
@@ -236,6 +236,84 @@ func TestCalendarHTMXReturnsSectionOnly(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCalendarFilterMultiValueTagsFromForm reproduces m's bug report —
|
||||
// /calendar filters don't work when the chip-strip form submits a
|
||||
// multi-select. <select multiple name="tag"> serialises as
|
||||
// `?tag=foo&tag=bar` (two URL params with the same key); the prior
|
||||
// ParseTreeFilter implementation used `r.URL.Query().Get("tag")` which
|
||||
// returns only the FIRST value, so the second tag silently dropped and
|
||||
// items matching only the first tag bled through. AND-across-tags is
|
||||
// the contract per TreeFilter.Matches.
|
||||
func TestCalendarFilterMultiValueTagsFromForm(t *testing.T) {
|
||||
srv, pool := mustServer(t)
|
||||
defer pool.Close()
|
||||
h := srv.Routes()
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000000"), ".", "")
|
||||
tagA := "cal-multibug-a-" + stamp
|
||||
tagB := "cal-multibug-b-" + stamp
|
||||
var dev 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)
|
||||
}
|
||||
|
||||
type seed struct {
|
||||
slug, note string
|
||||
tags []string
|
||||
}
|
||||
now := time.Now().UTC().Format("150405")
|
||||
seeds := []seed{
|
||||
{slug: "ab-" + stamp, note: "cal-AB-note-" + now, tags: []string{tagA, tagB}},
|
||||
{slug: "a-" + stamp, note: "cal-A-note-" + now, tags: []string{tagA}},
|
||||
{slug: "b-" + stamp, note: "cal-B-note-" + now, tags: []string{tagB}},
|
||||
}
|
||||
var ids []string
|
||||
for _, s := range seeds {
|
||||
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::text[])
|
||||
returning id`,
|
||||
s.slug, s.slug, dev, s.tags,
|
||||
).Scan(&id); err != nil {
|
||||
t.Fatalf("seed %s: %v", s.slug, err)
|
||||
}
|
||||
ids = append(ids, id)
|
||||
if _, err := pool.Exec(ctx,
|
||||
`insert into projax.item_links (item_id, ref_type, ref_id, rel, note, event_date)
|
||||
values ($1, 'document', $2, 'contains', $3, current_date)`,
|
||||
id, "https://example.com/cal-multibug-"+s.slug, s.note,
|
||||
); err != nil {
|
||||
t.Fatalf("seed link %s: %v", s.slug, err)
|
||||
}
|
||||
}
|
||||
for _, id := range ids {
|
||||
defer pool.Exec(context.Background(), `delete from projax.items where id=$1`, id)
|
||||
}
|
||||
|
||||
// HTMX-style multi-value submission: two `tag=` params, not comma-joined.
|
||||
url := "/calendar?refresh=1&tag=" + tagA + "&tag=" + tagB
|
||||
_, body := get(t, h, url)
|
||||
|
||||
// Item AB has BOTH tags — must appear.
|
||||
if !strings.Contains(body, seeds[0].note) {
|
||||
t.Errorf("expected AB note %q to appear with tag=%s&tag=%s (AND match), got body excerpt: %s",
|
||||
seeds[0].note, tagA, tagB, truncate(body, 600))
|
||||
}
|
||||
// Item A has only tagA — must NOT appear (AND semantics fail tagB).
|
||||
if strings.Contains(body, seeds[1].note) {
|
||||
t.Errorf("BUG: A-only note %q leaked through tag=%s&tag=%s — second tag silently dropped by q.Get(\"tag\")",
|
||||
seeds[1].note, tagA, tagB)
|
||||
}
|
||||
// Item B has only tagB — must NOT appear (AND semantics fail tagA).
|
||||
if strings.Contains(body, seeds[2].note) {
|
||||
t.Errorf("BUG: B-only note %q leaked through tag=%s&tag=%s — second tag silently dropped by q.Get(\"tag\")",
|
||||
seeds[2].note, tagA, tagB)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCalendarCellCarriesLongLabel proves the per-cell long German label
|
||||
// is in the rendered HTML so the mobile breakpoint CSS (≤480px) can
|
||||
// reveal it. The label compensates for the column-header weekday strip
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"log/slog"
|
||||
"mime"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -733,6 +734,23 @@ func parseTimelineExcludeList(raw []string) []string {
|
||||
// parseCSV splits a comma/space-delimited chip input into a deduplicated,
|
||||
// trimmed lowercase string slice. Empty input → []string{} (nil avoided so
|
||||
// JSON/SQL writes get an explicit empty array).
|
||||
// parseValues collects every value for `key` from a url.Values map and
|
||||
// splits each on the same comma/whitespace separators parseCSV accepts.
|
||||
// Handles both filter-strip styles:
|
||||
// - `?tag=foo,bar` (tree page hidden-input chip pattern)
|
||||
// - `?tag=foo&tag=bar` (HTMX multi-select form submission)
|
||||
//
|
||||
// Mixed shapes work too (`?tag=foo,bar&tag=baz` → [foo bar baz]).
|
||||
// Without this, `q.Get(key)` returned only the first value, so the
|
||||
// second tag/mgmt/has selection from any <select multiple> filter strip
|
||||
// silently dropped.
|
||||
func parseValues(q url.Values, key string) []string {
|
||||
if vs, ok := q[key]; ok {
|
||||
return parseCSV(strings.Join(vs, ","))
|
||||
}
|
||||
return []string{}
|
||||
}
|
||||
|
||||
func parseCSV(raw string) []string {
|
||||
if strings.TrimSpace(raw) == "" {
|
||||
return []string{}
|
||||
|
||||
@@ -36,13 +36,20 @@ func (f TreeFilter) Active() bool {
|
||||
|
||||
// ParseTreeFilter pulls the filter state from a URL query.
|
||||
// Defaults: Status=["active"], ShowArchived=false. Other dimensions empty.
|
||||
// Multi-value dimensions (tag/mgmt/status/has) use parseValues so BOTH the
|
||||
// comma-joined hidden-input style (`?tag=foo,bar`, tree page) AND the
|
||||
// repeated-param HTMX multi-select style (`?tag=foo&tag=bar`, every
|
||||
// filter-strip form) round-trip into the same TreeFilter shape. The
|
||||
// prior `q.Get(key)` calls silently dropped every second-and-beyond
|
||||
// value from any multi-select submission — see
|
||||
// TestCalendarFilterMultiValueTagsFromForm for the regression.
|
||||
func ParseTreeFilter(q url.Values) TreeFilter {
|
||||
f := TreeFilter{
|
||||
Q: strings.TrimSpace(q.Get("q")),
|
||||
Tags: parseCSV(q.Get("tag")),
|
||||
Management: parseCSV(q.Get("mgmt")),
|
||||
Status: parseCSV(q.Get("status")),
|
||||
HasLinks: parseCSV(q.Get("has")),
|
||||
Tags: parseValues(q, "tag"),
|
||||
Management: parseValues(q, "mgmt"),
|
||||
Status: parseValues(q, "status"),
|
||||
HasLinks: parseValues(q, "has"),
|
||||
ShowArchived: q.Get("show-archived") == "1",
|
||||
}
|
||||
if v := strings.TrimSpace(q.Get("public")); v != "" {
|
||||
|
||||
Reference in New Issue
Block a user