Keep what was typed when a target or webhook edit is refused (closes #381)

A refused save on the target edit page now shows the edit form again,
with the reason above it and every value submitted, instead of a bare
text page; the status codes are unchanged. The webhook edit page keeps
the submitted name, description and retention the same way, while the
page still reports the stored retention.

Target edits are validated by setTargetFromForm, which newTarget now
uses too, so the add and edit forms accept and refuse the same things.
An empty max_retries keeps the target's own count.

The browser test also saves both edit pages with refused values.

Model: opus-5-5

internal/handlers/target_edit.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"errors"
 	"net/http"
+	"strconv"
 
 	"github.com/go-chi/chi"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -13,10 +14,13 @@ import (
 const targetEditTemplate = "target_edit.html"
 
 // tmplKeyTarget is the template data key for the target being
-// edited, and tmplKeyMaxTimeout for the timeout ceiling the form
-// tells the user about.
+// edited, tmplKeyTargetForm for the values its form shows, and
+// tmplKeyMaxTimeout for the timeout ceiling the form tells the user
+// about. The add target form on the webhook page takes its values
+// under the same key as the edit form.
 const (
 	tmplKeyTarget     = "Target"
+	tmplKeyTargetForm = "TargetForm"
 	tmplKeyMaxTimeout = "MaxTimeout"
 )
 
@@ -28,20 +32,19 @@ const configUnreadableMessage = "The stored configuration for this " +
 	"target could not be read. Enter the values below; saving " +
 	"replaces the stored configuration."
 
-// targetEditView is the display model for the target edit page.
+// targetEditView is the display model for the target edit page: the
+// target's row fields as stored. The values the form shows, the
+// UNMASKED configuration among them, come separately, as a
+// targetFormInput.
 //
-// It carries the target's row fields alongside its UNMASKED
-// configuration, and deliberately omits database.Target's raw
-// Config blob: the form renders named fields, and giving the
-// template the blob as well would put an unreviewed second path to
-// the credential on the page.
+// It deliberately omits database.Target's raw Config blob: the form
+// renders named fields, and giving the template the blob as well
+// would put an unreviewed second path to the credential on the page.
 type targetEditView struct {
-	ID         string
-	Name       string
-	Type       database.TargetType
-	Active     bool
-	MaxRetries int
-	Config     delivery.TargetConfigForm
+	ID     string
+	Name   string
+	Type   database.TargetType
+	Active bool
 }
 
 // HandleTargetEdit shows the form to edit a target.
@@ -73,7 +76,18 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
 			msg = configUnreadableMessage
 		}
 
-		h.renderTargetEdit(w, r, webhook, target, cfg, msg)
+		form := targetFormInput{
+			Name:       target.Name,
+			URL:        cfg.URL,
+			Headers:    cfg.Headers,
+			Timeout:    cfg.Timeout,
+			MaxRetries: strconv.Itoa(target.MaxRetries),
+			Expiry:     cfg.Expiry,
+		}
+
+		h.renderTargetEdit(
+			w, r, webhook, target, form, msg, http.StatusOK,
+		)
 	}
 }
 
@@ -101,11 +115,12 @@ func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
 	}
 }
 
-// applyTargetEdit validates and saves target edits.
+// applyTargetEdit validates and saves target edits. A refused save
+// shows the edit form again with the values submitted and the reason.
 //
-// The submitted configuration goes through buildTargetConfig, the
-// same builder the create path uses, so an edited destination is
-// SSRF-validated exactly as a new one is.
+// The submission goes through setTargetFromForm, as a new target
+// does, so an edited destination is SSRF-validated exactly as a new
+// one is.
 //
 // The target's type is not editable. Each type stores a different
 // configuration shape and its delivery history is recorded against
@@ -118,16 +133,13 @@ func (h *Handlers) applyTargetEdit(
 	webhook database.Webhook,
 	target *database.Target,
 ) {
-	name := r.PostFormValue("name")
-	if name == "" {
-		http.Error(w, "Name is required", http.StatusBadRequest)
+	in := targetFormInputFrom(r)
 
-		return
-	}
+	// edited is the target as the submission leaves it; target stays
+	// as stored, for the page shown again when the save is refused.
+	edited := *target
 
-	configJSON, errMsg, err := h.buildTargetConfig(
-		r.Context(), target.Type, targetFormInputFrom(r),
-	)
+	errMsg, err := h.setTargetFromForm(r.Context(), &edited, in)
 	if err != nil {
 		h.serverError(w, r, "failed to encode target config", err)
 
@@ -135,45 +147,26 @@ func (h *Handlers) applyTargetEdit(
 	}
 
 	if errMsg != "" {
-		http.Error(w, errMsg, http.StatusBadRequest)
+		h.renderTargetEdit(
+			w, r, webhook, target, in, errMsg, http.StatusBadRequest,
+		)
 
 		return
 	}
 
-	// Retries are offered only by the forms for target types that
-	// retry, so an absent field means "this form does not edit
-	// retries" rather than "set them to zero". Reading it
-	// unconditionally would silently disable retries on any target
-	// saved from a form that does not render the input.
-	//
-	// A field that IS submitted but does not parse is a 400, through
-	// the same validator the create path uses. It is rejected before
-	// anything is written, so a typo cannot destroy the retry count
-	// the target is already delivering with.
-	if r.PostForm.Has("max_retries") {
-		retries, ok := targetMaxRetries(w, r, target.MaxRetries)
-		if !ok {
-			return
-		}
-
-		target.MaxRetries = retries
-	}
-
-	oldName := target.Name
-	target.Name = name
-	target.Config = configJSON
-
 	// A new name renames the archive file before it is saved (see
 	// delivery.Engine.Rename). If either step fails, it goes back to
 	// the name that is still stored.
-	err = h.renameTargetArchive(target, webhook.Name, oldName, name)
+	err = h.renameTargetArchive(
+		target, webhook.Name, target.Name, edited.Name,
+	)
 	if err == nil {
-		err = h.db.DB().Save(target).Error
+		err = h.db.DB().Save(&edited).Error
 	}
 
 	if err != nil {
 		restoreErr := h.renameTargetArchive(
-			target, webhook.Name, name, oldName,
+			target, webhook.Name, edited.Name, target.Name,
 		)
 		if restoreErr != nil {
 			h.log.Error(
@@ -184,8 +177,8 @@ func (h *Handlers) applyTargetEdit(
 		}
 
 		if errors.Is(err, delivery.ErrArchiveNameTaken) {
-			http.Error(
-				w,
+			h.renderTargetEdit(
+				w, r, webhook, target, in,
 				"Not saved: "+err.Error()+
 					". Move that archive out of the data directory, "+
 					"its .db together with any -wal and -shm beside "+
@@ -222,15 +215,17 @@ func (h *Handlers) renameTargetArchive(
 	return h.archives.Rename(target.ID, webhookName, newName)
 }
 
-// renderTargetEdit renders the target edit page with an optional
-// error message.
+// renderTargetEdit renders the target edit page for the target as
+// stored, its form showing form's values, with an optional error
+// message above it.
 func (h *Handlers) renderTargetEdit(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
 	target *database.Target,
-	cfg delivery.TargetConfigForm,
+	form targetFormInput,
 	errMsg string,
+	status int,
 ) {
 	// The template calls Webhook methods, which take pointer
 	// receivers; html/template cannot address a value stored in a
@@ -238,18 +233,17 @@ func (h *Handlers) renderTargetEdit(
 	data := map[string]any{
 		tmplKeyWebhook: &webhook,
 		tmplKeyTarget: targetEditView{
-			ID:         target.ID,
-			Name:       target.Name,
-			Type:       target.Type,
-			Active:     target.Active,
-			MaxRetries: target.MaxRetries,
-			Config:     cfg,
+			ID:     target.ID,
+			Name:   target.Name,
+			Type:   target.Type,
+			Active: target.Active,
 		},
+		tmplKeyTargetForm: form,
 		tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds,
 		tmplKeyError:      errMsg,
 	}
 
-	h.renderTemplate(w, r, targetEditTemplate, data)
+	h.renderTemplateStatus(w, r, targetEditTemplate, data, status)
 }
 
 // ownedTarget resolves the request's sourceID and targetID