From f3074ddaf9056da31f4650d3825a1909f7e9a33f Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Sun, 30 Apr 2023 17:02:47 +0200
Subject: [PATCH] Some small code refactorings (#1727)

Refactorings taken from pull requests #1722 and #1725
---
 server/api/pipeline.go            | 15 +++++++--------
 server/forge/configFetcher.go     |  4 +++-
 server/model/const.go             | 13 ++++++++++---
 server/model/secret.go            |  4 ++--
 web/src/lib/api/index.ts          |  1 +
 web/src/lib/api/types/pipeline.ts |  4 +++-
 6 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/server/api/pipeline.go b/server/api/pipeline.go
index b6c082f1a..1444bda90 100644
--- a/server/api/pipeline.go
+++ b/server/api/pipeline.go
@@ -43,9 +43,9 @@ func CreatePipeline(c *gin.Context) {
 	_store := store.FromContext(c)
 	repo := session.Repo(c)
 
-	var p model.PipelineOptions
-
-	err := json.NewDecoder(c.Request.Body).Decode(&p)
+	// parse create options
+	var opts model.PipelineOptions
+	err := json.NewDecoder(c.Request.Body).Decode(&opts)
 	if err != nil {
 		_ = c.AbortWithError(http.StatusBadRequest, err)
 		return
@@ -53,9 +53,9 @@ func CreatePipeline(c *gin.Context) {
 
 	user := session.User(c)
 
-	lastCommit, _ := server.Config.Services.Forge.BranchHead(c, user, repo, p.Branch)
+	lastCommit, _ := server.Config.Services.Forge.BranchHead(c, user, repo, opts.Branch)
 
-	tmpBuild := createTmpPipeline(model.EventManual, lastCommit, repo, user, &p)
+	tmpBuild := createTmpPipeline(model.EventManual, lastCommit, repo, user, &opts)
 
 	pl, err := pipeline.Create(c, _store, repo, tmpBuild)
 	if err != nil {
@@ -368,9 +368,8 @@ func PostPipeline(c *gin.Context) {
 	if event, ok := c.GetQuery("event"); ok {
 		pl.Event = model.WebhookEvent(event)
 
-		if !model.ValidateWebhookEvent(pl.Event) {
-			msg := fmt.Sprintf("pipeline event '%s' is invalid", event)
-			c.String(http.StatusBadRequest, msg)
+		if err := model.ValidateWebhookEvent(pl.Event); err != nil {
+			_ = c.AbortWithError(http.StatusBadRequest, err)
 			return
 		}
 	}
diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go
index 98cac9411..042934e21 100644
--- a/server/forge/configFetcher.go
+++ b/server/forge/configFetcher.go
@@ -39,6 +39,7 @@ type configFetcher struct {
 	repo            *model.Repo
 	pipeline        *model.Pipeline
 	configExtension config.Extension
+	configPath      string
 	timeout         time.Duration
 }
 
@@ -49,6 +50,7 @@ func NewConfigFetcher(forge Forge, timeout time.Duration, configExtension config
 		repo:            repo,
 		pipeline:        pipeline,
 		configExtension: configExtension,
+		configPath:      repo.Config,
 		timeout:         timeout,
 	}
 }
@@ -59,7 +61,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er
 
 	// try to fetch 3 times
 	for i := 0; i < 3; i++ {
-		files, err = cf.fetch(ctx, time.Second*cf.timeout, strings.TrimSpace(cf.repo.Config))
+		files, err = cf.fetch(ctx, time.Second*cf.timeout, strings.TrimSpace(cf.configPath))
 		if err != nil {
 			log.Trace().Err(err).Msgf("%d. try failed", i+1)
 		}
diff --git a/server/model/const.go b/server/model/const.go
index 15a9c97f2..1496d49c1 100644
--- a/server/model/const.go
+++ b/server/model/const.go
@@ -15,6 +15,11 @@
 
 package model
 
+import (
+	"errors"
+	"fmt"
+)
+
 type WebhookEvent string
 
 const (
@@ -32,12 +37,14 @@ func (wel WebhookEventList) Len() int           { return len(wel) }
 func (wel WebhookEventList) Swap(i, j int)      { wel[i], wel[j] = wel[j], wel[i] }
 func (wel WebhookEventList) Less(i, j int) bool { return wel[i] < wel[j] }
 
-func ValidateWebhookEvent(s WebhookEvent) bool {
+var ErrInvalidWebhookEvent = errors.New("invalid webhook event")
+
+func ValidateWebhookEvent(s WebhookEvent) error {
 	switch s {
 	case EventPush, EventPull, EventTag, EventDeploy, EventCron, EventManual:
-		return true
+		return nil
 	default:
-		return false
+		return fmt.Errorf("%w: %s", ErrInvalidWebhookEvent, s)
 	}
 }
 
diff --git a/server/model/secret.go b/server/model/secret.go
index 16013f429..a669fa8ae 100644
--- a/server/model/secret.go
+++ b/server/model/secret.go
@@ -125,8 +125,8 @@ var validDockerImageString = regexp.MustCompile(
 // Validate validates the required fields and formats.
 func (s *Secret) Validate() error {
 	for _, event := range s.Events {
-		if !ValidateWebhookEvent(event) {
-			return fmt.Errorf("%w: '%s'", ErrSecretEventInvalid, event)
+		if err := ValidateWebhookEvent(event); err != nil {
+			return errors.Join(err, ErrSecretEventInvalid)
 		}
 	}
 	if len(s.Events) == 0 {
diff --git a/web/src/lib/api/index.ts b/web/src/lib/api/index.ts
index 8ebcee042..ef8c4c23e 100644
--- a/web/src/lib/api/index.ts
+++ b/web/src/lib/api/index.ts
@@ -22,6 +22,7 @@ type RepoListOptions = {
   all?: boolean;
 };
 
+// PipelineOptions is the data for creating a new pipeline
 type PipelineOptions = {
   branch: string;
   variables: Record<string, string>;
diff --git a/web/src/lib/api/types/pipeline.ts b/web/src/lib/api/types/pipeline.ts
index 1d9669850..052595fa9 100644
--- a/web/src/lib/api/types/pipeline.ts
+++ b/web/src/lib/api/types/pipeline.ts
@@ -1,3 +1,5 @@
+import { WebhookEvents } from './webhook';
+
 // A pipeline for a repository.
 export type Pipeline = {
   id: number;
@@ -8,7 +10,7 @@ export type Pipeline = {
 
   parent: number;
 
-  event: 'push' | 'tag' | 'pull_request' | 'deployment' | 'cron' | 'manual';
+  event: WebhookEvents;
 
   //  The current status of the pipeline.
   status: PipelineStatus;