tom/forkjo
tom/forkjo
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions 1

Support requested_reviewers data in comment webhook events (#26178)

Browse source 
close #25833

Currently, the information for "requested_reviewers" is only included in
the webhook event for reviews. I would like to suggest adding this
information to the webhook event for "PullRequest comment" as well, as
they both pertain to the "PullRequest" event.

Also, The reviewer information for the Pull Request is not displayed
when it is approved or rejected.

(cherry picked from commit d50ed0abf731a10741831d4b6dd54791e3e567ec)
This commit is contained in:
谈笑风生间 2024-10-16 17:10:05 +08:00 committed by Gergely Nagy
parent 3aaf6f72d6
commit c3741d7fb0
No known key found for this signature in database
2 changed files with 50 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
modules/structs/hook.go
View file

@ -223,6 +223,7 @@ const (
type IssueCommentPayload struct { type IssueCommentPayload struct {
Action HookIssueCommentAction `json:"action"` Action HookIssueCommentAction `json:"action"`
Issue *Issue `json:"issue"` Issue *Issue `json:"issue"`
PullRequest *PullRequest `json:"pull_request,omitempty"`
Comment *Comment `json:"comment"` Comment *Comment `json:"comment"`
Changes *ChangesPayload `json:"changes,omitempty"` Changes *ChangesPayload `json:"changes,omitempty"`
Repository *Repository `json:"repository"` Repository *Repository `json:"repository"`

36
services/webhook/notifier.go
View file

@ -59,7 +59,7 @@ func (m *webhookNotifier) IssueClearLabels(ctx context.Context, doer *user_model
err = PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequestLabel, &api.PullRequestPayload{ err = PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequestLabel, &api.PullRequestPayload{
Action: api.HookIssueLabelCleared, Action: api.HookIssueLabelCleared,
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}) })
@ -150,7 +150,7 @@ func (m *webhookNotifier) IssueChangeAssignee(ctx context.Context, doer *user_mo
} }
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
} }
@ -201,7 +201,7 @@ func (m *webhookNotifier) IssueChangeTitle(ctx context.Context, doer *user_model
From: oldTitle, From: oldTitle,
}, },
}, },
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}) })
@ -236,7 +236,7 @@ func (m *webhookNotifier) IssueChangeStatus(ctx context.Context, doer *user_mode
// Merge pull request calls issue.changeStatus so we need to handle separately. // Merge pull request calls issue.changeStatus so we need to handle separately.
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
CommitID: commitID, CommitID: commitID,
@ -307,7 +307,7 @@ func (m *webhookNotifier) NewPullRequest(ctx context.Context, pull *issues_model
if err := PrepareWebhooks(ctx, EventSource{Repository: pull.Issue.Repo}, webhook_module.HookEventPullRequest, &api.PullRequestPayload{ if err := PrepareWebhooks(ctx, EventSource{Repository: pull.Issue.Repo}, webhook_module.HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueOpened, Action: api.HookIssueOpened,
Index: pull.Issue.Index, Index: pull.Issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, pull, nil), PullRequest: convert.ToAPIPullRequest(ctx, pull, pull.Issue.Poster),
Repository: convert.ToRepo(ctx, pull.Issue.Repo, permission), Repository: convert.ToRepo(ctx, pull.Issue.Repo, permission),
Sender: convert.ToUser(ctx, pull.Issue.Poster, nil), Sender: convert.ToUser(ctx, pull.Issue.Poster, nil),
}); err != nil { }); err != nil {
@ -336,7 +336,7 @@ func (m *webhookNotifier) IssueChangeContent(ctx context.Context, doer *user_mod
From: oldContent, From: oldContent,
}, },
}, },
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}) })
@ -375,8 +375,10 @@ func (m *webhookNotifier) UpdateComment(ctx context.Context, doer *user_model.Us
} }
var eventType webhook_module.HookEventType var eventType webhook_module.HookEventType
var pullRequest *api.PullRequest
if c.Issue.IsPull { if c.Issue.IsPull {
eventType = webhook_module.HookEventPullRequestComment eventType = webhook_module.HookEventPullRequestComment
pullRequest = convert.ToAPIPullRequest(ctx, c.Issue.PullRequest, doer)
} else { } else {
eventType = webhook_module.HookEventIssueComment eventType = webhook_module.HookEventIssueComment
} }
@ -385,6 +387,7 @@ func (m *webhookNotifier) UpdateComment(ctx context.Context, doer *user_model.Us
if err := PrepareWebhooks(ctx, EventSource{Repository: c.Issue.Repo}, eventType, &api.IssueCommentPayload{ if err := PrepareWebhooks(ctx, EventSource{Repository: c.Issue.Repo}, eventType, &api.IssueCommentPayload{
Action: api.HookIssueCommentEdited, Action: api.HookIssueCommentEdited,
Issue: convert.ToAPIIssue(ctx, doer, c.Issue), Issue: convert.ToAPIIssue(ctx, doer, c.Issue),
PullRequest: pullRequest,
Comment: convert.ToAPIComment(ctx, c.Issue.Repo, c), Comment: convert.ToAPIComment(ctx, c.Issue.Repo, c),
Changes: &api.ChangesPayload{ Changes: &api.ChangesPayload{
Body: &api.ChangesFromPayload{ Body: &api.ChangesFromPayload{
@ -403,8 +406,10 @@ func (m *webhookNotifier) CreateIssueComment(ctx context.Context, doer *user_mod
issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User,
) { ) {
var eventType webhook_module.HookEventType var eventType webhook_module.HookEventType
var pullRequest *api.PullRequest
if issue.IsPull { if issue.IsPull {
eventType = webhook_module.HookEventPullRequestComment eventType = webhook_module.HookEventPullRequestComment
pullRequest = convert.ToAPIPullRequest(ctx, issue.PullRequest, doer)
} else { } else {
eventType = webhook_module.HookEventIssueComment eventType = webhook_module.HookEventIssueComment
} }
@ -413,6 +418,7 @@ func (m *webhookNotifier) CreateIssueComment(ctx context.Context, doer *user_mod
if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, eventType, &api.IssueCommentPayload{ if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, eventType, &api.IssueCommentPayload{
Action: api.HookIssueCommentCreated, Action: api.HookIssueCommentCreated,
Issue: convert.ToAPIIssue(ctx, doer, issue), Issue: convert.ToAPIIssue(ctx, doer, issue),
PullRequest: pullRequest,
Comment: convert.ToAPIComment(ctx, repo, comment), Comment: convert.ToAPIComment(ctx, repo, comment),
Repository: convert.ToRepo(ctx, repo, permission), Repository: convert.ToRepo(ctx, repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
@ -440,8 +446,10 @@ func (m *webhookNotifier) DeleteComment(ctx context.Context, doer *user_model.Us
} }
var eventType webhook_module.HookEventType var eventType webhook_module.HookEventType
var pullRequest *api.PullRequest
if comment.Issue.IsPull { if comment.Issue.IsPull {
eventType = webhook_module.HookEventPullRequestComment eventType = webhook_module.HookEventPullRequestComment
pullRequest = convert.ToAPIPullRequest(ctx, comment.Issue.PullRequest, doer)
} else { } else {
eventType = webhook_module.HookEventIssueComment eventType = webhook_module.HookEventIssueComment
} }
@ -450,6 +458,7 @@ func (m *webhookNotifier) DeleteComment(ctx context.Context, doer *user_model.Us
if err := PrepareWebhooks(ctx, EventSource{Repository: comment.Issue.Repo}, eventType, &api.IssueCommentPayload{ if err := PrepareWebhooks(ctx, EventSource{Repository: comment.Issue.Repo}, eventType, &api.IssueCommentPayload{
Action: api.HookIssueCommentDeleted, Action: api.HookIssueCommentDeleted,
Issue: convert.ToAPIIssue(ctx, doer, comment.Issue), Issue: convert.ToAPIIssue(ctx, doer, comment.Issue),
PullRequest: pullRequest,
Comment: convert.ToAPIComment(ctx, comment.Issue.Repo, comment), Comment: convert.ToAPIComment(ctx, comment.Issue.Repo, comment),
Repository: convert.ToRepo(ctx, comment.Issue.Repo, permission), Repository: convert.ToRepo(ctx, comment.Issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
@ -525,7 +534,7 @@ func (m *webhookNotifier) IssueChangeLabels(ctx context.Context, doer *user_mode
err = PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequestLabel, &api.PullRequestPayload{ err = PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequestLabel, &api.PullRequestPayload{
Action: api.HookIssueLabelUpdated, Action: api.HookIssueLabelUpdated,
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm.AccessModeOwner}), Repository: convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm.AccessModeOwner}),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}) })
@ -567,7 +576,7 @@ func (m *webhookNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m
err = PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequestMilestone, &api.PullRequestPayload{ err = PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, webhook_module.HookEventPullRequestMilestone, &api.PullRequestPayload{
Action: hookAction, Action: hookAction,
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}) })
@ -640,7 +649,7 @@ func (*webhookNotifier) MergePullRequest(ctx context.Context, doer *user_model.U
// Merge pull request calls issue.changeStatus so we need to handle separately. // Merge pull request calls issue.changeStatus so we need to handle separately.
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: pr.Issue.Index, Index: pr.Issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), PullRequest: convert.ToAPIPullRequest(ctx, pr, doer),
Repository: convert.ToRepo(ctx, pr.Issue.Repo, permission), Repository: convert.ToRepo(ctx, pr.Issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
Action: api.HookIssueClosed, Action: api.HookIssueClosed,
@ -668,7 +677,7 @@ func (m *webhookNotifier) PullRequestChangeTargetBranch(ctx context.Context, doe
From: oldBranch, From: oldBranch,
}, },
}, },
PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), PullRequest: convert.ToAPIPullRequest(ctx, pr, doer),
Repository: convert.ToRepo(ctx, issue.Repo, mode), Repository: convert.ToRepo(ctx, issue.Repo, mode),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}); err != nil { }); err != nil {
@ -705,7 +714,8 @@ func (m *webhookNotifier) PullRequestReview(ctx context.Context, pr *issues_mode
if err := PrepareWebhooks(ctx, EventSource{Repository: review.Issue.Repo}, reviewHookType, &api.PullRequestPayload{ if err := PrepareWebhooks(ctx, EventSource{Repository: review.Issue.Repo}, reviewHookType, &api.PullRequestPayload{
Action: api.HookIssueReviewed, Action: api.HookIssueReviewed,
Index: review.Issue.Index, Index: review.Issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), PullRequest: convert.ToAPIPullRequest(ctx, pr, review.Reviewer),
RequestedReviewer: convert.ToUser(ctx, review.Reviewer, nil),
Repository: convert.ToRepo(ctx, review.Issue.Repo, permission), Repository: convert.ToRepo(ctx, review.Issue.Repo, permission),
Sender: convert.ToUser(ctx, review.Reviewer, nil), Sender: convert.ToUser(ctx, review.Reviewer, nil),
Review: &api.ReviewPayload{ Review: &api.ReviewPayload{
@ -729,7 +739,7 @@ func (m *webhookNotifier) PullRequestReviewRequest(ctx context.Context, doer *us
} }
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer),
RequestedReviewer: convert.ToUser(ctx, reviewer, nil), RequestedReviewer: convert.ToUser(ctx, reviewer, nil),
Repository: convert.ToRepo(ctx, issue.Repo, permission), Repository: convert.ToRepo(ctx, issue.Repo, permission),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
@ -773,7 +783,7 @@ func (m *webhookNotifier) PullRequestSynchronized(ctx context.Context, doer *use
if err := PrepareWebhooks(ctx, EventSource{Repository: pr.Issue.Repo}, webhook_module.HookEventPullRequestSync, &api.PullRequestPayload{ if err := PrepareWebhooks(ctx, EventSource{Repository: pr.Issue.Repo}, webhook_module.HookEventPullRequestSync, &api.PullRequestPayload{
Action: api.HookIssueSynchronized, Action: api.HookIssueSynchronized,
Index: pr.Issue.Index, Index: pr.Issue.Index,
PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), PullRequest: convert.ToAPIPullRequest(ctx, pr, doer),
Repository: convert.ToRepo(ctx, pr.Issue.Repo, access_model.Permission{AccessMode: perm.AccessModeOwner}), Repository: convert.ToRepo(ctx, pr.Issue.Repo, access_model.Permission{AccessMode: perm.AccessModeOwner}),
Sender: convert.ToUser(ctx, doer, nil), Sender: convert.ToUser(ctx, doer, nil),
}); err != nil { }); err != nil {