Improve the `issue_comment` workflow trigger event (#29277)

Fix #29175
Replace #29207

This PR makes some improvements to the `issue_comment` workflow trigger
event.

1. Fix the bug that pull requests cannot trigger `issue_comment`
workflows
2. Previously the `issue_comment` event only supported the `created`
activity type. This PR adds support for the missing `edited` and
`deleted` activity types.
3. Some events (including `issue_comment`, `issues`, etc. ) only trigger
workflows that belong to the workflow file on the default branch. This
PR introduces the `IsDefaultBranchWorkflow` function to check for these
events.

diff --git a/modules/actions/github.go b/modules/actions/github.go
index 18917c5118d6639a92b22c392807dbbbcb05b14b..68116ec83a539930c2bf1ff96cd20e26e2becf6e 100644 (file)
--- a/modules/actions/github.go
+++ b/modules/actions/github.go
@@ -25,6 +25,45 @@ const (
        GithubEventSchedule                 = "schedule"
 )
 
+// IsDefaultBranchWorkflow returns true if the event only triggers workflows on the default branch
+func IsDefaultBranchWorkflow(triggedEvent webhook_module.HookEventType) bool {
+       switch triggedEvent {
+       case webhook_module.HookEventDelete:
+               // GitHub "delete" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#delete
+               return true
+       case webhook_module.HookEventFork:
+               // GitHub "fork" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#fork
+               return true
+       case webhook_module.HookEventIssueComment:
+               // GitHub "issue_comment" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment
+               return true
+       case webhook_module.HookEventPullRequestComment:
+               // GitHub "pull_request_comment" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment
+               return true
+       case webhook_module.HookEventWiki:
+               // GitHub "gollum" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#gollum
+               return true
+       case webhook_module.HookEventSchedule:
+               // GitHub "schedule" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
+               return true
+       case webhook_module.HookEventIssues,
+               webhook_module.HookEventIssueAssign,
+               webhook_module.HookEventIssueLabel,
+               webhook_module.HookEventIssueMilestone:
+               // Github "issues" event
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues
+               return true
+       }
+
+       return false
+}
+
 // canGithubEventMatch check if the input Github event can match any Gitea event.
 func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEventType) bool {
        switch eventName {
@@ -75,6 +114,11 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent
        case GithubEventSchedule:
                return triggedEvent == webhook_module.HookEventSchedule
 
+       case GithubEventIssueComment:
+               // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment
+               return triggedEvent == webhook_module.HookEventIssueComment ||
+                       triggedEvent == webhook_module.HookEventPullRequestComment
+
        default:
                return eventName == string(triggedEvent)
        }

diff --git a/modules/actions/github_test.go b/modules/actions/github_test.go
index 4bf55ae03fccda7f4c3d80fd885775a5566390ca..6652ff6eac22e0fd33ca1085aed1ece67e066b25 100644 (file)
--- a/modules/actions/github_test.go
+++ b/modules/actions/github_test.go
@@ -103,6 +103,12 @@ func TestCanGithubEventMatch(t *testing.T) {
                        webhook_module.HookEventCreate,
                        true,
                },
+               {
+                       "create pull request comment",
+                       GithubEventIssueComment,
+                       webhook_module.HookEventPullRequestComment,
+                       true,
+               },
        }
 
        for _, tc := range testCases {

diff --git a/services/actions/notifier.go b/services/actions/notifier.go
index 77848a3f589782cc9ee06417ef54e4100bbab9ea..e144484dabda2846b6ec07d6d3c305e4cde5ad1f 100644 (file)
--- a/services/actions/notifier.go
+++ b/services/actions/notifier.go
@@ -224,37 +224,88 @@ func (n *actionsNotifier) CreateIssueComment(ctx context.Context, doer *user_mod
 ) {
        ctx = withMethod(ctx, "CreateIssueComment")
 
-       permission, _ := access_model.GetUserRepoPermission(ctx, repo, doer)
-
        if issue.IsPull {
-               if err := issue.LoadPullRequest(ctx); err != nil {
+               notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventPullRequestComment, api.HookIssueCommentCreated)
+               return
+       }
+       notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventIssueComment, api.HookIssueCommentCreated)
+}
+
+func (n *actionsNotifier) UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment, oldContent string) {
+       ctx = withMethod(ctx, "UpdateComment")
+
+       if err := c.LoadIssue(ctx); err != nil {
+               log.Error("LoadIssue: %v", err)
+               return
+       }
+
+       if c.Issue.IsPull {
+               notifyIssueCommentChange(ctx, doer, c, oldContent, webhook_module.HookEventPullRequestComment, api.HookIssueCommentEdited)
+               return
+       }
+       notifyIssueCommentChange(ctx, doer, c, oldContent, webhook_module.HookEventIssueComment, api.HookIssueCommentEdited)
+}
+
+func (n *actionsNotifier) DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) {
+       ctx = withMethod(ctx, "DeleteComment")
+
+       if err := comment.LoadIssue(ctx); err != nil {
+               log.Error("LoadIssue: %v", err)
+               return
+       }
+
+       if comment.Issue.IsPull {
+               notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventPullRequestComment, api.HookIssueCommentDeleted)
+               return
+       }
+       notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventIssueComment, api.HookIssueCommentDeleted)
+}
+
+func notifyIssueCommentChange(ctx context.Context, doer *user_model.User, comment *issues_model.Comment, oldContent string, event webhook_module.HookEventType, action api.HookIssueCommentAction) {
+       if err := comment.LoadIssue(ctx); err != nil {
+               log.Error("LoadIssue: %v", err)
+               return
+       }
+       if err := comment.Issue.LoadAttributes(ctx); err != nil {
+               log.Error("LoadAttributes: %v", err)
+               return
+       }
+
+       permission, _ := access_model.GetUserRepoPermission(ctx, comment.Issue.Repo, doer)
+
+       payload := &api.IssueCommentPayload{
+               Action:     action,
+               Issue:      convert.ToAPIIssue(ctx, comment.Issue),
+               Comment:    convert.ToAPIComment(ctx, comment.Issue.Repo, comment),
+               Repository: convert.ToRepo(ctx, comment.Issue.Repo, permission),
+               Sender:     convert.ToUser(ctx, doer, nil),
+               IsPull:     comment.Issue.IsPull,
+       }
+
+       if action == api.HookIssueCommentEdited {
+               payload.Changes = &api.ChangesPayload{
+                       Body: &api.ChangesFromPayload{
+                               From: oldContent,
+                       },
+               }
+       }
+
+       if comment.Issue.IsPull {
+               if err := comment.Issue.LoadPullRequest(ctx); err != nil {
                        log.Error("LoadPullRequest: %v", err)
                        return
                }
-               newNotifyInputFromIssue(issue, webhook_module.HookEventPullRequestComment).
+               newNotifyInputFromIssue(comment.Issue, event).
                        WithDoer(doer).
-                       WithPayload(&api.IssueCommentPayload{
-                               Action:     api.HookIssueCommentCreated,
-                               Issue:      convert.ToAPIIssue(ctx, issue),
-                               Comment:    convert.ToAPIComment(ctx, repo, comment),
-                               Repository: convert.ToRepo(ctx, repo, permission),
-                               Sender:     convert.ToUser(ctx, doer, nil),
-                               IsPull:     true,
-                       }).
-                       WithPullRequest(issue.PullRequest).
+                       WithPayload(payload).
+                       WithPullRequest(comment.Issue.PullRequest).
                        Notify(ctx)
                return
        }
-       newNotifyInputFromIssue(issue, webhook_module.HookEventIssueComment).
+
+       newNotifyInputFromIssue(comment.Issue, event).
                WithDoer(doer).
-               WithPayload(&api.IssueCommentPayload{
-                       Action:     api.HookIssueCommentCreated,
-                       Issue:      convert.ToAPIIssue(ctx, issue),
-                       Comment:    convert.ToAPIComment(ctx, repo, comment),
-                       Repository: convert.ToRepo(ctx, repo, permission),
-                       Sender:     convert.ToUser(ctx, doer, nil),
-                       IsPull:     false,
-               }).
+               WithPayload(payload).
                Notify(ctx)
 }
 
@@ -496,7 +547,6 @@ func (n *actionsNotifier) DeleteRef(ctx context.Context, pusher *user_model.User
        apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeNone})
 
        newNotifyInput(repo, pusher, webhook_module.HookEventDelete).
-               WithRef(refFullName.ShortName()). // FIXME: should we use a full ref name
                WithPayload(&api.DeletePayload{
                        Ref:        refFullName.ShortName(),
                        RefType:    refFullName.RefType(),

diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go
index 8852f23c5f7130bc428e9919ae449843ca4eac71..c20335af6f30db4032568d61946a258d93c1beee 100644 (file)
--- a/services/actions/notifier_helper.go
+++ b/services/actions/notifier_helper.go
@@ -136,12 +136,15 @@ func notify(ctx context.Context, input *notifyInput) error {
        defer gitRepo.Close()
 
        ref := input.Ref
-       if input.Event == webhook_module.HookEventDelete {
-               // The event is deleting a reference, so it will fail to get the commit for a deleted reference.
-               // Set ref to empty string to fall back to the default branch.
-               ref = ""
+       if ref != input.Repo.DefaultBranch && actions_module.IsDefaultBranchWorkflow(input.Event) {
+               if ref != "" {
+                       log.Warn("Event %q should only trigger workflows on the default branch, but its ref is %q. Will fall back to the default branch",
+                               input.Event, ref)
+               }
+               ref = input.Repo.DefaultBranch
        }
        if ref == "" {
+               log.Warn("Ref of event %q is empty, will fall back to the default branch", input.Event)
                ref = input.Repo.DefaultBranch
        }