Targets section: one Add, then a target type and Next, then that type's fields (closes #370)

The targets section of the webhook page showed its add form open with every field, a URL included for types that use none. It now lists only its targets until "+ Add" is clicked; "+ Add" shows a choice of target type with Next and Cancel on one row, and Next shows the name and only that type's fields. The database and log types show no URL field and the server stores none for them; the slack form gains its retry field. A refused target brings the page back with the form open on its type, the values entered and the reason, and Cancel empties it. An encoding failure stays a logged 500. Target validation returns its message, so the new-webhook page can reuse it. Model: opus-5-5
2026-10-02 22:24:38 +02:00
parent 719d7013ee
commit da75950e91
11 changed files with 701 additions and 379 deletions
@@ -87,8 +87,37 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {

 	page := srv.URL + "/hook/" + webhook.ID

-	checkAddForms(ctx, t, page)
-	checkTargetType(ctx, t, page+"/events")
+	checkAddEntrypoint(ctx, t, page)
+
+	// Each target type, with the fields its add target form submits, in
+	// page order. Only http and slack have a url field.
+	targetTypes := []struct {
+		name   string
+		fields string
+		values map[string]string
+	}{
+		{
+			"http", "csrf_token name type url headers timeout max_retries",
+			map[string]string{"url": publicTargetURL},
+		},
+		{
+			"slack", "csrf_token name type url max_retries",
+			map[string]string{"url": publicTargetURL},
+		},
+		{
+			"database", "csrf_token name type expiry",
+			map[string]string{"expiry": "720h"},
+		},
+		{"log", "csrf_token name type", nil},
+	}
+
+	for _, tt := range targetTypes {
+		checkAddTarget(
+			ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values,
+		)
+	}
+
+	checkRefusedTarget(ctx, t, page)
 	checkCopy(ctx, t, page)
 	checkEntrypointEdit(ctx, t, page, page+"/events")
 	checkRecentEvents(ctx, t, page)
@@ -233,115 +262,194 @@ func click(ctx context.Context, t *testing.T, xpath string) {
 	))
 }

-// checkAddForms loads a webhook page and checks that each section's add
-// form stays hidden until the Add button beside its heading is clicked.
-// The click looks for a button element there, so it also checks that
-// Add is one.
-func checkAddForms(ctx context.Context, t *testing.T, url string) {
+// checkAddEntrypoint loads a webhook page and checks that the add
+// entrypoint form stays hidden until the Add button beside its heading
+// is clicked. The click looks for a button element there, so it also
+// checks that Add is one.
+func checkAddEntrypoint(ctx context.Context, t *testing.T, url string) {
+	t.Helper()
+
+	form := `form[action$="/entrypoints"]`
+
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+
+	assert.True(t, hidden(ctx, form),
+		"the add entrypoint form shows before Add is clicked")
+
+	click(ctx, t, `//h2[text()="Entrypoints"]/following-sibling::button`)
+
+	assert.True(t, shown(ctx, form),
+		"the add entrypoint form stays hidden when Add is clicked")
+}
+
+// publicTargetURL is a destination the server accepts for an http or
+// slack target. It is a literal public address, so accepting it needs
+// no DNS.
+const publicTargetURL = "https://93.184.216.34/hook"
+
+// The parts of the targets section's add target form the checks below
+// find and click. Add is the button beside the Targets heading; each
+// Cancel is found from the button beside it, since both are on the
+// page at once.
+const (
+	addTarget     = `//h2[text()="Targets"]/following-sibling::button`
+	typeSelect    = `//select[@aria-label="Target type"]`
+	nextButton    = `//button[text()="Next"]`
+	cancelChoice  = nextButton + `/following-sibling::button[text()="Cancel"]`
+	saveButton    = `//form[contains(@action, "/targets")]//button[text()="Save"]`
+	cancelFields  = saveButton + `/following-sibling::button[text()="Cancel"]`
+	targetName    = `form[action$="/targets"] input[name="name"]`
+	submittedKeys = `[...new FormData(
+		document.querySelector('form[action$="/targets"]')).keys()]`
+)
+
+// checkAddTarget loads a webhook page and walks the add target form for
+// one target type. The form shows nothing until Add is clicked; Add
+// shows only the type choice; Cancel there closes it; Next shows the
+// type's own fields in place of the choice, and the form then submits
+// exactly fields, so a field another type uses, such as url, is absent;
+// Cancel closes it again. It then adds a target of the type, filling in
+// values, and checks that the section lists it with that type.
+func checkAddTarget(
+	ctx context.Context,
+	t *testing.T,
+	url, targetType string,
+	fields []string,
+	values map[string]string,
+) {
 	t.Helper()

 	require.NoError(t, chromedp.Run(ctx, loadPage(url)))

-	sections := []struct{ heading, form string }{
-		{"Entrypoints", `form[action$="/entrypoints"]`},
-		{"Targets", `form[action$="/targets"]`},
+	assert.Truef(t, hidden(ctx, typeSelect),
+		"%s: the type choice shows before Add is clicked", targetType)
+	assert.Truef(t, hidden(ctx, targetName),
+		"%s: the fields show before Add is clicked", targetType)
+
+	click(ctx, t, addTarget)
+	assert.Truef(t, shown(ctx, typeSelect),
+		"%s: Add does not show the type choice", targetType)
+	assert.Truef(t, hidden(ctx, targetName),
+		"%s: Add shows the fields before Next", targetType)
+
+	click(ctx, t, cancelChoice)
+	assert.Truef(t, hidden(ctx, typeSelect),
+		"%s: Cancel does not close the type choice", targetType)
+
+	chooseTargetType(ctx, t, targetType)
+
+	var submitted []string
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Evaluate(submittedKeys, &submitted),
+	))
+	assert.Equalf(t, fields, submitted,
+		"%s: the form does not submit exactly the type's fields", targetType)
+
+	click(ctx, t, cancelFields)
+	assert.Truef(t, hidden(ctx, targetName),
+		"%s: Cancel does not close the fields", targetType)
+	assert.Truef(t, shown(ctx, addTarget),
+		"%s: Add does not come back after Cancel", targetType)
+
+	name := "added-" + targetType
+
+	chooseTargetType(ctx, t, targetType)
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.SetValue(targetName, name, chromedp.ByQuery),
+	))
+
+	for field, value := range values {
+		require.NoError(t, chromedp.Run(ctx, chromedp.SetValue(
+			`form[action$="/targets"] [name="`+field+`"]`, value,
+			chromedp.ByQuery,
+		)))
 	}

-	for _, s := range sections {
-		assert.Truef(
-			t, hidden(ctx, s.form),
-			"%s: the add form shows before Add is clicked", s.heading,
-		)
-
-		click(ctx, t, `//h2[text()="`+s.heading+
-			`"]/following-sibling::button`)
-
-		assert.Truef(
-			t, shown(ctx, s.form),
-			"%s: the add form stays hidden when Add is clicked", s.heading,
-		)
-	}
+	click(ctx, t, saveButton)
+	assert.Truef(t, shown(ctx, `//span[text()="`+name+
+		`"]/following-sibling::div/span[text()="`+targetType+`"]`),
+		"%s: the added target is not listed with its type", targetType)
 }

-// checkTargetType chooses Slack in the open add target form and checks
-// what the form would then submit: one url field, the Slack one, and
-// not the HTTP url, headers or timeout, which are hidden and disabled.
-//
-// It then opens the page at elsewhere and goes back. The browser loads
-// the webhook page again and restores the form as it was left, Slack
-// chosen, without a change event; the form must again show and submit
-// Slack's fields, not the HTTP ones.
-func checkTargetType(ctx context.Context, t *testing.T, elsewhere string) {
+// chooseTargetType clicks Add, picks targetType and clicks Next, and
+// checks that the type's fields then show in place of the type choice.
+func chooseTargetType(ctx context.Context, t *testing.T, targetType string) {
+	t.Helper()
+
+	click(ctx, t, addTarget)
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.SetValue(typeSelect, targetType, chromedp.BySearch),
+	))
+	click(ctx, t, nextButton)
+
+	assert.Truef(t, shown(ctx, targetName),
+		"%s: Next does not show the fields", targetType)
+	assert.Truef(t, hidden(ctx, typeSelect),
+		"%s: Next leaves the type choice showing", targetType)
+	assert.Truef(t, hidden(ctx, addTarget),
+		"%s: Add still shows while the form is open", targetType)
+}
+
+// checkRefusedTarget submits an http target the server refuses, a
+// loopback destination, and checks that the page comes back with the
+// form open on the http fields, the values entered and the reason, and
+// that after Cancel the next Add starts with an empty form and no
+// reason.
+func checkRefusedTarget(ctx context.Context, t *testing.T, url string) {
 	t.Helper()

 	const (
-		chooseSlack = `(() => {
-			const type = document.querySelector('select[name="type"]');
-			type.value = "slack";
-			type.dispatchEvent(new Event("change"));
-		})()`
-		chosen    = `document.querySelector('select[name="type"]').value`
-		howLoaded = `performance.getEntriesByType("navigation")[0].type`
-		submitted = `[...new FormData(
-			document.querySelector('form[action$="/targets"]')).keys()]`
-		slackURL = `input[placeholder^="https://hooks.slack.com/"]`
-		httpURL  = `input[placeholder="https://example.com/webhook"]`
+		refusedURL = "http://127.0.0.1/hook"
+		urlField   = `form[action$="/targets"] input[name="url"]`
+		reason     = `//div[@class="alert-error"]`
 	)

-	slackFields := strings.Fields("csrf_token name type max_retries url")
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))

-	var fields []string
+	chooseTargetType(ctx, t, "http")
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.SetValue(targetName, "refused", chromedp.ByQuery),
+		chromedp.SetValue(urlField, refusedURL, chromedp.ByQuery),
+	))
+
+	click(ctx, t, saveButton)
+
+	assert.True(t, shown(ctx, reason),
+		"a refused target does not show the reason")
+
+	var name, typed string

 	require.NoError(t, chromedp.Run(
 		ctx,
-		chromedp.Evaluate(chooseSlack, nil),
-		chromedp.Evaluate(submitted, &fields),
+		chromedp.Value(targetName, &name, chromedp.ByQuery),
+		chromedp.Value(urlField, &typed, chromedp.ByQuery),
 	))

-	assert.Equal(
-		t, slackFields, fields,
-		"with Slack chosen, the HTTP fields must not be submitted",
-	)
+	assert.Equal(t, "refused", name,
+		"a refused target does not keep the name entered")
+	assert.Equal(t, refusedURL, typed,
+		"a refused target does not keep the url entered")
+	assert.True(t, shown(ctx, targetName),
+		"a refused target does not come back with the form open")
+	assert.True(t, hidden(ctx, typeSelect),
+		"a refused target comes back on the type choice")

-	var loaded, restored string
+	click(ctx, t, cancelFields)
+	chooseTargetType(ctx, t, "http")
+
+	assert.True(t, hidden(ctx, reason),
+		"after Cancel, the next Add still shows the reason")

-	// Going back waits for the load event, after which the browser has
-	// restored the form.
 	require.NoError(t, chromedp.Run(
 		ctx,
-		loadPage(elsewhere),
-		chromedp.NavigateBack(),
-		chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
-		chromedp.Evaluate(howLoaded, &loaded),
-		chromedp.Evaluate(chosen, &restored),
+		chromedp.Value(targetName, &name, chromedp.ByQuery),
+		chromedp.Value(urlField, &typed, chromedp.ByQuery),
 	))

-	// A page the browser kept in memory and showed again as it was
-	// would prove nothing here.
-	require.Equal(
-		t, "back_forward", loaded,
-		"going back, the browser did not load the page again",
-	)
-	require.Equal(
-		t, "slack", restored,
-		"going back, the browser did not restore the chosen type",
-	)
-
-	click(ctx, t, `//h2[text()="Targets"]/following-sibling::button`)
-
-	assert.True(t, shown(ctx, slackURL),
-		"going back with Slack chosen, the Slack fields are not shown")
-	assert.True(t, hidden(ctx, httpURL),
-		"going back with Slack chosen, the HTTP fields are shown")
-
-	require.NoError(t, chromedp.Run(
-		ctx, chromedp.Evaluate(submitted, &fields),
-	))
-
-	assert.Equal(
-		t, slackFields, fields,
-		"going back with Slack chosen, the HTTP fields must not be submitted",
-	)
+	assert.Empty(t, name, "after Cancel, the next Add keeps the name entered")
+	assert.Empty(t, typed, "after Cancel, the next Add keeps the url entered")
 }

 // checkCopy loads a webhook page and checks that the Copy control beside
@@ -381,9 +489,13 @@ func checkEntrypointEdit(
 ) {
 	t.Helper()

+	// Cancel and Save are found inside the edit form, since the add
+	// target form has buttons of the same names.
 	const (
 		editForm    = `form[action$="/edit"]`
 		input       = editForm + ` input[name="description"]`
+		cancelEdit  = `//form[contains(@action, "/edit")]/button[text()="Cancel"]`
+		saveEdit    = `//form[contains(@action, "/edit")]/button[text()="Save"]`
 		description = `//span[text()="Entrypoint"]`
 		edit        = `//button[text()="Edit"]`
 	)
@@ -404,7 +516,7 @@ func checkEntrypointEdit(
 	require.NoError(t, chromedp.Run(
 		ctx, chromedp.SendKeys(input, "draft", chromedp.ByQuery),
 	))
-	click(ctx, t, `//button[text()="Cancel"]`)
+	click(ctx, t, cancelEdit)
 	assert.True(t, hidden(ctx, editForm),
 		"clicking Cancel does not hide the edit form")
 	assert.True(t, shown(ctx, description),
@@ -446,7 +558,7 @@ func checkEntrypointEdit(
 	require.NoError(t, chromedp.Run(
 		ctx, chromedp.SendKeys(input, "Billing sender", chromedp.ByQuery),
 	))
-	click(ctx, t, `//button[text()="Save"]`)
+	click(ctx, t, saveEdit)

 	assert.True(t, shown(ctx, `//span[text()="Billing sender"]`),
 		"saving the edit form does not change the description")