From c3741d7fb0114691da73f00ae0ac9dced87e884d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=88=E7=AC=91=E9=A3=8E=E7=94=9F=E9=97=B4?= Date: Wed, 16 Oct 2024 17:10:05 +0800 Subject: [PATCH] Support requested_reviewers data in comment webhook events (#26178) 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) --- modules/structs/hook.go | 15 ++++---- services/webhook/notifier.go | 74 ++++++++++++++++++++---------------- 2 files changed, 50 insertions(+), 39 deletions(-) diff --git a/modules/structs/hook.go b/modules/structs/hook.go index b7f8861b76..56afed6ee6 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -221,13 +221,14 @@ const ( // IssueCommentPayload represents a payload information of issue comment event. type IssueCommentPayload struct { - Action HookIssueCommentAction `json:"action"` - Issue *Issue `json:"issue"` - Comment *Comment `json:"comment"` - Changes *ChangesPayload `json:"changes,omitempty"` - Repository *Repository `json:"repository"` - Sender *User `json:"sender"` - IsPull bool `json:"is_pull"` + Action HookIssueCommentAction `json:"action"` + Issue *Issue `json:"issue"` + PullRequest *PullRequest `json:"pull_request,omitempty"` + Comment *Comment `json:"comment"` + Changes *ChangesPayload `json:"changes,omitempty"` + Repository *Repository `json:"repository"` + Sender *User `json:"sender"` + IsPull bool `json:"is_pull"` } // JSONPayload implements Payload diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index a9b3422653..8bfd03024f 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -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{ Action: api.HookIssueLabelCleared, Index: issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), Repository: convert.ToRepo(ctx, issue.Repo, permission), Sender: convert.ToUser(ctx, doer, nil), }) @@ -150,7 +150,7 @@ func (m *webhookNotifier) IssueChangeAssignee(ctx context.Context, doer *user_mo } apiPullRequest := &api.PullRequestPayload{ Index: issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), Repository: convert.ToRepo(ctx, issue.Repo, permission), Sender: convert.ToUser(ctx, doer, nil), } @@ -201,7 +201,7 @@ func (m *webhookNotifier) IssueChangeTitle(ctx context.Context, doer *user_model From: oldTitle, }, }, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), Repository: convert.ToRepo(ctx, issue.Repo, permission), 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. apiPullRequest := &api.PullRequestPayload{ Index: issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), Repository: convert.ToRepo(ctx, issue.Repo, permission), Sender: convert.ToUser(ctx, doer, nil), 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{ Action: api.HookIssueOpened, 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), Sender: convert.ToUser(ctx, pull.Issue.Poster, nil), }); err != nil { @@ -336,7 +336,7 @@ func (m *webhookNotifier) IssueChangeContent(ctx context.Context, doer *user_mod From: oldContent, }, }, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), Repository: convert.ToRepo(ctx, issue.Repo, permission), Sender: convert.ToUser(ctx, doer, nil), }) @@ -375,17 +375,20 @@ func (m *webhookNotifier) UpdateComment(ctx context.Context, doer *user_model.Us } var eventType webhook_module.HookEventType + var pullRequest *api.PullRequest if c.Issue.IsPull { eventType = webhook_module.HookEventPullRequestComment + pullRequest = convert.ToAPIPullRequest(ctx, c.Issue.PullRequest, doer) } else { eventType = webhook_module.HookEventIssueComment } permission, _ := access_model.GetUserRepoPermission(ctx, c.Issue.Repo, doer) if err := PrepareWebhooks(ctx, EventSource{Repository: c.Issue.Repo}, eventType, &api.IssueCommentPayload{ - Action: api.HookIssueCommentEdited, - Issue: convert.ToAPIIssue(ctx, doer, c.Issue), - Comment: convert.ToAPIComment(ctx, c.Issue.Repo, c), + Action: api.HookIssueCommentEdited, + Issue: convert.ToAPIIssue(ctx, doer, c.Issue), + PullRequest: pullRequest, + Comment: convert.ToAPIComment(ctx, c.Issue.Repo, c), Changes: &api.ChangesPayload{ Body: &api.ChangesFromPayload{ From: oldContent, @@ -403,20 +406,23 @@ func (m *webhookNotifier) CreateIssueComment(ctx context.Context, doer *user_mod issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, ) { var eventType webhook_module.HookEventType + var pullRequest *api.PullRequest if issue.IsPull { eventType = webhook_module.HookEventPullRequestComment + pullRequest = convert.ToAPIPullRequest(ctx, issue.PullRequest, doer) } else { eventType = webhook_module.HookEventIssueComment } permission, _ := access_model.GetUserRepoPermission(ctx, repo, doer) if err := PrepareWebhooks(ctx, EventSource{Repository: issue.Repo}, eventType, &api.IssueCommentPayload{ - Action: api.HookIssueCommentCreated, - Issue: convert.ToAPIIssue(ctx, doer, issue), - Comment: convert.ToAPIComment(ctx, repo, comment), - Repository: convert.ToRepo(ctx, repo, permission), - Sender: convert.ToUser(ctx, doer, nil), - IsPull: issue.IsPull, + Action: api.HookIssueCommentCreated, + Issue: convert.ToAPIIssue(ctx, doer, issue), + PullRequest: pullRequest, + Comment: convert.ToAPIComment(ctx, repo, comment), + Repository: convert.ToRepo(ctx, repo, permission), + Sender: convert.ToUser(ctx, doer, nil), + IsPull: issue.IsPull, }); err != nil { log.Error("PrepareWebhooks [comment_id: %d]: %v", comment.ID, err) } @@ -440,20 +446,23 @@ func (m *webhookNotifier) DeleteComment(ctx context.Context, doer *user_model.Us } var eventType webhook_module.HookEventType + var pullRequest *api.PullRequest if comment.Issue.IsPull { eventType = webhook_module.HookEventPullRequestComment + pullRequest = convert.ToAPIPullRequest(ctx, comment.Issue.PullRequest, doer) } else { eventType = webhook_module.HookEventIssueComment } permission, _ := access_model.GetUserRepoPermission(ctx, comment.Issue.Repo, doer) if err := PrepareWebhooks(ctx, EventSource{Repository: comment.Issue.Repo}, eventType, &api.IssueCommentPayload{ - Action: api.HookIssueCommentDeleted, - Issue: convert.ToAPIIssue(ctx, doer, 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, + Action: api.HookIssueCommentDeleted, + Issue: convert.ToAPIIssue(ctx, doer, comment.Issue), + PullRequest: pullRequest, + 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, }); err != nil { log.Error("PrepareWebhooks [comment_id: %d]: %v", comment.ID, err) } @@ -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{ Action: api.HookIssueLabelUpdated, 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}), 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{ Action: hookAction, Index: issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), Repository: convert.ToRepo(ctx, issue.Repo, permission), 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. apiPullRequest := &api.PullRequestPayload{ Index: pr.Issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), + PullRequest: convert.ToAPIPullRequest(ctx, pr, doer), Repository: convert.ToRepo(ctx, pr.Issue.Repo, permission), Sender: convert.ToUser(ctx, doer, nil), Action: api.HookIssueClosed, @@ -668,7 +677,7 @@ func (m *webhookNotifier) PullRequestChangeTargetBranch(ctx context.Context, doe From: oldBranch, }, }, - PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), + PullRequest: convert.ToAPIPullRequest(ctx, pr, doer), Repository: convert.ToRepo(ctx, issue.Repo, mode), Sender: convert.ToUser(ctx, doer, nil), }); err != nil { @@ -703,11 +712,12 @@ func (m *webhookNotifier) PullRequestReview(ctx context.Context, pr *issues_mode return } if err := PrepareWebhooks(ctx, EventSource{Repository: review.Issue.Repo}, reviewHookType, &api.PullRequestPayload{ - Action: api.HookIssueReviewed, - Index: review.Issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, pr, nil), - Repository: convert.ToRepo(ctx, review.Issue.Repo, permission), - Sender: convert.ToUser(ctx, review.Reviewer, nil), + Action: api.HookIssueReviewed, + Index: review.Issue.Index, + PullRequest: convert.ToAPIPullRequest(ctx, pr, review.Reviewer), + RequestedReviewer: convert.ToUser(ctx, review.Reviewer, nil), + Repository: convert.ToRepo(ctx, review.Issue.Repo, permission), + Sender: convert.ToUser(ctx, review.Reviewer, nil), Review: &api.ReviewPayload{ Type: string(reviewHookType), Content: review.Content, @@ -729,7 +739,7 @@ func (m *webhookNotifier) PullRequestReviewRequest(ctx context.Context, doer *us } apiPullRequest := &api.PullRequestPayload{ Index: issue.Index, - PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil), + PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, doer), RequestedReviewer: convert.ToUser(ctx, reviewer, nil), Repository: convert.ToRepo(ctx, issue.Repo, permission), 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{ Action: api.HookIssueSynchronized, 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}), Sender: convert.ToUser(ctx, doer, nil), }); err != nil {