From beab2df1227f9b7e556aa5716d94feb3a3e2088e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 13 Jan 2019 22:42:55 +0800 Subject: [PATCH] Refactor mail notification (#5110) * mail notification implement interface * fix file comment year * use NullNotifier as parent struct of notifiers --- models/issue.go | 3 - models/issue_comment.go | 9 +- models/pull.go | 2 - .../base/{base.go => notifier.go} | 0 modules/notification/base/null.go | 108 ++++++++++++++++++ modules/notification/mail/mail.go | 74 ++++++++++++ modules/notification/notification.go | 2 + modules/notification/ui/ui.go | 47 +------- 8 files changed, 190 insertions(+), 55 deletions(-) rename modules/notification/base/{base.go => notifier.go} (100%) create mode 100644 modules/notification/base/null.go create mode 100644 modules/notification/mail/mail.go diff --git a/models/issue.go b/models/issue.go index c982931c0..f81281b0e 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1121,9 +1121,6 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in }); err != nil { log.Error(4, "NotifyWatchers: %v", err) } - if err = issue.MailParticipants(); err != nil { - log.Error(4, "MailParticipants: %v", err) - } mode, _ := AccessLevel(issue.Poster, issue.Repo) if err = PrepareWebhooks(repo, HookEventIssues, &api.IssuePayload{ diff --git a/models/issue_comment.go b/models/issue_comment.go index 651cbdfad..03096414e 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -361,7 +361,11 @@ func (c *Comment) LoadDepIssueDetails() (err error) { // MailParticipants sends new comment emails to repository watchers // and mentioned people. -func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) { +func (c *Comment) MailParticipants(opType ActionType, issue *Issue) (err error) { + return c.mailParticipants(x, opType, issue) +} + +func (c *Comment) mailParticipants(e Engine, opType ActionType, issue *Issue) (err error) { mentions := markup.FindAllMentions(c.Content) if err = UpdateIssueMentions(e, c.IssueID, mentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err) @@ -632,9 +636,6 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen if err = notifyWatchers(e, act); err != nil { log.Error(4, "notifyWatchers: %v", err) } - if err = comment.MailParticipants(e, act.OpType, opts.Issue); err != nil { - log.Error(4, "MailParticipants: %v", err) - } } return nil } diff --git a/models/pull.go b/models/pull.go index cd21e494a..d7e11de03 100644 --- a/models/pull.go +++ b/models/pull.go @@ -845,8 +845,6 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str IsPrivate: repo.IsPrivate, }); err != nil { log.Error(4, "NotifyWatchers: %v", err) - } else if err = pull.MailParticipants(); err != nil { - log.Error(4, "MailParticipants: %v", err) } pr.Issue = pull diff --git a/modules/notification/base/base.go b/modules/notification/base/notifier.go similarity index 100% rename from modules/notification/base/base.go rename to modules/notification/base/notifier.go diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go new file mode 100644 index 000000000..608bd0dca --- /dev/null +++ b/modules/notification/base/null.go @@ -0,0 +1,108 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package base + +import ( + "code.gitea.io/git" + "code.gitea.io/gitea/models" +) + +// NullNotifier implements a blank notifier +type NullNotifier struct { +} + +var ( + _ Notifier = &NullNotifier{} +) + +// Run places a place holder function +func (*NullNotifier) Run() { +} + +// NotifyCreateIssueComment places a place holder function +func (*NullNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, + issue *models.Issue, comment *models.Comment) { +} + +// NotifyNewIssue places a place holder function +func (*NullNotifier) NotifyNewIssue(issue *models.Issue) { +} + +// NotifyIssueChangeStatus places a place holder function +func (*NullNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) { +} + +// NotifyNewPullRequest places a place holder function +func (*NullNotifier) NotifyNewPullRequest(pr *models.PullRequest) { +} + +// NotifyPullRequestReview places a place holder function +func (*NullNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment) { +} + +// NotifyMergePullRequest places a place holder function +func (*NullNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, baseRepo *git.Repository) { +} + +// NotifyUpdateComment places a place holder function +func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { +} + +// NotifyDeleteComment places a place holder function +func (*NullNotifier) NotifyDeleteComment(doer *models.User, c *models.Comment) { +} + +// NotifyDeleteRepository places a place holder function +func (*NullNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) { +} + +// NotifyForkRepository places a place holder function +func (*NullNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) { +} + +// NotifyNewRelease places a place holder function +func (*NullNotifier) NotifyNewRelease(rel *models.Release) { +} + +// NotifyUpdateRelease places a place holder function +func (*NullNotifier) NotifyUpdateRelease(doer *models.User, rel *models.Release) { +} + +// NotifyDeleteRelease places a place holder function +func (*NullNotifier) NotifyDeleteRelease(doer *models.User, rel *models.Release) { +} + +// NotifyIssueChangeMilestone places a place holder function +func (*NullNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue) { +} + +// NotifyIssueChangeContent places a place holder function +func (*NullNotifier) NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) { +} + +// NotifyIssueChangeAssignee places a place holder function +func (*NullNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, removed bool) { +} + +// NotifyIssueClearLabels places a place holder function +func (*NullNotifier) NotifyIssueClearLabels(doer *models.User, issue *models.Issue) { +} + +// NotifyIssueChangeTitle places a place holder function +func (*NullNotifier) NotifyIssueChangeTitle(doer *models.User, issue *models.Issue, oldTitle string) { +} + +// NotifyIssueChangeLabels places a place holder function +func (*NullNotifier) NotifyIssueChangeLabels(doer *models.User, issue *models.Issue, + addedLabels []*models.Label, removedLabels []*models.Label) { +} + +// NotifyCreateRepository places a place holder function +func (*NullNotifier) NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) { +} + +// NotifyMigrateRepository places a place holder function +func (*NullNotifier) NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository) { +} diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go new file mode 100644 index 000000000..8bb417344 --- /dev/null +++ b/modules/notification/mail/mail.go @@ -0,0 +1,74 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package mail + +import ( + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/notification/base" +) + +type mailNotifier struct { + base.NullNotifier +} + +var ( + _ base.Notifier = &mailNotifier{} +) + +// NewNotifier create a new mailNotifier notifier +func NewNotifier() base.Notifier { + return &mailNotifier{} +} + +func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, + issue *models.Issue, comment *models.Comment) { + var act models.ActionType + if comment.Type == models.CommentTypeClose { + act = models.ActionCloseIssue + } else if comment.Type == models.CommentTypeReopen { + act = models.ActionReopenIssue + } else if comment.Type == models.CommentTypeComment { + act = models.ActionCommentIssue + } else if comment.Type == models.CommentTypeCode { + act = models.ActionCommentIssue + } + + if err := comment.MailParticipants(act, issue); err != nil { + log.Error(4, "MailParticipants: %v", err) + } +} + +func (m *mailNotifier) NotifyNewIssue(issue *models.Issue) { + if err := issue.MailParticipants(); err != nil { + log.Error(4, "MailParticipants: %v", err) + } +} + +func (m *mailNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) { + if err := issue.MailParticipants(); err != nil { + log.Error(4, "MailParticipants: %v", err) + } +} + +func (m *mailNotifier) NotifyNewPullRequest(pr *models.PullRequest) { + if err := pr.Issue.MailParticipants(); err != nil { + log.Error(4, "MailParticipants: %v", err) + } +} + +func (m *mailNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, comment *models.Comment) { + var act models.ActionType + if comment.Type == models.CommentTypeClose { + act = models.ActionCloseIssue + } else if comment.Type == models.CommentTypeReopen { + act = models.ActionReopenIssue + } else if comment.Type == models.CommentTypeComment { + act = models.ActionCommentIssue + } + if err := comment.MailParticipants(act, pr.Issue); err != nil { + log.Error(4, "MailParticipants: %v", err) + } +} diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 3f3579394..6f482e881 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/git" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/notification/base" + "code.gitea.io/gitea/modules/notification/mail" "code.gitea.io/gitea/modules/notification/ui" ) @@ -23,6 +24,7 @@ func RegisterNotifier(notifier base.Notifier) { func init() { RegisterNotifier(ui.NewNotifier()) + RegisterNotifier(mail.NewNotifier()) } // NotifyCreateIssueComment notifies issue comment related message to notifiers diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index f0d708eb5..135e92abf 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -13,6 +13,7 @@ import ( type ( notificationService struct { + base.NullNotifier issueQueue chan issueNotificationOpts } @@ -86,49 +87,3 @@ func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r r.Reviewer.ID, } } - -func (ns *notificationService) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { -} - -func (ns *notificationService) NotifyDeleteComment(doer *models.User, c *models.Comment) { -} - -func (ns *notificationService) NotifyDeleteRepository(doer *models.User, repo *models.Repository) { -} - -func (ns *notificationService) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) { -} - -func (ns *notificationService) NotifyNewRelease(rel *models.Release) { -} - -func (ns *notificationService) NotifyUpdateRelease(doer *models.User, rel *models.Release) { -} - -func (ns *notificationService) NotifyDeleteRelease(doer *models.User, rel *models.Release) { -} - -func (ns *notificationService) NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue) { -} - -func (ns *notificationService) NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) { -} - -func (ns *notificationService) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, removed bool) { -} - -func (ns *notificationService) NotifyIssueClearLabels(doer *models.User, issue *models.Issue) { -} - -func (ns *notificationService) NotifyIssueChangeTitle(doer *models.User, issue *models.Issue, oldTitle string) { -} - -func (ns *notificationService) NotifyIssueChangeLabels(doer *models.User, issue *models.Issue, - addedLabels []*models.Label, removedLabels []*models.Label) { -} - -func (ns *notificationService) NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) { -} - -func (ns *notificationService) NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository) { -}