Skip to content

Commit a4fe1cd

Browse files
authored
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.
1 parent a70c00b commit a4fe1cd

File tree

4 files changed

+130
-27
lines changed

4 files changed

+130
-27
lines changed

modules/actions/github.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,45 @@ const (
2525
GithubEventSchedule = "schedule"
2626
)
2727

28+
// IsDefaultBranchWorkflow returns true if the event only triggers workflows on the default branch
29+
func IsDefaultBranchWorkflow(triggedEvent webhook_module.HookEventType) bool {
30+
switch triggedEvent {
31+
case webhook_module.HookEventDelete:
32+
// GitHub "delete" event
33+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#delete
34+
return true
35+
case webhook_module.HookEventFork:
36+
// GitHub "fork" event
37+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#fork
38+
return true
39+
case webhook_module.HookEventIssueComment:
40+
// GitHub "issue_comment" event
41+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issue_comment
42+
return true
43+
case webhook_module.HookEventPullRequestComment:
44+
// GitHub "pull_request_comment" event
45+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment
46+
return true
47+
case webhook_module.HookEventWiki:
48+
// GitHub "gollum" event
49+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#gollum
50+
return true
51+
case webhook_module.HookEventSchedule:
52+
// GitHub "schedule" event
53+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
54+
return true
55+
case webhook_module.HookEventIssues,
56+
webhook_module.HookEventIssueAssign,
57+
webhook_module.HookEventIssueLabel,
58+
webhook_module.HookEventIssueMilestone:
59+
// Github "issues" event
60+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#issues
61+
return true
62+
}
63+
64+
return false
65+
}
66+
2867
// canGithubEventMatch check if the input Github event can match any Gitea event.
2968
func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEventType) bool {
3069
switch eventName {
@@ -75,6 +114,11 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent
75114
case GithubEventSchedule:
76115
return triggedEvent == webhook_module.HookEventSchedule
77116

117+
case GithubEventIssueComment:
118+
// https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_comment-use-issue_comment
119+
return triggedEvent == webhook_module.HookEventIssueComment ||
120+
triggedEvent == webhook_module.HookEventPullRequestComment
121+
78122
default:
79123
return eventName == string(triggedEvent)
80124
}

modules/actions/github_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ func TestCanGithubEventMatch(t *testing.T) {
103103
webhook_module.HookEventCreate,
104104
true,
105105
},
106+
{
107+
"create pull request comment",
108+
GithubEventIssueComment,
109+
webhook_module.HookEventPullRequestComment,
110+
true,
111+
},
106112
}
107113

108114
for _, tc := range testCases {

services/actions/notifier.go

Lines changed: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -224,37 +224,88 @@ func (n *actionsNotifier) CreateIssueComment(ctx context.Context, doer *user_mod
224224
) {
225225
ctx = withMethod(ctx, "CreateIssueComment")
226226

227-
permission, _ := access_model.GetUserRepoPermission(ctx, repo, doer)
228-
229227
if issue.IsPull {
230-
if err := issue.LoadPullRequest(ctx); err != nil {
228+
notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventPullRequestComment, api.HookIssueCommentCreated)
229+
return
230+
}
231+
notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventIssueComment, api.HookIssueCommentCreated)
232+
}
233+
234+
func (n *actionsNotifier) UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment, oldContent string) {
235+
ctx = withMethod(ctx, "UpdateComment")
236+
237+
if err := c.LoadIssue(ctx); err != nil {
238+
log.Error("LoadIssue: %v", err)
239+
return
240+
}
241+
242+
if c.Issue.IsPull {
243+
notifyIssueCommentChange(ctx, doer, c, oldContent, webhook_module.HookEventPullRequestComment, api.HookIssueCommentEdited)
244+
return
245+
}
246+
notifyIssueCommentChange(ctx, doer, c, oldContent, webhook_module.HookEventIssueComment, api.HookIssueCommentEdited)
247+
}
248+
249+
func (n *actionsNotifier) DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) {
250+
ctx = withMethod(ctx, "DeleteComment")
251+
252+
if err := comment.LoadIssue(ctx); err != nil {
253+
log.Error("LoadIssue: %v", err)
254+
return
255+
}
256+
257+
if comment.Issue.IsPull {
258+
notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventPullRequestComment, api.HookIssueCommentDeleted)
259+
return
260+
}
261+
notifyIssueCommentChange(ctx, doer, comment, "", webhook_module.HookEventIssueComment, api.HookIssueCommentDeleted)
262+
}
263+
264+
func notifyIssueCommentChange(ctx context.Context, doer *user_model.User, comment *issues_model.Comment, oldContent string, event webhook_module.HookEventType, action api.HookIssueCommentAction) {
265+
if err := comment.LoadIssue(ctx); err != nil {
266+
log.Error("LoadIssue: %v", err)
267+
return
268+
}
269+
if err := comment.Issue.LoadAttributes(ctx); err != nil {
270+
log.Error("LoadAttributes: %v", err)
271+
return
272+
}
273+
274+
permission, _ := access_model.GetUserRepoPermission(ctx, comment.Issue.Repo, doer)
275+
276+
payload := &api.IssueCommentPayload{
277+
Action: action,
278+
Issue: convert.ToAPIIssue(ctx, comment.Issue),
279+
Comment: convert.ToAPIComment(ctx, comment.Issue.Repo, comment),
280+
Repository: convert.ToRepo(ctx, comment.Issue.Repo, permission),
281+
Sender: convert.ToUser(ctx, doer, nil),
282+
IsPull: comment.Issue.IsPull,
283+
}
284+
285+
if action == api.HookIssueCommentEdited {
286+
payload.Changes = &api.ChangesPayload{
287+
Body: &api.ChangesFromPayload{
288+
From: oldContent,
289+
},
290+
}
291+
}
292+
293+
if comment.Issue.IsPull {
294+
if err := comment.Issue.LoadPullRequest(ctx); err != nil {
231295
log.Error("LoadPullRequest: %v", err)
232296
return
233297
}
234-
newNotifyInputFromIssue(issue, webhook_module.HookEventPullRequestComment).
298+
newNotifyInputFromIssue(comment.Issue, event).
235299
WithDoer(doer).
236-
WithPayload(&api.IssueCommentPayload{
237-
Action: api.HookIssueCommentCreated,
238-
Issue: convert.ToAPIIssue(ctx, issue),
239-
Comment: convert.ToAPIComment(ctx, repo, comment),
240-
Repository: convert.ToRepo(ctx, repo, permission),
241-
Sender: convert.ToUser(ctx, doer, nil),
242-
IsPull: true,
243-
}).
244-
WithPullRequest(issue.PullRequest).
300+
WithPayload(payload).
301+
WithPullRequest(comment.Issue.PullRequest).
245302
Notify(ctx)
246303
return
247304
}
248-
newNotifyInputFromIssue(issue, webhook_module.HookEventIssueComment).
305+
306+
newNotifyInputFromIssue(comment.Issue, event).
249307
WithDoer(doer).
250-
WithPayload(&api.IssueCommentPayload{
251-
Action: api.HookIssueCommentCreated,
252-
Issue: convert.ToAPIIssue(ctx, issue),
253-
Comment: convert.ToAPIComment(ctx, repo, comment),
254-
Repository: convert.ToRepo(ctx, repo, permission),
255-
Sender: convert.ToUser(ctx, doer, nil),
256-
IsPull: false,
257-
}).
308+
WithPayload(payload).
258309
Notify(ctx)
259310
}
260311

@@ -496,7 +547,6 @@ func (n *actionsNotifier) DeleteRef(ctx context.Context, pusher *user_model.User
496547
apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeNone})
497548

498549
newNotifyInput(repo, pusher, webhook_module.HookEventDelete).
499-
WithRef(refFullName.ShortName()). // FIXME: should we use a full ref name
500550
WithPayload(&api.DeletePayload{
501551
Ref: refFullName.ShortName(),
502552
RefType: refFullName.RefType(),

services/actions/notifier_helper.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,15 @@ func notify(ctx context.Context, input *notifyInput) error {
136136
defer gitRepo.Close()
137137

138138
ref := input.Ref
139-
if input.Event == webhook_module.HookEventDelete {
140-
// The event is deleting a reference, so it will fail to get the commit for a deleted reference.
141-
// Set ref to empty string to fall back to the default branch.
142-
ref = ""
139+
if ref != input.Repo.DefaultBranch && actions_module.IsDefaultBranchWorkflow(input.Event) {
140+
if ref != "" {
141+
log.Warn("Event %q should only trigger workflows on the default branch, but its ref is %q. Will fall back to the default branch",
142+
input.Event, ref)
143+
}
144+
ref = input.Repo.DefaultBranch
143145
}
144146
if ref == "" {
147+
log.Warn("Ref of event %q is empty, will fall back to the default branch", input.Event)
145148
ref = input.Repo.DefaultBranch
146149
}
147150

0 commit comments

Comments
 (0)