From 8d799c236c854c8bb132b38c97385beaf53d9327 Mon Sep 17 00:00:00 2001 From: badhezi Date: Tue, 15 Apr 2025 19:56:19 +0300 Subject: [PATCH 1/8] use the correct context data for PR link template in issue card --- templates/repo/issue/card.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/card.tmpl b/templates/repo/issue/card.tmpl index c7bbe91885012..41fe6cea8fbae 100644 --- a/templates/repo/issue/card.tmpl +++ b/templates/repo/issue/card.tmpl @@ -45,7 +45,7 @@ {{if $.Page.LinkedPRs}} {{range index $.Page.LinkedPRs .ID}}
- + {{svg "octicon-git-merge" 16 "tw-mr-1 tw-align-middle"}} {{.Title}} #{{.Index}} From c511c712036efa86fe508c60c616579e80814b92 Mon Sep 17 00:00:00 2001 From: badhezi Date: Tue, 3 Jun 2025 09:02:09 +0300 Subject: [PATCH 2/8] add issue delete notifier --- modules/structs/hook.go | 2 ++ services/webhook/notifier.go | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/modules/structs/hook.go b/modules/structs/hook.go index aaa9fbc9d364d..4dab6410e802c 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -306,6 +306,8 @@ const ( HookIssueReviewRequested HookIssueAction = "review_requested" // HookIssueReviewRequestRemoved is an issue action for removing a review request to someone on a pull request. HookIssueReviewRequestRemoved HookIssueAction = "review_request_removed" + // HookIssueDeleted is an issue action for deleting an issue + HookIssueDeleted HookIssueAction = "deleted" ) // IssuePayload represents the payload information that is sent along with an issue event. diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 9e3f21de290ee..322e926364057 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -295,6 +295,28 @@ func (m *webhookNotifier) NewIssue(ctx context.Context, issue *issues_model.Issu } } +func (m *webhookNotifier) DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) { + if err := issue.LoadRepo(ctx); err != nil { + log.Error("issue.LoadRepo: %v", err) + return + } + if err := issue.LoadPoster(ctx); err != nil { + log.Error("issue.LoadPoster: %v", err) + return + } + + permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) + if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventIssues, &api.IssuePayload{ + Action: api.HookIssueDeleted, + Index: issue.Index, + Issue: convert.ToAPIIssue(ctx, issue.Poster, issue), + Repository: convert.ToRepo(ctx, issue.Repo, permission), + Sender: convert.ToUser(ctx, doer, nil), + }); err != nil { + log.Error("PrepareWebhooks: %v", err) + } +} + func (m *webhookNotifier) NewPullRequest(ctx context.Context, pull *issues_model.PullRequest, mentions []*user_model.User) { if err := pull.LoadIssue(ctx); err != nil { log.Error("pull.LoadIssue: %v", err) From 1e3d908c312e3156e2e904dbf187fe461896e8e2 Mon Sep 17 00:00:00 2001 From: badhezi Date: Tue, 3 Jun 2025 09:11:59 +0300 Subject: [PATCH 3/8] use doer permissions for issue delete webhook payload --- services/webhook/notifier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 322e926364057..e57b60c854d48 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -305,7 +305,7 @@ func (m *webhookNotifier) DeleteIssue(ctx context.Context, doer *user_model.User return } - permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) + permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, doer) if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventIssues, &api.IssuePayload{ Action: api.HookIssueDeleted, Index: issue.Index, From 650d32686619918fa10ba764133fcddcc96f8278 Mon Sep 17 00:00:00 2001 From: badhezi Date: Tue, 3 Jun 2025 09:39:10 +0300 Subject: [PATCH 4/8] add case for pull request deletion --- services/webhook/notifier.go | 49 +++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index e57b60c854d48..6b7eae93a76ca 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -296,24 +296,39 @@ func (m *webhookNotifier) NewIssue(ctx context.Context, issue *issues_model.Issu } func (m *webhookNotifier) DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) { - if err := issue.LoadRepo(ctx); err != nil { - log.Error("issue.LoadRepo: %v", err) - return - } - if err := issue.LoadPoster(ctx); err != nil { - log.Error("issue.LoadPoster: %v", err) - return - } - permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, doer) - if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventIssues, &api.IssuePayload{ - Action: api.HookIssueDeleted, - Index: issue.Index, - Issue: convert.ToAPIIssue(ctx, issue.Poster, issue), - Repository: convert.ToRepo(ctx, issue.Repo, permission), - Sender: convert.ToUser(ctx, doer, nil), - }); err != nil { - log.Error("PrepareWebhooks: %v", err) + if issue.IsPull { + if err := issue.LoadPullRequest(ctx); err != nil { + log.Error("LoadPullRequest: %v", err) + return + } + if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequest, &api.PullRequestPayload{ + Action: api.HookIssueDeleted, + Index: issue.Index, + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), + Repository: convert.ToRepo(ctx, issue.Repo, permission), + Sender: convert.ToUser(ctx, doer, nil), + }); err != nil { + log.Error("PrepareWebhooks: %v", err) + } + } else { + if err := issue.LoadRepo(ctx); err != nil { + log.Error("issue.LoadRepo: %v", err) + return + } + if err := issue.LoadPoster(ctx); err != nil { + log.Error("issue.LoadPoster: %v", err) + return + } + if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventIssues, &api.IssuePayload{ + Action: api.HookIssueDeleted, + Index: issue.Index, + Issue: convert.ToAPIIssue(ctx, issue.Poster, issue), + Repository: convert.ToRepo(ctx, issue.Repo, permission), + Sender: convert.ToUser(ctx, doer, nil), + }); err != nil { + log.Error("PrepareWebhooks: %v", err) + } } } From 2ba9a243322e11e39292cfba89e8b84aaa494758 Mon Sep 17 00:00:00 2001 From: badhezi Date: Tue, 3 Jun 2025 21:37:49 +0300 Subject: [PATCH 5/8] move const to relevant line --- modules/structs/hook.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/structs/hook.go b/modules/structs/hook.go index 4dab6410e802c..76ba97539a88d 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -286,6 +286,8 @@ const ( HookIssueReOpened HookIssueAction = "reopened" // HookIssueEdited edited HookIssueEdited HookIssueAction = "edited" + // HookIssueDeleted is an issue action for deleting an issue + HookIssueDeleted HookIssueAction = "deleted" // HookIssueAssigned assigned HookIssueAssigned HookIssueAction = "assigned" // HookIssueUnassigned unassigned @@ -306,8 +308,6 @@ const ( HookIssueReviewRequested HookIssueAction = "review_requested" // HookIssueReviewRequestRemoved is an issue action for removing a review request to someone on a pull request. HookIssueReviewRequestRemoved HookIssueAction = "review_request_removed" - // HookIssueDeleted is an issue action for deleting an issue - HookIssueDeleted HookIssueAction = "deleted" ) // IssuePayload represents the payload information that is sent along with an issue event. From e729fc9e024c517e88e22202679a2d8ab265ae4d Mon Sep 17 00:00:00 2001 From: badhezi Date: Wed, 4 Jun 2025 21:46:53 +0300 Subject: [PATCH 6/8] add test cases for issue delete and pull request delete webhooks --- options/locale/locale_en-US.ini | 4 +- tests/integration/issue_test.go | 7 +++ tests/integration/repo_webhook_test.go | 72 ++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index f7c2e5049bbc2..1eb23f3cc1ac3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2371,7 +2371,7 @@ settings.event_repository = Repository settings.event_repository_desc = Repository created or deleted. settings.event_header_issue = Issue Events settings.event_issues = Issues -settings.event_issues_desc = Issue opened, closed, reopened, or edited. +settings.event_issues_desc = Issue opened, closed, reopened, edited or deleted. settings.event_issue_assign = Issue Assigned settings.event_issue_assign_desc = Issue assigned or unassigned. settings.event_issue_label = Issue Labeled @@ -2382,7 +2382,7 @@ settings.event_issue_comment = Issue Comment settings.event_issue_comment_desc = Issue comment created, edited, or deleted. settings.event_header_pull_request = Pull Request Events settings.event_pull_request = Pull Request -settings.event_pull_request_desc = Pull request opened, closed, reopened, or edited. +settings.event_pull_request_desc = Pull request opened, closed, reopened, edited or deleted. settings.event_pull_request_assign = Pull Request Assigned settings.event_pull_request_assign_desc = Pull request assigned or unassigned. settings.event_pull_request_label = Pull Request Labeled diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 2e6a12df2cf47..bcc05b1515ab2 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -151,6 +151,13 @@ func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content return issueURL } +func testIssueDelete(t *testing.T, session *TestSession, issueUrl string) { + req := NewRequestWithValues(t, "POST", path.Join(issueUrl, "delete"), map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + }) + session.MakeRequest(t, req, http.StatusSeeOther) +} + func testIssueAssign(t *testing.T, session *TestSession, repoLink string, issueID, assigneeID int64) { req := NewRequestWithValues(t, "POST", fmt.Sprintf(repoLink+"/issues/assignee?issue_ids=%d", issueID), map[string]string{ "_csrf": GetUserCSRFToken(t, session), diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 438dd3211d3c0..2ecb75d834f7b 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "path" "strings" "testing" "time" @@ -450,6 +451,39 @@ func Test_WebhookIssue(t *testing.T) { }) } +func Test_WebhookIssueDelete(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + var payloads []api.IssuePayload + var triggeredEvent string + provider := newMockWebhookProvider(func(r *http.Request) { + content, _ := io.ReadAll(r.Body) + var payload api.IssuePayload + err := json.Unmarshal(content, &payload) + assert.NoError(t, err) + payloads = append(payloads, payload) + triggeredEvent = "issue" + }, http.StatusOK) + defer provider.Close() + + // 1. create a new webhook with special webhook for repo1 + session := loginUser(t, "user2") + testAPICreateWebhookForRepo(t, session, "user2", "repo1", provider.URL(), "issues") + issueUrl := testNewIssue(t, session, "user2", "repo1", "Title1", "Description1") + + // 2. trigger the webhook + testIssueDelete(t, session, issueUrl) + + // 3. validate the webhook is triggered + assert.Equal(t, "issue", triggeredEvent) + require.Len(t, payloads, 2) + assert.EqualValues(t, "deleted", payloads[1].Action) + assert.Equal(t, "repo1", payloads[1].Issue.Repo.Name) + assert.Equal(t, "user2/repo1", payloads[1].Issue.Repo.FullName) + assert.Equal(t, "Title1", payloads[1].Issue.Title) + assert.Equal(t, "Description1", payloads[1].Issue.Body) + }) +} + func Test_WebhookIssueAssign(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { var payloads []api.PullRequestPayload @@ -596,6 +630,44 @@ func Test_WebhookPullRequest(t *testing.T) { }) } +func Test_WebhookPullRequestDelete(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + var payloads []api.PullRequestPayload + var triggeredEvent string + provider := newMockWebhookProvider(func(r *http.Request) { + content, _ := io.ReadAll(r.Body) + var payload api.PullRequestPayload + err := json.Unmarshal(content, &payload) + assert.NoError(t, err) + payloads = append(payloads, payload) + triggeredEvent = "pull_request" + }, http.StatusOK) + defer provider.Close() + + // 1. create a new webhook with special webhook for repo1 + session := loginUser(t, "user2") + testAPICreateWebhookForRepo(t, session, "user2", "repo1", provider.URL(), "pull_request") + + testAPICreateBranch(t, session, "user2", "repo1", "master", "master2", http.StatusCreated) + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 1}) + issueUrl := testCreatePullToDefaultBranch(t, session, repo1, repo1, "master2", "first pull request") + + // 2. trigger the webhook + testIssueDelete(t, session, path.Join(repo1.Link(), "pulls", issueUrl)) + + // 3. validate the webhook is triggered + assert.Equal(t, "pull_request", triggeredEvent) + require.Len(t, payloads, 2) + assert.EqualValues(t, "deleted", payloads[1].Action) + assert.Equal(t, "repo1", payloads[1].PullRequest.Base.Repository.Name) + assert.Equal(t, "user2/repo1", payloads[1].PullRequest.Base.Repository.FullName) + assert.Equal(t, 0, *payloads[1].PullRequest.Additions) + assert.Equal(t, 0, *payloads[1].PullRequest.ChangedFiles) + assert.Equal(t, 0, *payloads[1].PullRequest.Deletions) + }) +} + func Test_WebhookPullRequestComment(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { var payloads []api.IssueCommentPayload From 1ac0719a8432876b33ee4f4d0e223468ae7698a4 Mon Sep 17 00:00:00 2001 From: badhezi Date: Wed, 4 Jun 2025 21:59:28 +0300 Subject: [PATCH 7/8] fix linting --- tests/integration/repo_webhook_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 2ecb75d834f7b..77d00a8a39d5c 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -468,10 +468,10 @@ func Test_WebhookIssueDelete(t *testing.T) { // 1. create a new webhook with special webhook for repo1 session := loginUser(t, "user2") testAPICreateWebhookForRepo(t, session, "user2", "repo1", provider.URL(), "issues") - issueUrl := testNewIssue(t, session, "user2", "repo1", "Title1", "Description1") + issueURL := testNewIssue(t, session, "user2", "repo1", "Title1", "Description1") // 2. trigger the webhook - testIssueDelete(t, session, issueUrl) + testIssueDelete(t, session, issueURL) // 3. validate the webhook is triggered assert.Equal(t, "issue", triggeredEvent) @@ -651,10 +651,10 @@ func Test_WebhookPullRequestDelete(t *testing.T) { testAPICreateBranch(t, session, "user2", "repo1", "master", "master2", http.StatusCreated) repo1 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 1}) - issueUrl := testCreatePullToDefaultBranch(t, session, repo1, repo1, "master2", "first pull request") + issueURL := testCreatePullToDefaultBranch(t, session, repo1, repo1, "master2", "first pull request") // 2. trigger the webhook - testIssueDelete(t, session, path.Join(repo1.Link(), "pulls", issueUrl)) + testIssueDelete(t, session, path.Join(repo1.Link(), "pulls", issueURL)) // 3. validate the webhook is triggered assert.Equal(t, "pull_request", triggeredEvent) From 8ed6da502a11c080f9b378eeb9804cf7c91758a7 Mon Sep 17 00:00:00 2001 From: badhezi Date: Wed, 4 Jun 2025 22:11:44 +0300 Subject: [PATCH 8/8] fix linting --- tests/integration/issue_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index bcc05b1515ab2..7969634cdfdbc 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -151,8 +151,8 @@ func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content return issueURL } -func testIssueDelete(t *testing.T, session *TestSession, issueUrl string) { - req := NewRequestWithValues(t, "POST", path.Join(issueUrl, "delete"), map[string]string{ +func testIssueDelete(t *testing.T, session *TestSession, issueURL string) { + req := NewRequestWithValues(t, "POST", path.Join(issueURL, "delete"), map[string]string{ "_csrf": GetUserCSRFToken(t, session), }) session.MakeRequest(t, req, http.StatusSeeOther)