Skip to content

Commit

Permalink
Send notifications for mentions in pulls, issues, (code-)comments (#1…
Browse files Browse the repository at this point in the history
…4218)

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"
  • Loading branch information
jpraet authored Jan 2, 2021
1 parent ac88b0e commit e6acce6
Show file tree
Hide file tree
Showing 15 changed files with 205 additions and 88 deletions.
14 changes: 14 additions & 0 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/references"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
api "code.gitea.io/gitea/modules/structs"
Expand Down Expand Up @@ -1848,6 +1849,19 @@ func (issue *Issue) updateClosedNum(e Engine) (err error) {
return
}

// FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database.
func (issue *Issue) FindAndUpdateIssueMentions(ctx DBContext, doer *User, content string) (mentions []*User, err error) {
rawMentions := references.FindAllMentionsMarkdown(content)
mentions, err = issue.ResolveMentionsByVisibility(ctx, doer, rawMentions)
if err != nil {
return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
}
if err = UpdateIssueMentions(ctx, issue.ID, mentions); err != nil {
return nil, fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
}
return
}

// ResolveMentionsByVisibility returns the users mentioned in an issue, removing those that
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
func (issue *Issue) ResolveMentionsByVisibility(ctx DBContext, doer *User, mentions []string) (users []*User, err error) {
Expand Down
8 changes: 4 additions & 4 deletions modules/notification/action/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewNotifier() base.Notifier {
return &actionNotifier{}
}

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

// NotifyCreateIssueComment notifies comment on an issue to notifiers
func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
issue *models.Issue, comment *models.Comment) {
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
act := &models.Action{
ActUserID: doer.ID,
ActUser: doer,
Expand Down Expand Up @@ -120,7 +120,7 @@ func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *model
}
}

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

func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User) {
if err := review.LoadReviewer(); err != nil {
log.Error("LoadReviewer '%d/%d': %v", review.ID, review.ReviewerID, err)
return
Expand Down
11 changes: 6 additions & 5 deletions modules/notification/base/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Notifier interface {
NotifyRenameRepository(doer *models.User, repo *models.Repository, oldRepoName string)
NotifyTransferRepository(doer *models.User, repo *models.Repository, oldOwnerName string)

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

NotifyNewPullRequest(*models.PullRequest)
NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User)
NotifyMergePullRequest(*models.PullRequest, *models.User)
NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest)
NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment)
NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User)
NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User)
NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string)
NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment)

NotifyCreateIssueComment(*models.User, *models.Repository,
*models.Issue, *models.Comment)
NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
issue *models.Issue, comment *models.Comment, mentions []*models.User)
NotifyUpdateComment(*models.User, *models.Comment, string)
NotifyDeleteComment(*models.User, *models.Comment)

Expand Down
12 changes: 8 additions & 4 deletions modules/notification/base/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,27 @@ func (*NullNotifier) Run() {

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

// NotifyNewIssue places a place holder function
func (*NullNotifier) NotifyNewIssue(issue *models.Issue) {
func (*NullNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
}

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

// NotifyNewPullRequest places a place holder function
func (*NullNotifier) NotifyNewPullRequest(pr *models.PullRequest) {
func (*NullNotifier) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
}

// NotifyPullRequestReview places a place holder function
func (*NullNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment) {
func (*NullNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment, mentions []*models.User) {
}

// NotifyPullRequestCodeComment places a place holder function
func (*NullNotifier) NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) {
}

// NotifyMergePullRequest places a place holder function
Expand Down
6 changes: 3 additions & 3 deletions modules/notification/indexer/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewNotifier() base.Notifier {
}

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

func (r *indexerNotifier) NotifyNewIssue(issue *models.Issue) {
func (r *indexerNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
issue_indexer.UpdateIssueIndexer(issue)
}

func (r *indexerNotifier) NotifyNewPullRequest(pr *models.PullRequest) {
func (r *indexerNotifier) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
issue_indexer.UpdateIssueIndexer(pr.Issue)
}

Expand Down
28 changes: 17 additions & 11 deletions modules/notification/mail/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewNotifier() base.Notifier {
}

func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
issue *models.Issue, comment *models.Comment) {
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
var act models.ActionType
if comment.Type == models.CommentTypeClose {
act = models.ActionCloseIssue
Expand All @@ -41,13 +41,13 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.
act = 0
}

if err := mailer.MailParticipantsComment(comment, act, issue); err != nil {
if err := mailer.MailParticipantsComment(comment, act, issue, mentions); err != nil {
log.Error("MailParticipantsComment: %v", err)
}
}

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

if err := mailer.MailParticipants(issue, doer, actionType); err != nil {
if err := mailer.MailParticipants(issue, doer, actionType, nil); err != nil {
log.Error("MailParticipants: %v", err)
}
}

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

func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment) {
func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment, mentions []*models.User) {
var act models.ActionType
if comment.Type == models.CommentTypeClose {
act = models.ActionCloseIssue
Expand All @@ -89,11 +89,17 @@ func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models
} else if comment.Type == models.CommentTypeComment {
act = models.ActionCommentPull
}
if err := mailer.MailParticipantsComment(comment, act, pr.Issue); err != nil {
if err := mailer.MailParticipantsComment(comment, act, pr.Issue, mentions); err != nil {
log.Error("MailParticipantsComment: %v", err)
}
}

func (m *mailNotifier) NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) {
if err := mailer.MailMentionsComment(pr, comment, mentions); err != nil {
log.Error("MailMentionsComment: %v", err)
}
}

func (m *mailNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) {
// mail only sent to added assignees and not self-assignee
if !removed && doer.ID != assignee.ID && assignee.EmailNotifications() == models.EmailNotificationsEnabled {
Expand All @@ -115,7 +121,7 @@ func (m *mailNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode
return
}
pr.Issue.Content = ""
if err := mailer.MailParticipants(pr.Issue, doer, models.ActionMergePullRequest); err != nil {
if err := mailer.MailParticipants(pr.Issue, doer, models.ActionMergePullRequest, nil); err != nil {
log.Error("MailParticipants: %v", err)
}
}
Expand Down Expand Up @@ -143,7 +149,7 @@ func (m *mailNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *model
}
comment.Content = ""

m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment)
m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment, nil)
}

func (m *mailNotifier) NotifyNewRelease(rel *models.Release) {
Expand Down
23 changes: 15 additions & 8 deletions modules/notification/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,16 @@ func NewContext() {

// NotifyCreateIssueComment notifies issue comment related message to notifiers
func NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
issue *models.Issue, comment *models.Comment) {
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
for _, notifier := range notifiers {
notifier.NotifyCreateIssueComment(doer, repo, issue, comment)
notifier.NotifyCreateIssueComment(doer, repo, issue, comment, mentions)
}
}

// NotifyNewIssue notifies new issue to notifiers
func NotifyNewIssue(issue *models.Issue) {
func NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
for _, notifier := range notifiers {
notifier.NotifyNewIssue(issue)
notifier.NotifyNewIssue(issue, mentions)
}
}

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

// NotifyNewPullRequest notifies new pull request to notifiers
func NotifyNewPullRequest(pr *models.PullRequest) {
func NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
for _, notifier := range notifiers {
notifier.NotifyNewPullRequest(pr)
notifier.NotifyNewPullRequest(pr, mentions)
}
}

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

// NotifyPullRequestReview notifies new pull request review
func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User) {
for _, notifier := range notifiers {
notifier.NotifyPullRequestReview(pr, review, comment)
notifier.NotifyPullRequestReview(pr, review, comment, mentions)
}
}

// NotifyPullRequestCodeComment notifies new pull request code comment
func NotifyPullRequestCodeComment(pr *models.PullRequest, comment *models.Comment, mentions []*models.User) {
for _, notifier := range notifiers {
notifier.NotifyPullRequestCodeComment(pr, comment, mentions)
}
}

Expand Down
55 changes: 51 additions & 4 deletions modules/notification/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (ns *notificationService) Run() {
}

func (ns *notificationService) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
issue *models.Issue, comment *models.Comment) {
issue *models.Issue, comment *models.Comment, mentions []*models.User) {
var opts = issueNotificationOpts{
IssueID: issue.ID,
NotificationAuthorID: doer.ID,
Expand All @@ -60,13 +60,31 @@ func (ns *notificationService) NotifyCreateIssueComment(doer *models.User, repo
opts.CommentID = comment.ID
}
_ = ns.issueQueue.Push(opts)
for _, mention := range mentions {
var opts = issueNotificationOpts{
IssueID: issue.ID,
NotificationAuthorID: doer.ID,
ReceiverID: mention.ID,
}
if comment != nil {
opts.CommentID = comment.ID
}
_ = ns.issueQueue.Push(opts)
}
}

func (ns *notificationService) NotifyNewIssue(issue *models.Issue) {
func (ns *notificationService) NotifyNewIssue(issue *models.Issue, mentions []*models.User) {
_ = ns.issueQueue.Push(issueNotificationOpts{
IssueID: issue.ID,
NotificationAuthorID: issue.Poster.ID,
})
for _, mention := range mentions {
_ = ns.issueQueue.Push(issueNotificationOpts{
IssueID: issue.ID,
NotificationAuthorID: issue.Poster.ID,
ReceiverID: mention.ID,
})
}
}

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

func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest) {
func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest, mentions []*models.User) {
if err := pr.LoadIssue(); err != nil {
log.Error("Unable to load issue: %d for pr: %d: Error: %v", pr.IssueID, pr.ID, err)
return
Expand All @@ -92,9 +110,16 @@ func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest) {
IssueID: pr.Issue.ID,
NotificationAuthorID: pr.Issue.PosterID,
})
for _, mention := range mentions {
_ = ns.issueQueue.Push(issueNotificationOpts{
IssueID: pr.Issue.ID,
NotificationAuthorID: pr.Issue.PosterID,
ReceiverID: mention.ID,
})
}
}

func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, c *models.Comment) {
func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, c *models.Comment, mentions []*models.User) {
var opts = issueNotificationOpts{
IssueID: pr.Issue.ID,
NotificationAuthorID: r.Reviewer.ID,
Expand All @@ -103,6 +128,28 @@ func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r
opts.CommentID = c.ID
}
_ = ns.issueQueue.Push(opts)
for _, mention := range mentions {
var opts = issueNotificationOpts{
IssueID: pr.Issue.ID,
NotificationAuthorID: r.Reviewer.ID,
ReceiverID: mention.ID,
}
if c != nil {
opts.CommentID = c.ID
}
_ = ns.issueQueue.Push(opts)
}
}

func (ns *notificationService) NotifyPullRequestCodeComment(pr *models.PullRequest, c *models.Comment, mentions []*models.User) {
for _, mention := range mentions {
_ = ns.issueQueue.Push(issueNotificationOpts{
IssueID: pr.Issue.ID,
NotificationAuthorID: c.Poster.ID,
CommentID: c.ID,
ReceiverID: mention.ID,
})
}
}

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

0 comments on commit e6acce6

Please sign in to comment.