Merge branch 'mai/kahn/fix-views-edit-filters' (fix: views edit UI + URL chip overlay on saved-view pages)

web/server.go

@@ -151,7 +151,7 @@ func New(s *store.Store, logger *slog.Logger) (*Server, error) {
 		},
 	}
 	pages := map[string]*template.Template{}
-	for _, name := range []string{"new", "classify", "caldav_admin", "caldav_disabled", "error", "views"} {
+	for _, name := range []string{"new", "classify", "caldav_admin", "caldav_disabled", "error", "views", "view_edit"} {
 		t, err := template.New(name).Funcs(funcs).ParseFS(templatesFS,
 			"templates/layout.tmpl",
 			"templates/"+name+".tmpl",
@@ -384,6 +384,7 @@ func (s *Server) Routes() http.Handler {
 	mux.HandleFunc("POST /admin/caldav/unlink", s.handleCalDAVUnlink)
 	mux.HandleFunc("GET /views", s.handleViewsIndex)
 	mux.HandleFunc("POST /views", s.handleViewCreate)
+	mux.HandleFunc("GET /views/{id}/edit", s.handleViewEdit)
 	mux.HandleFunc("GET /views/", s.handleViewRedirect)
 	mux.HandleFunc("POST /views/", s.handleViewWrite)
 	mux.HandleFunc("GET /login", s.handleLoginForm)

web/templates/view_edit.tmpl

@@ -0,0 +1,42 @@
+{{define "content"}}
+<h1>Edit view</h1>
+<p class="muted"><a href="/views">← back to views</a></p>
+
+<section class="views-create">
+  <form method="post" action="/views/{{.View.ID}}">
+    <label>Name <input type="text" name="name" required maxlength="80" value="{{.View.Name}}"></label>
+    <label>Description <input type="text" name="description" maxlength="200" value="{{.View.Description}}"></label>
+    <label>View type
+      <select name="view_type" required>
+        {{$cur := .View.ViewType}}
+        {{range .AllViewTypes}}<option value="{{.}}"{{if eq . $cur}} selected{{end}}>{{.}}</option>{{end}}
+      </select>
+    </label>
+    <label>Default for
+      <select name="is_default_for">
+        {{$d := deref .View.IsDefaultFor}}
+        {{range .DefaultForOptions}}<option value="{{.}}"{{if eq . $d}} selected{{end}}>{{if eq . ""}}—{{else}}{{.}}{{end}}</option>{{end}}
+      </select>
+    </label>
+    <label>Group by
+      <select name="group_by">
+        {{$g := deref .View.GroupBy}}
+        {{range .GroupByOptions}}<option value="{{.}}"{{if eq . $g}} selected{{end}}>{{if eq . ""}}—{{else}}{{.}}{{end}}</option>{{end}}
+      </select>
+    </label>
+    <label>Sort field <input type="text" name="sort_field" placeholder="title / updated_at / start_time" maxlength="40" value="{{deref .View.SortField}}"></label>
+    <label>Sort dir
+      <select name="sort_dir">
+        {{$sd := deref .View.SortDir}}
+        {{range .SortDirOptions}}<option value="{{.}}"{{if eq . $sd}} selected{{end}}>{{if eq . ""}}—{{else}}{{.}}{{end}}</option>{{end}}
+      </select>
+    </label>
+    <label><input type="checkbox" name="pinned" value="1"{{if .View.Pinned}} checked{{end}}> Pinned</label>
+    <label>Filter (URL query form)
+      <input type="text" name="filter_query" placeholder="tag=work&mgmt=mai" value="{{.FilterQuery}}">
+    </label>
+    <button type="submit">Save changes</button>
+    <a class="muted" href="/views">cancel</a>
+  </form>
+</section>
+{{end}}

web/templates/views.tmpl

@@ -20,6 +20,7 @@
         <td>{{if .IsDefaultFor}}{{deref .IsDefaultFor}}{{else}}<span class="muted">—</span>{{end}}</td>
         <td>{{if .GroupBy}}{{deref .GroupBy}}{{else}}<span class="muted">—</span>{{end}}</td>
         <td>
+          <a href="/views/{{.ID}}/edit">edit</a>
           <form method="post" action="/views/{{.ID}}/delete" style="display:inline">
             <button type="submit" class="link-button" onclick="return confirm('Delete view {{.Name}}?')">delete</button>
           </form>

web/tree_filter.go

@@ -26,6 +26,12 @@ type TreeFilter struct {
 	// exposes an explicit on/off toggle.
 	ProjectPath        string
 	IncludeDescendants bool
+	// Phase 5i fix-shift — saved-view anchor. When set, the URL was
+	// `?view=<uuid>`; chip clicks need to round-trip the value so the user
+	// stays inside the saved view while narrowing further. Not a "filter"
+	// dimension in the matching sense — Matches ignores it — but it lives
+	// in the URL state and on the struct so QueryString preserves it.
+	ViewID string
 }
 
 // Active reports whether any filter dimension is set to something other than
@@ -64,6 +70,7 @@ func ParseTreeFilter(q url.Values) TreeFilter {
 		ShowArchived:       q.Get("show-archived") == "1",
 		ProjectPath:        strings.TrimSpace(q.Get("project")),
 		IncludeDescendants: true,
+		ViewID:             strings.TrimSpace(q.Get("view")),
 	}
 	if v := strings.TrimSpace(q.Get("public")); v != "" {
 		// Treat 1/true/yes/on as true; 0/false/no/off as false; anything else nil.
@@ -125,6 +132,9 @@ func (f TreeFilter) QueryString() string {
 			v.Set("project_descendants", "0")
 		}
 	}
+	if f.ViewID != "" {
+		v.Set("view", f.ViewID)
+	}
 	return v.Encode()
 }
 

web/views.go

@@ -62,6 +62,55 @@ func (s *Server) handleViewCreate(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/views/"+v.ID, http.StatusSeeOther)
 }
 
+// handleViewEdit renders the edit form for an existing view, pre-populated
+// with the row's current values. Submit posts back to /views/<id>.
+func (s *Server) handleViewEdit(w http.ResponseWriter, r *http.Request) {
+	id := strings.TrimSuffix(strings.TrimPrefix(r.URL.Path, "/views/"), "/edit")
+	if id == "" {
+		http.NotFound(w, r)
+		return
+	}
+	v, err := s.Store.GetView(r.Context(), id)
+	if err != nil {
+		if errors.Is(err, store.ErrViewNotFound) {
+			http.NotFound(w, r)
+			return
+		}
+		s.fail(w, r, err)
+		return
+	}
+	filterQuery, err := filterJSONToQuery(v.FilterJSON)
+	if err != nil {
+		s.Logger.Warn("filterJSONToQuery", "id", id, "err", err)
+	}
+	s.render(w, r, "view_edit", map[string]any{
+		"Title":             "edit view",
+		"View":              v,
+		"FilterQuery":       filterQuery,
+		"AllViewTypes":      allViewTypes,
+		"DefaultForOptions": []string{"", "tree", "dashboard", "calendar", "timeline"},
+		"SortDirOptions":    []string{"", "asc", "desc"},
+		"GroupByOptions":    []string{"", "status", "area", "tag", "management"},
+	})
+}
+
+// filterJSONToQuery rebuilds a URL-query representation of a stored
+// filter_json so the edit form can pre-populate the `filter_query` input
+// field. Inverse of filterQueryToJSON.
+func filterJSONToQuery(filterJSON []byte) (string, error) {
+	if len(filterJSON) == 0 {
+		return "", nil
+	}
+	payload := map[string]any{}
+	if err := json.Unmarshal(filterJSON, &payload); err != nil {
+		return "", err
+	}
+	f := filterFromJSONPayload(payload)
+	// QueryString re-emits the canonical URL query form; that's exactly the
+	// shape the form's `filter_query` input expects on round-trip.
+	return f.QueryString(), nil
+}
+
 // handleViewWrite dispatches the /views/<id> POST routes: bare path is
 // update; /views/<id>/delete is soft-delete.
 func (s *Server) handleViewWrite(w http.ResponseWriter, r *http.Request) {
@@ -106,13 +155,19 @@ func (s *Server) handleViewDelete(w http.ResponseWriter, r *http.Request, id str
 // handleViewRedirect resolves /views/<uuid> GET into a redirect to the
 // appropriate Views-supporting page with ?view=<uuid> appended. The target
 // page resolves the saved filter+view_type at render time via
-// applySavedView.
+// applySavedView. /views/<id>/edit is dispatched separately via the more
+// specific route registered first; this handler ignores the edit suffix
+// defensively when the routing pattern doesn't match for some reason.
 func (s *Server) handleViewRedirect(w http.ResponseWriter, r *http.Request) {
 	id := strings.TrimPrefix(r.URL.Path, "/views/")
 	if id == "" {
 		http.NotFound(w, r)
 		return
 	}
+	if strings.HasSuffix(id, "/edit") {
+		s.handleViewEdit(w, r)
+		return
+	}
 	v, err := s.Store.GetView(r.Context(), id)
 	if err != nil {
 		if errors.Is(err, store.ErrViewNotFound) {
@@ -247,9 +302,13 @@ func (s *Server) applyDefaultView(r *http.Request, page string, filter *TreeFilt
 
 // applySavedView resolves a `?view=<uuid>` reference and folds the persisted
 // filter + view_type back into the supplied TreeFilter + view-type slot.
-// Called by every Views-supporting page handler at the top of their render
-// path. Returns the saved view (for chip labelling) or nil when no `?view=`
-// was given. Errors are logged + returned (handlers can choose to ignore).
+// URL chip params OVERLAY the saved filter — a saved view scoped to
+// `dev` with `?tag=work` added narrows further. Transient overlays don't
+// auto-save back to the view (the URL is bookmarkable, but to persist the
+// drift the user opens /views/<id>/edit).
+//
+// Returns the saved view (for chip labelling) or nil when no `?view=` was
+// given. Errors are logged + returned (handlers can choose to ignore).
 func (s *Server) applySavedView(r *http.Request, filter *TreeFilter, viewType *string) (*store.View, error) {
 	id := strings.TrimSpace(r.URL.Query().Get("view"))
 	if id == "" {
@@ -265,13 +324,52 @@ func (s *Server) applySavedView(r *http.Request, filter *TreeFilter, viewType *s
 			return v, fmt.Errorf("decode filter_json: %w", err)
 		}
 	}
-	// Replace filter dimensions with persisted values. Empty / missing keys
-	// reset to TreeFilter defaults so a saved view is the canonical state.
-	*filter = filterFromJSONPayload(payload)
-	*viewType = v.ViewType
+	saved := filterFromJSONPayload(payload)
+	saved.ViewID = id
+	q := r.URL.Query()
+	overlayURLFields(&saved, *filter, q)
+	*filter = saved
+	// view_type: URL wins when explicitly set, otherwise the saved value.
+	if strings.TrimSpace(q.Get("view_type")) == "" {
+		*viewType = v.ViewType
+	}
 	return v, nil
 }
 
+// overlayURLFields lets URL-provided chip values override the saved-view
+// baseline. The URL filter is the parsed-from-query TreeFilter; q is the
+// raw url.Values so we can detect "field was set in the URL" distinct from
+// "field's value happens to equal the zero value".
+func overlayURLFields(base *TreeFilter, urlFilter TreeFilter, q url.Values) {
+	if q.Get("q") != "" {
+		base.Q = urlFilter.Q
+	}
+	if _, ok := q["tag"]; ok {
+		base.Tags = urlFilter.Tags
+	}
+	if _, ok := q["mgmt"]; ok {
+		base.Management = urlFilter.Management
+	}
+	if _, ok := q["status"]; ok {
+		base.Status = urlFilter.Status
+	}
+	if _, ok := q["has"]; ok {
+		base.HasLinks = urlFilter.HasLinks
+	}
+	if q.Get("show-archived") != "" {
+		base.ShowArchived = urlFilter.ShowArchived
+	}
+	if q.Get("public") != "" {
+		base.Public = urlFilter.Public
+	}
+	if q.Get("project") != "" {
+		base.ProjectPath = urlFilter.ProjectPath
+	}
+	if q.Get("project_descendants") != "" {
+		base.IncludeDescendants = urlFilter.IncludeDescendants
+	}
+}
+
 // filterFromJSONPayload is the inverse of filterQueryToJSON. Keys absent
 // from the payload land at their TreeFilter zero value (Status defaults to
 // ["active"] to match ParseTreeFilter).

web/views_test.go

@@ -89,6 +89,156 @@ func TestViewsCRUDRoundTrip(t *testing.T) {
 	}
 }
 
+// TestViewEditFlow exercises the fix for m's bug "we cant edit views yet".
+// GET /views/<id>/edit renders the pre-filled form; POST /views/<id> updates
+// the row in place. Verifies name + view_type + filter_json round-trip.
+func TestViewEditFlow(t *testing.T) {
+	srv, pool := mustServer(t)
+	defer pool.Close()
+	h := srv.Routes()
+	ctx := context.Background()
+
+	stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "")
+	name := "p5i-fix-edit-" + stamp
+	defer pool.Exec(context.Background(),
+		`UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL OR name = $2`,
+		name, name+"-renamed")
+
+	var id string
+	if err := pool.QueryRow(ctx, `
+INSERT INTO projax.views (name, view_type, filter_json)
+VALUES ($1, 'list', $2::jsonb)
+RETURNING id`, name, []byte(`{"tags":["dev"]}`)).Scan(&id); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+
+	// GET /views/<id>/edit renders the pre-filled form (not the redirect).
+	code, body := get(t, h, "/views/"+id+"/edit")
+	if code != 200 {
+		t.Fatalf("GET /views/<id>/edit status=%d, want 200", code)
+	}
+	if !strings.Contains(body, `value="`+name+`"`) {
+		t.Error("edit form should pre-fill the name input")
+	}
+	if !strings.Contains(body, `value="tag=dev"`) {
+		t.Error("edit form should pre-fill filter_query from filter_json")
+	}
+
+	// Index page now shows an edit link per row.
+	_, idx := get(t, h, "/views")
+	if !strings.Contains(idx, `/views/`+id+`/edit`) {
+		t.Error("/views should expose an edit link per row")
+	}
+
+	// POST /views/<id> updates the row.
+	form := url.Values{}
+	form.Set("name", name+"-renamed")
+	form.Set("view_type", "card")
+	form.Set("filter_query", "tag=work&mgmt=mai")
+	code, _ = post(t, h, "/views/"+id, form)
+	if code != 303 {
+		t.Fatalf("POST /views/<id> status=%d, want 303", code)
+	}
+
+	var newName, newType string
+	var newFilter []byte
+	if err := pool.QueryRow(ctx,
+		`SELECT name, view_type, filter_json FROM projax.views WHERE id = $1`, id,
+	).Scan(&newName, &newType, &newFilter); err != nil {
+		t.Fatalf("post-update read: %v", err)
+	}
+	if newName != name+"-renamed" {
+		t.Errorf("name = %q, want %q", newName, name+"-renamed")
+	}
+	if newType != "card" {
+		t.Errorf("view_type = %q, want 'card'", newType)
+	}
+	payload := map[string]any{}
+	_ = json.Unmarshal(newFilter, &payload)
+	tags, _ := payload["tags"].([]any)
+	if len(tags) != 1 || tags[0] != "work" {
+		t.Errorf("filter_json tags = %v, want [work] post-update", payload["tags"])
+	}
+}
+
+// TestSavedViewPageFilterApply exercises the fix for m's bug "the filters on
+// custom views dont seem to work". A request to /?view=<id>&tag=work narrows
+// the saved view further by overlaying the URL chip onto the persisted
+// filter_json. Previously the saved filter clobbered the URL chips
+// wholesale.
+func TestSavedViewPageFilterApply(t *testing.T) {
+	srv, pool := mustServer(t)
+	defer pool.Close()
+	h := srv.Routes()
+	ctx := context.Background()
+
+	stamp := strings.ReplaceAll(time.Now().UTC().Format("150405.000"), ".", "")
+	name := "p5i-fix-overlay-" + stamp
+	devSlug := "p5i-fix-overlay-d-" + stamp
+	homeSlug := "p5i-fix-overlay-h-" + stamp
+
+	defer pool.Exec(context.Background(),
+		`UPDATE projax.views SET deleted_at = now() WHERE name = $1 AND deleted_at IS NULL`, name)
+
+	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)
+	}
+	var devID, homeID string
+	if err := pool.QueryRow(ctx, `
+INSERT INTO projax.items (kind, title, slug, parent_ids, tags)
+VALUES (array['project']::text[], 'Fix Dev', $1, ARRAY[$2]::uuid[], ARRAY['work'])
+RETURNING id`, devSlug, dev).Scan(&devID); err != nil {
+		t.Fatalf("seed dev item: %v", err)
+	}
+	if err := pool.QueryRow(ctx, `
+INSERT INTO projax.items (kind, title, slug, parent_ids, tags)
+VALUES (array['project']::text[], 'Fix Home', $1, ARRAY[$2]::uuid[], ARRAY['home'])
+RETURNING id`, homeSlug, home).Scan(&homeID); err != nil {
+		t.Fatalf("seed home item: %v", err)
+	}
+	defer pool.Exec(context.Background(), `delete from projax.items where id in ($1,$2)`, devID, homeID)
+
+	// Saved view with view_type=list and NO tag filter — both items should pass.
+	var id string
+	if err := pool.QueryRow(ctx, `
+INSERT INTO projax.views (name, view_type, filter_json)
+VALUES ($1, 'list', '{}'::jsonb)
+RETURNING id`, name).Scan(&id); err != nil {
+		t.Fatalf("seed view: %v", err)
+	}
+
+	devLink := `href="/i/dev.` + devSlug + `"`
+	homeLink := `href="/i/home.` + homeSlug + `"`
+
+	// Open view alone — both rows should appear.
+	_, baseBody := get(t, h, "/?view="+id)
+	if !strings.Contains(baseBody, devLink) {
+		t.Error("saved view without tag filter should show dev row")
+	}
+	if !strings.Contains(baseBody, homeLink) {
+		t.Error("saved view without tag filter should show home row")
+	}
+
+	// Overlay ?tag=work — home row should disappear; dev should remain.
+	_, narrowedBody := get(t, h, "/?view="+id+"&tag=work")
+	if !strings.Contains(narrowedBody, devLink) {
+		t.Error("?view=<id>&tag=work should still show dev row (work-tagged)")
+	}
+	if strings.Contains(narrowedBody, homeLink) {
+		t.Error("?view=<id>&tag=work should hide home row — URL chip must overlay saved filter")
+	}
+
+	// Chip URLs inside the saved view must round-trip the view= param so
+	// chip clicks don't strip the saved view.
+	if !strings.Contains(narrowedBody, "view="+id) {
+		t.Error("chip URLs inside a saved view should carry view=<id> forward")
+	}
+}
+
 // TestDefaultViewAppliedOnCleanURL verifies the Slice E behaviour: when /
 // is requested with no chip params and a default view exists for the page,
 // the saved filter + view_type apply and a "Showing default view: …"