refactor webhook *NewPost (#20729)

* refactor webhook *NewPost

* remove empty values

* always show errs.Message

* remove utils.IsValidSlackChannel

* move IsValidSlackChannel to services/webhook package

* binding: handle empty Message case

* make IsValidSlackChannel more strict

modules/web/middleware/binding.go

@@ -136,7 +136,16 @@ func Validate(errs binding.Errors, data map[string]interface{}, f Form, l transl
 			case validation.ErrRegexPattern:
 				data["ErrorMsg"] = trName + l.Tr("form.regex_pattern_error", errs[0].Message)
 			default:
-				data["ErrorMsg"] = l.Tr("form.unknown_error") + " " + errs[0].Classification
+				msg := errs[0].Classification
+				if msg != "" && errs[0].Message != "" {
+					msg += ": "
+				}
+
+				msg += errs[0].Message
+				if msg == "" {
+					msg = l.Tr("form.unknown_error")
+				}
+				data["ErrorMsg"] = trName + ": " + msg
 			}
 			return errs
 		}

routers/api/v1/utils/hook.go

@@ -15,7 +15,6 @@ import (
 	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
-	"code.gitea.io/gitea/routers/utils"
 	webhook_service "code.gitea.io/gitea/services/webhook"
 )
 
@@ -141,14 +140,15 @@ func addHook(ctx *context.APIContext, form *api.CreateHookOption, orgID, repoID
 			ctx.Error(http.StatusUnprocessableEntity, "", "Missing config option: channel")
 			return nil, false
 		}
+		channel = strings.TrimSpace(channel)
 
-		if !utils.IsValidSlackChannel(channel) {
+		if !webhook_service.IsValidSlackChannel(channel) {
 			ctx.Error(http.StatusBadRequest, "", "Invalid slack channel name")
 			return nil, false
 		}
 
 		meta, err := json.Marshal(&webhook_service.SlackMeta{
-			Channel:  strings.TrimSpace(channel),
+			Channel:  channel,
 			Username: form.Config["username"],
 			IconURL:  form.Config["icon_url"],
 			Color:    form.Config["color"],

routers/utils/utils.go

@@ -20,25 +20,6 @@ func RemoveUsernameParameterSuffix(name string) string {
 	return name
 }
 
-// IsValidSlackChannel validates a channel name conforms to what slack expects.
-// It makes sure a channel name cannot be empty and invalid ( only an # )
-func IsValidSlackChannel(channelName string) bool {
-	switch len(strings.TrimSpace(channelName)) {
-	case 0:
-		return false
-	case 1:
-		// Keep default behaviour where a channel name is still
-		// valid without an #
-		// But if it contains only an #, it should be regarded as
-		// invalid
-		if channelName[0] == '#' {
-			return false
-		}
-	}
-
-	return true
-}
-
 // SanitizeFlashErrorString will sanitize a flash error string
 func SanitizeFlashErrorString(x string) string {
 	return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")

routers/utils/utils_test.go

@@ -18,23 +18,6 @@ func TestRemoveUsernameParameterSuffix(t *testing.T) {
 	assert.Equal(t, "", RemoveUsernameParameterSuffix(""))
 }
 
-func TestIsValidSlackChannel(t *testing.T) {
-	tt := []struct {
-		channelName string
-		expected    bool
-	}{
-		{"gitea", true},
-		{"  ", false},
-		{"#", false},
-		{"gitea   ", true},
-		{"  gitea", true},
-	}
-
-	for _, v := range tt {
-		assert.Equal(t, v.expected, IsValidSlackChannel(v.channelName))
-	}
-}
-
 func TestIsExternalURL(t *testing.T) {
 	setting.AppURL = "https://try.gitea.io/"
 	type test struct {

services/forms/repo_form.go

@@ -17,7 +17,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/web/middleware"
-	"code.gitea.io/gitea/routers/utils"
+	"code.gitea.io/gitea/services/webhook"
 
 	"gitea.com/go-chi/binding"
 )
@@ -305,14 +305,16 @@ type NewSlackHookForm struct {
 // Validate validates the fields
 func (f *NewSlackHookForm) Validate(req *http.Request, errs binding.Errors) binding.Errors {
 	ctx := context.GetContext(req)
+	if !webhook.IsValidSlackChannel(strings.TrimSpace(f.Channel)) {
+		errs = append(errs, binding.Error{
+			FieldNames:     []string{"Channel"},
+			Classification: "",
+			Message:        ctx.Tr("repo.settings.add_webhook.invalid_channel_name"),
+		})
+	}
 	return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
 }
 
-// HasInvalidChannel validates the channel name is in the right format
-func (f NewSlackHookForm) HasInvalidChannel() bool {
-	return !utils.IsValidSlackChannel(f.Channel)
-}
-
 // NewDiscordHookForm form for creating discord hook
 type NewDiscordHookForm struct {
 	PayloadURL string `binding:"Required;ValidUrl"`

services/webhook/slack.go

@@ -7,6 +7,7 @@ package webhook
 import (
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 
 	webhook_model "code.gitea.io/gitea/models/webhook"
@@ -286,3 +287,13 @@ func GetSlackPayload(p api.Payloader, event webhook_model.HookEventType, meta st
 
 	return convertPayloader(s, p, event)
 }
+
+var slackChannel = regexp.MustCompile(`^#?[a-z0-9_-]{1,80}$`)
+
+// IsValidSlackChannel validates a channel name conforms to what slack expects:
+// https://api.slack.com/methods/conversations.rename#naming
+// Conversation names can only contain lowercase letters, numbers, hyphens, and underscores, and must be 80 characters or less.
+// Gitea accepts if it starts with a #.
+func IsValidSlackChannel(name string) bool {
+	return slackChannel.MatchString(name)
+}

services/webhook/slack_test.go

@@ -170,3 +170,22 @@ func TestSlackJSONPayload(t *testing.T) {
 	require.NoError(t, err)
 	assert.NotEmpty(t, json)
 }
+
+func TestIsValidSlackChannel(t *testing.T) {
+	tt := []struct {
+		channelName string
+		expected    bool
+	}{
+		{"gitea", true},
+		{"#gitea", true},
+		{"  ", false},
+		{"#", false},
+		{" #", false},
+		{"gitea   ", false},
+		{"  gitea", false},
+	}
+
+	for _, v := range tt {
+		assert.Equal(t, v.expected, IsValidSlackChannel(v.channelName))
+	}
+}