From 5b1d283d0603463a9469b2733f79bbb238353bc7 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Fri, 2 Oct 2026 10:30:31 +0200
Subject: [PATCH] Say what each action did in a one-line notice (closes #383)
Saving, deleting, activating or deactivating a webhook, entrypoint or target, and signing out, now land on their page with a one-line notice such as "Webhook deleted." or "Signed out.". The redirect carries a fixed code that maps to fixed text; an unknown code shows nothing, so nothing from the URL is ever echoed. One partial in the page layout shows the notice on every page, and replay and resubmit now use the same codes and partial. Error pages show no notice.
Model: opus-5-5
---
internal/handlers/auth.go | 6 +-
internal/handlers/delivery_replay.go | 74 ++++---------
internal/handlers/delivery_replay_test.go | 16 +--
internal/handlers/event_resubmit.go | 59 +----------
internal/handlers/event_resubmit_test.go | 8 +-
internal/handlers/handlers.go | 25 +++--
internal/handlers/notice.go | 109 ++++++++++++++++++++
internal/handlers/source_delete_test.go | 4 +-
internal/handlers/source_management.go | 87 ++++++++--------
internal/handlers/target_edit.go | 3 +-
internal/server/error_page_test.go | 16 +++
internal/server/routes_test.go | 120 ++++++++++++++++------
templates/base.html | 1 +
templates/notice.html | 7 ++
templates/source_logs.html | 8 --
15 files changed, 327 insertions(+), 216 deletions(-)
create mode 100644 internal/handlers/notice.go
create mode 100644 templates/notice.html
diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go
index 6f037af..74ad68d 100644
--- a/internal/handlers/auth.go
+++ b/internal/handlers/auth.go
@@ -333,7 +333,9 @@ func (h *Handlers) HandleLogout() http.HandlerFunc {
)
}
- // Redirect to login page
- http.Redirect(w, r, "/pages/login", http.StatusSeeOther)
+ http.Redirect(
+ w, r, withNotice("/pages/login", signedOut),
+ http.StatusSeeOther,
+ )
}
}
diff --git a/internal/handlers/delivery_replay.go b/internal/handlers/delivery_replay.go
index ced78b8..4ef278b 100644
--- a/internal/handlers/delivery_replay.go
+++ b/internal/handlers/delivery_replay.go
@@ -11,72 +11,37 @@ import (
"sneak.berlin/go/webhooker/internal/delivery"
)
-// replayOutcomeParam is the query parameter the replay POST redirects
-// with and the event log page reads its banner from.
-const replayOutcomeParam = "replay"
-
-// replayOutcomeCode is the outcome of a replay POST. The redirect
-// carries one of these fixed codes rather than a message, so nothing a
-// client submits can reach the rendered page through it.
-type replayOutcomeCode string
-
+// The outcomes of a replay POST, as the notice codes its redirect
+// carries. noticeFor holds the line each one shows.
const (
// replayQueued reports that a new delivery was created and handed
// to the delivery engine.
- replayQueued replayOutcomeCode = "queued"
+ replayQueued noticeCode = "replay-queued"
// replayTargetDeleted reports a target that once existed and has
// since been deleted. Deletes are soft and deliveries carry no
// foreign key to the target row, so the history survives its
// target and this is the ordinary case for an old event.
- replayTargetDeleted replayOutcomeCode = "target-deleted"
+ replayTargetDeleted noticeCode = "replay-target-deleted"
// replayTargetMissing reports a target id that names no row at
// all, deleted or otherwise.
- replayTargetMissing replayOutcomeCode = "target-missing"
+ replayTargetMissing noticeCode = "replay-target-missing"
// replayTargetInactive reports a target the operator has
// deactivated. A deactivated target receives no new deliveries, so
// a replay to it would be a delivery they switched off.
- replayTargetInactive replayOutcomeCode = "target-inactive"
+ replayTargetInactive noticeCode = "replay-target-inactive"
// replayNotTerminal reports a delivery the engine has not finished
// with.
- replayNotTerminal replayOutcomeCode = "not-terminal"
+ replayNotTerminal noticeCode = "replay-not-terminal"
// replayInFlight reports that an earlier replay of this event to
// this target is still running.
- replayInFlight replayOutcomeCode = "in-flight"
+ replayInFlight noticeCode = "replay-in-flight"
)
-// replayOutcome returns the banner the event log page shows for an
-// outcome code, and whether the replay was queued. An unrecognised
-// code yields no banner.
-func replayOutcome(code string) (string, bool) {
- switch replayOutcomeCode(code) {
- case replayQueued:
- return "Replay queued: a new delivery was created against " +
- "the target's current configuration.", true
- case replayTargetDeleted:
- return "Not replayed: the target this delivery was for has " +
- "been deleted. Recreate the target, then replay.", false
- case replayTargetMissing:
- return "Not replayed: the target this delivery was for no " +
- "longer exists.", false
- case replayTargetInactive:
- return "Not replayed: the target this delivery was for is " +
- "deactivated. Activate it, then replay.", false
- case replayNotTerminal:
- return "Not replayed: this delivery has not finished yet.",
- false
- case replayInFlight:
- return "Not replayed: a delivery of this event to this " +
- "target is already in flight.", false
- default:
- return "", false
- }
-}
-
// HandleDeliveryReplay re-sends a finished delivery's event to its
// target.
//
@@ -140,14 +105,14 @@ func (h *Handlers) replayDelivery(
}
if !original.Status.Terminal() {
- h.finishReplay(w, r, webhook, replayNotTerminal)
+ redirectToEventLog(w, r, webhook, replayNotTerminal)
return
}
target, code := h.replayTarget(webhook.ID, original.TargetID)
if target == nil {
- h.finishReplay(w, r, webhook, code)
+ redirectToEventLog(w, r, webhook, code)
return
}
@@ -200,7 +165,7 @@ func (h *Handlers) queueReplay(
}
if inFlight > 0 {
- h.finishReplay(w, r, webhook, replayInFlight)
+ redirectToEventLog(w, r, webhook, replayInFlight)
return
}
@@ -238,7 +203,7 @@ func (h *Handlers) queueReplay(
"delivery_id", task.DeliveryID,
)
- h.finishReplay(w, r, webhook, replayQueued)
+ redirectToEventLog(w, r, webhook, replayQueued)
}
// replayTarget loads the delivery's target as it stands now.
@@ -251,7 +216,7 @@ func (h *Handlers) queueReplay(
// with the returned code saying why.
func (h *Handlers) replayTarget(
webhookID, targetID string,
-) (*database.Target, replayOutcomeCode) {
+) (*database.Target, noticeCode) {
var target database.Target
err := h.db.DB().Unscoped().Where(
@@ -361,17 +326,16 @@ func replayBody(body string) *string {
return &body
}
-// finishReplay redirects back to the event log the replay was
-// triggered from, carrying the outcome code the page turns into a
-// banner and the page number the form submitted.
-func (h *Handlers) finishReplay(
+// redirectToEventLog redirects a replay or resubmit back to the event
+// log it was triggered from, carrying the outcome as its notice and
+// the page number the form submitted.
+func redirectToEventLog(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
- code replayOutcomeCode,
+ code noticeCode,
) {
- dest := "/hook/" + webhook.ID + "/events?" +
- replayOutcomeParam + "=" + string(code)
+ dest := withNotice("/hook/"+webhook.ID+"/events", code)
// The page is read from the form rather than the query string:
// this is a POST, and its query string is what logs and Referer
diff --git a/internal/handlers/delivery_replay_test.go b/internal/handlers/delivery_replay_test.go
index a301237..f155c17 100644
--- a/internal/handlers/delivery_replay_test.go
+++ b/internal/handlers/delivery_replay_test.go
@@ -212,7 +212,7 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?replay=queued",
+ "/hook/"+wh.ID+"/events?notice=replay-queued",
w.Header().Get("Location"),
)
@@ -362,7 +362,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?replay=target-deleted",
+ "/hook/"+wh.ID+"/events?notice=replay-target-deleted",
w.Header().Get("Location"),
)
@@ -390,7 +390,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
require.Equal(t, http.StatusSeeOther, missing.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?replay=target-missing",
+ "/hook/"+wh.ID+"/events?notice=replay-target-missing",
missing.Header().Get("Location"),
)
}
@@ -431,7 +431,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
require.Equal(t, http.StatusSeeOther, first.Code)
require.Equal(
t,
- "/hook/"+wh.ID+"/events?replay=queued",
+ "/hook/"+wh.ID+"/events?notice=replay-queued",
first.Header().Get("Location"),
)
@@ -439,7 +439,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
require.Equal(t, http.StatusSeeOther, second.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?replay=in-flight",
+ "/hook/"+wh.ID+"/events?notice=replay-in-flight",
second.Header().Get("Location"),
)
@@ -465,7 +465,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
require.Equal(t, http.StatusSeeOther, pending.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?replay=not-terminal",
+ "/hook/"+wh.ID+"/events?notice=replay-not-terminal",
pending.Header().Get("Location"),
)
}
@@ -509,7 +509,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
assert.Contains(t, body, ">Replay<")
refused := renderSourceLogsPageWithQuery(
- t, h, sess, wh.ID, "?replay=target-deleted",
+ t, h, sess, wh.ID, "?notice=replay-target-deleted",
)
assert.Contains(t, refused, "alert-error")
@@ -517,7 +517,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
// An outcome code nobody issued renders no banner at all.
unknown := renderSourceLogsPageWithQuery(
- t, h, sess, wh.ID, "?replay=made-up",
+ t, h, sess, wh.ID, "?notice=made-up",
)
assert.NotContains(t, unknown, "alert-error")
diff --git a/internal/handlers/event_resubmit.go b/internal/handlers/event_resubmit.go
index dcff688..63b51c4 100644
--- a/internal/handlers/event_resubmit.go
+++ b/internal/handlers/event_resubmit.go
@@ -3,7 +3,6 @@ package handlers
import (
"errors"
"net/http"
- "strconv"
"github.com/go-chi/chi"
"github.com/google/uuid"
@@ -11,43 +10,19 @@ import (
"sneak.berlin/go/webhooker/internal/database"
)
-// resubmitOutcomeParam is the query parameter the resubmit POST
-// redirects with and the event log page reads its banner from.
-const resubmitOutcomeParam = "resubmit"
-
-// resubmitOutcomeCode is the outcome of a resubmit POST. The redirect
-// carries one of these fixed codes rather than a message, so nothing a
-// client submits can reach the rendered page through it.
-type resubmitOutcomeCode string
-
+// The outcomes of a resubmit POST, as the notice codes its redirect
+// carries. noticeFor holds the line each one shows.
const (
// resubmitQueued reports that a new event was stored and its
// deliveries handed to the delivery engine.
- resubmitQueued resubmitOutcomeCode = "queued"
+ resubmitQueued noticeCode = "resubmit-queued"
// resubmitNoTargets reports a source with no active targets. The
// new event is stored either way, exactly as a received event
// with no targets is.
- resubmitNoTargets resubmitOutcomeCode = "no-targets"
+ resubmitNoTargets noticeCode = "resubmit-no-targets"
)
-// resubmitOutcome returns the banner the event log page shows for an
-// outcome code, and whether the resubmit was queued. An unrecognised
-// code yields no banner.
-func resubmitOutcome(code string) (string, bool) {
- switch resubmitOutcomeCode(code) {
- case resubmitQueued:
- return "Resubmitted: a new event was created from the stored " +
- "one and queued to every active target.", true
- case resubmitNoTargets:
- return "Resubmitted: a new event was created, but this " +
- "source has no active targets, so nothing was queued.",
- true
- default:
- return "", false
- }
-}
-
// resubmitSource is the stored event a resubmit copies. Its body is
// read as bytes rather than as a string so the copy is byte-identical
// to what was received, whatever the payload's encoding.
@@ -245,29 +220,5 @@ func (h *Handlers) queueResubmit(
code = resubmitNoTargets
}
- h.finishResubmit(w, r, webhook, code)
-}
-
-// finishResubmit redirects back to the event log the resubmit was
-// triggered from, carrying the outcome code the page turns into a
-// banner and the page number the form submitted.
-func (h *Handlers) finishResubmit(
- w http.ResponseWriter,
- r *http.Request,
- webhook database.Webhook,
- code resubmitOutcomeCode,
-) {
- dest := "/hook/" + webhook.ID + "/events?" +
- resubmitOutcomeParam + "=" + string(code)
-
- // The page is read from the form rather than the query string:
- // this is a POST, and its query string is what logs and Referer
- // headers record.
- if page := pageOrFirst(
- r.PostFormValue("page"),
- ); page > 1 {
- dest += "&page=" + strconv.Itoa(page)
- }
-
- http.Redirect(w, r, dest, http.StatusSeeOther)
+ redirectToEventLog(w, r, webhook, code)
}
diff --git a/internal/handlers/event_resubmit_test.go b/internal/handlers/event_resubmit_test.go
index 921139d..a7c92d2 100644
--- a/internal/handlers/event_resubmit_test.go
+++ b/internal/handlers/event_resubmit_test.go
@@ -154,7 +154,7 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?resubmit=queued",
+ "/hook/"+wh.ID+"/events?notice=resubmit-queued",
w.Header().Get("Location"),
)
@@ -282,7 +282,7 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?resubmit=queued",
+ "/hook/"+wh.ID+"/events?notice=resubmit-queued",
w.Header().Get("Location"),
"a resubmit must not be refused while an earlier "+
"one is in flight",
@@ -436,7 +436,7 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?resubmit=queued",
+ "/hook/"+wh.ID+"/events?notice=resubmit-queued",
w.Header().Get("Location"),
"an inactive target is skipped, not an error",
)
@@ -482,7 +482,7 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
- "/hook/"+wh.ID+"/events?resubmit=no-targets",
+ "/hook/"+wh.ID+"/events?notice=resubmit-no-targets",
w.Header().Get("Location"),
)
diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go
index b0f6e87..713a629 100644
--- a/internal/handlers/handlers.go
+++ b/internal/handlers/handlers.go
@@ -97,10 +97,10 @@ type Handlers struct {
// parsePageTemplate parses a page-specific template set from the
// embedded FS. Each page template is combined with the shared
-// base, htmlheader, and navbar templates, and with any further files
-// the page includes. The page file must be listed first so that its
-// root action ({{template "base" .}}) becomes the template set's entry
-// point.
+// base, htmlheader, navbar and notice templates, and with any further
+// files the page includes. The page file must be listed first so that
+// its root action ({{template "base" .}}) becomes the template set's
+// entry point.
func parsePageTemplate(
pageFile string, included ...string,
) *template.Template {
@@ -109,6 +109,7 @@ func parsePageTemplate(
"base.html",
"htmlheader.html",
"navbar.html",
+ "notice.html",
}, included...)
return template.Must(
@@ -209,11 +210,13 @@ func (s *Handlers) renderError(
// served outside the routes where NoCache runs.
w.Header().Set("Cache-Control", "no-store")
+ // No notice: one would say an action worked above a page saying
+ // the request failed.
data := s.pageData(r, map[string]any{
"Status": status,
"StatusText": http.StatusText(status),
"Message": errorPageText(status),
- })
+ }, nil)
var buf bytes.Buffer
@@ -267,6 +270,7 @@ type templateDataWrapper struct {
User *UserInfo
CSRFToken string
Version string
+ Notice *notice
Data any
}
@@ -311,12 +315,15 @@ func (s *Handlers) renderTemplate(
return
}
- s.executeTemplate(w, r, tmpl, s.pageData(r, data))
+ s.executeTemplate(w, r, tmpl, s.pageData(r, data, noticeFor(r)))
}
// pageData adds the fields the shared layout renders to a page's own
-// data.
-func (s *Handlers) pageData(r *http.Request, data any) any {
+// data. The layout shows the notice, when there is one, above the
+// page.
+func (s *Handlers) pageData(
+ r *http.Request, data any, pageNotice *notice,
+) any {
userInfo := s.getUserInfo(r)
csrfToken := middleware.CSRFToken(r)
@@ -330,6 +337,7 @@ func (s *Handlers) pageData(r *http.Request, data any) any {
m["User"] = userInfo
m["CSRFToken"] = csrfToken
m["Version"] = version
+ m["Notice"] = pageNotice
return m
}
@@ -338,6 +346,7 @@ func (s *Handlers) pageData(r *http.Request, data any) any {
User: userInfo,
CSRFToken: csrfToken,
Version: version,
+ Notice: pageNotice,
Data: data,
}
}
diff --git a/internal/handlers/notice.go b/internal/handlers/notice.go
new file mode 100644
index 0000000..72b97e5
--- /dev/null
+++ b/internal/handlers/notice.go
@@ -0,0 +1,109 @@
+package handlers
+
+import "net/http"
+
+// noticeParam is the query parameter an action's redirect carries its
+// notice code in.
+const noticeParam = "notice"
+
+// noticeCode names one of the fixed lines noticeFor knows. An action
+// redirects with the code rather than the line, so nothing a client
+// puts in the URL reaches the page: a code noticeFor does not know
+// shows nothing.
+type noticeCode string
+
+// The codes of the actions on the webhook pages and of signing out.
+// Replay's codes, with the reasons a replay can be refused, and
+// resubmit's codes are defined beside those actions.
+const (
+ webhookCreated noticeCode = "webhook-created"
+ webhookSaved noticeCode = "webhook-saved"
+ webhookDeleted noticeCode = "webhook-deleted"
+ entrypointAdded noticeCode = "entrypoint-added"
+ entrypointDeleted noticeCode = "entrypoint-deleted"
+ entrypointActivated noticeCode = "entrypoint-activated"
+ entrypointDeactivated noticeCode = "entrypoint-deactivated"
+ targetAdded noticeCode = "target-added"
+ targetSaved noticeCode = "target-saved"
+ targetDeleted noticeCode = "target-deleted"
+ targetActivated noticeCode = "target-activated"
+ targetDeactivated noticeCode = "target-deactivated"
+ signedOut noticeCode = "signed-out"
+)
+
+// notice is the line templates/notice.html shows above a page to say
+// what an action did.
+type notice struct {
+ Text string
+
+ // Failed shows the line as an error: the action was refused.
+ Failed bool
+}
+
+// noticeFor returns the notice the request's URL names, or nil when it
+// names none or an unknown code.
+func noticeFor(r *http.Request) *notice {
+ n, ok := map[noticeCode]notice{
+ webhookCreated: {Text: "Webhook created."},
+ webhookSaved: {Text: "Webhook saved."},
+ webhookDeleted: {Text: "Webhook deleted."},
+ entrypointAdded: {Text: "Entrypoint added."},
+ entrypointDeleted: {Text: "Entrypoint deleted."},
+ entrypointActivated: {Text: "Entrypoint activated."},
+ entrypointDeactivated: {Text: "Entrypoint deactivated."},
+ targetAdded: {Text: "Target added."},
+ targetSaved: {Text: "Target saved."},
+ targetDeleted: {Text: "Target deleted."},
+ targetActivated: {Text: "Target activated."},
+ targetDeactivated: {Text: "Target deactivated."},
+ signedOut: {Text: "Signed out."},
+
+ replayQueued: {
+ Text: "Replay queued: a new delivery was created " +
+ "against the target's current configuration.",
+ },
+ replayTargetDeleted: {
+ Text: "Not replayed: the target this delivery was for " +
+ "has been deleted. Recreate the target, then replay.",
+ Failed: true,
+ },
+ replayTargetMissing: {
+ Text: "Not replayed: the target this delivery was for " +
+ "no longer exists.",
+ Failed: true,
+ },
+ replayTargetInactive: {
+ Text: "Not replayed: the target this delivery was for " +
+ "is deactivated. Activate it, then replay.",
+ Failed: true,
+ },
+ replayNotTerminal: {
+ Text: "Not replayed: this delivery has not finished yet.",
+ Failed: true,
+ },
+ replayInFlight: {
+ Text: "Not replayed: a delivery of this event to this " +
+ "target is already in flight.",
+ Failed: true,
+ },
+
+ resubmitQueued: {
+ Text: "Resubmitted: a new event was created from the " +
+ "stored one and queued to every active target.",
+ },
+ resubmitNoTargets: {
+ Text: "Resubmitted: a new event was created, but this " +
+ "source has no active targets, so nothing was queued.",
+ },
+ }[noticeCode(r.URL.Query().Get(noticeParam))]
+ if !ok {
+ return nil
+ }
+
+ return &n
+}
+
+// withNotice returns path with code added as its notice.
+func withNotice(path string, code noticeCode) string {
+ return path + "?" + noticeParam + "=" + string(code)
+}
diff --git a/internal/handlers/source_delete_test.go b/internal/handlers/source_delete_test.go
index 58db5c5..1bdd9fc 100644
--- a/internal/handlers/source_delete_test.go
+++ b/internal/handlers/source_delete_test.go
@@ -411,7 +411,9 @@ func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
h.HandleSourceDelete().ServeHTTP(w, req)
require.Equal(t, http.StatusSeeOther, w.Code)
- assert.Equal(t, "/hooks", w.Header().Get("Location"))
+ assert.Equal(
+ t, "/hooks?notice=webhook-deleted", w.Header().Get("Location"),
+ )
assert.Equal(
t, int64(0),
diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go
index 330e787..1596101 100644
--- a/internal/handlers/source_management.go
+++ b/internal/handlers/source_management.go
@@ -322,7 +322,8 @@ func (h *Handlers) createWebhookWithEntrypoint(
)
http.Redirect(
- w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
+ w, r, withNotice("/hook/"+webhook.ID, webhookCreated),
+ http.StatusSeeOther,
)
}
@@ -579,7 +580,8 @@ func (h *Handlers) applyWebhookEdit(
}
http.Redirect(
- w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
+ w, r, withNotice("/hook/"+webhook.ID, webhookSaved),
+ http.StatusSeeOther,
)
}
@@ -662,7 +664,9 @@ func (h *Handlers) deleteWebhookResources(
return
}
- http.Redirect(w, r, "/hooks", http.StatusSeeOther)
+ http.Redirect(
+ w, r, withNotice("/hooks", webhookDeleted), http.StatusSeeOther,
+ )
}
// commitWebhookDeletion soft-deletes a webhook's entrypoints,
@@ -841,31 +845,16 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
totalPages++
}
- // The banner a replay or resubmit POST redirected back
- // with. The message comes from a fixed set keyed by the
- // outcome code, never from the query string itself.
- replayMsg, replayOK := replayOutcome(
- r.URL.Query().Get(replayOutcomeParam),
- )
-
- resubmitMsg, resubmitOK := resubmitOutcome(
- r.URL.Query().Get(resubmitOutcomeParam),
- )
-
data := map[string]any{
- tmplKeyWebhook: &webhook,
- "Events": evts,
- "ReplayMessage": replayMsg,
- "ReplayQueued": replayOK,
- "ResubmitMessage": resubmitMsg,
- "ResubmitQueued": resubmitOK,
- "Page": page,
- "TotalPages": totalPages,
- "TotalEvents": total,
- "HasPrev": page > 1,
- "HasNext": page < totalPages,
- "PrevPage": page - 1,
- "NextPage": page + 1,
+ tmplKeyWebhook: &webhook,
+ "Events": evts,
+ "Page": page,
+ "TotalPages": totalPages,
+ "TotalEvents": total,
+ "HasPrev": page > 1,
+ "HasNext": page < totalPages,
+ "PrevPage": page - 1,
+ "NextPage": page + 1,
}
h.renderTemplate(w, r, "source_logs.html", data)
@@ -1254,7 +1243,8 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
}
http.Redirect(
- w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
+ w, r, withNotice("/hook/"+webhook.ID, entrypointAdded),
+ http.StatusSeeOther,
)
}
}
@@ -1365,7 +1355,8 @@ func (h *Handlers) processTargetCreate(
}
http.Redirect(
- w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
+ w, r, withNotice("/hook/"+webhook.ID, targetAdded),
+ http.StatusSeeOther,
)
}
@@ -1643,6 +1634,7 @@ func (h *Handlers) HandleEntrypointDelete() http.HandlerFunc {
"entrypointID", &database.Entrypoint{},
"failed to delete entrypoint",
nil,
+ entrypointDeleted,
)
}
@@ -1655,18 +1647,21 @@ func (h *Handlers) HandleTargetDelete() http.HandlerFunc {
"targetID", &database.Target{},
"failed to delete target",
h.evictArchiveWriterIfUnused,
+ targetDeleted,
)
}
// deleteChildResource returns a handler that deletes a child
// resource (entrypoint or target) belonging to a webhook. The
// optional afterDelete hook runs with the webhook's id once the
-// delete has succeeded, before the redirect.
+// delete has succeeded, before the redirect, which carries done as
+// its notice.
func (h *Handlers) deleteChildResource(
idParam string,
model any,
errMsg string,
afterDelete func(webhookID string),
+ done noticeCode,
) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
userID, ok := h.getUserID(r)
@@ -1708,7 +1703,7 @@ func (h *Handlers) deleteChildResource(
http.Redirect(
w, r,
- "/hook/"+webhook.ID,
+ withNotice("/hook/"+webhook.ID, done),
http.StatusSeeOther,
)
}
@@ -1719,7 +1714,7 @@ func (h *Handlers) deleteChildResource(
func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
return h.toggleChildResource(
"entrypointID",
- func(webhookID, childID string) error {
+ func(webhookID, childID string) (bool, error) {
var ep database.Entrypoint
err := h.db.DB().Where(
@@ -1727,14 +1722,15 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
childID, webhookID,
).First(&ep).Error
if err != nil {
- return err
+ return false, err
}
ep.Active = !ep.Active
- return h.db.DB().Save(&ep).Error
+ return ep.Active, h.db.DB().Save(&ep).Error
},
"failed to toggle entrypoint",
+ entrypointActivated, entrypointDeactivated,
)
}
@@ -1742,7 +1738,7 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
return h.toggleChildResource(
"targetID",
- func(webhookID, childID string) error {
+ func(webhookID, childID string) (bool, error) {
var tgt database.Target
err := h.db.DB().Where(
@@ -1750,23 +1746,27 @@ func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
childID, webhookID,
).First(&tgt).Error
if err != nil {
- return err
+ return false, err
}
tgt.Active = !tgt.Active
- return h.db.DB().Save(&tgt).Error
+ return tgt.Active, h.db.DB().Save(&tgt).Error
},
"failed to toggle target",
+ targetActivated, targetDeactivated,
)
}
// toggleChildResource returns a handler that toggles the active
-// state of a child resource belonging to a webhook.
+// state of a child resource belonging to a webhook. toggleFn returns
+// the new state, and the redirect carries activated or deactivated as
+// its notice to match.
func (h *Handlers) toggleChildResource(
idParam string,
- toggleFn func(webhookID, childID string) error,
+ toggleFn func(webhookID, childID string) (bool, error),
errMsg string,
+ activated, deactivated noticeCode,
) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
userID, ok := h.getUserID(r)
@@ -1792,16 +1792,21 @@ func (h *Handlers) toggleChildResource(
return
}
- err = toggleFn(webhook.ID, childID)
+ active, err := toggleFn(webhook.ID, childID)
if err != nil {
h.serverError(w, r, errMsg, err)
return
}
+ done := deactivated
+ if active {
+ done = activated
+ }
+
http.Redirect(
w, r,
- "/hook/"+webhook.ID,
+ withNotice("/hook/"+webhook.ID, done),
http.StatusSeeOther,
)
}
diff --git a/internal/handlers/target_edit.go b/internal/handlers/target_edit.go
index 810b801..d3264e3 100644
--- a/internal/handlers/target_edit.go
+++ b/internal/handlers/target_edit.go
@@ -161,7 +161,8 @@ func (h *Handlers) applyTargetEdit(
}
http.Redirect(
- w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
+ w, r, withNotice("/hook/"+webhook.ID, targetSaved),
+ http.StatusSeeOther,
)
}
diff --git a/internal/server/error_page_test.go b/internal/server/error_page_test.go
index b907de8..06abf2e 100644
--- a/internal/server/error_page_test.go
+++ b/internal/server/error_page_test.go
@@ -83,6 +83,22 @@ func TestErrorPage_DeletedTarget(t *testing.T) {
assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
}
+// TestErrorPage_ShowsNoNotice pins that a notice code in the URL of a
+// page that fails is not shown above the error.
+func TestErrorPage_ShowsNoNotice(t *testing.T) {
+ t.Parallel()
+
+ env := newTestEnv(t)
+
+ userID, _ := env.seedUser(t, "owner", "somepassword")
+ cookies := env.authCookies(t, userID, "owner")
+
+ w := env.get("/hook/no-such-webhook?notice=webhook-saved", cookies)
+
+ assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
+ assert.NotContains(t, w.Body.String(), "Webhook saved.")
+}
+
func TestErrorPage_UnknownPath(t *testing.T) {
t.Parallel()
diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go
index e9766a7..0c12126 100644
--- a/internal/server/routes_test.go
+++ b/internal/server/routes_test.go
@@ -263,6 +263,24 @@ func (e *testEnv) urlFrom(
return html.UnescapeString(match[1])
}
+// requireNotice requires w to redirect to dest carrying the notice
+// code, then renders that page and requires it to show text.
+func (e *testEnv) requireNotice(
+ t *testing.T,
+ w *httptest.ResponseRecorder,
+ dest, code, text string,
+ cookies []*http.Cookie,
+) {
+ t.Helper()
+
+ require.Equal(t, http.StatusSeeOther, w.Code)
+ require.Equal(t, dest+"?notice="+code, w.Header().Get("Location"))
+
+ page := e.get(w.Header().Get("Location"), cookies)
+ require.Equal(t, http.StatusOK, page.Code)
+ assert.Contains(t, page.Body.String(), text)
+}
+
// authCookies forges an authenticated session for the given user.
func (e *testEnv) authCookies(
t *testing.T,
@@ -741,6 +759,31 @@ func TestPagesLogin_ReturnsToTheRequestedPage(t *testing.T) {
assert.Equal(t, asked, w.Header().Get("Location"))
}
+// TestPagesLogout_SaysSignedOut signs out with the navbar's form and
+// lands on the sign-in page, which says so.
+func TestPagesLogout_SaysSignedOut(t *testing.T) {
+ t.Parallel()
+
+ env := newTestEnv(t)
+
+ userID, _ := env.seedUser(t, "leaver", "somepassword")
+ token, cookies := env.csrfFrom(
+ t, "/hooks", env.authCookies(t, userID, "leaver"),
+ )
+
+ form := url.Values{}
+ form.Set("csrf_token", token)
+
+ w := env.post(
+ env.urlFrom(t, "/hooks", `action="(/pages/logout)"`, cookies),
+ form, cookies,
+ )
+
+ // The sign-in page is requested without the session cookie, which
+ // the logout told the browser to delete.
+ env.requireNotice(t, w, "/pages/login", "signed-out", "Signed out.", nil)
+}
+
// --- /user/{username} group ---
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -858,9 +901,9 @@ func TestHooks_ListAndNewWebhookForm(t *testing.T) {
require.NoError(t,
env.db.DB().Where("name = ?", "created").First(&created).Error,
)
- assert.Equal(
- t, "/hook/"+created.ID, w.Header().Get("Location"),
- "creating a webhook should redirect to its page",
+ env.requireNotice(
+ t, w, "/hook/"+created.ID, "webhook-created", "Webhook created.",
+ cookies,
)
}
@@ -891,8 +934,7 @@ func TestHook_EditFormAndDelete(t *testing.T) {
env.urlFrom(t, editPage, `action="(/hook/[^/"]+/edit)"`, cookies),
form, cookies,
)
- require.Equal(t, http.StatusSeeOther, w.Code)
- assert.Equal(t, page, w.Header().Get("Location"))
+ env.requireNotice(t, w, page, "webhook-saved", "Webhook saved.", cookies)
var edited database.Webhook
@@ -906,16 +948,17 @@ func TestHook_EditFormAndDelete(t *testing.T) {
env.urlFrom(t, page, `action="(/hook/[^/"]+/delete)"`, cookies),
form, cookies,
)
- require.Equal(t, http.StatusSeeOther, w.Code)
- assert.Equal(t, "/hooks", w.Header().Get("Location"))
+ env.requireNotice(
+ t, w, "/hooks", "webhook-deleted", "Webhook deleted.", cookies,
+ )
assert.Equal(
t, http.StatusNotFound, env.get(page, cookies).Code,
"a deleted webhook's page should be gone",
)
}
-// TestHook_EntrypointActions adds, deactivates and deletes an
-// entrypoint with the forms on the webhook page, each submitted to
+// TestHook_EntrypointActions adds, deactivates, activates and deletes
+// an entrypoint with the forms on the webhook page, each submitted to
// the action and with the token the page rendered.
func TestHook_EntrypointActions(t *testing.T) {
t.Parallel()
@@ -933,16 +976,19 @@ func TestHook_EntrypointActions(t *testing.T) {
form.Set("csrf_token", token)
// submit posts the webhook page's form whose action pattern
- // captures, and requires the redirect back to that page.
- submit := func(pattern string) {
+ // captures, and requires the redirect back to that page with the
+ // notice code, and the page to show text.
+ submit := func(pattern, code, text string) {
t.Helper()
w := env.post(env.urlFrom(t, page, pattern, cookies), form, cookies)
- require.Equal(t, http.StatusSeeOther, w.Code)
- require.Equal(t, page, w.Header().Get("Location"))
+ env.requireNotice(t, w, page, code, text, cookies)
}
- submit(`action="(/hook/[^/"]+/entrypoints)"`)
+ toggle := `action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`
+
+ submit(`action="(/hook/[^/"]+/entrypoints)"`,
+ "entrypoint-added", "Entrypoint added.")
var added database.Entrypoint
@@ -951,7 +997,7 @@ func TestHook_EntrypointActions(t *testing.T) {
)
require.True(t, added.Active)
- submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`)
+ submit(toggle, "entrypoint-deactivated", "Entrypoint deactivated.")
var toggled database.Entrypoint
@@ -960,7 +1006,10 @@ func TestHook_EntrypointActions(t *testing.T) {
)
assert.False(t, toggled.Active, "the toggle should deactivate it")
- submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`)
+ submit(toggle, "entrypoint-activated", "Entrypoint activated.")
+
+ submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`,
+ "entrypoint-deleted", "Entrypoint deleted.")
var left int64
@@ -971,8 +1020,8 @@ func TestHook_EntrypointActions(t *testing.T) {
// TestHook_TargetActions adds a target with the form on the webhook
// page, follows its Edit link to the target edit form and submits
-// it, then deactivates and deletes it, every URL and token taken from
-// the rendered pages.
+// it, then deactivates, activates and deletes it, every URL and token
+// taken from the rendered pages.
func TestHook_TargetActions(t *testing.T) {
t.Parallel()
@@ -987,27 +1036,29 @@ func TestHook_TargetActions(t *testing.T) {
// submit posts form, with the token, to the action pattern
// captures on the page at from, and requires the redirect back to
- // the webhook page.
- submit := func(from, pattern string, form url.Values) {
+ // the webhook page with the notice code, and that page to show
+ // text.
+ submit := func(from, pattern string, form url.Values, code, text string) {
t.Helper()
form.Set("csrf_token", token)
w := env.post(env.urlFrom(t, from, pattern, cookies), form, cookies)
- require.Equal(t, http.StatusSeeOther, w.Code)
- require.Equal(t, page, w.Header().Get("Location"))
+ env.requireNotice(t, w, page, code, text, cookies)
}
+ toggle := `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`
+
submit(page, `action="(/hook/[^/"]+/targets)"`, url.Values{
"name": {"added"},
"type": {string(database.TargetTypeLog)},
- })
+ }, "target-added", "Target added.")
editPage := env.urlFrom(
t, page, `href="(/hook/[^/"]+/targets/[^/"]+/edit)"`, cookies,
)
submit(editPage, `action="(/hook/[^/"]+/targets/[^/"]+/edit)"`,
- url.Values{"name": {"renamed"}})
+ url.Values{"name": {"renamed"}}, "target-saved", "Target saved.")
var edited database.Target
@@ -1017,8 +1068,8 @@ func TestHook_TargetActions(t *testing.T) {
assert.Equal(t, "renamed", edited.Name)
require.True(t, edited.Active)
- submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`,
- url.Values{})
+ submit(page, toggle, url.Values{},
+ "target-deactivated", "Target deactivated.")
var toggled database.Target
@@ -1027,8 +1078,11 @@ func TestHook_TargetActions(t *testing.T) {
)
assert.False(t, toggled.Active, "the toggle should deactivate it")
+ submit(page, toggle, url.Values{},
+ "target-activated", "Target activated.")
+
submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/delete)"`,
- url.Values{})
+ url.Values{}, "target-deleted", "Target deleted.")
var left int64
@@ -1064,9 +1118,9 @@ func TestHook_ResubmitFromEventLog(t *testing.T) {
env.urlFrom(t, logsPath, `action="(/hook/[^"]+/resubmit)"`, cookies),
form, cookies,
)
- require.Equal(t, http.StatusSeeOther, w.Code)
- assert.Equal(
- t, logsPath+"?resubmit=no-targets", w.Header().Get("Location"),
+ env.requireNotice(
+ t, w, logsPath, "resubmit-no-targets",
+ "this source has no active targets", cookies,
)
webhookDB, err := env.dbMgr.GetDB(wh.ID)
@@ -1302,10 +1356,8 @@ func TestDeliveryReplay_PostOnlyAndCSRFProtected(t *testing.T) {
html.UnescapeString(action[1]), form, cookies,
)
- require.Equal(t, http.StatusSeeOther, w.Code)
- assert.Equal(
- t, logsPath+"?replay=queued",
- w.Header().Get("Location"),
+ env.requireNotice(
+ t, w, logsPath, "replay-queued", "Replay queued:", cookies,
)
assert.Equal(
t, int64(2), env.countDeliveries(t, wh.ID),
diff --git a/templates/base.html b/templates/base.html
index b300710..a193d19 100644
--- a/templates/base.html
+++ b/templates/base.html
@@ -7,6 +7,7 @@
{{template "navbar" .}}
+ {{template "notice" .}}
{{block "content" .}}{{end}}
{{template "footer" .}}
diff --git a/templates/notice.html b/templates/notice.html
new file mode 100644
index 0000000..f8206df
--- /dev/null
+++ b/templates/notice.html
@@ -0,0 +1,7 @@
+{{define "notice"}}
+{{with .Notice}}
+
+{{end}}
+{{end}}
diff --git a/templates/source_logs.html b/templates/source_logs.html
index 91ecfcb..66a4601 100644
--- a/templates/source_logs.html
+++ b/templates/source_logs.html
@@ -12,14 +12,6 @@
- {{if .ReplayMessage}}
- {{.ReplayMessage}}
- {{end}}
-
- {{if .ResubmitMessage}}
- {{.ResubmitMessage}}
- {{end}}
-