Keep a chosen image older than the picker's list, and the layout on a rejected save

The image picker's select lists the newest recentImages library images and
was meant to add the chosen one when it is older than those, but the fallback
option was gated on the filename being unknown, which designData makes known:
the option never rendered, the browser selected "None", and every save of the
design form cleared a logo or favicon older than the 48 newest images (the
post form's featured image the same way). The partial now checks whether the
choice is listed and otherwise emits it, named.

handleDesign ran the uploads before parsing the module and menu rows and gave
up at the first problem, so a rejected upload rendered the form without any
rows — and, since a missing row means delete, the next save would have wiped
the layout and menu. Everything is read before the first problem is reported.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sd8UPWrvyYCLj97JexNw3A

4 files changed, 83 insertions, 32 deletions

diff --git a/AGENTS.md b/AGENTS.md
index 5753d26..b7bd557 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -144,7 +144,10 @@ internal/web/         server.go (host router, middleware, render helpers)
   the modules of every area (with their settings inline, folded in a
   `<details>`) and the menu — stored atomically by `BlogStore.SaveDesign`
   (theme + full module list + full menu in one transaction: rows with an id
-  are updated, without one inserted, missing ones deleted). Sections are cards
+  are updated, without one inserted, missing ones deleted). Because a missing
+  row means delete, `handleDesign` reads *everything* — modules, menu, then
+  the uploads — before reporting the first problem, and the rejected form is
+  rendered from all of it; never bail out half-way. Sections are cards
   with ids (`designSections`) a sticky side index jumps to; the hidden `at`
   field says which one was in view so the flash returns there. Module rows are
   `mod.<i>.{id,area,kind,title,body,count,pos,del}` and menu rows
diff --git a/internal/web/handlers_design.go b/internal/web/handlers_design.go
index 5853436..acc7569 100644
--- a/internal/web/handlers_design.go
+++ b/internal/web/handlers_design.go
@@ -141,39 +141,49 @@ func (s *Server) handleDesign(w http.ResponseWriter, r *http.Request) {
 	if p, ok := PresetByKey(r.FormValue("preset")); ok {
 		v.Theme = v.Theme.WithPreset(p)
 	}
-	// The form is shown again with everything that was sent, plus the message.
-	fail := func(msg string) {
-		d, err := s.designData(r, v)
-		if err != nil {
-			s.serverError(w, err)
-			return
-		}
-		d["error"] = msg
-		s.renderStatus(w, r, http.StatusBadRequest, "dashboard/design.html", d)
-	}
-	// Optional direct uploads from the pickers.
-	for field, dst := range map[string]*string{"bg_image_file": &v.Theme.BgImage, "header_image_file": &v.Theme.HeaderImage, "logo_file": &v.Theme.Logo, "favicon_file": &v.Theme.Favicon} {
-		img, err := s.readUpload(r, field, true)
-		if err != nil {
-			fail(err.Error())
-			return
-		}
-		if img != nil {
-			*dst = img.ID.String()
+	// Every part of the form is read before any problem is reported, so the
+	// form shown again carries all of it: a module or menu row that was left
+	// out here would be deleted by the next save. The first problem is the
+	// one reported.
+	var problem error
+	note := func(err error) {
+		if problem == nil {
+			problem = err
 		}
 	}
 	var err error
-	if v.Modules, err = parseModules(r.Form, lang, blog.Language); err != nil {
-		fail(err.Error())
-		return
-	}
+	v.Modules, err = parseModules(r.Form, lang, blog.Language)
+	note(err)
 	pages, err := blogStore(r).ListPages(r.Context())
 	if err != nil {
 		s.serverError(w, err)
 		return
 	}
-	if v.Menu, err = parseMenu(r.Form, lang, pages); err != nil {
-		fail(err.Error())
+	v.Menu, err = parseMenu(r.Form, lang, pages)
+	note(err)
+	// Optional direct uploads from the pickers. One that is stored stays
+	// chosen in the form even when something else is rejected.
+	for _, u := range []struct {
+		field string
+		dst   *string
+	}{{"bg_image_file", &v.Theme.BgImage}, {"header_image_file", &v.Theme.HeaderImage}, {"logo_file", &v.Theme.Logo}, {"favicon_file", &v.Theme.Favicon}} {
+		img, err := s.readUpload(r, u.field, true)
+		if err != nil {
+			note(err)
+			continue
+		}
+		if img != nil {
+			*u.dst = img.ID.String()
+		}
+	}
+	if problem != nil {
+		d, err := s.designData(r, v)
+		if err != nil {
+			s.serverError(w, err)
+			return
+		}
+		d["error"] = problem.Error()
+		s.renderStatus(w, r, http.StatusBadRequest, "dashboard/design.html", d)
 		return
 	}
 	if err := blogStore(r).SaveDesign(r.Context(), v.Theme.JSON(), v.Modules, v.Menu); err != nil {
diff --git a/internal/web/templates/partials/imagepick.html b/internal/web/templates/partials/imagepick.html
index ed9368c..5062d42 100644
--- a/internal/web/templates/partials/imagepick.html
+++ b/internal/web/templates/partials/imagepick.html
@@ -1,9 +1,11 @@
 {{/* One image field of a form (the design page, a post's featured image).
-     The select lists the recent images (plus the chosen one) so it works
-     without the script; with it the select is hidden and "Choose from
-     library…" opens the form's imagelib panel, which fetches the library
-     page by page and marks the pickers with .js. */}}
-{{define "imagepick"}}{{$cur := .current}}<div class="imagepick" data-name="{{.name}}">
+     The select lists the recent images so it works without the script; a
+     chosen image older than those gets its own option (named through .names,
+     or "Current image" once the file is gone), so the select always carries
+     the choice and a save keeps it. With the script the select is hidden and
+     "Choose from library…" opens the form's imagelib panel, which fetches the
+     library page by page and marks the pickers with .js. */}}
+{{define "imagepick"}}{{$cur := .current}}{{$listed := false}}{{range .images}}{{if eq .ID.String $cur}}{{$listed = true}}{{end}}{{end}}<div class="imagepick" data-name="{{.name}}">
   <label>{{.label}}</label>
   {{if .hint}}<p class="muted small">{{.hint}}</p>{{end}}
   <div class="pickrow">
@@ -12,7 +14,7 @@
       <select name="{{.name}}" class="ip-nojs">
         <option value="none"{{if not $cur}} selected{{end}}>{{t "None"}}</option>
         {{range .images}}<option value="{{.ID}}"{{if eq .ID.String $cur}} selected{{end}}>{{.Filename}}</option>{{end}}
-        {{if and $cur (not (index .names $cur))}}<option value="{{$cur}}" selected>{{t "Current image"}}</option>{{end}}
+        {{if and $cur (not $listed)}}<option value="{{$cur}}" selected>{{with index .names $cur}}{{.}}{{else}}{{t "Current image"}}{{end}}</option>{{end}}
       </select>
       <span class="ip-jsonly"><button type="button" class="secondary small" data-pick="open">{{t "Choose from library…"}}</button> <button type="button" class="secondary small" data-pick="clear">{{t "None"}}</button></span>
       <label class="upload">{{t "…or upload a new one"}}<br><input type="file" name="{{.name}}_file" accept="{{if .accept}}{{.accept}}{{else}}image/*{{end}}"></label>
diff --git a/internal/web/web_test.go b/internal/web/web_test.go
index b707339..fcd1e41 100644
--- a/internal/web/web_test.go
+++ b/internal/web/web_test.go
@@ -691,3 +691,39 @@ func TestPostSummary(t *testing.T) {
 		t.Error("summary: expected a cut")
 	}
 }
+
+// A chosen image older than the recent ones the picker lists must still be
+// the select's selected option, else saving the form clears it (the logo of
+// a blog with more than recentImages images vanished on every save).
+func TestImagePickKeepsOlderChoice(t *testing.T) {
+	tpl, err := newTemplates(false).get("en", "dashboard/design.html")
+	if err != nil {
+		t.Fatal(err)
+	}
+	recent := uuid.New()
+	old := uuid.New()
+	for _, tc := range []struct {
+		name  string
+		names map[string]string
+		want  string
+	}{
+		{"known name", map[string]string{recent.String(): "new.png", old.String(): "logo.png"}, `<option value="` + old.String() + `" selected>logo.png</option>`},
+		{"unknown name", map[string]string{recent.String(): "new.png"}, `<option value="` + old.String() + `" selected>Current image</option>`},
+	} {
+		var b strings.Builder
+		err := tpl.ExecuteTemplate(&b, "imagepick", map[string]any{
+			"name": "logo", "current": old.String(), "sub": "x", "label": "Logo",
+			"images": []store.File{{ID: recent, Filename: "new.png"}}, "names": tc.names,
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		out := b.String()
+		if !strings.Contains(out, tc.want) {
+			t.Errorf("%s: the chosen image is not the selected option:\n%s", tc.name, out)
+		}
+		if strings.Contains(out, `<option value="none" selected>`) {
+			t.Errorf("%s: None must not be selected when an image is chosen", tc.name)
+		}
+	}
+}