sneak/webhooker
1
0
Fork 0
Code Issues 67 Pull Requests Actions Packages Projects Releases Wiki Activity

Deleting a target blanks its name on every historical delivery in the event log #211

New Issue
Closed
opened 2026-08-20 05:48:49 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-20 05:48:49 +02:00
Collaborator
Copy Link

Deletes are soft (internal/database/base_model.go:16, deleteChildResource at internal/handlers/source_management.go:1295-1298) and deliveries carry no FK to the target row, so deleting a target correctly preserves all its delivery history — verified, the rows survive intact.

But loadTargetMap (source_management.go:800-820) queries under GORM's default scope, so the soft-deleted target is excluded and targetMap[TargetID] yields a zero TargetView. templates/source_logs.html:26-30 then renders {{.Target.Name}}: {{.Status}} as:

: failed

Verified before and after: the target name disappeared from the log page while its delivery rows remained. The history survives; its label does not.

This is the practical cost of having no target edit form (#127) — changing a destination URL means delete and recreate, and that silently unlabels everything the old target ever did.

Definition of done:

  • loadTargetMap includes soft-deleted targets (Unscoped(), or a dedicated lookup) so historical deliveries keep their name
  • a deleted target's name renders with a marker making clear it no longer exists, e.g. echo-sink (deleted)
  • masking rules for target config are unchanged by this — a deleted target's config stays masked
  • a handler test asserts a delivery produced by a since-deleted target still renders its name

Not milestoned to 1.0.0: it is a display defect on an already-degraded path, and #127 plus the replay work may reshape it.

Deletes are soft (`internal/database/base_model.go:16`, `deleteChildResource` at `internal/handlers/source_management.go:1295-1298`) and deliveries carry no FK to the target row, so deleting a target correctly preserves all its delivery history — verified, the rows survive intact. But `loadTargetMap` (`source_management.go:800-820`) queries under GORM's default scope, so the soft-deleted target is excluded and `targetMap[TargetID]` yields a zero `TargetView`. `templates/source_logs.html:26-30` then renders `{{.Target.Name}}: {{.Status}}` as: ``` : failed ``` Verified before and after: the target name disappeared from the log page while its delivery rows remained. The history survives; its label does not. This is the practical cost of having no target edit form (#127) — changing a destination URL means delete and recreate, and that silently unlabels everything the old target ever did. Definition of done: - `loadTargetMap` includes soft-deleted targets (`Unscoped()`, or a dedicated lookup) so historical deliveries keep their name - a deleted target's name renders with a marker making clear it no longer exists, e.g. `echo-sink (deleted)` - masking rules for target config are unchanged by this — a deleted target's config stays masked - a handler test asserts a delivery produced by a since-deleted target still renders its name Not milestoned to 1.0.0: it is a display defect on an already-degraded path, and #127 plus the replay work may reshape it.
clawbot added this to the 1.0.0 milestone 2026-08-24 00:58:21 +02:00
clawbot commented 2026-08-24 01:36:00 +02:00
Author
Collaborator
Copy Link

Plan:

  • loadTargetMap (internal/handlers/source_management.go) already loads Unscoped() for the redactor half and then discards soft-deleted rows before building the view half. Build the view half from every loaded row instead, so a historical delivery resolves to a named view.
  • delivery.TargetView gains a Deleted flag set from DeletedAt.Valid, plus a DisplayName() that renders name (deleted). Views keep coming from NewTargetViews, so config masking stays a single chokepoint and a deleted target is masked by exactly the same code as a live one.
  • templates/source_logs.html renders DisplayName at both places a delivery's target is named.
  • Scope of the widening: the view half of loadTargetMap feeds only DeliveryView.Target on the event log page. The target list on the source detail page has its own scoped query, and the replay path resolves its target separately and already refuses a deleted one. No delivery-engine code is touched.

Tests: a delivery from a since-deleted target renders its name with the marker, and a credential stored in a deleted target's config does not appear in the rendered page.

Plan: - `loadTargetMap` (`internal/handlers/source_management.go`) already loads `Unscoped()` for the redactor half and then discards soft-deleted rows before building the view half. Build the view half from every loaded row instead, so a historical delivery resolves to a named view. - `delivery.TargetView` gains a `Deleted` flag set from `DeletedAt.Valid`, plus a `DisplayName()` that renders `name (deleted)`. Views keep coming from `NewTargetViews`, so config masking stays a single chokepoint and a deleted target is masked by exactly the same code as a live one. - `templates/source_logs.html` renders `DisplayName` at both places a delivery's target is named. - Scope of the widening: the view half of `loadTargetMap` feeds only `DeliveryView.Target` on the event log page. The target list on the source detail page has its own scoped query, and the replay path resolves its target separately and already refuses a deleted one. No delivery-engine code is touched. Tests: a delivery from a since-deleted target renders its name with the marker, and a credential stored in a deleted target's config does not appear in the rendered page.
clawbot commented 2026-08-24 01:51:34 +02:00
Author
Collaborator
Copy Link

Built in #266 (branch issue-211-deleted-target-name, base next).

loadTargetMap now builds its view half from the soft-deleted rows it was already loading Unscoped() for the redactor half, and delivery.TargetView renders name (deleted) through a new DisplayName() that templates/source_logs.html uses at both places a delivery's target is named.

Verified: both new handler tests fail on unmodified next (page renders : delivered and an empty name span) and pass on the branch. On a running instance, with the same script against a build of next and a build of the branch — real event delivered through a target named NAMED-TARGET-211, then deleted through the UI — next shows zero occurrences of the name afterwards and the branch shows NAMED-TARGET-211 (deleted): delivered, while the source detail target list still shows nothing. A credential stored in a deleted target's config stays masked, asserted by a test. make check green with GOFLAGS=-count=1, lint run uncached in Docker.

The PR body records what else consumes the widened lookup and what stays scoped.

Built in https://git.eeqj.de/sneak/webhooker/pulls/266 (branch `issue-211-deleted-target-name`, base `next`). `loadTargetMap` now builds its view half from the soft-deleted rows it was already loading `Unscoped()` for the redactor half, and `delivery.TargetView` renders `name (deleted)` through a new `DisplayName()` that `templates/source_logs.html` uses at both places a delivery's target is named. Verified: both new handler tests fail on unmodified `next` (page renders `: delivered` and an empty name span) and pass on the branch. On a running instance, with the same script against a build of `next` and a build of the branch — real event delivered through a target named `NAMED-TARGET-211`, then deleted through the UI — `next` shows zero occurrences of the name afterwards and the branch shows `NAMED-TARGET-211 (deleted): delivered`, while the source detail target list still shows nothing. A credential stored in a deleted target's config stays masked, asserted by a test. `make check` green with `GOFLAGS=-count=1`, lint run uncached in Docker. The PR body records what else consumes the widened lookup and what stays scoped.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#211
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?