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

Backport #29277 by @Zettat123

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.

Co-authored-by: Zettat123 <zettat123@gmail.com>
This commit is contained in:
Giteabot 2024-02-23 00:19:48 +08:00 committed by GitHub
parent ed5e0c8c27
commit fdb0d03083
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 130 additions and 27 deletions

View File

@ -25,6 +25,45 @@ const (
GithubEventSchedule = "schedule" 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. // canGithubEventMatch check if the input Github event can match any Gitea event.
func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEventType) bool { func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEventType) bool {
switch eventName { switch eventName {
@ -75,6 +114,11 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent
case GithubEventSchedule: case GithubEventSchedule:
return triggedEvent == webhook_module.HookEventSchedule 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: default:
return eventName == string(triggedEvent) return eventName == string(triggedEvent)
} }

View File

@ -103,6 +103,12 @@ func TestCanGithubEventMatch(t *testing.T) {
webhook_module.HookEventCreate, webhook_module.HookEventCreate,
true, true,
}, },
{
"create pull request comment",
GithubEventIssueComment,
webhook_module.HookEventPullRequestComment,
true,
},
} }
for _, tc := range testCases { for _, tc := range testCases {

View File

@ -225,37 +225,88 @@ func (n *actionsNotifier) CreateIssueComment(ctx context.Context, doer *user_mod
) { ) {
ctx = withMethod(ctx, "CreateIssueComment") ctx = withMethod(ctx, "CreateIssueComment")
permission, _ := access_model.GetUserRepoPermission(ctx, repo, doer)
if issue.IsPull { 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) log.Error("LoadPullRequest: %v", err)
return return
} }
newNotifyInputFromIssue(issue, webhook_module.HookEventPullRequestComment). newNotifyInputFromIssue(comment.Issue, event).
WithDoer(doer). WithDoer(doer).
WithPayload(&api.IssueCommentPayload{ WithPayload(payload).
Action: api.HookIssueCommentCreated, WithPullRequest(comment.Issue.PullRequest).
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).
Notify(ctx) Notify(ctx)
return return
} }
newNotifyInputFromIssue(issue, webhook_module.HookEventIssueComment).
newNotifyInputFromIssue(comment.Issue, event).
WithDoer(doer). WithDoer(doer).
WithPayload(&api.IssueCommentPayload{ WithPayload(payload).
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,
}).
Notify(ctx) Notify(ctx)
} }
@ -497,7 +548,6 @@ func (n *actionsNotifier) DeleteRef(ctx context.Context, pusher *user_model.User
apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}) apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeNone})
newNotifyInput(repo, pusher, webhook_module.HookEventDelete). newNotifyInput(repo, pusher, webhook_module.HookEventDelete).
WithRef(refFullName.ShortName()). // FIXME: should we use a full ref name
WithPayload(&api.DeletePayload{ WithPayload(&api.DeletePayload{
Ref: refFullName.ShortName(), Ref: refFullName.ShortName(),
RefType: refFullName.RefType(), RefType: refFullName.RefType(),

View File

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