Name a deleted target on its historical deliveries (closes #211)

Deleting a target soft deletes its row while its deliveries survive
in the per-webhook database, so the event log kept rendering the
history and lost the label: every historical row read ": delivered"
and the deliveries block was headed by an empty name.

loadTargetMap already loaded soft-deleted rows Unscoped for the
redactor half, then discarded them before building the view half.
It now builds both halves from every loaded row, and TargetView
carries a Deleted flag with a DisplayName that renders
"name (deleted)" — an operator debugging an old delivery needs to
know the target is gone, not just what it was called.

The views still come from NewTargetViews, so a deleted target's
configuration is masked by exactly the code that masks a live
one's. The widening is confined to this map, which feeds only
DeliveryView.Target on the event log page: the source detail
target list, the target edit form, the receiver and resubmit fan-out
and the delivery engine each resolve targets through their own
scoped queries, and the replay path keeps refusing a deleted target.

internal/handlers/source_logs_deleted_target_test.go

@@ -0,0 +1,144 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/handlers"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// deletedMarker is the suffix the event log appends to the name
+// of a target that no longer exists.
+const deletedMarker = " (deleted)"
+
+// deleteTargetThroughHandler removes a target through the real
+// deletion handler, so the test soft-deletes exactly the way the
+// UI does rather than by writing the timestamp itself.
+func deleteTargetThroughHandler(
+	t *testing.T,
+	h *handlers.Handlers,
+	sess *session.Session,
+	webhookID, targetID string,
+) {
+	t.Helper()
+
+	req := postRequest(
+		"/source/"+webhookID+"/targets/"+targetID+"/delete",
+		authenticatedCookies(
+			t, sess, deleteTestUserID, deleteTestUsername,
+		),
+		map[string]string{
+			paramSourceID: webhookID,
+			paramTargetID: targetID,
+		},
+	)
+	w := httptest.NewRecorder()
+
+	h.HandleTargetDelete().ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusSeeOther, w.Code)
+}
+
+// TestHandleSourceLogs_NamesDeletedTarget proves a delivery
+// produced by a since-deleted target still names it on the event
+// log, marked as deleted.
+//
+// Deletes are soft and deliveries carry no foreign key to the
+// target row, so the history outlives the target. Against a
+// scoped lookup the delivery resolves to a zero view and the page
+// renders ": delivered" with nothing saying what it was delivered
+// to.
+func TestHandleSourceLogs_NamesDeletedTarget(t *testing.T) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	tgt := seedTarget(t, db, wh.ID, database.TargetTypeLog)
+
+	seedDeliveredEvent(t, dbMgr, wh.ID, tgt.ID)
+
+	// The control: the name is on the page while the target
+	// lives, and is not yet marked as deleted.
+	before := renderSourceLogsPage(t, h, sess, wh.ID)
+	assert.Contains(t, before, tgt.Name)
+	assert.NotContains(t, before, tgt.Name+deletedMarker)
+
+	deleteTargetThroughHandler(t, h, sess, wh.ID, tgt.ID)
+
+	after := renderSourceLogsPage(t, h, sess, wh.ID)
+
+	assert.Contains(
+		t, after, tgt.Name+deletedMarker,
+		"a delivery from a deleted target must keep its name, "+
+			"marked as no longer existing",
+	)
+	assert.Contains(
+		t, after, "delivered",
+		"the delivery history itself must survive the delete",
+	)
+}
+
+// TestHandleSourceLogs_MasksDeletedTargetConfig proves that
+// naming a deleted target does not widen what the page shows of
+// it: its stored configuration stays masked by exactly the rules
+// a live target's is.
+//
+// The lookup behind the name reads soft-deleted rows, so it
+// carries a full target row — credential blob included — into the
+// place a zero value used to sit. The projection to TargetView is
+// what keeps that blob away from the template, and it must hold
+// for a deleted row too.
+func TestHandleSourceLogs_MasksDeletedTargetConfig(t *testing.T) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	tgt := seedConfiguredTarget(
+		t, db, wh.ID,
+		database.TargetTypeSlack,
+		`{"webhookUrl":"`+slackWebhookURL+`"}`,
+	)
+
+	seedDeliveredEvent(t, dbMgr, wh.ID, tgt.ID)
+	deleteTargetThroughHandler(t, h, sess, wh.ID, tgt.ID)
+
+	body := renderSourceLogsPage(t, h, sess, wh.ID)
+
+	assert.NotContains(t, body, slackSecretPath)
+	assert.NotContains(t, body, "T00000000")
+	assert.NotContains(t, body, "B00000000")
+	assert.NotContains(
+		t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
+	)
+	assert.NotContains(t, body, "webhookUrl")
+
+	// The name is there; only the credential is not.
+	assert.Contains(t, body, tgt.Name+deletedMarker)
+}

internal/handlers/source_management.go

@@ -860,11 +860,16 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 //
 // The load is Unscoped because deleting a target only soft
 // deletes the row while its deliveries survive in the
-// per-webhook database: a scoped load leaves those deliveries
-// with a zero redactor, which renders their response bodies
-// unredacted. Only the redactor half of the map is built from
-// deleted rows. The view half, which is what the page lists,
-// stays scoped.
+// per-webhook database. Both halves of the map need those rows:
+// a scoped load leaves an old delivery with a zero redactor,
+// which renders its response bodies unredacted, and with a zero
+// view, which renders its target as a blank name.
+//
+// This map is historical display only. It is built for the event
+// log page and reaches nothing but DeliveryView.Target: the
+// target list on the source detail page, the edit form and the
+// replay path each resolve targets themselves, and a deleted row
+// is refused there as before.
 func (h *Handlers) loadTargetMap(
 	webhookID string,
 ) (map[string]eventLogTarget, error) {
@@ -880,21 +885,18 @@ func (h *Handlers) loadTargetMap(
 	targetMap := make(
 		map[string]eventLogTarget, len(targets),
 	)
-	live := make([]database.Target, 0, len(targets))
 
 	for i := range targets {
 		targetMap[targets[i].ID] = eventLogTarget{
 			Redactor: delivery.NewRedactor(&targets[i]),
 		}
-
-		if !targets[i].DeletedAt.Valid {
-			live = append(live, targets[i])
-		}
 	}
 
 	// The views come from NewTargetViews rather than being
-	// rebuilt here, so the masking rules stay in one place.
-	for _, v := range delivery.NewTargetViews(live) {
+	// rebuilt here, so the masking rules stay in one place and a
+	// deleted target's configuration is masked by the same code
+	// that masks a live one's.
+	for _, v := range delivery.NewTargetViews(targets) {
 		entry := targetMap[v.ID]
 		entry.View = v
 		targetMap[v.ID] = entry