Skip to content

Commit e6acce6

Browse files
jpraetjpraet
authored
Send notifications for mentions in pulls, issues, (code-)comments (#14218)
Fixes #14187: mention handling extracted from email notification code Fixes #14013: add notification for mentions in pull request code comments Fixes #13450: Not receiving any emails with setting "Only Email on Mention"
1 parent ac88b0e commit e6acce6

File tree

15 files changed

+205
-88
lines changed

15 files changed

+205
-88
lines changed

‎models/issue.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"code.gitea.io/gitea/modules/base"
1616
"code.gitea.io/gitea/modules/log"
17+
"code.gitea.io/gitea/modules/references"
1718
"code.gitea.io/gitea/modules/setting"
1819
"code.gitea.io/gitea/modules/structs"
1920
api "code.gitea.io/gitea/modules/structs"
@@ -1848,6 +1849,19 @@ func (issue *Issue) updateClosedNum(e Engine) (err error) {
18481849
return
18491850
}
18501851

1852+
// FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database.
1853+
func (issue *Issue) FindAndUpdateIssueMentions(ctx DBContext, doer *User, content string) (mentions []*User, err error) {
1854+
rawMentions := references.FindAllMentionsMarkdown(content)
1855+
mentions, err = issue.ResolveMentionsByVisibility(ctx, doer, rawMentions)
1856+
if err != nil {
1857+
return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
1858+
}
1859+
if err = UpdateIssueMentions(ctx, issue.ID, mentions); err != nil {
1860+
return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
1861+
}
1862+
return
1863+
}
1864+
18511865
// ResolveMentionsByVisibility returns the users mentioned in an issue, removing those that
18521866
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
18531867
func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, mentions []string) (users []*User, err error) {

‎modules/notification/action/action.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func NewNotifier() base.Notifier {
2929
return &actionNotifier{}
3030
}
3131

32-
func (a *actionNotifier) NotifyNewIssue(issue *models.Issue) {
32+
func (a *actionNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
3333
if err := issue.LoadPoster(); err != nil {
3434
log.Error("issue.LoadPoster: %v", err)
3535
return
@@ -88,7 +88,7 @@ func (a *actionNotifier) NotifyIssueChangeStatus(doer *models.User, issue *model
8888

8989
// NotifyCreateIssueComment notifies comment on an issue to notifiers
9090
func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
91-
issue *models.Issue, comment *models.Comment) {
91+
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
9292
act := &models.Action{
9393
ActUserID: doer.ID,
9494
ActUser: doer,
@@ -120,7 +120,7 @@ func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *model
120120
}
121121
}
122122

123-
func (a *actionNotifier) NotifyNewPullRequest(pull *models.PullRequest) {
123+
func (a *actionNotifier) NotifyNewPullRequest(pull *models.PullRequest, mentions []*models.User) {
124124
if err := pull.LoadIssue(); err != nil {
125125
log.Error("pull.LoadIssue: %v", err)
126126
return
@@ -203,7 +203,7 @@ func (a *actionNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *
203203
}
204204
}
205205

206-
func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
206+
func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User) {
207207
if err := review.LoadReviewer(); err != nil {
208208
log.Error("LoadReviewer '%d/%d': %v", review.ID, review.ReviewerID, err)
209209
return

‎modules/notification/base/notifier.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type Notifier interface {
2020
NotifyRenameRepository(doer *models.User, repo *models.Repository, oldRepoName string)
2121
NotifyTransferRepository(doer *models.User, repo *models.Repository, oldOwnerName string)
2222

23-
NotifyNewIssue(*models.Issue)
23+
NotifyNewIssue(issue *models.Issue, mentions []*models.User)
2424
NotifyIssueChangeStatus(*models.User, *models.Issue, *models.Comment, bool)
2525
NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue, oldMilestoneID int64)
2626
NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment)
@@ -32,15 +32,16 @@ type Notifier interface {
3232
NotifyIssueChangeLabels(doer *models.User, issue *models.Issue,
3333
addedLabels []*models.Label, removedLabels []*models.Label)
3434

35-
NotifyNewPullRequest(*models.PullRequest)
35+
NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User)
3636
NotifyMergePullRequest(*models.PullRequest, *models.User)
3737
NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest)
38-
NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment)
38+
NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User)
39+
NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User)
3940
NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string)
4041
NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment)
4142

42-
NotifyCreateIssueComment(*models.User, *models.Repository,
43-
*models.Issue, *models.Comment)
43+
NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
44+
issue *models.Issue, comment *models.Comment, mentions []*models.User)
4445
NotifyUpdateComment(*models.User, *models.Comment, string)
4546
NotifyDeleteComment(*models.User, *models.Comment)
4647

‎modules/notification/base/null.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,27 @@ func (*NullNotifier) Run() {
2323

2424
// NotifyCreateIssueComment places a place holder function
2525
func (*NullNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
26-
issue *models.Issue, comment *models.Comment) {
26+
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
2727
}
2828

2929
// NotifyNewIssue places a place holder function
30-
func (*NullNotifier) NotifyNewIssue(issue *models.Issue) {
30+
func (*NullNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
3131
}
3232

3333
// NotifyIssueChangeStatus places a place holder function
3434
func (*NullNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) {
3535
}
3636

3737
// NotifyNewPullRequest places a place holder function
38-
func (*NullNotifier) NotifyNewPullRequest(pr *models.PullRequest) {
38+
func (*NullNotifier) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
3939
}
4040

4141
// NotifyPullRequestReview places a place holder function
42-
func (*NullNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment) {
42+
func (*NullNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment, mentions []*models.User) {
43+
}
44+
45+
// NotifyPullRequestCodeComment places a place holder function
46+
func (*NullNotifier) NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) {
4347
}
4448

4549
// NotifyMergePullRequest places a place holder function

‎modules/notification/indexer/indexer.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func NewNotifier() base.Notifier {
3030
}
3131

3232
func (r *indexerNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
33-
issue *models.Issue, comment *models.Comment) {
33+
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
3434
if comment.Type == models.CommentTypeComment {
3535
if issue.Comments == nil {
3636
if err := issue.LoadDiscussComments(); err != nil {
@@ -45,11 +45,11 @@ func (r *indexerNotifier) NotifyCreateIssueComment(doer *models.User, repo *mode
4545
}
4646
}
4747

48-
func (r *indexerNotifier) NotifyNewIssue(issue *models.Issue) {
48+
func (r *indexerNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
4949
issue_indexer.UpdateIssueIndexer(issue)
5050
}
5151

52-
func (r *indexerNotifier) NotifyNewPullRequest(pr *models.PullRequest) {
52+
func (r *indexerNotifier) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
5353
issue_indexer.UpdateIssueIndexer(pr.Issue)
5454
}
5555

‎modules/notification/mail/mail.go‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func NewNotifier() base.Notifier {
2727
}
2828

2929
func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
30-
issue *models.Issue, comment *models.Comment) {
30+
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
3131
var act models.ActionType
3232
if comment.Type == models.CommentTypeClose {
3333
act = models.ActionCloseIssue
@@ -41,13 +41,13 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.
4141
act = 0
4242
}
4343

44-
if err := mailer.MailParticipantsComment(comment, act, issue); err != nil {
44+
if err := mailer.MailParticipantsComment(comment, act, issue, mentions); err != nil {
4545
log.Error("MailParticipantsComment: %v", err)
4646
}
4747
}
4848

49-
func (m *mailNotifier) NotifyNewIssue(issue *models.Issue) {
50-
if err := mailer.MailParticipants(issue, issue.Poster, models.ActionCreateIssue); err != nil {
49+
func (m *mailNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
50+
if err := mailer.MailParticipants(issue, issue.Poster, models.ActionCreateIssue, mentions); err != nil {
5151
log.Error("MailParticipants: %v", err)
5252
}
5353
}
@@ -69,18 +69,18 @@ func (m *mailNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.
6969
}
7070
}
7171

72-
if err := mailer.MailParticipants(issue, doer, actionType); err != nil {
72+
if err := mailer.MailParticipants(issue, doer, actionType, nil); err != nil {
7373
log.Error("MailParticipants: %v", err)
7474
}
7575
}
7676

77-
func (m *mailNotifier) NotifyNewPullRequest(pr *models.PullRequest) {
78-
if err := mailer.MailParticipants(pr.Issue, pr.Issue.Poster, models.ActionCreatePullRequest); err != nil {
77+
func (m *mailNotifier) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
78+
if err := mailer.MailParticipants(pr.Issue, pr.Issue.Poster, models.ActionCreatePullRequest, mentions); err != nil {
7979
log.Error("MailParticipants: %v", err)
8080
}
8181
}
8282

83-
func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment) {
83+
func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment, mentions []*models.User) {
8484
var act models.ActionType
8585
if comment.Type == models.CommentTypeClose {
8686
act = models.ActionCloseIssue
@@ -89,11 +89,17 @@ func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models
8989
} else if comment.Type == models.CommentTypeComment {
9090
act = models.ActionCommentPull
9191
}
92-
if err := mailer.MailParticipantsComment(comment, act, pr.Issue); err != nil {
92+
if err := mailer.MailParticipantsComment(comment, act, pr.Issue, mentions); err != nil {
9393
log.Error("MailParticipantsComment: %v", err)
9494
}
9595
}
9696

97+
func (m *mailNotifier) NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) {
98+
if err := mailer.MailMentionsComment(pr, comment, mentions); err != nil {
99+
log.Error("MailMentionsComment: %v", err)
100+
}
101+
}
102+
97103
func (m *mailNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) {
98104
// mail only sent to added assignees and not self-assignee
99105
if !removed && doer.ID != assignee.ID && assignee.EmailNotifications() == models.EmailNotificationsEnabled {
@@ -115,7 +121,7 @@ func (m *mailNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode
115121
return
116122
}
117123
pr.Issue.Content = ""
118-
if err := mailer.MailParticipants(pr.Issue, doer, models.ActionMergePullRequest); err != nil {
124+
if err := mailer.MailParticipants(pr.Issue, doer, models.ActionMergePullRequest, nil); err != nil {
119125
log.Error("MailParticipants: %v", err)
120126
}
121127
}
@@ -143,7 +149,7 @@ func (m *mailNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *model
143149
}
144150
comment.Content = ""
145151

146-
m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment)
152+
m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment, nil)
147153
}
148154

149155
func (m *mailNotifier) NotifyNewRelease(rel *models.Release) {

‎modules/notification/notification.go‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,16 @@ func NewContext() {
3939

4040
// NotifyCreateIssueComment notifies issue comment related message to notifiers
4141
func NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
42-
issue *models.Issue, comment *models.Comment) {
42+
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
4343
for _, notifier := range notifiers {
44-
notifier.NotifyCreateIssueComment(doer, repo, issue, comment)
44+
notifier.NotifyCreateIssueComment(doer, repo, issue, comment, mentions)
4545
}
4646
}
4747

4848
// NotifyNewIssue notifies new issue to notifiers
49-
func NotifyNewIssue(issue *models.Issue) {
49+
func NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
5050
for _, notifier := range notifiers {
51-
notifier.NotifyNewIssue(issue)
51+
notifier.NotifyNewIssue(issue, mentions)
5252
}
5353
}
5454

@@ -67,9 +67,9 @@ func NotifyMergePullRequest(pr *models.PullRequest, doer *models.User) {
6767
}
6868

6969
// NotifyNewPullRequest notifies new pull request to notifiers
70-
func NotifyNewPullRequest(pr *models.PullRequest) {
70+
func NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
7171
for _, notifier := range notifiers {
72-
notifier.NotifyNewPullRequest(pr)
72+
notifier.NotifyNewPullRequest(pr, mentions)
7373
}
7474
}
7575

@@ -81,9 +81,16 @@ func NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest) {
8181
}
8282

8383
// NotifyPullRequestReview notifies new pull request review
84-
func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
84+
func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User) {
8585
for _, notifier := range notifiers {
86-
notifier.NotifyPullRequestReview(pr, review, comment)
86+
notifier.NotifyPullRequestReview(pr, review, comment, mentions)
87+
}
88+
}
89+
90+
// NotifyPullRequestCodeComment notifies new pull request code comment
91+
func NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) {
92+
for _, notifier := range notifiers {
93+
notifier.NotifyPullRequestCodeComment(pr, comment, mentions)
8794
}
8895
}
8996

‎modules/notification/ui/ui.go‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (ns *notificationService) Run() {
5151
}
5252

5353
func (ns *notificationService) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
54-
issue *models.Issue, comment *models.Comment) {
54+
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
5555
var opts = issueNotificationOpts{
5656
IssueID: issue.ID,
5757
NotificationAuthorID: doer.ID,
@@ -60,13 +60,31 @@ func (ns *notificationService) NotifyCreateIssueComment(doer *models.User, repo
6060
opts.CommentID = comment.ID
6161
}
6262
_ = ns.issueQueue.Push(opts)
63+
for _, mention := range mentions {
64+
var opts = issueNotificationOpts{
65+
IssueID: issue.ID,
66+
NotificationAuthorID: doer.ID,
67+
ReceiverID: mention.ID,
68+
}
69+
if comment != nil {
70+
opts.CommentID = comment.ID
71+
}
72+
_ = ns.issueQueue.Push(opts)
73+
}
6374
}
6475

65-
func (ns *notificationService) NotifyNewIssue(issue *models.Issue) {
76+
func (ns *notificationService) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
6677
_ = ns.issueQueue.Push(issueNotificationOpts{
6778
IssueID: issue.ID,
6879
NotificationAuthorID: issue.Poster.ID,
6980
})
81+
for _, mention := range mentions {
82+
_ = ns.issueQueue.Push(issueNotificationOpts{
83+
IssueID: issue.ID,
84+
NotificationAuthorID: issue.Poster.ID,
85+
ReceiverID: mention.ID,
86+
})
87+
}
7088
}
7189

7290
func (ns *notificationService) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) {
@@ -83,7 +101,7 @@ func (ns *notificationService) NotifyMergePullRequest(pr *models.PullRequest, do
83101
})
84102
}
85103

86-
func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest) {
104+
func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
87105
if err := pr.LoadIssue(); err != nil {
88106
log.Error("Unable to load issue: %d for pr: %d: Error: %v", pr.IssueID, pr.ID, err)
89107
return
@@ -92,9 +110,16 @@ func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest) {
92110
IssueID: pr.Issue.ID,
93111
NotificationAuthorID: pr.Issue.PosterID,
94112
})
113+
for _, mention := range mentions {
114+
_ = ns.issueQueue.Push(issueNotificationOpts{
115+
IssueID: pr.Issue.ID,
116+
NotificationAuthorID: pr.Issue.PosterID,
117+
ReceiverID: mention.ID,
118+
})
119+
}
95120
}
96121

97-
func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, c *models.Comment) {
122+
func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, c *models.Comment, mentions []*models.User) {
98123
var opts = issueNotificationOpts{
99124
IssueID: pr.Issue.ID,
100125
NotificationAuthorID: r.Reviewer.ID,
@@ -103,6 +128,28 @@ func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r
103128
opts.CommentID = c.ID
104129
}
105130
_ = ns.issueQueue.Push(opts)
131+
for _, mention := range mentions {
132+
var opts = issueNotificationOpts{
133+
IssueID: pr.Issue.ID,
134+
NotificationAuthorID: r.Reviewer.ID,
135+
ReceiverID: mention.ID,
136+
}
137+
if c != nil {
138+
opts.CommentID = c.ID
139+
}
140+
_ = ns.issueQueue.Push(opts)
141+
}
142+
}
143+
144+
func (ns *notificationService) NotifyPullRequestCodeComment(pr *models.PullRequest, c *models.Comment, mentions []*models.User) {
145+
for _, mention := range mentions {
146+
_ = ns.issueQueue.Push(issueNotificationOpts{
147+
IssueID: pr.Issue.ID,
148+
NotificationAuthorID: c.Poster.ID,
149+
CommentID: c.ID,
150+
ReceiverID: mention.ID,
151+
})
152+
}
106153
}
107154

108155
func (ns *notificationService) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) {

0 commit comments

Comments
 (0)