Let an entrypoint's description be edited in place (closes #392)

An entrypoint's description was set when it was added and could never change, so renaming one meant deleting it and adding a new one with a new URL every sender had to be given again. Each entrypoint on the webhook page now has an Edit button, in the shared secondary style, that opens its description in place with Save and Cancel and keeps its URL. The save goes through the same login, CSRF and ownership checks as the other entrypoint actions; an empty description shows as "Entrypoint". Activate and deactivate now write only the active column, so they cannot undo an edit. Tests cover each, through the router and the browser.

Model: opus-5-5

internal/handlers/entrypoint_toggle_test.go

@@ -0,0 +1,95 @@
+package handlers_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/go-chi/chi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// TestHandleEntrypointToggle_DoesNotUndoAnEdit proves that a toggle
+// which loaded the entrypoint before an edit of its description was
+// saved does not write the old description back over the edit. The
+// edit is submitted from a callback on the toggle's own read of the
+// entrypoint, so it is saved after that read and before the toggle
+// writes.
+func TestHandleEntrypointToggle_DoesNotUndoAnEdit(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	wh := seedWebhookWithRetention(t, env.db, 30)
+	ep := seedEntrypoint(t, env.db, wh.ID)
+	require.True(t, ep.Active)
+
+	router := chi.NewRouter()
+	router.Post(
+		"/hook/{sourceID}/entrypoints/{entrypointID}/edit",
+		env.handlers.HandleEntrypointEdit(),
+	)
+	router.Post(
+		"/hook/{sourceID}/entrypoints/{entrypointID}/toggle",
+		env.handlers.HandleEntrypointToggle(),
+	)
+
+	// post submits one of the entrypoint's forms as the test user and
+	// returns the response's status code.
+	post := func(action string, form url.Values) int {
+		req := httptest.NewRequestWithContext(
+			context.Background(), http.MethodPost,
+			"/hook/"+wh.ID+"/entrypoints/"+ep.ID+"/"+action,
+			strings.NewReader(form.Encode()),
+		)
+		req.Header.Set(
+			"Content-Type", "application/x-www-form-urlencoded",
+		)
+
+		for _, c := range env.cookies {
+			req.AddCookie(c)
+		}
+
+		w := httptest.NewRecorder()
+		router.ServeHTTP(w, req)
+
+		return w.Code
+	}
+
+	var (
+		edited   bool
+		editCode int
+	)
+
+	require.NoError(t, env.db.DB().Callback().Query().
+		After("gorm:query").
+		Register("test:edit_after_toggle_read", func(tx *gorm.DB) {
+			// Only the first read of an entrypoint, the toggle's,
+			// submits the edit.
+			if tx.Statement.Table != "entrypoints" || edited {
+				return
+			}
+
+			edited = true
+			editCode = post(
+				"edit", url.Values{"description": {"Billing sender"}},
+			)
+		}),
+	)
+
+	require.Equal(t, http.StatusSeeOther, post("toggle", nil))
+	require.Equal(t, http.StatusSeeOther, editCode)
+
+	var stored database.Entrypoint
+
+	require.NoError(
+		t, env.db.DB().First(&stored, "id = ?", ep.ID).Error,
+	)
+	assert.False(t, stored.Active)
+	assert.Equal(t, "Billing sender", stored.Description)
+}

internal/handlers/notice.go

@@ -20,6 +20,7 @@ const (
 	webhookSaved          noticeCode = "webhook-saved"
 	webhookDeleted        noticeCode = "webhook-deleted"
 	entrypointAdded       noticeCode = "entrypoint-added"
+	entrypointSaved       noticeCode = "entrypoint-saved"
 	entrypointDeleted     noticeCode = "entrypoint-deleted"
 	entrypointActivated   noticeCode = "entrypoint-activated"
 	entrypointDeactivated noticeCode = "entrypoint-deactivated"
@@ -48,6 +49,7 @@ func noticeFor(r *http.Request) *notice {
 		webhookSaved:          {Text: "Webhook saved."},
 		webhookDeleted:        {Text: "Webhook deleted."},
 		entrypointAdded:       {Text: "Entrypoint added."},
+		entrypointSaved:       {Text: "Entrypoint description saved."},
 		entrypointDeleted:     {Text: "Entrypoint deleted."},
 		entrypointActivated:   {Text: "Entrypoint activated."},
 		entrypointDeactivated: {Text: "Entrypoint deactivated."},

internal/handlers/source_delete_test.go

@@ -86,12 +86,13 @@ var errInjectedDelete = errors.New("injected delete failure")
 // save of an existing row.
 var errInjectedSave = errors.New("injected save failure")
 
-// seedEntrypoint inserts an entrypoint for a webhook.
+// seedEntrypoint inserts an active entrypoint for a webhook and
+// returns it.
 func seedEntrypoint(
 	t *testing.T,
 	db *database.Database,
 	webhookID string,
-) {
+) *database.Entrypoint {
 	t.Helper()
 
 	ep := &database.Entrypoint{
@@ -104,6 +105,8 @@ func seedEntrypoint(
 		t,
 		db.DB().Omit(clause.Associations).Create(ep).Error,
 	)
+
+	return ep
 }
 
 // countRows counts the live (not soft-deleted) rows of a model

internal/handlers/source_management.go

@@ -1396,6 +1396,70 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 	}
 }
 
+// HandleEntrypointEdit handles changing an entrypoint's description.
+// It writes only the description column, so the entrypoint keeps its
+// URL, and an activate or deactivate saved since the page was shown
+// is not undone.
+func (h *Handlers) HandleEntrypointEdit() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		userID, ok := h.getUserID(r)
+		if !ok {
+			http.Redirect(
+				w, r, "/pages/login", http.StatusSeeOther,
+			)
+
+			return
+		}
+
+		sourceID := chi.URLParam(r, "sourceID")
+		entrypointID := chi.URLParam(r, "entrypointID")
+
+		var webhook database.Webhook
+
+		err := h.db.DB().Where(
+			"id = ? AND user_id = ?", sourceID, userID,
+		).First(&webhook).Error
+		if err != nil {
+			h.renderError(w, r, http.StatusNotFound)
+
+			return
+		}
+
+		// The body size cap is enforced by the MaxBodySize
+		// middleware, which runs before CSRF parses the form.
+		err = r.ParseForm()
+		if err != nil {
+			h.renderError(w, r, http.StatusBadRequest)
+
+			return
+		}
+
+		result := h.db.DB().Model(&database.Entrypoint{}).Where(
+			"id = ? AND webhook_id = ?", entrypointID, webhook.ID,
+		).Update("description", r.PostFormValue("description"))
+		if result.Error != nil {
+			h.serverError(
+				w, r, "failed to edit entrypoint", result.Error,
+			)
+
+			return
+		}
+
+		// The id came from the URL and may name another webhook's
+		// entrypoint, which this webhook does not have.
+		if result.RowsAffected == 0 {
+			h.renderError(w, r, http.StatusNotFound)
+
+			return
+		}
+
+		http.Redirect(
+			w, r, withNotice("/hook/"+webhook.ID, entrypointSaved),
+			http.StatusSeeOther,
+		)
+	}
+}
+
 // HandleTargetCreate handles adding a new target to a webhook.
 func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -1877,9 +1941,13 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 				return false, err
 			}
 
-			ep.Active = !ep.Active
+			// Only the active column: saving the whole row would
+			// write back the description read above over an edit
+			// saved since.
+			active := !ep.Active
 
-			return ep.Active, h.db.DB().Save(&ep).Error
+			return active, h.db.DB().Model(&ep).
+				Update("active", active).Error
 		},
 		"failed to toggle entrypoint",
 		entrypointActivated, entrypointDeactivated,

internal/server/alpine_browser_test.go

@@ -86,6 +86,7 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
 	checkAddForms(ctx, t, page)
 	checkTargetType(ctx, t, page+"/events")
 	checkCopy(ctx, t, page)
+	checkEntrypointEdit(ctx, t, page, page+"/events")
 	checkEventLog(ctx, t, page+"/events", event.ID, target.Name)
 	checkMobileMenu(ctx, t, page)
 
@@ -363,6 +364,89 @@ func checkCopy(ctx context.Context, t *testing.T, url string) {
 		`clicking Copy does not show "Copied"`)
 }
 
+// checkEntrypointEdit loads a webhook page whose entrypoint has no
+// description, and checks that Edit shows the edit form in place of
+// the description and hides until the form closes, so the form always
+// opens on the saved description; that Cancel hides it and drops what
+// was typed; that after typing, opening the page at elsewhere and going
+// back, Edit again opens the form on the saved description; and that
+// Save changes the description the page shows.
+func checkEntrypointEdit(
+	ctx context.Context, t *testing.T, url, elsewhere string,
+) {
+	t.Helper()
+
+	const (
+		editForm    = `form[action$="/edit"]`
+		input       = editForm + ` input[name="description"]`
+		description = `//span[text()="Entrypoint"]`
+		edit        = `//button[text()="Edit"]`
+	)
+
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+
+	assert.True(t, hidden(ctx, editForm),
+		"the edit form shows before Edit is clicked")
+
+	click(ctx, t, edit)
+	assert.True(t, shown(ctx, editForm),
+		"clicking Edit does not show the edit form")
+	assert.True(t, hidden(ctx, description),
+		"the description stays shown beside the edit form")
+	assert.True(t, hidden(ctx, edit),
+		"Edit stays shown while the edit form is open")
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.SendKeys(input, "draft", chromedp.ByQuery),
+	))
+	click(ctx, t, `//button[text()="Cancel"]`)
+	assert.True(t, hidden(ctx, editForm),
+		"clicking Cancel does not hide the edit form")
+	assert.True(t, shown(ctx, description),
+		"clicking Cancel does not show the description again")
+	assert.True(t, shown(ctx, edit),
+		"clicking Cancel does not show Edit again")
+
+	var typed string
+
+	click(ctx, t, edit)
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Value(input, &typed, chromedp.ByQuery),
+	))
+	assert.Empty(t, typed, "Cancel keeps what was typed")
+
+	var loaded string
+
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.SendKeys(input, "draft", chromedp.ByQuery),
+		loadPage(elsewhere),
+		chromedp.NavigateBack(),
+		chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
+		chromedp.Evaluate(
+			`performance.getEntriesByType("navigation")[0].type`, &loaded,
+		),
+	))
+	require.Equal(
+		t, "back_forward", loaded,
+		"going back, the browser did not load the page again",
+	)
+
+	click(ctx, t, edit)
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Value(input, &typed, chromedp.ByQuery),
+	))
+	assert.Empty(t, typed, "going back puts what was typed back in the form")
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.SendKeys(input, "Billing sender", chromedp.ByQuery),
+	))
+	click(ctx, t, `//button[text()="Save"]`)
+
+	assert.True(t, shown(ctx, `//span[text()="Billing sender"]`),
+		"saving the edit form does not change the description")
+}
+
 // checkEventLog loads the event log and checks that clicking an event's
 // row expands it, that in there clicking its delivery shows the
 // delivery's attempts and clicking again hides them, and that clicking

internal/server/routes.go

@@ -289,6 +289,10 @@ func (s *Server) setupSourceRoutes() {
 			"/entrypoints",
 			s.h.HandleEntrypointCreate(),
 		)
+		r.Post(
+			"/entrypoints/{entrypointID}/edit",
+			s.h.HandleEntrypointEdit(),
+		)
 		r.Post(
 			"/entrypoints/{entrypointID}/delete",
 			s.h.HandleEntrypointDelete(),

internal/server/routes_test.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/fx"
@@ -398,6 +399,44 @@ func (e *testEnv) seedTarget(
 	return tgt
 }
 
+// seedEntrypoint creates an active entrypoint for a webhook.
+func (e *testEnv) seedEntrypoint(
+	t *testing.T,
+	webhookID string,
+) *database.Entrypoint {
+	t.Helper()
+
+	ep := &database.Entrypoint{
+		WebhookID:   webhookID,
+		Path:        uuid.New().String(),
+		Description: "Default entrypoint",
+		Active:      true,
+	}
+
+	require.NoError(
+		t,
+		e.db.DB().Omit(clause.Associations).Create(ep).Error,
+	)
+
+	return ep
+}
+
+// storedEntrypoint reloads an entrypoint row.
+func (e *testEnv) storedEntrypoint(
+	t *testing.T,
+	entrypointID string,
+) database.Entrypoint {
+	t.Helper()
+
+	var ep database.Entrypoint
+
+	require.NoError(
+		t, e.db.DB().First(&ep, "id = ?", entrypointID).Error,
+	)
+
+	return ep
+}
+
 // seedFailedDelivery records a terminally failed delivery of an event
 // to a target in the webhook's own database.
 func (e *testEnv) seedFailedDelivery(
@@ -1087,6 +1126,119 @@ func TestHook_EntrypointActions(t *testing.T) {
 	assert.Zero(t, left, "the delete should remove the entrypoint")
 }
 
+// TestHook_EntrypointEdit changes an entrypoint's description with the
+// edit form on the webhook page, then empties it. The entrypoint keeps
+// its URL, and with no description it shows as "Entrypoint". Without
+// the CSRF token, or without a session, the edit is refused.
+func TestHook_EntrypointEdit(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "epeditor", "somepassword")
+	cookies := env.authCookies(t, userID, "epeditor")
+	wh := env.seedWebhook(t, userID)
+	ep := env.seedEntrypoint(t, wh.ID)
+	page := "/hook/" + wh.ID
+
+	token, cookies := env.csrfFrom(t, page, cookies)
+	action := env.urlFrom(
+		t, page, `action="(/hook/[^/"]+/entrypoints/[^/"]+/edit)"`,
+		cookies,
+	)
+
+	assert.Equal(
+		t, http.StatusForbidden,
+		env.post(action, entrypointEditForm("", "no token"), cookies).Code,
+		"an edit without a CSRF token must be refused",
+	)
+
+	// The request without a session carries a valid CSRF token from
+	// the login page, so only the session check can refuse it.
+	anonToken, anon := env.csrfFrom(t, "/pages/login", nil)
+	refused := env.post(
+		action, entrypointEditForm(anonToken, "no session"), anon,
+	)
+	assert.Equal(t, http.StatusSeeOther, refused.Code)
+	assert.Equal(
+		t, "/pages/login", refused.Header().Get("Location"),
+		"an edit without a session must be refused",
+	)
+
+	assert.Equal(
+		t, ep.Description, env.storedEntrypoint(t, ep.ID).Description,
+		"a refused edit must not change the description",
+	)
+
+	// edit submits the form with description and requires the
+	// redirect back to the webhook page with the notice.
+	edit := func(description string) {
+		t.Helper()
+
+		w := env.post(
+			action, entrypointEditForm(token, description), cookies,
+		)
+		env.requireNotice(t, w, page, "entrypoint-saved",
+			"Entrypoint description saved.", cookies)
+	}
+
+	edit("Billing sender")
+
+	stored := env.storedEntrypoint(t, ep.ID)
+	assert.Equal(t, "Billing sender", stored.Description)
+	assert.Equal(t, ep.Path, stored.Path,
+		"the edit must keep the entrypoint's URL")
+
+	body := env.get(page, cookies).Body.String()
+	assert.Contains(t, body, ">Billing sender</span>")
+	assert.Contains(t, body, "/h/"+ep.Path+"</code>")
+
+	edit("")
+
+	assert.Empty(t, env.storedEntrypoint(t, ep.ID).Description)
+	assert.Contains(t, env.get(page, cookies).Body.String(),
+		">Entrypoint</span>", "no description shows as Entrypoint")
+}
+
+// TestHook_EntrypointEdit_OtherUser404s has another logged-in user, with
+// a CSRF token of their own, try to edit an entrypoint: through the
+// owner's webhook, and through a webhook of their own. Both are 404s
+// and the description stays as it was.
+func TestHook_EntrypointEdit_OtherUser404s(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	ownerID, _ := env.seedUser(t, "epowner", "somepassword")
+	wh := env.seedWebhook(t, ownerID)
+	ep := env.seedEntrypoint(t, wh.ID)
+
+	otherID, _ := env.seedUser(t, "epother", "somepassword")
+	other := env.authCookies(t, otherID, "epother")
+	theirs := env.seedWebhook(t, otherID)
+	token, other := env.csrfFrom(t, "/hook/"+theirs.ID, other)
+
+	for _, webhookID := range []string{wh.ID, theirs.ID} {
+		path := "/hook/" + webhookID + "/entrypoints/" + ep.ID + "/edit"
+
+		w := env.post(path, entrypointEditForm(token, "not theirs"), other)
+		assert.Equal(t, http.StatusNotFound, w.Code, path)
+	}
+
+	assert.Equal(
+		t, ep.Description, env.storedEntrypoint(t, ep.ID).Description,
+		"another user must not change the description",
+	)
+}
+
+// entrypointEditForm fills in the webhook page's entrypoint edit form.
+func entrypointEditForm(token, description string) url.Values {
+	return url.Values{
+		"csrf_token":  {token},
+		"description": {description},
+	}
+}
+
 // 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, activates and deletes it, every URL and token