Use one name for each thing the UI shows (closes #399)
check / check (push) Successful in 3m31s

The database target type is called an archive on its badge, in the add target form's type list and on its edit page, and its settings read "Archive expiry" and "Archive rotation" everywhere, the new webhook page included. The retry field is labelled "Delivery attempts", with its help text and error messages to match, on both target forms and in the target list, where a stored 0 shows as one attempt. The target list's other labels take the same capitalisation. The navbar says "Sign out" and the sign-in page "Sign in", and the README follows. The resubmit notice says "webhook". The stored values (`database`, `max_retries`) and their meaning are unchanged.

Model: opus-5-5
This commit is contained in:
2026-10-03 02:54:47 +00:00
committed by sneak
parent 74a96b2226
commit 12deb0d79b
23 changed files with 180 additions and 98 deletions
+8 -7
View File
@@ -177,16 +177,17 @@ func httpConfigFields(t *database.Target) []ConfigField {
}
// maxRetriesField describes a target's retry count, which lives
// on the target row rather than in its configuration blob.
// on the target row rather than in its configuration blob. A
// stored 0 makes a single attempt, so it is shown as 1.
func maxRetriesField(t *database.Target) ConfigField {
retries := strconv.Itoa(t.MaxRetries)
attempts := strconv.Itoa(t.MaxRetries)
if t.MaxRetries == 0 {
retries += " (fire-and-forget)"
attempts = "1 (fire-and-forget: no retries, no circuit breaker)"
}
return ConfigField{
Label: "Max Retries",
Value: retries,
Label: "Delivery attempts",
Value: attempts,
}
}
@@ -213,10 +214,10 @@ func databaseConfigFields(configJSON string) []ConfigField {
}
return []ConfigField{{
Label: "Archive Expiry",
Label: "Archive expiry",
Value: value,
}, {
Label: "Archive Rotation",
Label: "Archive rotation",
Value: rotation,
}}
}
+6 -6
View File
@@ -32,7 +32,7 @@ const (
viewMaskedOrigin = viewExampleOrigin + "/..."
viewUnavailable = "(unavailable)"
viewExpiryNever = "never"
viewMaxRetries = "Max Retries"
viewMaxRetries = "Delivery attempts"
)
func TestMaskedWebhookURL(t *testing.T) {
@@ -190,7 +190,7 @@ func TestNewTargetViews_Slack(t *testing.T) {
t,
map[string]string{
"Webhook URL": slackMaskedURL,
viewMaxRetries: "0 (fire-and-forget)",
viewMaxRetries: "1 (fire-and-forget: no retries, no circuit breaker)",
},
fieldMap(view.Config),
)
@@ -258,7 +258,7 @@ func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
t,
map[string]string{
"Destination URL": viewMaskedOrigin,
viewMaxRetries: "0 (fire-and-forget)",
viewMaxRetries: "1 (fire-and-forget: no retries, no circuit breaker)",
},
fieldMap(view.Config),
)
@@ -329,8 +329,8 @@ func TestNewTargetViews_Database(t *testing.T) {
assert.Equal(
t,
map[string]string{
"Archive Expiry": tc.want,
"Archive Rotation": rotationNone,
"Archive expiry": tc.want,
"Archive rotation": rotationNone,
},
fieldMap(view.Config),
)
@@ -365,7 +365,7 @@ func TestNewTargetViews_DatabaseRotation(t *testing.T) {
})
assert.Equal(
t, want, fieldMap(view.Config)["Archive Rotation"],
t, want, fieldMap(view.Config)["Archive rotation"],
)
})
}
+1 -1
View File
@@ -45,7 +45,7 @@ func expiryShown(
require.Equal(t, http.StatusOK, w.Code)
return matched(
`Archive Expiry:</span>\s*<span>([^<]*)</span>`, w.Body.String(),
`Archive expiry:</span>\s*<span>([^<]*)</span>`, w.Body.String(),
)
}
+2 -2
View File
@@ -27,7 +27,7 @@ func rotationShown(
t.Helper()
return matched(
`Archive Rotation:</span>\s*<span>([^<]*)</span>`,
`Archive rotation:</span>\s*<span>([^<]*)</span>`,
renderedPage(t, env, webhookID),
)
}
@@ -211,7 +211,7 @@ func TestArchiveFileView_Rotated(t *testing.T) {
assert.Equal(t, "2.0 kB", view.Size)
page := targetList(t, renderedPage(t, env, webhook.ID))
assert.Contains(t, page, "Archive Size: 2.0 kB in 2 files")
assert.Contains(t, page, "Archive size: 2.0 kB in 2 files")
}
// renderedPage returns the webhook page.
+1 -1
View File
@@ -268,7 +268,7 @@ func (h *Handlers) rejectLogin(
)))
h.renderLoginError(
w, r,
"Too many failed login attempts. Please try again later.",
"Too many failed sign-in attempts. Please try again later.",
http.StatusTooManyRequests,
)
}
+1 -1
View File
@@ -96,7 +96,7 @@ func noticeFor(r *http.Request) *notice {
},
resubmitNoTargets: {
Text: "Resubmitted: a new event was created, but this " +
"source has no active targets, so nothing was queued.",
"webhook has no active targets, so nothing was queued.",
},
}[noticeCode(r.URL.Query().Get(noticeParam))]
if !ok {
+1 -1
View File
@@ -40,7 +40,7 @@ func TestEveryPageRendersItsOwnTitle(t *testing.T) {
data map[string]any
title string
}{
{"login.html", map[string]any{}, "Login - Webhooker"},
{"login.html", map[string]any{}, "Sign in - Webhooker"},
{"profile.html", map[string]any{}, "Profile - Webhooker"},
{"settings.html", map[string]any{}, "Settings - Webhooker"},
{"sources_list.html", map[string]any{}, "Webhooks - Webhooker"},
+7 -2
View File
@@ -233,8 +233,13 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
assert.Contains(t, body, "1 configured")
assert.NotContains(t, body, "sekrit")
assert.Contains(t, body, "Archive Expiry")
assert.Contains(t, body, "30 days")
// The database type is called an archive: on its badge, in the
// add target form's type list and in its settings.
list := targetList(t, body)
assert.Contains(t, list, "t-database archive Active")
assert.Contains(t, list, "Archive expiry: 30 days")
assert.Contains(t, list, "Archive rotation: none")
assert.Contains(t, body, `<option value="database">Archive</option>`)
// An unknown type gets the neutral placeholder, never the
// stored blob.
+1 -1
View File
@@ -1749,7 +1749,7 @@ func (h *Handlers) setTargetFromForm(
// cannot destroy the count a target is delivering with.
maxRetries, err := parseMaxRetries(in.MaxRetries, target.MaxRetries)
if err != nil {
return "Invalid max retries: " + retriesErrorMessage(err), nil
return "Invalid delivery attempts: " + retriesErrorMessage(err), nil
}
target.Name = in.Name
+25 -1
View File
@@ -418,6 +418,30 @@ func TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked(
assert.Contains(t, page, "original-name")
}
// TestHandleTargetEdit_CallsTheDatabaseTypeArchive pins the names the
// edit page of a database target gives its type and its settings to
// the ones its badge and the add target form use.
func TestHandleTargetEdit_CallsTheDatabaseTypeArchive(t *testing.T) {
t.Parallel()
env := setupSourceTest(t)
webhook := seedWebhookWithRetention(t, env.db, 30)
target := seedTarget(t, env.db, webhook.ID, database.TargetTypeDatabase)
w := serveTarget(
env, http.MethodGet,
"/hook/"+webhook.ID+"/targets/"+target.ID+"/edit",
nil,
)
require.Equal(t, http.StatusOK, w.Code)
page := w.Body.String()
assert.Contains(t, page, "Type: archive.")
assert.Contains(t, page, `class="label">Archive expiry</label>`)
assert.Contains(t, page, `class="label">Archive rotation</label>`)
}
// TestHandleTargetEditSubmit_Rejects covers every submission that
// must not reach storage.
//
@@ -588,7 +612,7 @@ func TestHandleTargetEditSubmit_RefusedFormComesBack(t *testing.T) {
{
database.TargetTypeSlack,
"name=edited&url=" + editOriginalURL + "&max_retries=25",
"Invalid max retries",
"Invalid delivery attempts",
},
{
database.TargetTypeDatabase,
+4 -4
View File
@@ -44,10 +44,10 @@ func TestHandleSourceDetail_ShowsArchiveFile(t *testing.T) {
path := delivery.ArchivePath(dbMgr, wh, archive)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.Equal(t, 1, strings.Count(body, "Archive File:"))
assert.Equal(t, 1, strings.Count(body, "Archive file:"))
assert.Contains(t, body, filepath.Base(path))
assert.Contains(t, body, "not created yet")
assert.NotContains(t, body, "Archive Size:")
assert.NotContains(t, body, "Archive size:")
seedArchive(t, path, 1, 100)
@@ -58,7 +58,7 @@ func TestHandleSourceDetail_ShowsArchiveFile(t *testing.T) {
assert.Contains(t, body, filepath.Base(path))
assert.NotContains(t, body, "not created yet")
assert.Regexp(t,
`Archive Size:</span>\s*<span>[1-9][0-9.]* [kM]?B</span>`, body,
`Archive size:</span>\s*<span>[1-9][0-9.]* [kM]?B</span>`, body,
)
assert.Contains(t, body,
`title="`+file.ModTime().UTC().Format(time.DateTime)+` UTC"`,
@@ -69,7 +69,7 @@ func TestHandleSourceDetail_ShowsArchiveFile(t *testing.T) {
body = renderSourceDetailPage(t, h, sess, wh.ID)
assert.Contains(t, body, filepath.Base(path))
assert.Contains(t, body, "not created yet")
assert.NotContains(t, body, "Archive Size:")
assert.NotContains(t, body, "Archive size:")
}
// targetList returns the text of the targets section in a rendered
+4 -4
View File
@@ -81,10 +81,10 @@ func TestPausedTarget_ShownUntilBreakerCloses(t *testing.T) {
list := targetList(t, renderSourceDetailPage(t, h, sess, wh.ID))
assert.Regexp(t, "t-http http Active Edit Deactivate Delete "+
"Deliveries Paused: after repeated failures, until "+cooldownEnds+
"Deliveries paused: after repeated failures, until "+cooldownEnds+
", then one waiting delivery is sent to test the target while "+
"the others wait at least one more cooldown", list)
assert.Equal(t, 1, strings.Count(list, "Paused"))
assert.Equal(t, 1, strings.Count(list, "Deliveries paused"))
log := renderSourceLogsPage(t, h, sess, wh.ID)
assert.Equal(t, 2, strings.Count(log, "t-http: waiting"))
@@ -105,7 +105,7 @@ func TestPausedTarget_ShownUntilBreakerCloses(t *testing.T) {
list = targetList(t, renderSourceDetailPage(t, h, sess, wh.ID))
assert.Contains(t, list, "t-http http Active Edit Deactivate Delete "+
"Deliveries Paused: held while one delivery tests whether the "+
"Deliveries paused: held while one delivery tests whether the "+
"target has recovered")
// Not the whole list: the add target form above the rows says UTC.
assert.NotContains(t, targetRow(list, "t-http", "t-log"), "UTC")
@@ -115,7 +115,7 @@ func TestPausedTarget_ShownUntilBreakerCloses(t *testing.T) {
breakers.Set(target.ID, delivery.CircuitClosed, 0)
list = targetList(t, renderSourceDetailPage(t, h, sess, wh.ID))
assert.NotContains(t, list, "Paused")
assert.NotContains(t, list, "Deliveries paused")
assertRetryingNotWaiting(t, h, sess, wh.ID, retrying, backedOff)
}
+3 -3
View File
@@ -25,14 +25,14 @@ var (
// errRetriesInvalid signals a max_retries form value that is not
// a non-negative whole number.
errRetriesInvalid = errors.New(
"retries must be a whole number of attempts",
"must be a whole number",
)
// errRetriesTooLarge signals a max_retries form value that is a
// whole number but above maxTargetRetries. It is distinguished
// from errRetriesInvalid so the message can name the ceiling
// instead of implying the input was not a number.
errRetriesTooLarge = errors.New("retries out of range")
errRetriesTooLarge = errors.New("out of range")
)
// parseMaxRetries interprets a max_retries form value.
@@ -82,7 +82,7 @@ func retriesErrorMessage(err error) string {
if errors.Is(err, errRetriesTooLarge) {
return errRetriesTooLarge.Error() +
": at most " + strconv.Itoa(maxTargetRetries) +
" retries"
" attempts"
}
return errRetriesInvalid.Error() +
+32 -7
View File
@@ -354,15 +354,14 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
// and target_http gives up once the attempt number reaches
// max_retries), and 0 is special-cased to a single fire-and-forget
// attempt with no circuit breaker.
const maxRetriesHelp = "This is the total number of delivery attempts, " +
"not retries on top of the first: a value of 3 makes three attempts " +
"in all. 0 means a single attempt with no retries and no circuit " +
"breaker."
const maxRetriesHelp = "How many times each delivery is attempted in " +
"all, the first attempt included. 0 means a single attempt with no " +
"retries and no circuit breaker."
// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
// help text on both the create form (the add-target form on the webhook
// detail page) and the edit form, so the copy cannot drift back to
// calling the number a retry count.
// label, "Delivery attempts", and help text on both the create form
// (the add-target form on the webhook detail page) and the edit form,
// so the copy cannot drift back to calling the number a retry count.
func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
t.Parallel()
@@ -393,6 +392,7 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
},
)
assert.Contains(t, createBody, "Delivery attempts:</label>")
assert.Contains(
t, createBody, maxRetriesHelp,
"the add-target form must explain max_retries as total attempts",
@@ -419,8 +419,33 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
},
)
assert.Contains(t, editBody, `class="label">Delivery attempts</label>`)
assert.Contains(
t, editBody, maxRetriesHelp,
"the target edit form must explain max_retries as total attempts",
)
}
// TestCreateFormCallsTheDatabaseTargetAnArchive pins the names the new
// webhook page gives the database target its Archive checkbox creates,
// and that target's settings, to the ones the target forms use.
func TestCreateFormCallsTheDatabaseTargetAnArchive(t *testing.T) {
t.Parallel()
var h *handlers.Handlers
var sess *session.Session
app := newTestApp(t, &h, &sess)
app.RequireStart()
t.Cleanup(app.RequireStop)
body := renderPage(t, h, sess, "sources_new.html", map[string]any{
dataKeyError: "",
})
assert.Contains(t, body, "created with an archive target")
assert.Contains(t, body, `class="label">Archive expiry</label>`)
assert.Contains(t, body, `class="label">Archive rotation</label>`)
}
+17 -13
View File
@@ -318,31 +318,35 @@ const (
func checkAddEachTargetType(ctx context.Context, t *testing.T, url string) {
t.Helper()
// Each target type, with the fields its add target form submits, in
// page order. Only http and slack have a url field.
// Each target type, with the badge its targets are listed with and
// the fields its add target form submits, in page order. Only http
// and slack have a url field.
targetTypes := []struct {
name string
badge string
fields string
values map[string]string
}{
{
"http", "csrf_token name type url headers timeout max_retries",
"http", "http",
"csrf_token name type url headers timeout max_retries",
map[string]string{"url": publicTargetURL},
},
{
"slack", "csrf_token name type url max_retries",
"slack", "slack", "csrf_token name type url max_retries",
map[string]string{"url": publicTargetURL},
},
{
"database", "csrf_token name type expiry rotation",
"database", "archive", "csrf_token name type expiry rotation",
map[string]string{"expiry": "720h", "rotation": "daily"},
},
{"log", "csrf_token name type", nil},
{"log", "log", "csrf_token name type", nil},
}
for _, tt := range targetTypes {
checkAddTarget(
ctx, t, url, tt.name, strings.Fields(tt.fields), tt.values,
ctx, t, url, tt.name, tt.badge,
strings.Fields(tt.fields), tt.values,
)
}
}
@@ -353,11 +357,11 @@ func checkAddEachTargetType(ctx context.Context, t *testing.T, url string) {
// type's own fields in place of the choice, and the form then submits
// exactly fields, so a field another type uses, such as url, is absent;
// Cancel closes it again. It then adds a target of the type, filling in
// values, and checks that the section lists it with that type.
// values, and checks that the section lists it with badge.
func checkAddTarget(
ctx context.Context,
t *testing.T,
url, targetType string,
url, targetType, badge string,
fields []string,
values map[string]string,
) {
@@ -412,8 +416,8 @@ func checkAddTarget(
click(ctx, t, saveButton)
assert.Truef(t, shown(ctx, `//span[text()="`+name+
`"]/following-sibling::div/span[text()="`+targetType+`"]`),
"%s: the added target is not listed with its type", targetType)
`"]/following-sibling::div/span[text()="`+badge+`"]`),
"%s: the added target is not listed as %s", targetType, badge)
}
// chooseTargetType clicks Add, picks targetType and clicks Next, and
@@ -466,10 +470,10 @@ func checkArchiveChoices(ctx context.Context, t *testing.T, url string) {
assert.Equal(t, "none", startRotation,
"the add target form's archive rotation does not start on none")
assert.True(t, shown(ctx, row+`//span[text()="Archive Expiry:"]`+
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")
assert.True(t, shown(ctx, row+`//span[text()="Archive Rotation:"]`+
assert.True(t, shown(ctx, row+`//span[text()="Archive rotation:"]`+
`/following-sibling::span[text()="daily"]`),
"a database target added with daily is not listed as daily")
+3 -3
View File
@@ -72,7 +72,7 @@ func TestEventResubmit_SignedOutRequestsNeverReachTheRateLimit(
env.requireNotice(
t, env.post(path, csrfForm(token), cookies),
logsPath, "resubmit-no-targets",
"this source has no active targets", cookies,
"this webhook has no active targets", cookies,
)
}
@@ -117,7 +117,7 @@ func TestEventResubmit_RefusedWithoutAValidCSRFToken(t *testing.T) {
env.requireNotice(
t, env.post(path, csrfForm(token), cookies),
logsPath, "resubmit-no-targets",
"this source has no active targets", cookies,
"this webhook has no active targets", cookies,
)
}
@@ -171,7 +171,7 @@ func TestEventResubmit_AnotherWebhooksEvent404s(t *testing.T) {
csrfForm(token), cookies,
),
intrudersLogs, "resubmit-no-targets",
"this source has no active targets", cookies,
"this webhook has no active targets", cookies,
)
}
+21 -1
View File
@@ -914,6 +914,26 @@ func TestPagesLogout_SaysSignedOut(t *testing.T) {
env.requireNotice(t, w, "/pages/login", "signed-out", "Signed out.", nil)
}
// TestSignInAndSignOutWording pins one wording for both: the sign-in
// page's button reads "Sign in" and the navbar's buttons "Sign out", as
// the sign-in page's heading, the error page's link and the notice
// after signing out do.
func TestSignInAndSignOutWording(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
assert.Contains(
t, env.get("/pages/login", nil).Body.String(), ">Sign in</button>",
)
userID, _ := env.seedUser(t, "reader", "somepassword")
page := env.get("/hooks", env.authCookies(t, userID, "reader")).Body.String()
assert.Equal(t, 2, strings.Count(page, ">Sign out</button>"),
"the desktop and the mobile navbar each say Sign out")
}
// --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -1363,7 +1383,7 @@ func TestHook_ResubmitFromEventLog(t *testing.T) {
)
env.requireNotice(
t, w, logsPath, "resubmit-no-targets",
"this source has no active targets", cookies,
"this webhook has no active targets", cookies,
)
webhookDB, err := env.dbMgr.GetDB(wh.ID)