Skip to content

Commit

Permalink
Support requested_reviewers data in comment webhook events (#26178)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Makonike
Makonike authored Oct 16, 2024
1 parent 5242e52 commit d50ed0a
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 39 deletions.
15 changes: 8 additions & 7 deletions modules/structs/hook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,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
Expand Down
74 changes: 42 additions & 32 deletions services/webhook/notifier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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),
})
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
})
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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),
})
Expand Down Expand Up @@ -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),
})
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -774,7 +784,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 {
Expand Down

0 comments on commit d50ed0a

Please sign in to comment.