Offer archive expiry choices on the target forms, show plain units (closes #396)

A database target's archive expiry was typed by hand as never or a raw duration such as 720h, and the target list showed it back raw. Adding or editing a database target now offers the new-webhook page's list of choices (never, 1h, 12h, 24h, 30d, 90d, 365d), defined once and shared by all three forms. The edit form starts on the stored expiry, or on the submitted one after a refused save; a stored value outside the choices is listed under its own value, so saving unchanged keeps it. The target list shows the expiry in plain units: "30 days", "12 hours", "never".

Model: opus-5-5

README.md

@@ -1721,8 +1721,11 @@ events should be forwarded.
   own archive database
   (`archive-{webhook_name}-{target_name}-{target_uuid}.db`) for long-term
   retention, with an optional creation-validated expiry (default: keep
-  forever). No external delivery and no retries; an archive write
-  failure fails the delivery. See the database target section under
+  forever). The new webhook form, the add target form and the target edit
+  form all offer the same expiries: never, 1h, 12h, 24h, 30d, 90d or 365d.
+  The target list shows the expiry in plain units, such as "30 days". No
+  external delivery and no retries; an archive write failure fails the
+  delivery. See the database target section under
   "Per-Webhook Event Databases" for the full semantics.
 - **`log`** — Write the event to the application log (stdout). Useful
   for debugging.

internal/delivery/target_config_edit.go

@@ -86,9 +86,9 @@ func NewTargetConfigForm(
 }
 
 // databaseConfigForm parses an archive target's optional expiry.
-// An absent or empty configuration is the keep-forever default and
-// yields an empty field, so re-saving the form unchanged stores the
-// same empty configuration it started with. An expiry that is set
+// An absent, empty or never expiry yields an empty expiry, on which
+// the edit form starts at never; saving it unchanged stores never,
+// which means the same as an empty expiry. An expiry that is set
 // but not a valid duration is an error, not a blank field.
 func databaseConfigForm(
 	configJSON string,

internal/delivery/target_config_view.go

@@ -1,9 +1,9 @@
 package delivery
 
 import (
-	"encoding/json"
 	"fmt"
 	"strconv"
+	"time"
 
 	"sneak.berlin/go/webhooker/internal/database"
 )
@@ -197,37 +197,61 @@ func maxRetriesField(t *database.Target) ConfigField {
 	}
 }
 
-// databaseConfigFields describes an archive target. Its
-// configuration is optional, and an absent or empty expiry
-// means the archive is kept forever. An expiry that is set
-// but not a valid duration is reported as unavailable rather
-// than echoed back.
+// databaseConfigFields describes an archive target by its
+// expiry in plain units, such as "30 days", or "never" when
+// the archive is kept forever. An expiry that is set but not
+// a valid duration is reported as unavailable rather than
+// echoed back.
 func databaseConfigFields(configJSON string) []ConfigField {
-	expiry := archiveExpiryNever
+	expiry, err := parseArchiveExpiry(configJSON)
+	if err != nil {
+		return unavailableConfigFields()
+	}
 
-	if configJSON != "" {
-		var cfg databaseTargetConfig
-
-		err := json.Unmarshal([]byte(configJSON), &cfg)
-		if err != nil {
-			return unavailableConfigFields()
-		}
-
-		if cfg.Expiry != "" {
-			if ValidateArchiveExpiry(cfg.Expiry) != nil {
-				return unavailableConfigFields()
-			}
-
-			expiry = cfg.Expiry
-		}
+	value := archiveExpiryNever
+	if expiry > 0 {
+		value = plainDuration(expiry)
 	}
 
 	return []ConfigField{{
 		Label: "Archive Expiry",
-		Value: expiry,
+		Value: value,
 	}}
 }
 
+// plainDuration writes a positive duration as a count of the
+// largest whole unit it divides into: "30 days", "12 hours",
+// "1 minute". A duration with a fraction of a second is
+// written as Go writes it.
+func plainDuration(d time.Duration) string {
+	const day = 24 * time.Hour
+
+	units := []struct {
+		size time.Duration
+		name string
+	}{
+		{day, "day"},
+		{time.Hour, "hour"},
+		{time.Minute, "minute"},
+		{time.Second, "second"},
+	}
+
+	for _, unit := range units {
+		if d%unit.size != 0 {
+			continue
+		}
+
+		count := int64(d / unit.size)
+		if count == 1 {
+			return "1 " + unit.name
+		}
+
+		return fmt.Sprintf("%d %ss", count, unit.name)
+	}
+
+	return d.String()
+}
+
 // MaskedWebhookURL returns the Slack webhook URL reduced to
 // its scheme and host, with the path, query and any userinfo
 // elided. The path segments are the credential, so none of

internal/delivery/target_config_view_test.go

@@ -305,14 +305,20 @@ func TestNewTargetViews_Database(t *testing.T) {
 	}{
 		"empty config": {config: "", want: viewExpiryNever},
 		"empty expiry": {config: `{}`, want: viewExpiryNever},
-		"explicit": {
-			config: `{"expiry":"720h"}`,
-			want:   "720h",
-		},
 		"never literal": {
 			config: `{"expiry":"` + viewExpiryNever + `"}`,
 			want:   viewExpiryNever,
 		},
+		"1h":    {config: `{"expiry":"1h"}`, want: "1 hour"},
+		"12h":   {config: `{"expiry":"12h"}`, want: "12 hours"},
+		"24h":   {config: `{"expiry":"24h"}`, want: "1 day"},
+		"720h":  {config: `{"expiry":"720h"}`, want: "30 days"},
+		"2160h": {config: `{"expiry":"2160h"}`, want: "90 days"},
+		"8760h": {config: `{"expiry":"8760h"}`, want: "365 days"},
+		"36h":   {config: `{"expiry":"36h"}`, want: "36 hours"},
+		"1h30m": {config: `{"expiry":"1h30m"}`, want: "90 minutes"},
+		"45s":   {config: `{"expiry":"45s"}`, want: "45 seconds"},
+		"1.5s":  {config: `{"expiry":"1.5s"}`, want: "1.5s"},
 	}
 
 	for name, tc := range tests {

internal/delivery/target_headers_test.go

@@ -216,8 +216,9 @@ func TestNewTargetConfigForm(t *testing.T) {
 	assert.Empty(t, form.URL)
 }
 
-// A keep-forever archive target must pre-fill as an empty field, so
-// saving the form back unchanged stores the same empty config.
+// A keep-forever archive target yields an empty expiry, so the edit
+// form starts on never; saving it unchanged stores never, which means
+// the same as an empty expiry.
 func TestNewTargetConfigForm_DatabaseNeverIsBlank(t *testing.T) {
 	t.Parallel()
 

internal/handlers/archive_expiry.go

@@ -0,0 +1,58 @@
+package handlers
+
+const (
+	// archiveExpiryNever is the archive expiry that keeps archived
+	// events forever. A stored empty expiry means the same.
+	archiveExpiryNever = "never"
+
+	// tmplKeyArchiveExpiryChoices is the template data key for the
+	// entries of a page's archive expiry select.
+	tmplKeyArchiveExpiryChoices = "ArchiveExpiryChoices"
+)
+
+// archiveExpiryChoice is one entry of a database target's archive
+// expiry select: the expiry stored, the label shown, and whether the
+// select starts on it.
+type archiveExpiryChoice struct {
+	Value    string
+	Label    string
+	Selected bool
+}
+
+// archiveExpiryChoices lists the archive expiries offered by the new
+// webhook page, the add target form and the target edit form.
+func archiveExpiryChoices() []archiveExpiryChoice {
+	return []archiveExpiryChoice{
+		{Value: archiveExpiryNever, Label: archiveExpiryNever},
+		{Value: "1h", Label: "1h"},
+		{Value: "12h", Label: "12h"},
+		{Value: "24h", Label: "24h"},
+		{Value: "720h", Label: "30d"},
+		{Value: "2160h", Label: "90d"},
+		{Value: "8760h", Label: "365d"},
+	}
+}
+
+// archiveExpiryOptions returns the choices with expiry selected; an
+// empty expiry selects never. An expiry that is not one of the
+// choices comes first as its own selected entry, so saving the form
+// unchanged keeps it.
+func archiveExpiryOptions(expiry string) []archiveExpiryChoice {
+	if expiry == "" {
+		expiry = archiveExpiryNever
+	}
+
+	options := archiveExpiryChoices()
+
+	for i := range options {
+		if options[i].Value == expiry {
+			options[i].Selected = true
+
+			return options
+		}
+	}
+
+	own := archiveExpiryChoice{Value: expiry, Label: expiry, Selected: true}
+
+	return append([]archiveExpiryChoice{own}, options...)
+}

internal/handlers/archive_expiry_test.go

@@ -0,0 +1,166 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"regexp"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// expiryNever is the archive expiry that keeps archived events
+// forever.
+const expiryNever = "never"
+
+// matched returns what the one group of pattern matched in page, at
+// each match.
+func matched(pattern, page string) []string {
+	matches := regexp.MustCompile(pattern).FindAllStringSubmatch(page, -1)
+	groups := make([]string, 0, len(matches))
+
+	for _, m := range matches {
+		groups = append(groups, m[1])
+	}
+
+	return groups
+}
+
+// expiryShown returns the archive expiries the webhook page's target
+// list shows.
+func expiryShown(
+	t *testing.T, env *sourceTestEnv, webhookID string,
+) []string {
+	t.Helper()
+
+	w := httptest.NewRecorder()
+	env.handlers.HandleSourceDetail().ServeHTTP(w, getRequest(
+		t, "/hook/"+webhookID, env.cookies,
+		map[string]string{sourceIDParam: webhookID},
+	))
+	require.Equal(t, http.StatusOK, w.Code)
+
+	return matched(
+		`Archive Expiry:</span>\s*<span>([^<]*)</span>`, w.Body.String(),
+	)
+}
+
+// expirySelected returns the target edit page and the expiries its
+// select starts on.
+func expirySelected(
+	t *testing.T, env *sourceTestEnv, webhookID, targetID string,
+) (string, []string) {
+	t.Helper()
+
+	w := serveTarget(
+		env, http.MethodGet,
+		"/hook/"+webhookID+"/targets/"+targetID+"/edit", nil,
+	)
+	require.Equal(t, http.StatusOK, w.Code)
+
+	page := w.Body.String()
+
+	return page, matched(`<option value="([^"]*)" selected>`, page)
+}
+
+// TestArchiveExpiryChoices adds a database target with each archive
+// expiry the forms offer, and checks that it is stored as chosen,
+// shown in plain units in the target list, and that the target edit
+// form starts on it.
+func TestArchiveExpiryChoices(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	choices := []struct{ value, shown string }{
+		{expiryNever, expiryNever},
+		{"1h", "1 hour"},
+		{"12h", "12 hours"},
+		{"24h", "1 day"},
+		{"720h", "30 days"},
+		{"2160h", "90 days"},
+		{"8760h", "365 days"},
+	}
+
+	for _, choice := range choices {
+		t.Run(choice.value, func(t *testing.T) {
+			t.Parallel()
+
+			webhook := seedWebhookWithRetention(t, env.db, 30)
+
+			form := url.Values{}
+			form.Set("name", "archive")
+			form.Set("type", string(database.TargetTypeDatabase))
+			form.Set("expiry", choice.value)
+
+			w := serveTarget(
+				env, http.MethodPost, "/hook/"+webhook.ID+"/targets", form,
+			)
+			require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+			targets := targetsForWebhook(t, env.db, webhook.ID)
+			require.Len(t, targets, 1)
+			assert.JSONEq(
+				t, `{"expiry":"`+choice.value+`"}`, targets[0].Config,
+			)
+
+			assert.Equal(
+				t, []string{choice.shown},
+				expiryShown(t, env, webhook.ID),
+			)
+
+			_, selected := expirySelected(t, env, webhook.ID, targets[0].ID)
+			assert.Equal(t, []string{choice.value}, selected)
+		})
+	}
+}
+
+// TestArchiveExpiryEditStartsOnStoredValue checks the edit form of a
+// database target whose stored expiry is empty, which selects never,
+// and of one whose expiry is not one of the choices, which is listed
+// first as its own selected entry and saved unchanged.
+func TestArchiveExpiryEditStartsOnStoredValue(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	webhook := seedWebhookWithRetention(t, env.db, 30)
+	empty := seedConfiguredTarget(
+		t, env.db, webhook.ID, database.TargetTypeDatabase, "",
+	)
+
+	_, selected := expirySelected(t, env, webhook.ID, empty.ID)
+	assert.Equal(t, []string{expiryNever}, selected)
+
+	webhook = seedWebhookWithRetention(t, env.db, 30)
+	unlisted := seedConfiguredTarget(
+		t, env.db, webhook.ID, database.TargetTypeDatabase,
+		`{"expiry":"36h"}`,
+	)
+
+	assert.Equal(t, []string{"36 hours"}, expiryShown(t, env, webhook.ID))
+
+	page, selected := expirySelected(t, env, webhook.ID, unlisted.ID)
+	assert.Equal(t, []string{"36h"}, selected)
+	assert.Regexp(
+		t,
+		`<select id="expiry" name="expiry" class="input">\s*`+
+			`<option value="36h" selected>36h</option>\s*`+
+			`<option value="never">never</option>`,
+		page,
+	)
+	assert.Contains(t, page, `<option value="8760h">365d</option>`)
+
+	form := url.Values{}
+	form.Set("name", unlisted.Name)
+	form.Set("expiry", "36h")
+
+	w := submitTargetEdit(env, webhook.ID, unlisted.ID, form)
+	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+	assert.JSONEq(
+		t, `{"expiry":"36h"}`, storedTarget(t, env, unlisted.ID).Config,
+	)
+}

internal/handlers/source_detail_test.go

@@ -234,7 +234,7 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
 	assert.NotContains(t, body, "sekrit")
 
 	assert.Contains(t, body, "Archive Expiry")
-	assert.Contains(t, body, "720h")
+	assert.Contains(t, body, "30 days")
 
 	// An unknown type gets the neutral placeholder, never the
 	// stored blob.

internal/handlers/source_management.go

@@ -312,6 +312,9 @@ func newSourceFormData(
 		tmplKeyError:           errMsg,
 		"Form":                 in,
 		"DefaultRetentionDays": database.DefaultRetentionDays,
+		tmplKeyArchiveExpiryChoices: archiveExpiryOptions(
+			in.ArchiveExpiry,
+		),
 	}
 }
 
@@ -623,6 +626,9 @@ func (h *Handlers) renderSourceDetail(
 		"Stats":           h.loadWebhookStats(webhook.ID, entrypoints, targets),
 		tmplKeyTargetForm: targetForm,
 		"TargetError":     targetErr,
+		// The add target form's select starts on its expiry
+		// through Alpine, so no choice is selected here.
+		tmplKeyArchiveExpiryChoices: archiveExpiryChoices(),
 	}
 
 	status := http.StatusOK

internal/handlers/target_edit.go

@@ -238,9 +238,10 @@ func (h *Handlers) renderTargetEdit(
 			Type:   target.Type,
 			Active: target.Active,
 		},
-		tmplKeyTargetForm: form,
-		tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds,
-		tmplKeyError:      errMsg,
+		tmplKeyTargetForm:           form,
+		tmplKeyMaxTimeout:           delivery.MaxTargetTimeoutSeconds,
+		tmplKeyError:                errMsg,
+		tmplKeyArchiveExpiryChoices: archiveExpiryOptions(form.Expiry),
 	}
 
 	h.renderTemplateStatus(w, r, targetEditTemplate, data, status)

internal/handlers/target_edit_test.go

@@ -613,12 +613,16 @@ func TestHandleTargetEditSubmit_RefusedFormComesBack(t *testing.T) {
 			page := w.Body.String()
 			assert.Contains(t, page, `class="alert-error">`+tc.reason)
 
-			// headers is the form's one textarea; every other field is
-			// an input.
+			// headers is the form's one textarea and expiry its one
+			// select; every other field is an input.
 			for field := range form {
 				shown := `name="` + field + `" value="` + form.Get(field) + `"`
-				if field == "headers" {
+
+				switch field {
+				case "headers":
 					shown = ">" + form.Get(field) + "</textarea>"
+				case "expiry":
+					shown = `<option value="` + form.Get(field) + `" selected>`
 				}
 
 				assert.Contains(t, page, shown)

internal/server/alpine_browser_test.go

@@ -71,6 +71,7 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	// this order. A new check is one more line here.
 	checkAddEntrypoint(ctx, t, page)
 	checkAddEachTargetType(ctx, t, page)
+	checkArchiveExpiry(ctx, t, page)
 	checkRefusedTarget(ctx, t, page)
 	checkTargetDeliveries(ctx, t, page, target.Name,
 		"0 in total, 0 in the last 24 hours",
@@ -421,6 +422,43 @@ func chooseTargetType(ctx context.Context, t *testing.T, targetType string) {
 		"%s: Add still shows while the form is open", targetType)
 }
 
+// checkArchiveExpiry loads a webhook page and checks that the add
+// target form's archive expiry starts on never, that the database
+// target checkAddTarget added with 720h is listed as 30 days, and that
+// its edit form starts on 720h.
+func checkArchiveExpiry(ctx context.Context, t *testing.T, url string) {
+	t.Helper()
+
+	const expiry = `form[action$="/targets"] select[name="expiry"]`
+
+	row := `//span[text()="added-database"]/ancestor::div[@class="p-4"][1]`
+
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+
+	chooseTargetType(ctx, t, "database")
+
+	var start, edited string
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Value(expiry, &start, chromedp.ByQuery),
+	))
+	assert.Equal(t, "never", start,
+		"the add target form's archive expiry does not start on never")
+
+	assert.True(t, shown(ctx, row+`//span[text()="Archive Expiry:"]`+
+		`/following-sibling::span[text()="30 days"]`),
+		"a database target added with 720h is not listed as 30 days")
+
+	click(ctx, t, row+`//a[text()="Edit"]`)
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.WaitReady("#expiry", chromedp.ByQuery),
+		chromedp.Value("#expiry", &edited, chromedp.ByQuery),
+	))
+	assert.Equal(t, "720h", edited,
+		"the edit form does not start on the stored archive expiry")
+}
+
 // checkRefusedTarget submits an http target the server refuses, a
 // loopback destination, and checks that the page comes back with the
 // form open on the http fields, the values entered and the reason, and

templates/source_detail.html

@@ -203,8 +203,15 @@
                     <template x-if="isDatabase">
                         <div>
                             <input type="hidden" name="type" value="database">
-                            <input type="text" name="expiry" :value="expiry" placeholder="never" class="input text-sm">
-                            <p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
+                            <div class="flex gap-2 items-center">
+                                <label class="text-sm text-gray-700">Archive expiry:</label>
+                                <select name="expiry" :value="expiry" class="input text-sm w-24">
+                                    {{range .ArchiveExpiryChoices}}
+                                    <option value="{{.Value}}">{{.Label}}</option>
+                                    {{end}}
+                                </select>
+                            </div>
+                            <p class="text-xs text-gray-500 mt-1">Archived events older than this are deleted from the archive; never keeps them all.</p>
                         </div>
                     </template>
                     <template x-if="isLog">

templates/sources_new.html

@@ -50,13 +50,9 @@
                 <div x-show="open" x-cloak class="mt-3">
                     <label for="archive_expiry" class="label">Archive pruning</label>
                     <select id="archive_expiry" name="archive_expiry" class="input">
-                        <option value="never"{{if eq .Form.ArchiveExpiry "never"}} selected{{end}}>never</option>
-                        <option value="1h"{{if eq .Form.ArchiveExpiry "1h"}} selected{{end}}>1h</option>
-                        <option value="12h"{{if eq .Form.ArchiveExpiry "12h"}} selected{{end}}>12h</option>
-                        <option value="24h"{{if eq .Form.ArchiveExpiry "24h"}} selected{{end}}>24h</option>
-                        <option value="720h"{{if eq .Form.ArchiveExpiry "720h"}} selected{{end}}>30d</option>
-                        <option value="2160h"{{if eq .Form.ArchiveExpiry "2160h"}} selected{{end}}>90d</option>
-                        <option value="8760h"{{if eq .Form.ArchiveExpiry "8760h"}} selected{{end}}>365d</option>
+                        {{range .ArchiveExpiryChoices}}
+                        <option value="{{.Value}}"{{if .Selected}} selected{{end}}>{{.Label}}</option>
+                        {{end}}
                     </select>
                     <p class="text-xs text-gray-500 mt-1">Archived events older than this are deleted from the archive; never keeps them all.</p>
                 </div>

templates/target_edit.html

@@ -60,8 +60,12 @@
             {{if eq .Target.Type "database"}}
             <div class="form-group">
                 <label for="expiry" class="label">Archive Expiry</label>
-                <input type="text" id="expiry" name="expiry" value="{{.TargetForm.Expiry}}" placeholder="never" class="input">
-                <p class="text-xs text-gray-500 mt-1">"never" (the default when blank) keeps archived rows forever, or a Go duration like "720h" prunes older rows.</p>
+                <select id="expiry" name="expiry" class="input">
+                    {{range .ArchiveExpiryChoices}}
+                    <option value="{{.Value}}"{{if .Selected}} selected{{end}}>{{.Label}}</option>
+                    {{end}}
+                </select>
+                <p class="text-xs text-gray-500 mt-1">Archived events older than this are deleted from the archive; never keeps them all.</p>
             </div>
             {{end}}