Merge pull request '[REFACTOR] webhook endpoint logic cleanup' (#2847) from oliverpool/forgejo:webhook_4_cleanup_endpoints into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2847
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

modules/setting/webhook.go

@@ -15,7 +15,6 @@ var Webhook = struct {
 	DeliverTimeout  int
 	SkipTLSVerify   bool
 	AllowedHostList string
-	Types           []string
 	PagingNum       int
 	ProxyURL        string
 	ProxyURLFixed   *url.URL
@@ -35,7 +34,6 @@ func loadWebhookFrom(rootCfg ConfigProvider) {
 	Webhook.DeliverTimeout = sec.Key("DELIVER_TIMEOUT").MustInt(5)
 	Webhook.SkipTLSVerify = sec.Key("SKIP_TLS_VERIFY").MustBool()
 	Webhook.AllowedHostList = sec.Key("ALLOWED_HOST_LIST").MustString("")
-	Webhook.Types = []string{"forgejo", "gitea", "gogs", "slack", "discord", "dingtalk", "telegram", "msteams", "feishu", "matrix", "wechatwork", "packagist"}
 	Webhook.PagingNum = sec.Key("PAGING_NUM").MustInt(10)
 	Webhook.ProxyURL = sec.Key("PROXY_URL").MustString("")
 	if Webhook.ProxyURL != "" {

routers/web/repo/setting/webhook.go

@@ -10,7 +10,6 @@ import (
 	"net/http"
 	"net/url"
 	"path"
-	"strings"
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/perm"
@@ -22,7 +21,6 @@ import (
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web/middleware"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 	"code.gitea.io/gitea/services/context"
@@ -41,8 +39,8 @@ const (
 	tplAdminHookNew base.TplName = "admin/hook_new"
 )
 
-// Webhooks render web hooks list page
-func Webhooks(ctx *context.Context) {
+// WebhookList render web hooks list page
+func WebhookList(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.hooks")
 	ctx.Data["PageIsSettingsHooks"] = true
 	ctx.Data["BaseLink"] = ctx.Repo.RepoLink + "/settings/hooks"
@@ -111,17 +109,8 @@ func getOwnerRepoCtx(ctx *context.Context) (*ownerRepoCtx, error) {
 	return nil, errors.New("unable to set OwnerRepo context")
 }
 
-func checkHookType(ctx *context.Context) string {
-	hookType := strings.ToLower(ctx.Params(":type"))
-	if !util.SliceContainsString(setting.Webhook.Types, hookType, true) {
-		ctx.NotFound("checkHookType", nil)
-		return ""
-	}
-	return hookType
-}
-
-// WebhooksNew render creating webhook page
-func WebhooksNew(ctx *context.Context) {
+// WebhookNew render creating webhook page
+func WebhookNew(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook")
 	ctx.Data["Webhook"] = webhook.Webhook{HookEvent: &webhook_module.HookEvent{}}
 
@@ -142,16 +131,12 @@ func WebhooksNew(ctx *context.Context) {
 		ctx.Data["PageIsSettingsHooksNew"] = true
 	}
 
-	hookType := checkHookType(ctx)
-	ctx.Data["HookType"] = hookType
-	if ctx.Written() {
+	hookType := ctx.Params(":type")
+	if webhook_service.GetWebhookHandler(hookType) == nil {
+		ctx.NotFound("GetWebhookHandler", nil)
 		return
 	}
-	if hookType == "discord" {
-		ctx.Data["DiscordHook"] = map[string]any{
-			"Username": "Gitea",
-		}
-	}
+	ctx.Data["HookType"] = hookType
 	ctx.Data["BaseLink"] = orCtx.LinkNew
 	ctx.Data["BaseLinkNew"] = orCtx.LinkNew
 
@@ -191,21 +176,9 @@ func ParseHookEvent(form forms.WebhookForm) *webhook_module.HookEvent {
 	}
 }
 
-type webhookParams struct {
-	// Type should be imported from webhook package (webhook.XXX)
-	Type string
-
-	URL         string
-	ContentType webhook.HookContentType
-	Secret      string
-	HTTPMethod  string
-	WebhookForm forms.WebhookForm
-	Meta        any
-}
-
 func WebhookCreate(ctx *context.Context) {
-	typ := ctx.Params(":type")
-	handler := webhook_service.GetWebhookHandler(typ)
+	hookType := ctx.Params(":type")
+	handler := webhook_service.GetWebhookHandler(hookType)
 	if handler == nil {
 		ctx.NotFound("GetWebhookHandler", nil)
 		return
@@ -213,25 +186,14 @@ func WebhookCreate(ctx *context.Context) {
 
 	fields := handler.FormFields(func(form any) {
 		errs := binding.Bind(ctx.Req, form)
-		middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error will be checked later in ctx.HasError
+		middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error checked below in ctx.HasError
 	})
-	createWebhook(ctx, webhookParams{
-		Type:        typ,
-		URL:         fields.URL,
-		ContentType: fields.ContentType,
-		Secret:      fields.Secret,
-		HTTPMethod:  fields.HTTPMethod,
-		WebhookForm: fields.WebhookForm,
-		Meta:        fields.Metadata,
-	})
-}
 
-func createWebhook(ctx *context.Context, params webhookParams) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook")
 	ctx.Data["PageIsSettingsHooks"] = true
 	ctx.Data["PageIsSettingsHooksNew"] = true
 	ctx.Data["Webhook"] = webhook.Webhook{HookEvent: &webhook_module.HookEvent{}}
-	ctx.Data["HookType"] = params.Type
+	ctx.Data["HookType"] = hookType
 
 	orCtx, err := getOwnerRepoCtx(ctx)
 	if err != nil {
@@ -242,13 +204,29 @@ func createWebhook(ctx *context.Context, params webhookParams) {
 	ctx.Data["BaseLinkNew"] = orCtx.LinkNew
 
 	if ctx.HasError() {
+		// pre-fill the form with the submitted data
+		var w webhook.Webhook
+		w.URL = fields.URL
+		w.ContentType = fields.ContentType
+		w.Secret = fields.Secret
+		w.HookEvent = ParseHookEvent(fields.WebhookForm)
+		w.IsActive = fields.WebhookForm.Active
+		w.HTTPMethod = fields.HTTPMethod
+		err := w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader)
+		if err != nil {
+			ctx.ServerError("SetHeaderAuthorization", err)
+			return
+		}
+		ctx.Data["Webhook"] = w
+		ctx.Data["HookMetadata"] = fields.Metadata
+
 		ctx.HTML(http.StatusUnprocessableEntity, orCtx.NewTemplate)
 		return
 	}
 
 	var meta []byte
-	if params.Meta != nil {
-		meta, err = json.Marshal(params.Meta)
+	if fields.Metadata != nil {
+		meta, err = json.Marshal(fields.Metadata)
 		if err != nil {
 			ctx.ServerError("Marshal", err)
 			return
@@ -257,18 +235,18 @@ func createWebhook(ctx *context.Context, params webhookParams) {
 
 	w := &webhook.Webhook{
 		RepoID:          orCtx.RepoID,
-		URL:             params.URL,
-		HTTPMethod:      params.HTTPMethod,
-		ContentType:     params.ContentType,
-		Secret:          params.Secret,
-		HookEvent:       ParseHookEvent(params.WebhookForm),
-		IsActive:        params.WebhookForm.Active,
-		Type:            params.Type,
+		URL:             fields.URL,
+		HTTPMethod:      fields.HTTPMethod,
+		ContentType:     fields.ContentType,
+		Secret:          fields.Secret,
+		HookEvent:       ParseHookEvent(fields.WebhookForm),
+		IsActive:        fields.WebhookForm.Active,
+		Type:            hookType,
 		Meta:            string(meta),
 		OwnerID:         orCtx.OwnerID,
 		IsSystemWebhook: orCtx.IsSystemWebhook,
 	}
-	err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader)
+	err = w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader)
 	if err != nil {
 		ctx.ServerError("SetHeaderAuthorization", err)
 		return
@@ -286,29 +264,6 @@ func createWebhook(ctx *context.Context, params webhookParams) {
 }
 
 func WebhookUpdate(ctx *context.Context) {
-	typ := ctx.Params(":type")
-	handler := webhook_service.GetWebhookHandler(typ)
-	if handler == nil {
-		ctx.NotFound("GetWebhookHandler", nil)
-		return
-	}
-
-	fields := handler.FormFields(func(form any) {
-		errs := binding.Bind(ctx.Req, form)
-		middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error will be checked later in ctx.HasError
-	})
-	editWebhook(ctx, webhookParams{
-		Type:        typ,
-		URL:         fields.URL,
-		ContentType: fields.ContentType,
-		Secret:      fields.Secret,
-		HTTPMethod:  fields.HTTPMethod,
-		WebhookForm: fields.WebhookForm,
-		Meta:        fields.Metadata,
-	})
-}
-
-func editWebhook(ctx *context.Context, params webhookParams) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.update_webhook")
 	ctx.Data["PageIsSettingsHooks"] = true
 	ctx.Data["PageIsSettingsHooksEdit"] = true
@@ -319,35 +274,48 @@ func editWebhook(ctx *context.Context, params webhookParams) {
 	}
 	ctx.Data["Webhook"] = w
 
+	handler := webhook_service.GetWebhookHandler(w.Type)
+	if handler == nil {
+		ctx.NotFound("GetWebhookHandler", nil)
+		return
+	}
+
+	fields := handler.FormFields(func(form any) {
+		errs := binding.Bind(ctx.Req, form)
+		middleware.Validate(errs, ctx.Data, form, ctx.Locale) // error checked below in ctx.HasError
+	})
+
+	// pre-fill the form with the submitted data
+	w.URL = fields.URL
+	w.ContentType = fields.ContentType
+	w.Secret = fields.Secret
+	w.HookEvent = ParseHookEvent(fields.WebhookForm)
+	w.IsActive = fields.WebhookForm.Active
+	w.HTTPMethod = fields.HTTPMethod
+
+	err := w.SetHeaderAuthorization(fields.WebhookForm.AuthorizationHeader)
+	if err != nil {
+		ctx.ServerError("SetHeaderAuthorization", err)
+		return
+	}
+
 	if ctx.HasError() {
+		ctx.Data["HookMetadata"] = fields.Metadata
 		ctx.HTML(http.StatusUnprocessableEntity, orCtx.NewTemplate)
 		return
 	}
 
 	var meta []byte
-	var err error
-	if params.Meta != nil {
-		meta, err = json.Marshal(params.Meta)
+	if fields.Metadata != nil {
+		meta, err = json.Marshal(fields.Metadata)
 		if err != nil {
 			ctx.ServerError("Marshal", err)
 			return
 		}
 	}
 
-	w.URL = params.URL
-	w.ContentType = params.ContentType
-	w.Secret = params.Secret
-	w.HookEvent = ParseHookEvent(params.WebhookForm)
-	w.IsActive = params.WebhookForm.Active
-	w.HTTPMethod = params.HTTPMethod
 	w.Meta = string(meta)
 
-	err = w.SetHeaderAuthorization(params.WebhookForm.AuthorizationHeader)
-	if err != nil {
-		ctx.ServerError("SetHeaderAuthorization", err)
-		return
-	}
-
 	if err := w.UpdateEvent(); err != nil {
 		ctx.ServerError("UpdateEvent", err)
 		return
@@ -399,8 +367,8 @@ func checkWebhook(ctx *context.Context) (*ownerRepoCtx, *webhook.Webhook) {
 	return orCtx, w
 }
 
-// WebHooksEdit render editing web hook page
-func WebHooksEdit(ctx *context.Context) {
+// WebhookEdit render editing web hook page
+func WebhookEdit(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.update_webhook")
 	ctx.Data["PageIsSettingsHooks"] = true
 	ctx.Data["PageIsSettingsHooksEdit"] = true
@@ -414,8 +382,8 @@ func WebHooksEdit(ctx *context.Context) {
 	ctx.HTML(http.StatusOK, orCtx.NewTemplate)
 }
 
-// TestWebhook test if web hook is work fine
-func TestWebhook(ctx *context.Context) {
+// WebhookTest test if web hook is work fine
+func WebhookTest(ctx *context.Context) {
 	hookID := ctx.ParamsInt64(":id")
 	w, err := webhook.GetWebhookByRepoID(ctx, ctx.Repo.Repository.ID, hookID)
 	if err != nil {
@@ -475,8 +443,8 @@ func TestWebhook(ctx *context.Context) {
 	}
 }
 
-// ReplayWebhook replays a webhook
-func ReplayWebhook(ctx *context.Context) {
+// WebhookReplay replays a webhook
+func WebhookReplay(ctx *context.Context) {
 	hookTaskUUID := ctx.Params(":uuid")
 
 	orCtx, w := checkWebhook(ctx)
@@ -497,8 +465,8 @@ func ReplayWebhook(ctx *context.Context) {
 	ctx.Redirect(fmt.Sprintf("%s/%d", orCtx.Link, w.ID))
 }
 
-// DeleteWebhook delete a webhook
-func DeleteWebhook(ctx *context.Context) {
+// WebhookDelete delete a webhook
+func WebhookDelete(ctx *context.Context) {
 	if err := webhook.DeleteWebhookByRepoID(ctx, ctx.Repo.Repository.ID, ctx.FormInt64("id")); err != nil {
 		ctx.Flash.Error("DeleteWebhookByRepoID: " + err.Error())
 	} else {

routers/web/web.go

@@ -400,15 +400,6 @@ func registerRoutes(m *web.Route) {
 		}
 	}
 
-	addWebhookAddRoutes := func() {
-		m.Get("/{type}/new", repo_setting.WebhooksNew)
-		m.Post("/{type}/new", repo_setting.WebhookCreate)
-	}
-
-	addWebhookEditRoutes := func() {
-		m.Post("/{type}/{id:[0-9]+}", repo_setting.WebhookUpdate)
-	}
-
 	addSettingsVariablesRoutes := func() {
 		m.Group("/variables", func() {
 			m.Get("", repo_setting.Variables)
@@ -618,12 +609,13 @@ func registerRoutes(m *web.Route) {
 		m.Group("/hooks", func() {
 			m.Get("", user_setting.Webhooks)
 			m.Post("/delete", user_setting.DeleteWebhook)
-			addWebhookAddRoutes()
+			m.Get("/{type}/new", repo_setting.WebhookNew)
+			m.Post("/{type}/new", repo_setting.WebhookCreate)
 			m.Group("/{id}", func() {
-				m.Get("", repo_setting.WebHooksEdit)
-				m.Post("/replay/{uuid}", repo_setting.ReplayWebhook)
+				m.Get("", repo_setting.WebhookEdit)
+				m.Post("", repo_setting.WebhookUpdate)
+				m.Post("/replay/{uuid}", repo_setting.WebhookReplay)
 			})
-			addWebhookEditRoutes()
 		}, webhooksEnabled)
 
 		m.Group("/blocked_users", func() {
@@ -722,14 +714,15 @@ func registerRoutes(m *web.Route) {
 			m.Get("", admin.DefaultOrSystemWebhooks)
 			m.Post("/delete", admin.DeleteDefaultOrSystemWebhook)
 			m.Group("/{id}", func() {
-				m.Get("", repo_setting.WebHooksEdit)
-				m.Post("/replay/{uuid}", repo_setting.ReplayWebhook)
+				m.Get("", repo_setting.WebhookEdit)
+				m.Post("", repo_setting.WebhookUpdate)
+				m.Post("/replay/{uuid}", repo_setting.WebhookReplay)
 			})
-			addWebhookEditRoutes()
 		}, webhooksEnabled)
 
 		m.Group("/{configType:default-hooks|system-hooks}", func() {
-			addWebhookAddRoutes()
+			m.Get("/{type}/new", repo_setting.WebhookNew)
+			m.Post("/{type}/new", repo_setting.WebhookCreate)
 		})
 
 		m.Group("/auths", func() {
@@ -887,12 +880,13 @@ func registerRoutes(m *web.Route) {
 				m.Group("/hooks", func() {
 					m.Get("", org.Webhooks)
 					m.Post("/delete", org.DeleteWebhook)
-					addWebhookAddRoutes()
+					m.Get("/{type}/new", repo_setting.WebhookNew)
+					m.Post("/{type}/new", repo_setting.WebhookCreate)
 					m.Group("/{id}", func() {
-						m.Get("", repo_setting.WebHooksEdit)
-						m.Post("/replay/{uuid}", repo_setting.ReplayWebhook)
+						m.Get("", repo_setting.WebhookEdit)
+						m.Post("", repo_setting.WebhookUpdate)
+						m.Post("/replay/{uuid}", repo_setting.WebhookReplay)
 					})
-					addWebhookEditRoutes()
 				}, webhooksEnabled)
 
 				m.Group("/labels", func() {
@@ -1059,15 +1053,16 @@ func registerRoutes(m *web.Route) {
 			}, context.GitHookService())
 
 			m.Group("/hooks", func() {
-				m.Get("", repo_setting.Webhooks)
-				m.Post("/delete", repo_setting.DeleteWebhook)
-				addWebhookAddRoutes()
+				m.Get("", repo_setting.WebhookList)
+				m.Post("/delete", repo_setting.WebhookDelete)
+				m.Get("/{type}/new", repo_setting.WebhookNew)
+				m.Post("/{type}/new", repo_setting.WebhookCreate)
 				m.Group("/{id}", func() {
-					m.Get("", repo_setting.WebHooksEdit)
-					m.Post("/test", repo_setting.TestWebhook)
-					m.Post("/replay/{uuid}", repo_setting.ReplayWebhook)
+					m.Get("", repo_setting.WebhookEdit)
+					m.Post("", repo_setting.WebhookUpdate)
+					m.Post("/test", repo_setting.WebhookTest)
+					m.Post("/replay/{uuid}", repo_setting.WebhookReplay)
 				})
-				addWebhookEditRoutes()
 			}, webhooksEnabled)
 
 			m.Group("/keys", func() {

templates/repo/settings/webhook/dingtalk.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "dingtalk"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://dingtalk.com" (ctx.Locale.Tr "repo.settings.web_hook_name_dingtalk")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/dingtalk/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "dingtalk/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/discord.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "discord"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://discord.com" (ctx.Locale.Tr "repo.settings.web_hook_name_discord")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/discord/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "discord/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/feishu.tmpl

@@ -1,7 +1,7 @@
 {{if eq .HookType "feishu"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://feishu.cn" (ctx.Locale.Tr "repo.settings.web_hook_name_feishu")}}</p>
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://larksuite.com" (ctx.Locale.Tr "repo.settings.web_hook_name_larksuite")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/feishu/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "feishu/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/forgejo.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "forgejo"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://forgejo.org/docs/latest/user/webhooks/" (ctx.Locale.Tr "repo.settings.web_hook_name_forgejo")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/forgejo/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "forgejo/new"}}" method="post">
 		{{template "base/disable_form_autofill"}}
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">

templates/repo/settings/webhook/gitea.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "gitea"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://forgejo.org/docs/latest/user/webhooks/" (ctx.Locale.Tr "repo.settings.web_hook_name_gitea")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/gitea/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "gitea/new"}}" method="post">
 		{{template "base/disable_form_autofill"}}
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">

templates/repo/settings/webhook/gogs.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "gogs"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://forgejo.org/docs/latest/user/webhooks/" (ctx.Locale.Tr "repo.settings.web_hook_name_gogs")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/gogs/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "gogs/new"}}" method="post">
 		{{template "base/disable_form_autofill"}}
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">

templates/repo/settings/webhook/matrix.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "matrix"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://matrix.org/" (ctx.Locale.Tr "repo.settings.web_hook_name_matrix")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/matrix/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "matrix/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_HomeserverURL}}error{{end}}">
 			<label for="homeserver_url">{{ctx.Locale.Tr "repo.settings.matrix.homeserver_url"}}</label>

templates/repo/settings/webhook/msteams.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "msteams"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://teams.microsoft.com" (ctx.Locale.Tr "repo.settings.web_hook_name_msteams")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/msteams/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "msteams/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/packagist.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "packagist"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://packagist.org" (ctx.Locale.Tr "repo.settings.web_hook_name_packagist")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/packagist/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "packagist/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_Username}}error{{end}}">
 			<label for="username">{{ctx.Locale.Tr "repo.settings.packagist_username"}}</label>

templates/repo/settings/webhook/settings.tmpl

@@ -259,7 +259,7 @@
 </div>
 
 <!-- Authorization Header -->
-<div class="field{{if eq .HookType "matrix"}} required{{end}}">
+<div class="field{{if eq .HookType "matrix"}} required{{end}} {{if .Err_AuthorizationHeader}}error{{end}}">
 	<label for="authorization_header">{{ctx.Locale.Tr "repo.settings.authorization_header"}}</label>
 	<input id="authorization_header" name="authorization_header" type="text" value="{{.Webhook.HeaderAuthorization}}"{{if eq .HookType "matrix"}} placeholder="Bearer $access_token" required{{end}}>
 	{{if ne .HookType "matrix"}}{{/* Matrix doesn't make the authorization optional but it is implied by the help string, should be changed.*/}}

templates/repo/settings/webhook/slack.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "slack"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://slack.com" (ctx.Locale.Tr "repo.settings.web_hook_name_slack")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/slack/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "slack/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>

templates/repo/settings/webhook/telegram.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "telegram"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://core.telegram.org/bots" (ctx.Locale.Tr "repo.settings.web_hook_name_telegram")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/telegram/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "telegram/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_BotToken}}error{{end}}">
 			<label for="bot_token">{{ctx.Locale.Tr "repo.settings.bot_token"}}</label>

templates/repo/settings/webhook/wechatwork.tmpl

@@ -1,6 +1,6 @@
 {{if eq .HookType "wechatwork"}}
 	<p>{{ctx.Locale.Tr "repo.settings.add_web_hook_desc" "https://work.weixin.qq.com" (ctx.Locale.Tr "repo.settings.web_hook_name_wechatwork")}}</p>
-	<form class="ui form" action="{{.BaseLink}}/wechatwork/{{or .Webhook.ID "new"}}" method="post">
+	<form class="ui form" action="{{.BaseLink}}/{{or .Webhook.ID "wechatwork/new"}}" method="post">
 		{{.CsrfTokenHtml}}
 		<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 			<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>

tests/integration/links_test.go

@@ -193,8 +193,8 @@ func TestRedirectsWebhooks(t *testing.T) {
 			{from: "/user2/repo1/settings/hooks/" + kind + "/new", to: "/", verb: "POST"},
 			{from: "/admin/system-hooks/" + kind + "/new", to: "/", verb: "POST"},
 			{from: "/admin/default-hooks/" + kind + "/new", to: "/", verb: "POST"},
-			{from: "/user2/repo1/settings/hooks/" + kind + "/1", to: "/", verb: "POST"},
-			{from: "/admin/hooks/" + kind + "/1", to: "/", verb: "POST"},
+			{from: "/user2/repo1/settings/hooks/1", to: "/", verb: "POST"},
+			{from: "/admin/hooks/1", to: "/", verb: "POST"},
 		}
 		for _, info := range redirects {
 			req := NewRequest(t, info.verb, info.from)

tests/integration/repo_webhook_test.go

@@ -52,7 +52,7 @@ func TestNewWebHookLink(t *testing.T) {
 	htmlDoc := NewHTMLParser(t, resp.Body)
 	assert.Equal(t, webhooksLen, htmlDoc.Find(`a[href^="`+baseurl+`/"][href$="/new"]`).Length(), "not all webhooks are listed in the 'new' dropdown on failure")
 
-	resp = session.MakeRequest(t, NewRequestWithValues(t, "POST", baseurl+"/gitea/1", map[string]string{"_csrf": csrfToken}), http.StatusUnprocessableEntity)
+	resp = session.MakeRequest(t, NewRequestWithValues(t, "POST", baseurl+"/1", map[string]string{"_csrf": csrfToken}), http.StatusUnprocessableEntity)
 	htmlDoc = NewHTMLParser(t, resp.Body)
 	assert.Equal(t, webhooksLen, htmlDoc.Find(`a[href^="`+baseurl+`/"][href$="/new"]`).Length(), "not all webhooks are listed in the 'new' dropdown on failure")
 }
@@ -330,7 +330,16 @@ func testWebhookForms(name string, session *TestSession, validFields map[string]
 						}
 					}
 
-					session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user2/repo1/settings/hooks/"+name+"/new", payload), http.StatusUnprocessableEntity)
+					resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user2/repo1/settings/hooks/"+name+"/new", payload), http.StatusUnprocessableEntity)
+					// check that the invalid form is pre-filled
+					htmlForm = NewHTMLParser(t, resp.Body).Find(`form[action^="/user2/repo1/settings/hooks/"]`)
+					for k, v := range payload {
+						if k == "_csrf" || k == "events" || v == "" {
+							// the 'events' is a radio input, which is buggy below
+							continue
+						}
+						assert.Equal(t, v, assertInput(t, htmlForm, k), "input %q did not contain value %q", k, v)
+					}
 					if t.Failed() {
 						t.Log(invalidPatch)
 					}