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

Adding or editing a database target now offers the archive expiry
choices of the new webhook page (never, 1h, 12h, 24h, 30d, 90d, 365d)
in place of a text field. The list is defined once, in
internal/handlers/archive_expiry.go, and all three forms render it. The
edit form starts on the stored expiry; one that is not among the choices
is listed first as its own entry, so saving unchanged keeps it. The
target list shows the expiry in plain units, such as "30 days" or
"12 hours", or "never".

Model: opus-5-5

README.md

@@ -1719,8 +1719,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_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/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),
 		"TargetForm":  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

@@ -245,8 +245,9 @@ func (h *Handlers) renderTargetEdit(
 			MaxRetries: target.MaxRetries,
 			Config:     cfg,
 		},
-		tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds,
-		tmplKeyError:      errMsg,
+		tmplKeyMaxTimeout:           delivery.MaxTargetTimeoutSeconds,
+		tmplKeyError:                errMsg,
+		tmplKeyArchiveExpiryChoices: archiveExpiryOptions(cfg.Expiry),
 	}
 
 	h.renderTemplate(w, r, targetEditTemplate, data)

internal/server/alpine_browser_test.go

@@ -89,35 +89,8 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	page := srv.URL + "/hook/" + webhook.ID
 
 	checkAddEntrypoint(ctx, t, page)
-
-	// Each target type, with the fields its add target form submits, in
-	// page order. Only http and slack have a url field.
-	targetTypes := []struct {
-		name   string
-		fields string
-		values map[string]string
-	}{
-		{
-			"http", "csrf_token name type url headers timeout max_retries",
-			map[string]string{"url": publicTargetURL},
-		},
-		{
-			"slack", "csrf_token name type url max_retries",
-			map[string]string{"url": publicTargetURL},
-		},
-		{
-			"database", "csrf_token name type expiry",
-			map[string]string{"expiry": "720h"},
-		},
-		{"log", "csrf_token name type", nil},
-	}
-
-	for _, tt := range targetTypes {
-		checkAddTarget(
-			ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values,
-		)
-	}
-
+	checkAddEveryTarget(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",
@@ -310,6 +283,40 @@ const (
 		document.querySelector('form[action$="/targets"]')).keys()]`
 )
 
+// checkAddEveryTarget adds a target of each type on a webhook page,
+// each through checkAddTarget, in page order.
+func checkAddEveryTarget(ctx context.Context, t *testing.T, page string) {
+	t.Helper()
+
+	// Each target type, with the fields its add target form submits.
+	// Only http and slack have a url field.
+	targetTypes := []struct {
+		name   string
+		fields string
+		values map[string]string
+	}{
+		{
+			"http", "csrf_token name type url headers timeout max_retries",
+			map[string]string{"url": publicTargetURL},
+		},
+		{
+			"slack", "csrf_token name type url max_retries",
+			map[string]string{"url": publicTargetURL},
+		},
+		{
+			"database", "csrf_token name type expiry",
+			map[string]string{"expiry": "720h"},
+		},
+		{"log", "csrf_token name type", nil},
+	}
+
+	for _, tt := range targetTypes {
+		checkAddTarget(
+			ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values,
+		)
+	}
+}
+
 // checkAddTarget loads a webhook page and walks the add target form for
 // one target type. The form shows nothing until Add is clicked; Add
 // shows only the type choice; Cancel there closes it; Next shows the
@@ -398,6 +405,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="{{.Target.Config.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}}