Skip to content

Commit c8139c0

Browse files
brechtvlbrechtvl
authored
Webhooks: for issue close/reopen action, add commit ID that caused it (#22583)
The `commit_id` property name is the same as equivalent functionality in GitHub. If the action was not caused by a commit, an empty string is used. This can for example be used to automatically add a Resolved label to an issue fixed by a commit, or clear it when the issue is reopened.
1 parent a31fedd commit c8139c0

File tree

15 files changed

+25
-21
lines changed

15 files changed

+25
-21
lines changed

‎modules/notification/action/action.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (a *actionNotifier) NotifyNewIssue(ctx context.Context, issue *issues_model
5656
}
5757

5858
// NotifyIssueChangeStatus notifies close or reopen issue to notifiers
59-
func (a *actionNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool) {
59+
func (a *actionNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool) {
6060
// Compose comment action, could be plain comment, close or reopen issue/pull request.
6161
// This object will be used to notify watchers in the end of function.
6262
act := &activities_model.Action{

‎modules/notification/base/notifier.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type Notifier interface {
2323
NotifyRenameRepository(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, oldRepoName string)
2424
NotifyTransferRepository(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, oldOwnerName string)
2525
NotifyNewIssue(ctx context.Context, issue *issues_model.Issue, mentions []*user_model.User)
26-
NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool)
26+
NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool)
2727
NotifyDeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue)
2828
NotifyIssueChangeMilestone(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldMilestoneID int64)
2929
NotifyIssueChangeAssignee(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, assignee *user_model.User, removed bool, comment *issues_model.Comment)

‎modules/notification/base/null.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func (*NullNotifier) NotifyNewIssue(ctx context.Context, issue *issues_model.Iss
3232
}
3333

3434
// NotifyIssueChangeStatus places a place holder function
35-
func (*NullNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
35+
func (*NullNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
3636
}
3737

3838
// NotifyDeleteIssue notify when some issue deleted

‎modules/notification/mail/mail.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (m *mailNotifier) NotifyNewIssue(ctx context.Context, issue *issues_model.I
5454
}
5555
}
5656

57-
func (m *mailNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
57+
func (m *mailNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
5858
var actionType activities_model.ActionType
5959
if issue.IsPull {
6060
if isClosed {

‎modules/notification/notification.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ func NotifyNewIssue(ctx context.Context, issue *issues_model.Issue, mentions []*
7777
}
7878

7979
// NotifyIssueChangeStatus notifies close or reopen issue to notifiers
80-
func NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool) {
80+
func NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, closeOrReopen bool) {
8181
for _, notifier := range notifiers {
82-
notifier.NotifyIssueChangeStatus(ctx, doer, issue, actionComment, closeOrReopen)
82+
notifier.NotifyIssueChangeStatus(ctx, doer, commitID, issue, actionComment, closeOrReopen)
8383
}
8484
}
8585

‎modules/notification/ui/ui.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (ns *notificationService) NotifyNewIssue(ctx context.Context, issue *issues
9393
}
9494
}
9595

96-
func (ns *notificationService) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
96+
func (ns *notificationService) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
9797
_ = ns.issueQueue.Push(issueNotificationOpts{
9898
IssueID: issue.ID,
9999
NotificationAuthorID: doer.ID,

‎modules/structs/hook.go

+2
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ type IssuePayload struct {
352352
Issue *Issue `json:"issue"`
353353
Repository *Repository `json:"repository"`
354354
Sender *User `json:"sender"`
355+
CommitID string `json:"commit_id"`
355356
}
356357

357358
// JSONPayload encodes the IssuePayload to JSON, with an indentation of two spaces.
@@ -386,6 +387,7 @@ type PullRequestPayload struct {
386387
PullRequest *PullRequest `json:"pull_request"`
387388
Repository *Repository `json:"repository"`
388389
Sender *User `json:"sender"`
390+
CommitID string `json:"commit_id"`
389391
Review *ReviewPayload `json:"review"`
390392
}
391393

‎routers/api/v1/repo/issue.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ func CreateIssue(ctx *context.APIContext) {
654654
}
655655

656656
if form.Closed {
657-
if err := issue_service.ChangeStatus(issue, ctx.Doer, true); err != nil {
657+
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", true); err != nil {
658658
if issues_model.IsErrDependenciesLeft(err) {
659659
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
660660
return
@@ -826,7 +826,7 @@ func EditIssue(ctx *context.APIContext) {
826826
}
827827

828828
if statusChangeComment != nil {
829-
notification.NotifyIssueChangeStatus(ctx, ctx.Doer, issue, statusChangeComment, issue.IsClosed)
829+
notification.NotifyIssueChangeStatus(ctx, ctx.Doer, "", issue, statusChangeComment, issue.IsClosed)
830830
}
831831

832832
// Refetch from database to assign some automatic values

‎routers/api/v1/repo/pull.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ func EditPullRequest(ctx *context.APIContext) {
600600
}
601601

602602
if statusChangeComment != nil {
603-
notification.NotifyIssueChangeStatus(ctx, ctx.Doer, issue, statusChangeComment, issue.IsClosed)
603+
notification.NotifyIssueChangeStatus(ctx, ctx.Doer, "", issue, statusChangeComment, issue.IsClosed)
604604
}
605605

606606
// change pull target branch

‎routers/web/repo/issue.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -2599,7 +2599,7 @@ func UpdateIssueStatus(ctx *context.Context) {
25992599
}
26002600
for _, issue := range issues {
26012601
if issue.IsClosed != isClosed {
2602-
if err := issue_service.ChangeStatus(issue, ctx.Doer, isClosed); err != nil {
2602+
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", isClosed); err != nil {
26032603
if issues_model.IsErrDependenciesLeft(err) {
26042604
ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{
26052605
"error": "cannot close this issue because it still has open dependencies",
@@ -2696,7 +2696,7 @@ func NewComment(ctx *context.Context) {
26962696
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
26972697
} else {
26982698
isClosed := form.Status == "close"
2699-
if err := issue_service.ChangeStatus(issue, ctx.Doer, isClosed); err != nil {
2699+
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", isClosed); err != nil {
27002700
log.Error("ChangeStatus: %v", err)
27012701

27022702
if issues_model.IsErrDependenciesLeft(err) {

‎services/issue/commit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func UpdateIssuesCommit(doer *user_model.User, repo *repo_model.Repository, comm
193193
}
194194
if close != refIssue.IsClosed {
195195
refIssue.Repo = refRepo
196-
if err := ChangeStatus(refIssue, doer, close); err != nil {
196+
if err := ChangeStatus(refIssue, doer, c.Sha1, close); err != nil {
197197
return err
198198
}
199199
}

‎services/issue/status.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import (
1414
)
1515

1616
// ChangeStatus changes issue status to open or closed.
17-
func ChangeStatus(issue *issues_model.Issue, doer *user_model.User, closed bool) error {
18-
return changeStatusCtx(db.DefaultContext, issue, doer, closed)
17+
func ChangeStatus(issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool) error {
18+
return changeStatusCtx(db.DefaultContext, issue, doer, commitID, closed)
1919
}
2020

2121
// changeStatusCtx changes issue status to open or closed.
2222
// TODO: if context is not db.DefaultContext we get a deadlock!!!
23-
func changeStatusCtx(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, closed bool) error {
23+
func changeStatusCtx(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool) error {
2424
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, closed)
2525
if err != nil {
2626
if issues_model.IsErrDependenciesLeft(err) && closed {
@@ -37,7 +37,7 @@ func changeStatusCtx(ctx context.Context, issue *issues_model.Issue, doer *user_
3737
}
3838
}
3939

40-
notification.NotifyIssueChangeStatus(ctx, doer, issue, comment, closed)
40+
notification.NotifyIssueChangeStatus(ctx, doer, commitID, issue, comment, closed)
4141

4242
return nil
4343
}

‎services/pull/merge.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
225225
}
226226
close := ref.RefAction == references.XRefActionCloses
227227
if close != ref.Issue.IsClosed {
228-
if err = issue_service.ChangeStatus(ref.Issue, doer, close); err != nil {
228+
if err = issue_service.ChangeStatus(ref.Issue, doer, pr.MergedCommitID, close); err != nil {
229229
// Allow ErrDependenciesLeft
230230
if !issues_model.IsErrDependenciesLeft(err) {
231231
return err

‎services/pull/pull.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ func CloseBranchPulls(doer *user_model.User, repoID int64, branch string) error
532532

533533
var errs errlist
534534
for _, pr := range prs {
535-
if err = issue_service.ChangeStatus(pr.Issue, doer, true); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
535+
if err = issue_service.ChangeStatus(pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
536536
errs = append(errs, err)
537537
}
538538
}
@@ -566,7 +566,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
566566
if pr.BaseRepoID == repo.ID {
567567
continue
568568
}
569-
if err = issue_service.ChangeStatus(pr.Issue, doer, true); err != nil && !issues_model.IsErrPullWasClosed(err) {
569+
if err = issue_service.ChangeStatus(pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) {
570570
errs = append(errs, err)
571571
}
572572
}

‎services/webhook/notifier.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(ctx context.Context, doer *user
229229
}
230230
}
231231

232-
func (m *webhookNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
232+
func (m *webhookNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) {
233233
mode, _ := access_model.AccessLevel(ctx, issue.Poster, issue.Repo)
234234
var err error
235235
if issue.IsPull {
@@ -243,6 +243,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *use
243243
PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil),
244244
Repository: convert.ToRepo(ctx, issue.Repo, mode),
245245
Sender: convert.ToUser(doer, nil),
246+
CommitID: commitID,
246247
}
247248
if isClosed {
248249
apiPullRequest.Action = api.HookIssueClosed
@@ -256,6 +257,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *use
256257
Issue: convert.ToAPIIssue(ctx, issue),
257258
Repository: convert.ToRepo(ctx, issue.Repo, mode),
258259
Sender: convert.ToUser(doer, nil),
260+
CommitID: commitID,
259261
}
260262
if isClosed {
261263
apiIssue.Action = api.HookIssueClosed

0 commit comments

Comments
 (0)