From ee500dacd01adf1735cb0f4643ddc385ce4cf026 Mon Sep 17 00:00:00 2001
From: Laura Hausmann <laura@hausmann.dev>
Date: Sun, 14 Apr 2024 23:45:18 +0200
Subject: [PATCH 1/2] Fix release published actions not triggering for releases
created from existing tags
(cherry picked from commit 46977b0f013420136926b6b96a5aee4dea1c3018)
---
routers/api/v1/repo/release.go | 4 ++--
routers/web/repo/release.go | 4 ++--
services/release/release.go | 8 ++++----
services/release/release_test.go | 14 +++++++-------
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go
index f0f3c0bbc7..c6495c3de8 100644
--- a/routers/api/v1/repo/release.go
+++ b/routers/api/v1/repo/release.go
@@ -267,7 +267,7 @@ func CreateRelease(ctx *context.APIContext) {
rel.Publisher = ctx.Doer
rel.Target = form.Target
- if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil {
+ if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil, true); err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateRelease", err)
return
}
@@ -341,7 +341,7 @@ func EditRelease(ctx *context.APIContext) {
if form.IsPrerelease != nil {
rel.IsPrerelease = *form.IsPrerelease
}
- if err := release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil {
+ if err := release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil, false); err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateRelease", err)
return
}
diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go
index 54e9aed207..908ce8138e 100644
--- a/routers/web/repo/release.go
+++ b/routers/web/repo/release.go
@@ -556,7 +556,7 @@ func NewReleasePost(ctx *context.Context) {
rel.PublisherID = ctx.Doer.ID
rel.IsTag = false
- if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil {
+ if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil, true); err != nil {
ctx.Data["Err_TagName"] = true
ctx.ServerError("UpdateRelease", err)
return
@@ -663,7 +663,7 @@ func EditReleasePost(ctx *context.Context) {
rel.IsDraft = len(form.Draft) > 0
rel.IsPrerelease = form.Prerelease
if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo,
- rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments); err != nil {
+ rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments, false); err != nil {
ctx.ServerError("UpdateRelease", err)
return
}
diff --git a/services/release/release.go b/services/release/release.go
index ba5fd1dd98..c349ab5143 100644
--- a/services/release/release.go
+++ b/services/release/release.go
@@ -199,7 +199,7 @@ func CreateNewTag(ctx context.Context, doer *user_model.User, repo *repo_model.R
// delAttachmentUUIDs accept a slice of attachments' uuids which will be deleted from the release
// editAttachments accept a map of attachment uuid to new attachment name which will be updated with attachments.
func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, rel *repo_model.Release,
- addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string,
+ addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string, createdFromTag bool,
) error {
if rel.ID == 0 {
return errors.New("UpdateRelease only accepts an exist release")
@@ -292,11 +292,11 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo
}
if !rel.IsDraft {
- if !isCreated {
- notify_service.UpdateRelease(gitRepo.Ctx, doer, rel)
+ if createdFromTag || isCreated {
+ notify_service.NewRelease(gitRepo.Ctx, rel)
return nil
}
- notify_service.NewRelease(gitRepo.Ctx, rel)
+ notify_service.UpdateRelease(gitRepo.Ctx, doer, rel)
}
return nil
}
diff --git a/services/release/release_test.go b/services/release/release_test.go
index 3d0681f1e1..eac1879f87 100644
--- a/services/release/release_test.go
+++ b/services/release/release_test.go
@@ -159,7 +159,7 @@ func TestRelease_Update(t *testing.T) {
releaseCreatedUnix := release.CreatedUnix
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
release.Note = "Changed note"
- assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil))
+ assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false))
release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID)
assert.NoError(t, err)
assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -183,7 +183,7 @@ func TestRelease_Update(t *testing.T) {
releaseCreatedUnix = release.CreatedUnix
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
release.Title = "Changed title"
- assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil))
+ assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false))
release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID)
assert.NoError(t, err)
assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -208,7 +208,7 @@ func TestRelease_Update(t *testing.T) {
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
release.Title = "Changed title"
release.Note = "Changed note"
- assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil))
+ assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false))
release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID)
assert.NoError(t, err)
assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -233,7 +233,7 @@ func TestRelease_Update(t *testing.T) {
release.IsDraft = false
tagName := release.TagName
- assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil))
+ assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, nil, false))
release, err = repo_model.GetReleaseByID(db.DefaultContext, release.ID)
assert.NoError(t, err)
assert.Equal(t, tagName, release.TagName)
@@ -247,7 +247,7 @@ func TestRelease_Update(t *testing.T) {
}, strings.NewReader(samplePayload), int64(len([]byte(samplePayload))))
assert.NoError(t, err)
- assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, []string{attach.UUID}, nil, nil))
+ assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, []string{attach.UUID}, nil, nil, false))
assert.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release))
assert.Len(t, release.Attachments, 1)
assert.EqualValues(t, attach.UUID, release.Attachments[0].UUID)
@@ -257,7 +257,7 @@ func TestRelease_Update(t *testing.T) {
// update the attachment name
assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, nil, map[string]string{
attach.UUID: "test2.txt",
- }))
+ }, false))
release.Attachments = nil
assert.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release))
assert.Len(t, release.Attachments, 1)
@@ -266,7 +266,7 @@ func TestRelease_Update(t *testing.T) {
assert.EqualValues(t, "test2.txt", release.Attachments[0].Name)
// delete the attachment
- assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, []string{attach.UUID}, nil))
+ assert.NoError(t, UpdateRelease(db.DefaultContext, user, gitRepo, release, nil, []string{attach.UUID}, nil, false))
release.Attachments = nil
assert.NoError(t, repo_model.GetReleaseAttachments(db.DefaultContext, release))
assert.Empty(t, release.Attachments)
From 145cac08657355e58186fb5a8ae9bbb874773da5 Mon Sep 17 00:00:00 2001
From: Laura Hausmann <laura@hausmann.dev>
Date: Tue, 16 Apr 2024 18:43:39 +0200
Subject: [PATCH 2/2] Add tests for webhook release events
Co-authored-by: oliverpool <git@olivier.pfad.fr>
(cherry picked from commit 8506dbe2e5a96b82b502be7d74b53527a2277df8)
---
tests/integration/webhook_test.go | 116 ++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
diff --git a/tests/integration/webhook_test.go b/tests/integration/webhook_test.go
index e5ecddab32..ec85d12b07 100644
--- a/tests/integration/webhook_test.go
+++ b/tests/integration/webhook_test.go
@@ -9,10 +9,16 @@ import (
"testing"
"code.gitea.io/gitea/models/db"
+ repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
+ user_model "code.gitea.io/gitea/models/user"
webhook_model "code.gitea.io/gitea/models/webhook"
+ "code.gitea.io/gitea/modules/git"
+ "code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/json"
webhook_module "code.gitea.io/gitea/modules/webhook"
+ "code.gitea.io/gitea/services/release"
+ "code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
)
@@ -70,3 +76,113 @@ func TestWebhookPayloadRef(t *testing.T) {
assert.Empty(t, expected)
})
}
+
+func TestWebhookReleaseEvents(t *testing.T) {
+ defer tests.PrepareTestEnv(t)()
+
+ user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+ repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+ w := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{
+ ID: 1,
+ RepoID: repo.ID,
+ })
+ w.HookEvent = &webhook_module.HookEvent{
+ SendEverything: true,
+ }
+ assert.NoError(t, w.UpdateEvent())
+ assert.NoError(t, webhook_model.UpdateWebhook(db.DefaultContext, w))
+
+ hookTasks := retrieveHookTasks(t, w.ID, true)
+
+ gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo)
+ assert.NoError(t, err)
+ defer gitRepo.Close()
+
+ t.Run("CreateRelease", func(t *testing.T) {
+ assert.NoError(t, release.CreateRelease(gitRepo, &repo_model.Release{
+ RepoID: repo.ID,
+ Repo: repo,
+ PublisherID: user.ID,
+ Publisher: user,
+ TagName: "v1.1.1",
+ Target: "master",
+ Title: "v1.1.1 is released",
+ Note: "v1.1.1 is released",
+ IsDraft: false,
+ IsPrerelease: false,
+ IsTag: false,
+ }, nil, ""))
+
+ // check the newly created hooktasks
+ hookTasksLenBefore := len(hookTasks)
+ hookTasks = retrieveHookTasks(t, w.ID, false)
+
+ checkHookTasks(t, map[webhook_module.HookEventType]string{
+ webhook_module.HookEventRelease: "published",
+ webhook_module.HookEventCreate: "", // a tag was created as well
+ webhook_module.HookEventPush: "", // the tag creation also means a push event
+ }, hookTasks[:len(hookTasks)-hookTasksLenBefore])
+
+ t.Run("UpdateRelease", func(t *testing.T) {
+ rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{RepoID: repo.ID, TagName: "v1.1.1"})
+ assert.NoError(t, release.UpdateRelease(db.DefaultContext, user, gitRepo, rel, nil, nil, nil, false))
+
+ // check the newly created hooktasks
+ hookTasksLenBefore := len(hookTasks)
+ hookTasks = retrieveHookTasks(t, w.ID, false)
+
+ checkHookTasks(t, map[webhook_module.HookEventType]string{
+ webhook_module.HookEventRelease: "updated",
+ }, hookTasks[:len(hookTasks)-hookTasksLenBefore])
+ })
+ })
+
+ t.Run("CreateNewTag", func(t *testing.T) {
+ assert.NoError(t, release.CreateNewTag(db.DefaultContext,
+ user,
+ repo,
+ "master",
+ "v1.1.2",
+ "v1.1.2 is tagged",
+ ))
+
+ // check the newly created hooktasks
+ hookTasksLenBefore := len(hookTasks)
+ hookTasks = retrieveHookTasks(t, w.ID, false)
+
+ checkHookTasks(t, map[webhook_module.HookEventType]string{
+ webhook_module.HookEventCreate: "", // tag was created as well
+ webhook_module.HookEventPush: "", // the tag creation also means a push event
+ }, hookTasks[:len(hookTasks)-hookTasksLenBefore])
+
+ t.Run("UpdateRelease", func(t *testing.T) {
+ rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{RepoID: repo.ID, TagName: "v1.1.2"})
+ assert.NoError(t, release.UpdateRelease(db.DefaultContext, user, gitRepo, rel, nil, nil, nil, true))
+
+ // check the newly created hooktasks
+ hookTasksLenBefore := len(hookTasks)
+ hookTasks = retrieveHookTasks(t, w.ID, false)
+
+ checkHookTasks(t, map[webhook_module.HookEventType]string{
+ webhook_module.HookEventRelease: "published",
+ }, hookTasks[:len(hookTasks)-hookTasksLenBefore])
+ })
+ })
+}
+
+func checkHookTasks(t *testing.T, expectedActions map[webhook_module.HookEventType]string, hookTasks []*webhook_model.HookTask) {
+ t.Helper()
+ for _, hookTask := range hookTasks {
+ expectedAction, ok := expectedActions[hookTask.EventType]
+ if !ok {
+ t.Errorf("unexpected (or duplicated) event %q", hookTask.EventType)
+ }
+ var payload struct {
+ Action string `json:"action"`
+ }
+ assert.NoError(t, json.Unmarshal([]byte(hookTask.PayloadContent), &payload))
+ assert.Equal(t, expectedAction, payload.Action, "unexpected action for %q event", hookTask.EventType)
+ delete(expectedActions, hookTask.EventType)
+ }
+ assert.Empty(t, expectedActions)
+}