From 93911f28f96f7889f2ce294ffe9f399bcb9cd725 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sat, 3 Oct 2026 00:19:13 +0200 Subject: [PATCH] Show a slack target's retry setting in the target list (closes #395) A slack target's edit page offers Max Retries and the delivery engine honours it, but the target list showed only its masked webhook URL, so setting retries changed nothing visible. The list now shows a slack target's Max Retries line exactly as an http target's, from the one function both use, so the label and the "0 (fire-and-forget)" wording cannot drift apart. The Max Queue Size line stays on http targets only. Tests cover a slack target with retries set, and one with a queue size stored that shows no queue-size line. Model: opus-5-5 --- internal/delivery/target_config_view.go | 43 ++++++++++---------- internal/delivery/target_config_view_test.go | 36 +++++++++++++--- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/internal/delivery/target_config_view.go b/internal/delivery/target_config_view.go index fc09a40..1053eac 100644 --- a/internal/delivery/target_config_view.go +++ b/internal/delivery/target_config_view.go @@ -97,7 +97,7 @@ func targetConfigFields( ) []ConfigField { switch t.Type { case database.TargetTypeSlack: - return slackConfigFields(t.Config) + return slackConfigFields(t) case database.TargetTypeHTTP: return httpConfigFields(t) case database.TargetTypeDatabase: @@ -119,10 +119,11 @@ func unavailableConfigFields() []ConfigField { }} } -// slackConfigFields describes a Slack target. Only the masked -// webhook URL is shown; the full URL is the credential. -func slackConfigFields(configJSON string) []ConfigField { - cfg, err := parseSlackConfig(configJSON) +// slackConfigFields describes a Slack target: its masked +// webhook URL and its retry count. Only the masked URL is +// shown; the full URL is the credential. +func slackConfigFields(t *database.Target) []ConfigField { + cfg, err := parseSlackConfig(t.Config) if err != nil { return unavailableConfigFields() } @@ -130,7 +131,7 @@ func slackConfigFields(configJSON string) []ConfigField { return []ConfigField{{ Label: "Webhook URL", Value: cfg.MaskedWebhookURL(), - }} + }, maxRetriesField(t)} } // httpConfigFields describes an HTTP target: its destination @@ -170,21 +171,7 @@ func httpConfigFields(t *database.Target) []ConfigField { }) } - return append(fields, retryFields(t)...) -} - -// retryFields describes a target's retry settings, which live -// on the target row rather than in its configuration blob. -func retryFields(t *database.Target) []ConfigField { - retries := strconv.Itoa(t.MaxRetries) - if t.MaxRetries == 0 { - retries += " (fire-and-forget)" - } - - fields := []ConfigField{{ - Label: "Max Retries", - Value: retries, - }} + fields = append(fields, maxRetriesField(t)) if t.MaxQueueSize > 0 { fields = append(fields, ConfigField{ @@ -196,6 +183,20 @@ func retryFields(t *database.Target) []ConfigField { return fields } +// maxRetriesField describes a target's retry count, which lives +// on the target row rather than in its configuration blob. +func maxRetriesField(t *database.Target) ConfigField { + retries := strconv.Itoa(t.MaxRetries) + if t.MaxRetries == 0 { + retries += " (fire-and-forget)" + } + + return ConfigField{ + Label: "Max Retries", + Value: retries, + } +} + // 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 diff --git a/internal/delivery/target_config_view_test.go b/internal/delivery/target_config_view_test.go index 41211fa..c9e58f5 100644 --- a/internal/delivery/target_config_view_test.go +++ b/internal/delivery/target_config_view_test.go @@ -32,6 +32,7 @@ const ( viewMaskedOrigin = viewExampleOrigin + "/..." viewUnavailable = "(unavailable)" viewExpiryNever = "never" + viewMaxRetries = "Max Retries" ) func TestMaskedWebhookURL(t *testing.T) { @@ -157,9 +158,7 @@ func TestNewTargetViews_DeletedTarget(t *testing.T) { t, slackTargetName+" (deleted)", view.DisplayName(), ) assert.Equal( - t, - map[string]string{"Webhook URL": slackMaskedURL}, - fieldMap(view.Config), + t, viewFor(t, slackTarget()).Config, view.Config, ) } @@ -189,7 +188,32 @@ func TestNewTargetViews_Slack(t *testing.T) { assert.Equal( t, - map[string]string{"Webhook URL": slackMaskedURL}, + map[string]string{ + "Webhook URL": slackMaskedURL, + viewMaxRetries: "0 (fire-and-forget)", + }, + fieldMap(view.Config), + ) +} + +// TestNewTargetViews_SlackRetries proves a Slack target shows +// its retry count the same way an HTTP target does, and no +// queue size even when one is stored: delivery never reads it. +func TestNewTargetViews_SlackRetries(t *testing.T) { + t.Parallel() + + target := slackTarget() + target.MaxRetries = 2 + target.MaxQueueSize = 100 + + view := viewFor(t, target) + + assert.Equal( + t, + map[string]string{ + "Webhook URL": slackMaskedURL, + viewMaxRetries: "2", + }, fieldMap(view.Config), ) } @@ -214,7 +238,7 @@ func TestNewTargetViews_HTTP(t *testing.T) { "Destination URL": viewMaskedOrigin, "Timeout": "30s", "Headers": "1 configured", - "Max Retries": "5", + viewMaxRetries: "5", "Max Queue Size": "100", }, fields, @@ -238,7 +262,7 @@ func TestNewTargetViews_HTTPFireAndForget(t *testing.T) { t, map[string]string{ "Destination URL": viewMaskedOrigin, - "Max Retries": "0 (fire-and-forget)", + viewMaxRetries: "0 (fire-and-forget)", }, fieldMap(view.Config), )