fix(hook/webhook)!: clean up rename repo code with EventAction and shift defer func

api/webhook.go

@@ -131,16 +131,12 @@ func PostWebhook(c *gin.Context) {
 
 	h, r, b := webhook.Hook, webhook.Repo, webhook.Build
 
-	defer func() {
-		// send API call to update the webhook
-		err = database.FromContext(c).UpdateHook(h)
-		if err != nil {
-			logrus.Errorf("unable to update webhook %s/%s: %v", r.GetFullName(), h.GetSourceID(), err)
-		}
-	}()
-
-	// check if build was parsed from webhook
-	if b == nil && h.GetEvent() != constants.EventRepositoryRename {
+	// check if build was parsed from webhook.
+	// build will be nil on repository events, but
+	// for renaming, we want to continue.
+	if b == nil &&
+		h.GetEvent() != constants.EventRepository &&
+		h.GetEventAction() != constants.ActionRenamed {
 		// typically, this should only happen on a webhook
 		// "ping" which gets sent when the webhook is created
 		c.JSON(http.StatusOK, "no build to process")
@@ -153,20 +149,23 @@ func PostWebhook(c *gin.Context) {
 		retErr := fmt.Errorf("%s: failed to parse repo from webhook", baseErr)
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		h.SetStatus(constants.StatusFailure)
-		h.SetError(retErr.Error())
-
 		return
 	}
 
-	if h.GetEvent() == constants.EventRepositoryRename {
-		err = renameRepository(h, r, c)
+	defer func() {
+		// send API call to update the webhook
+		err = database.FromContext(c).UpdateHook(h)
+		if err != nil {
+			logrus.Errorf("unable to update webhook %s/%s: %v", r.GetFullName(), h.GetSourceID(), err)
+		}
+	}()
+
+	if h.GetEvent() == constants.EventRepository {
+		err = renameRepository(h, r, c, m)
 		if err != nil {
 			util.HandleError(c, http.StatusBadRequest, err)
 			h.SetStatus(constants.StatusFailure)
 			h.SetError(err.Error())
-
-			return
 		}
 
 		return
@@ -672,7 +671,7 @@ func publishToQueue(queue queue.Service, db database.Service, p *pipeline.Build,
 // queries the database for the repo that matches that name and org, and updates
 // that repo to its new name in order to preserve it. It also updates the secrets
 // associated with that repo.
-func renameRepository(h *library.Hook, r *library.Repo, c *gin.Context) error {
+func renameRepository(h *library.Hook, r *library.Repo, c *gin.Context, m *types.Metadata) error {
 	// get the old name of the repo
 	previousName := r.GetPreviousName()
 	// get the repo from the database that matches the old name
@@ -706,8 +705,29 @@ func renameRepository(h *library.Hook, r *library.Repo, c *gin.Context) error {
 		return retErr
 	}
 
-	// get total number of secrets associated with repository
+	// update hook object which will be added to DB upon reaching deferred function in PostWebhook
+	h.SetRepoID(r.GetID())
+
+	// send API call to capture the last hook for the repo
+	lastHook, err := database.FromContext(c).GetLastHook(dbR)
+	if err != nil {
+		retErr := fmt.Errorf("unable to get last hook for repo %s: %w", r.GetFullName(), err)
+		util.HandleError(c, http.StatusInternalServerError, retErr)
+
+		h.SetStatus(constants.StatusFailure)
+		h.SetError(retErr.Error())
+
+		return retErr
+	}
 
+	// set the Number field
+	if lastHook != nil {
+		h.SetNumber(
+			lastHook.GetNumber() + 1,
+		)
+	}
+
+	// get total number of secrets associated with repository
 	t, err := database.FromContext(c).GetTypeSecretCount(constants.SecretRepo, r.GetOrg(), previousName, []string{})
 	if err != nil {
 		return fmt.Errorf("unable to get secret count for repo %s/%s: %w", r.GetOrg(), previousName, err)
@@ -737,6 +757,38 @@ func renameRepository(h *library.Hook, r *library.Repo, c *gin.Context) error {
 		}
 	}
 
+	// get total number of builds associated with repository
+	t, err = database.FromContext(c).GetRepoBuildCount(dbR, nil)
+	if err != nil {
+		return fmt.Errorf("unable to get build count for repo %s: %w", dbR.GetFullName(), err)
+	}
+
+	builds := []*library.Build{}
+	page = 1
+	// capture all builds belonging to repo in database
+	for build := int64(0); build < t; build += 100 {
+		b, _, err := database.FromContext(c).GetRepoBuildList(dbR, nil, time.Now().Unix(), 0, page, 100)
+		if err != nil {
+			return fmt.Errorf("unable to get build list for repo %s: %w", dbR.GetFullName(), err)
+		}
+
+		builds = append(builds, b...)
+
+		page++
+	}
+
+	// update build link to route to proper repo name
+	for _, build := range builds {
+		build.SetLink(
+			fmt.Sprintf("%s/%s/%d", m.Vela.WebAddress, dbR.GetFullName(), build.GetNumber()),
+		)
+
+		err = database.FromContext(c).UpdateBuild(build)
+		if err != nil {
+			return fmt.Errorf("unable to update build for repo %s: %w", dbR.GetFullName(), err)
+		}
+	}
+
 	c.JSON(http.StatusOK, r)
 
 	return nil

database/postgres/ddl/hook.go

@@ -11,19 +11,20 @@ const (
 CREATE TABLE
 IF NOT EXISTS
 hooks (
-	id         SERIAL PRIMARY KEY,
-	repo_id    INTEGER,
-	build_id   INTEGER,
-	number     INTEGER,
-	source_id  VARCHAR(250),
-	created    INTEGER,
-	host       VARCHAR(250),
-	event      VARCHAR(250),
-	branch     VARCHAR(500),
-	error      VARCHAR(500),
-	status     VARCHAR(250),
-	link       VARCHAR(1000),
-	webhook_id INTEGER,
+	id           SERIAL PRIMARY KEY,
+	repo_id      INTEGER,
+	build_id     INTEGER,
+	number       INTEGER,
+	source_id    VARCHAR(250),
+	created      INTEGER,
+	host         VARCHAR(250),
+	event        VARCHAR(250),
+	event_action VARCHAR(250),
+	branch       VARCHAR(500),
+	error        VARCHAR(500),
+	status       VARCHAR(250),
+	link         VARCHAR(1000),
+	webhook_id   INTEGER,
 	UNIQUE(repo_id, number)
 );
 `

database/postgres/hook_list_test.go

@@ -49,9 +49,9 @@ func TestPostgres_Client_GetHookList(t *testing.T) {
 
 	// create expected return in mock
 	_rows := sqlmock.NewRows(
-		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "branch", "error", "status", "link", "webhook_id"},
-	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", 1).
-		AddRow(2, 1, 2, 2, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", 1)
+		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"},
+	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1).
+		AddRow(2, 1, 2, 2, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1)
 
 	// ensure the mock expects the query
 	_mock.ExpectQuery(_query.SQL.String()).WillReturnRows(_rows)
@@ -129,9 +129,9 @@ func TestPostgres_Client_GetRepoHookList(t *testing.T) {
 
 	// create expected return in mock
 	_rows := sqlmock.NewRows(
-		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "branch", "error", "status", "link", "webhook_id"},
-	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", 1).
-		AddRow(2, 1, 2, 2, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", 1)
+		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"},
+	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1).
+		AddRow(2, 1, 2, 2, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1)
 
 	// ensure the mock expects the query
 	_mock.ExpectQuery(_query.SQL.String()).WillReturnRows(_rows)

database/postgres/hook_test.go

@@ -48,8 +48,8 @@ func TestPostgres_Client_GetHook(t *testing.T) {
 
 	// create expected return in mock
 	_rows := sqlmock.NewRows(
-		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "branch", "error", "status", "link", "webhook_id"},
-	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", 1)
+		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"},
+	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1)
 
 	// ensure the mock expects the query for test case 1
 	_mock.ExpectQuery(_query.SQL.String()).WillReturnRows(_rows)
@@ -125,8 +125,8 @@ func TestPostgres_Client_GetLastHook(t *testing.T) {
 
 	// create expected return in mock
 	_rows := sqlmock.NewRows(
-		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "branch", "error", "status", "link", "webhook_id"},
-	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", 1)
+		[]string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"},
+	).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 1)
 
 	// ensure the mock expects the query for test case 1
 	_mock.ExpectQuery(_query.SQL.String()).WillReturnRows(_rows)
@@ -192,8 +192,8 @@ func TestPostgres_Client_CreateHook(t *testing.T) {
 	_rows := sqlmock.NewRows([]string{"id"}).AddRow(1)
 
 	// ensure the mock expects the query
-	_mock.ExpectQuery(`INSERT INTO "hooks" ("repo_id","build_id","number","source_id","created","host","event","branch","error","status","link","webhook_id","id") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13) RETURNING "id"`).
-		WithArgs(1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", nil, nil, nil, nil, nil, nil, nil, 1, 1).
+	_mock.ExpectQuery(`INSERT INTO "hooks" ("repo_id","build_id","number","source_id","created","host","event","event_action","branch","error","status","link","webhook_id","id") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14) RETURNING "id"`).
+		WithArgs(1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", nil, nil, nil, nil, nil, nil, nil, nil, 1, 1).
 		WillReturnRows(_rows)
 
 	// setup tests
@@ -242,8 +242,8 @@ func TestPostgres_Client_UpdateHook(t *testing.T) {
 	defer func() { _sql, _ := _database.Postgres.DB(); _sql.Close() }()
 
 	// ensure the mock expects the query
-	_mock.ExpectExec(`UPDATE "hooks" SET "repo_id"=$1,"build_id"=$2,"number"=$3,"source_id"=$4,"created"=$5,"host"=$6,"event"=$7,"branch"=$8,"error"=$9,"status"=$10,"link"=$11,"webhook_id"=$12 WHERE "id" = $13`).
-		WithArgs(1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", nil, nil, nil, nil, nil, nil, nil, 1, 1).
+	_mock.ExpectExec(`UPDATE "hooks" SET "repo_id"=$1,"build_id"=$2,"number"=$3,"source_id"=$4,"created"=$5,"host"=$6,"event"=$7,"event_action"=$8,"branch"=$9,"error"=$10,"status"=$11,"link"=$12,"webhook_id"=$13 WHERE "id" = $14`).
+		WithArgs(1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", nil, nil, nil, nil, nil, nil, nil, nil, 1, 1).
 		WillReturnResult(sqlmock.NewResult(1, 1))
 
 	// setup tests
@@ -327,18 +327,19 @@ func testHook() *library.Hook {
 	str := ""
 
 	return &library.Hook{
-		ID:        &i64,
-		RepoID:    &i64,
-		BuildID:   &i64,
-		Number:    &i,
-		SourceID:  &str,
-		Created:   &i64,
-		Host:      &str,
-		Event:     &str,
-		Branch:    &str,
-		Error:     &str,
-		Status:    &str,
-		Link:      &str,
-		WebhookID: &i64,
+		ID:          &i64,
+		RepoID:      &i64,
+		BuildID:     &i64,
+		Number:      &i,
+		SourceID:    &str,
+		Created:     &i64,
+		Host:        &str,
+		Event:       &str,
+		EventAction: &str,
+		Branch:      &str,
+		Error:       &str,
+		Status:      &str,
+		Link:        &str,
+		WebhookID:   &i64,
 	}
 }

database/sqlite/ddl/hook.go

@@ -11,19 +11,20 @@ const (
 CREATE TABLE
 IF NOT EXISTS
 hooks (
-	id         INTEGER PRIMARY KEY AUTOINCREMENT,
-	repo_id    INTEGER,
-	build_id   INTEGER,
-	number     INTEGER,
-	source_id  TEXT,
-	created    INTEGER,
-	host       TEXT,
-	event      TEXT,
-	branch     TEXT,
-	error      TEXT,
-	status     TEXT,
-	link       TEXT,
-	webhook_id INTEGER,
+	id           INTEGER PRIMARY KEY AUTOINCREMENT,
+	repo_id      INTEGER,
+	build_id     INTEGER,
+	number       INTEGER,
+	source_id    TEXT,
+	created      INTEGER,
+	host         TEXT,
+	event        TEXT,
+	event_action TEXT,
+	branch       TEXT,
+	error        TEXT,
+	status       TEXT,
+	link         TEXT,
+	webhook_id   INTEGER,
 	UNIQUE(repo_id, build_id)
 );
 `

database/sqlite/hook_test.go

@@ -322,18 +322,19 @@ func testHook() *library.Hook {
 	str := ""
 
 	return &library.Hook{
-		ID:        &i64,
-		RepoID:    &i64,
-		BuildID:   &i64,
-		Number:    &i,
-		SourceID:  &str,
-		Created:   &i64,
-		Host:      &str,
-		Event:     &str,
-		Branch:    &str,
-		Error:     &str,
-		Status:    &str,
-		Link:      &str,
-		WebhookID: &i64,
+		ID:          &i64,
+		RepoID:      &i64,
+		BuildID:     &i64,
+		Number:      &i,
+		SourceID:    &str,
+		Created:     &i64,
+		Host:        &str,
+		Event:       &str,
+		EventAction: &str,
+		Branch:      &str,
+		Error:       &str,
+		Status:      &str,
+		Link:        &str,
+		WebhookID:   &i64,
 	}
 }

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/gin-gonic/gin v1.7.7
 	github.com/go-playground/assert/v2 v2.0.1
 	github.com/go-redis/redis/v8 v8.11.5
-	github.com/go-vela/types v0.14.0-rc1
+	github.com/go-vela/types v0.14.0-rc1.0.20220518151947-0cf0e4753e74
 	github.com/golang-jwt/jwt/v4 v4.4.1
 	github.com/google/go-cmp v0.5.8
 	github.com/google/go-github/v44 v44.0.0

go.sum

@@ -181,8 +181,8 @@ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/me
 github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE=
 github.com/go-test/deep v1.0.2 h1:onZX1rnHT3Wv6cqNgYyFOOlgVKJrksuCMCRvJStbMYw=
 github.com/go-test/deep v1.0.2/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
-github.com/go-vela/types v0.14.0-rc1 h1:zckr7Cywbw97IfYu/F/e1yscIW72HL0VhhNtp1NV5kQ=
-github.com/go-vela/types v0.14.0-rc1/go.mod h1:g2C+XdTuq2hzrsEUt+jVLLqmhgogoXryQEok3EK3HkE=
+github.com/go-vela/types v0.14.0-rc1.0.20220518151947-0cf0e4753e74 h1:AQoqAPwpVzP23G2sk7OUM7cRFEi1Rl5FIEcshJ6tzTI=
+github.com/go-vela/types v0.14.0-rc1.0.20220518151947-0cf0e4753e74/go.mod h1:g2C+XdTuq2hzrsEUt+jVLLqmhgogoXryQEok3EK3HkE=
 github.com/gofrs/uuid v4.0.0+incompatible h1:1SD/1F5pU8p29ybwgQSwpQk+mwdRrXCYuPhW6m+TnJw=
 github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=

scm/github/webhook.go

@@ -446,12 +446,10 @@ func (c *client) processRepositoryEvent(h *library.Hook, payload *github.Reposit
 	// if action is renamed, then get the previous name from payload
 	if payload.GetAction() == "renamed" {
 		r.SetPreviousName(payload.GetChanges().GetRepo().GetName().GetFrom())
-		// update hook object event type
-		h.SetEvent(constants.EventRepositoryRename)
-	} else {
-		h.SetEvent(constants.EventRepository)
 	}
 
+	h.SetEvent(constants.EventRepository)
+	h.SetEventAction(payload.GetAction())
 	h.SetBranch(r.GetBranch())
 	h.SetLink(
 		fmt.Sprintf("https://%s/%s/settings/hooks", h.GetHost(), r.GetFullName()),

scm/github/webhook_test.go

@@ -960,7 +960,8 @@ func TestGitHub_ProcessWebhook_RepositoryRename(t *testing.T) {
 	wantHook.SetWebhookID(123456)
 	wantHook.SetCreated(time.Now().UTC().Unix())
 	wantHook.SetHost("github.com")
-	wantHook.SetEvent("repositoryRename")
+	wantHook.SetEvent(constants.EventRepository)
+	wantHook.SetEventAction(constants.ActionRenamed)
 	wantHook.SetBranch("master")
 	wantHook.SetStatus(constants.StatusSuccess)
 	wantHook.SetLink("https://github.com/Codertocat/Hello-World/settings/hooks")
@@ -1022,7 +1023,8 @@ func TestGitHub_ProcessWebhook_Repository(t *testing.T) {
 	wantHook.SetWebhookID(123456)
 	wantHook.SetCreated(time.Now().UTC().Unix())
 	wantHook.SetHost("github.com")
-	wantHook.SetEvent("repository")
+	wantHook.SetEvent(constants.EventRepository)
+	wantHook.SetEventAction("publicized")
 	wantHook.SetBranch("master")
 	wantHook.SetStatus(constants.StatusSuccess)
 	wantHook.SetLink("https://github.com/Codertocat/Hello-World/settings/hooks")