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}}