Store a webhook's retention as a plain day count, 0 meaning forever (closes #539)

retention_days loses its column default of 30, and Webhook.RetentionDays
its default tag, so a 0 is stored as 0. The 365000-day stand-in
RetentionForeverDays and the BeforeSave hook that wrote it are gone. The
reaper and the "forever" label read 0 as forever, and the forms accept 0
or 1 to MaxFiniteRetentionDays; a blank field still gives 30 on the new
webhook form and keeps the stored value on the edit form. A negative
value, which the forms refuse, is still read as forever so it cannot put
the reaper's cutoff in the future.

Model: opus-5-5

internal/handlers/shared.go

@@ -16,21 +16,16 @@ import (
 //
 // An empty value yields fallback, which lets the create path apply the
 // default and the edit path leave the stored value unchanged. A value
-// of 0 is returned as 0 and is rewritten to the retain-forever
-// sentinel by database.Webhook's BeforeSave hook. Anything unparseable
-// or negative is refused rather than silently given a default.
+// of 0 means retain events forever and is returned as it is. Anything
+// unparseable or negative is refused rather than silently given a
+// default.
 //
 // The upper bound is not cosmetic. The reaper computes its cutoff as a
 // time.Duration, an int64 nanosecond count, so a day count above
 // database.MaxFiniteRetentionDays overflows, puts the cutoff in the
-// future, and deletes every event the webhook has. A finite value
-// above that ceiling is therefore refused, and the message names the
-// ceiling rather than implying the input was not a number.
-//
-// A value at or above the retain-forever sentinel is not out of range:
-// it is what the edit form pre-fills for a retain-forever webhook, so
-// submitting the form back unchanged has to keep meaning "forever"
-// rather than being rejected.
+// future, and deletes every event the webhook has. A value above that
+// ceiling is therefore refused, and the message names the ceiling
+// rather than implying the input was not a number.
 func parseRetentionDays(raw string, fallback int) (int, string) {
 	raw = strings.TrimSpace(raw)
 	if raw == "" {
@@ -43,10 +38,6 @@ func parseRetentionDays(raw string, fallback int) (int, string) {
 			"retain events forever."
 	}
 
-	if v >= database.RetentionForeverDays {
-		return database.RetentionForeverDays, ""
-	}
-
 	if v > database.MaxFiniteRetentionDays {
 		return 0, "Retention must be at most " +
 			strconv.Itoa(database.MaxFiniteRetentionDays) +

internal/handlers/source_management_test.go

@@ -137,8 +137,7 @@ func onlyWebhook(
 }
 
 // seedWebhookWithRetention inserts a webhook owned by the test user
-// with an exact stored retention value, bypassing Webhook.BeforeSave
-// via a column-level update so that legacy rows can be planted too.
+// with the given retention.
 func seedWebhookWithRetention(
 	t *testing.T,
 	db *database.Database,
@@ -155,13 +154,6 @@ func seedWebhookWithRetention(
 		t,
 		db.DB().Omit(clause.Associations).Create(wh).Error,
 	)
-	require.NoError(
-		t,
-		db.DB().Model(wh).
-			Update("retention_days", retentionDays).Error,
-	)
-
-	wh.RetentionDays = retentionDays
 
 	return *wh
 }
@@ -225,10 +217,9 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
 	}
 }
 
-// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
-// regression test for the bug: the create form's 0 must reach the
-// database as the retain-forever sentinel rather than being replaced by
-// the column's default of 30.
+// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever checks
+// that the create form's 0 reaches the database as 0, which means
+// retain forever.
 func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
 	t *testing.T,
 ) {
@@ -241,35 +232,60 @@ func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 
 	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+	assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
 	assert.True(t, wh.RetainsForever())
 }
 
-func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
+func TestHandleSourceCreateSubmit_DayCountIsStored(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	days := "45"
+
+	w := submitCreate(t, env.handlers, env.cookies, "finite", &days)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+
+	wh := onlyWebhook(t, env.db)
+	assert.Equal(t, 45, storedRetentionDays(t, env.db, wh.ID))
+}
+
+// TestHandleSourceCreateSubmit_BlankRetentionUsesDefault covers a
+// retention field left empty and one missing from the form: both
+// give the default, not forever.
+func TestHandleSourceCreateSubmit_BlankRetentionUsesDefault(
 	t *testing.T,
 ) {
 	t.Parallel()
 
-	env := setupSourceTest(t)
+	blank := ""
 
-	w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
-	require.Equal(t, http.StatusSeeOther, w.Code)
+	for name, retention := range map[string]*string{
+		"blank":   &blank,
+		"omitted": nil,
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
 
-	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.DefaultRetentionDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+			env := setupSourceTest(t)
+
+			w := submitCreate(
+				t, env.handlers, env.cookies, "defaulted", retention,
+			)
+			require.Equal(t, http.StatusSeeOther, w.Code)
+
+			wh := onlyWebhook(t, env.db)
+			assert.Equal(
+				t,
+				database.DefaultRetentionDays,
+				storedRetentionDays(t, env.db, wh.ID),
+			)
+		})
+	}
 }
 
-// TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
-// form's pre-filled retention from becoming a third hardcoded copy of
-// the 30-day policy.
+// TestHandleSourceCreate_PrefillsDefaultFromConstant checks that the
+// create form pre-fills its retention from database.DefaultRetentionDays
+// rather than from a hardcoded 30.
 func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
 	t.Parallel()
 
@@ -290,7 +306,7 @@ func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
 	)
 	assert.NotContains(
 		t, body, `max="365"`,
-		"a max below the sentinel would block retain-forever",
+		"a max of 365 would block any longer retention",
 	)
 	assert.Contains(t, body, `min="0"`)
 }
@@ -368,40 +384,6 @@ func TestHandleSourceCreateSubmit_OverflowingRetentionIsRejected(
 	)
 }
 
-// TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever guards the
-// boundary between "too large to represent" and "retain forever": the
-// sentinel is above MaxFiniteRetentionDays, but it is the value the
-// edit form pre-fills, so it must be accepted rather than rejected as
-// out of range. A value above the sentinel is stored as the sentinel.
-func TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	for _, days := range []int{
-		database.RetentionForeverDays,
-		database.RetentionForeverDays + 1,
-	} {
-		raw := strconv.Itoa(days)
-
-		t.Run(raw, func(t *testing.T) {
-			t.Parallel()
-
-			env := setupSourceTest(t)
-
-			w := submitCreate(t, env.handlers, env.cookies, "forever", &raw)
-			require.Equal(t, http.StatusSeeOther, w.Code)
-
-			wh := onlyWebhook(t, env.db)
-			assert.Equal(
-				t,
-				database.RetentionForeverDays,
-				storedRetentionDays(t, env.db, wh.ID),
-			)
-		})
-	}
-}
-
 // TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput checks that a
 // validation failure hands the user's typing back, matching what the
 // edit form already does. Losing a long description to a mistyped
@@ -480,11 +462,7 @@ func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
 	w := submitEdit(t, env, wh, "0")
 	require.Equal(t, http.StatusSeeOther, w.Code)
 
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+	assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
 }
 
 func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
@@ -867,18 +845,15 @@ func TestHandleSourceEditSubmit_ArchiveNameTaken(t *testing.T) {
 	assert.Equal(t, oldName, stored.Name)
 }
 
-// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
-// the removed max="365" cap used to break: render the edit form for a
-// retain-forever webhook, confirm the pre-filled sentinel is not capped
-// by browser validation, then submit that pre-filled value straight
-// back and confirm the retention policy survives untouched.
+// TestSourceEditForm_ForeverWebhookRoundTrips renders the edit form for
+// a retain-forever webhook, confirms it pre-fills 0, then submits that
+// pre-filled value straight back and confirms the retention policy
+// survives untouched.
 func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 	t.Parallel()
 
 	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(
-		t, env.db, database.RetentionForeverDays,
-	)
+	wh := seedWebhookWithRetention(t, env.db, 0)
 
 	req := getRequest(
 		t, "/hook/"+wh.ID+"/edit", env.cookies,
@@ -889,17 +864,12 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 
 	require.Equal(t, http.StatusOK, w.Code)
 
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
 	body := w.Body.String()
 
 	assert.Contains(
-		t, body, `value="`+sentinel+`"`,
+		t, body, `name="retention_days" value="0"`,
 		"the edit form pre-fills the stored retention",
 	)
-	assert.NotContains(
-		t, body, `max="365"`,
-		"a max below the sentinel would block saving any edit",
-	)
 	// "Currently forever." is the rendered RetentionLabel, not the
 	// static hint below the input, which says "Enter 0 to retain events
 	// forever." A bare Contains of "forever" would pass for any
@@ -910,29 +880,20 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 	)
 
 	// Submit the pre-filled value back, exactly as a browser would.
-	post := submitEdit(t, env, wh, sentinel)
+	post := submitEdit(t, env, wh, "0")
 	require.Equal(t, http.StatusSeeOther, post.Code)
 
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+	assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
 }
 
-// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
-// the retain-forever value is never rendered to the user as a raw day
-// count on either read-only view.
-func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
-	t *testing.T,
-) {
+// TestSourceListAndDetail_ShowForeverNotZeroDays checks that a
+// retention of 0 is shown as forever, never as a day count, on either
+// read-only view.
+func TestSourceListAndDetail_ShowForeverNotZeroDays(t *testing.T) {
 	t.Parallel()
 
 	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(
-		t, env.db, database.RetentionForeverDays,
-	)
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
+	wh := seedWebhookWithRetention(t, env.db, 0)
 
 	listW := httptest.NewRecorder()
 	env.handlers.HandleSourceList().ServeHTTP(
@@ -941,7 +902,7 @@ func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
 
 	require.Equal(t, http.StatusOK, listW.Code)
 	assert.Contains(t, listW.Body.String(), "Retention: forever")
-	assert.NotContains(t, listW.Body.String(), sentinel)
+	assert.NotContains(t, listW.Body.String(), "Retention: 0 days")
 
 	detailW := httptest.NewRecorder()
 	env.handlers.HandleSourceDetail().ServeHTTP(
@@ -954,5 +915,5 @@ func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
 
 	require.Equal(t, http.StatusOK, detailW.Code)
 	assert.Contains(t, detailW.Body.String(), "Retention: forever")
-	assert.NotContains(t, detailW.Body.String(), sentinel)
+	assert.NotContains(t, detailW.Body.String(), "Retention: 0 days")
 }

internal/handlers/ui_copy_test.go

@@ -184,8 +184,7 @@ func TestEventLogPageIsCalledFullEventLog(t *testing.T) {
 // TestCreateFormRetentionCopyMatchesBehaviour pins the create form's
 // retention copy to what the code does: the reaper permanently deletes
 // events past the cutoff, an empty field falls back to
-// DefaultRetentionDays, and 0 is rewritten to the retain-forever
-// sentinel by Webhook.BeforeSave.
+// DefaultRetentionDays, and 0 means retain forever.
 func TestCreateFormRetentionCopyMatchesBehaviour(t *testing.T) {
 	t.Parallel()
 
@@ -253,10 +252,7 @@ func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
 		"blank means unchanged, not forever",
 	)
 
-	forever := &database.Webhook{
-		Name:          "wh",
-		RetentionDays: database.RetentionForeverDays,
-	}
+	forever := &database.Webhook{Name: "wh", RetentionDays: 0}
 	forever.ID = "wh-2"
 
 	foreverBody := renderPage(

internal/handlers/webhook_stats_test.go

@@ -500,7 +500,7 @@ func TestWebhookStats_PaneShowsRetentionPeriod(t *testing.T) {
 		want          string
 	}{
 		{30, "30 days"},
-		{database.RetentionForeverDays, "forever"},
+		{0, "forever"},
 	}
 
 	for _, tt := range tests {