From e6acce649b348cc497b999100a170866a90c87b8 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Sat, 2 Jan 2021 18:04:02 +0100 Subject: [PATCH] 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" --- models/issue.go | 14 +++++++ modules/notification/action/action.go | 8 ++-- modules/notification/base/notifier.go | 11 ++--- modules/notification/base/null.go | 12 ++++-- modules/notification/indexer/indexer.go | 6 +-- modules/notification/mail/mail.go | 28 ++++++++----- modules/notification/notification.go | 23 +++++++---- modules/notification/ui/ui.go | 55 +++++++++++++++++++++++-- modules/notification/webhook/webhook.go | 8 ++-- services/comments/comments.go | 7 +++- services/issue/issue.go | 7 +++- services/mailer/mail_comment.go | 46 +++++++++++++-------- services/mailer/mail_issue.go | 34 ++++++--------- services/pull/pull.go | 7 +++- services/pull/review.go | 27 +++++++++++- 15 files changed, 205 insertions(+), 88 deletions(-) diff --git a/models/issue.go b/models/issue.go index eb47a74091..18d01d57d4 100644 --- a/models/issue.go +++ b/models/issue.go @@ -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" @@ -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) { diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 75d2aa3019..360906f076 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -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 @@ -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, @@ -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 @@ -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 diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index 7b8eb7b1f1..b01026dfc5 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -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) @@ -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) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index 7a9ad0b2ce..d80ba859f3 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -23,11 +23,11 @@ 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 @@ -35,11 +35,15 @@ func (*NullNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Is } // 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 diff --git a/modules/notification/indexer/indexer.go b/modules/notification/indexer/indexer.go index 8293d28570..205194ad3e 100644 --- a/modules/notification/indexer/indexer.go +++ b/modules/notification/indexer/indexer.go @@ -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 { @@ -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) } diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 847a2f30a7..ee8a0c436c 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -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 @@ -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) } } @@ -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 @@ -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 { @@ -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) } } @@ -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) { diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 21f9dc87a8..7ced57ce2d 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -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) } } @@ -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) } } @@ -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) } } diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index cadc9720d5..8e510e9cd4 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -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, @@ -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) { @@ -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 @@ -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, @@ -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) { diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 4b159f6248..a7357a51ca 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -261,7 +261,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *mode } } -func (m *webhookNotifier) NotifyNewIssue(issue *models.Issue) { +func (m *webhookNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models.User) { if err := issue.LoadRepo(); err != nil { log.Error("issue.LoadRepo: %v", err) return @@ -283,7 +283,7 @@ func (m *webhookNotifier) NotifyNewIssue(issue *models.Issue) { } } -func (m *webhookNotifier) NotifyNewPullRequest(pull *models.PullRequest) { +func (m *webhookNotifier) NotifyNewPullRequest(pull *models.PullRequest, mentions []*models.User) { if err := pull.LoadIssue(); err != nil { log.Error("pull.LoadIssue: %v", err) return @@ -399,7 +399,7 @@ func (m *webhookNotifier) NotifyUpdateComment(doer *models.User, c *models.Comme } func (m *webhookNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, - issue *models.Issue, comment *models.Comment) { + issue *models.Issue, comment *models.Comment, mentions []*models.User) { mode, _ := models.AccessLevel(doer, repo) var err error @@ -651,7 +651,7 @@ func (m *webhookNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User, } } -func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) { +func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment, mentions []*models.User) { var reviewHookType models.HookEventType switch review.Type { diff --git a/services/comments/comments.go b/services/comments/comments.go index ed479e7110..ad79eec4fb 100644 --- a/services/comments/comments.go +++ b/services/comments/comments.go @@ -22,8 +22,11 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model if err != nil { return nil, err } - - notification.NotifyCreateIssueComment(doer, repo, issue, comment) + mentions, err := issue.FindAndUpdateIssueMentions(models.DefaultDBContext(), doer, comment.Content) + if err != nil { + return nil, err + } + notification.NotifyCreateIssueComment(doer, repo, issue, comment, mentions) return comment, nil } diff --git a/services/issue/issue.go b/services/issue/issue.go index 0f90a2bcd0..14de0290a1 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -23,7 +23,12 @@ func NewIssue(repo *models.Repository, issue *models.Issue, labelIDs []int64, uu } } - notification.NotifyNewIssue(issue) + mentions, err := issue.FindAndUpdateIssueMentions(models.DefaultDBContext(), issue.Poster, issue.Content) + if err != nil { + return err + } + + notification.NotifyNewIssue(issue, mentions) return nil } diff --git a/services/mailer/mail_comment.go b/services/mailer/mail_comment.go index 9edbfabd48..2f166720db 100644 --- a/services/mailer/mail_comment.go +++ b/services/mailer/mail_comment.go @@ -5,31 +5,20 @@ package mailer import ( - "fmt" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/references" ) // MailParticipantsComment sends new comment emails to repository watchers // and mentioned people. -func MailParticipantsComment(c *models.Comment, opType models.ActionType, issue *models.Issue) error { - return mailParticipantsComment(models.DefaultDBContext(), c, opType, issue) +func MailParticipantsComment(c *models.Comment, opType models.ActionType, issue *models.Issue, mentions []*models.User) error { + return mailParticipantsComment(c, opType, issue, mentions) } -func mailParticipantsComment(ctx models.DBContext, c *models.Comment, opType models.ActionType, issue *models.Issue) (err error) { - rawMentions := references.FindAllMentionsMarkdown(c.Content) - userMentions, err := issue.ResolveMentionsByVisibility(ctx, c.Poster, rawMentions) - if err != nil { - return fmt.Errorf("ResolveMentionsByVisibility [%d]: %v", c.IssueID, err) - } - if err = models.UpdateIssueMentions(ctx, c.IssueID, userMentions); err != nil { - return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err) - } - mentions := make([]int64, len(userMentions)) - for i, u := range userMentions { - mentions[i] = u.ID +func mailParticipantsComment(c *models.Comment, opType models.ActionType, issue *models.Issue, mentions []*models.User) (err error) { + mentionedIDs := make([]int64, len(mentions)) + for i, u := range mentions { + mentionedIDs[i] = u.ID } if err = mailIssueCommentToParticipants( &mailCommentContext{ @@ -38,8 +27,29 @@ func mailParticipantsComment(ctx models.DBContext, c *models.Comment, opType mod ActionType: opType, Content: c.Content, Comment: c, - }, mentions); err != nil { + }, mentionedIDs); err != nil { log.Error("mailIssueCommentToParticipants: %v", err) } return nil } + +// MailMentionsComment sends email to users mentioned in a code comment +func MailMentionsComment(pr *models.PullRequest, c *models.Comment, mentions []*models.User) (err error) { + mentionedIDs := make([]int64, len(mentions)) + for i, u := range mentions { + mentionedIDs[i] = u.ID + } + visited := make(map[int64]bool, len(mentions)+1) + visited[c.Poster.ID] = true + if err = mailIssueCommentBatch( + &mailCommentContext{ + Issue: pr.Issue, + Doer: c.Poster, + ActionType: models.ActionCommentPull, + Content: c.Content, + Comment: c, + }, mentionedIDs, visited, true); err != nil { + log.Error("mailIssueCommentBatch: %v", err) + } + return nil +} diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index 30b54eb6cb..ffe2a4e471 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -9,7 +9,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/references" ) func fallbackMailSubject(issue *models.Issue) string { @@ -80,6 +79,12 @@ func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []int64) e // Avoid mailing the doer visited[ctx.Doer.ID] = true + + // =========== Mentions =========== + if err = mailIssueCommentBatch(ctx, mentions, visited, true); err != nil { + return fmt.Errorf("mailIssueCommentBatch() mentions: %v", err) + } + // Avoid mailing explicit unwatched ids, err = models.GetIssueWatchersIDs(ctx.Issue.ID, false) if err != nil { @@ -93,11 +98,6 @@ func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []int64) e return fmt.Errorf("mailIssueCommentBatch(): %v", err) } - // =========== Mentions =========== - if err = mailIssueCommentBatch(ctx, mentions, visited, true); err != nil { - return fmt.Errorf("mailIssueCommentBatch() mentions: %v", err) - } - return nil } @@ -145,22 +145,14 @@ func mailIssueCommentBatch(ctx *mailCommentContext, ids []int64, visited map[int // MailParticipants sends new issue thread created emails to repository watchers // and mentioned people. -func MailParticipants(issue *models.Issue, doer *models.User, opType models.ActionType) error { - return mailParticipants(models.DefaultDBContext(), issue, doer, opType) +func MailParticipants(issue *models.Issue, doer *models.User, opType models.ActionType, mentions []*models.User) error { + return mailParticipants(issue, doer, opType, mentions) } -func mailParticipants(ctx models.DBContext, issue *models.Issue, doer *models.User, opType models.ActionType) (err error) { - rawMentions := references.FindAllMentionsMarkdown(issue.Content) - userMentions, err := issue.ResolveMentionsByVisibility(ctx, doer, rawMentions) - if err != nil { - return fmt.Errorf("ResolveMentionsByVisibility [%d]: %v", issue.ID, err) - } - if err = models.UpdateIssueMentions(ctx, issue.ID, userMentions); err != nil { - return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) - } - mentions := make([]int64, len(userMentions)) - for i, u := range userMentions { - mentions[i] = u.ID +func mailParticipants(issue *models.Issue, doer *models.User, opType models.ActionType, mentions []*models.User) (err error) { + mentionedIDs := make([]int64, len(mentions)) + for i, u := range mentions { + mentionedIDs[i] = u.ID } if err = mailIssueCommentToParticipants( &mailCommentContext{ @@ -169,7 +161,7 @@ func mailParticipants(ctx models.DBContext, issue *models.Issue, doer *models.Us ActionType: opType, Content: issue.Content, Comment: nil, - }, mentions); err != nil { + }, mentionedIDs); err != nil { log.Error("mailIssueCommentToParticipants: %v", err) } return nil diff --git a/services/pull/pull.go b/services/pull/pull.go index 476c5dad54..35dcbf9604 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -53,7 +53,12 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 return err } - notification.NotifyNewPullRequest(pr) + mentions, err := pull.FindAndUpdateIssueMentions(models.DefaultDBContext(), pull.Poster, pull.Content) + if err != nil { + return err + } + + notification.NotifyNewPullRequest(pr, mentions) // add first push codes comment baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) diff --git a/services/pull/review.go b/services/pull/review.go index 6781136061..8994a9e78a 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -57,7 +57,12 @@ func CreateCodeComment(doer *models.User, gitRepo *git.Repository, issue *models return nil, err } - notification.NotifyCreateIssueComment(doer, issue.Repo, issue, comment) + mentions, err := issue.FindAndUpdateIssueMentions(models.DefaultDBContext(), doer, comment.Content) + if err != nil { + return nil, err + } + + notification.NotifyCreateIssueComment(doer, issue.Repo, issue, comment, mentions) return comment, nil } @@ -226,7 +231,25 @@ func SubmitReview(doer *models.User, gitRepo *git.Repository, issue *models.Issu return nil, nil, err } - notification.NotifyPullRequestReview(pr, review, comm) + ctx := models.DefaultDBContext() + mentions, err := issue.FindAndUpdateIssueMentions(ctx, doer, comm.Content) + if err != nil { + return nil, nil, err + } + + notification.NotifyPullRequestReview(pr, review, comm, mentions) + + for _, lines := range review.CodeComments { + for _, comments := range lines { + for _, codeComment := range comments { + mentions, err := issue.FindAndUpdateIssueMentions(ctx, doer, codeComment.Content) + if err != nil { + return nil, nil, err + } + notification.NotifyPullRequestCodeComment(pr, codeComment, mentions) + } + } + } return review, comm, nil }