Add notification interface and refactor UI notifications (#5085)

* add notification interface and refactor UI notifications

* add missing methods on notification interface and notifiy only issue status really changed

* implement NotifyPullRequestReview for ui notification

models/issue.go

@@ -112,6 +112,10 @@ func (issue *Issue) GetPullRequest() (pr *PullRequest, err error) {
 	}
 
 	pr, err = getPullRequestByIssueID(x, issue.ID)
+	if err != nil {
+		return nil, err
+	}
+	pr.Issue = issue
 	return
 }
 

models/review.go

@@ -239,6 +239,8 @@ func getCurrentReview(e Engine, reviewer *User, issue *Issue) (*Review, error) {
 	if len(reviews) == 0 {
 		return nil, ErrReviewNotExist{}
 	}
+	reviews[0].Reviewer = reviewer
+	reviews[0].Issue = issue
 	return reviews[0], nil
 }
 

modules/notification/base/base.go

@@ -0,0 +1,43 @@
+// Copyright 2018 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"
+)
+
+// Notifier defines an interface to notify receiver
+type Notifier interface {
+	Run()
+
+	NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository)
+	NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository)
+	NotifyDeleteRepository(doer *models.User, repo *models.Repository)
+	NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository)
+
+	NotifyNewIssue(*models.Issue)
+	NotifyIssueChangeStatus(*models.User, *models.Issue, bool)
+	NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue)
+	NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, removed bool)
+	NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string)
+	NotifyIssueClearLabels(doer *models.User, issue *models.Issue)
+	NotifyIssueChangeTitle(doer *models.User, issue *models.Issue, oldTitle string)
+	NotifyIssueChangeLabels(doer *models.User, issue *models.Issue,
+		addedLabels []*models.Label, removedLabels []*models.Label)
+
+	NotifyNewPullRequest(*models.PullRequest)
+	NotifyMergePullRequest(*models.PullRequest, *models.User, *git.Repository)
+	NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment)
+
+	NotifyCreateIssueComment(*models.User, *models.Repository,
+		*models.Issue, *models.Comment)
+	NotifyUpdateComment(*models.User, *models.Comment, string)
+	NotifyDeleteComment(*models.User, *models.Comment)
+
+	NotifyNewRelease(rel *models.Release)
+	NotifyUpdateRelease(doer *models.User, rel *models.Release)
+	NotifyDeleteRelease(doer *models.User, rel *models.Release)
+}

modules/notification/notification.go

@@ -1,50 +1,175 @@
-// Copyright 2016 The Gitea Authors. All rights reserved.
+// Copyright 2018 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 notification
 
 import (
+	"code.gitea.io/git"
 	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/notification/base"
+	"code.gitea.io/gitea/modules/notification/ui"
 )
 
-type (
+var (
-	notificationService struct {
+	notifiers []base.Notifier
-		issueQueue chan issueNotificationOpts
+)
+
+// RegisterNotifier providers method to receive notify messages
+func RegisterNotifier(notifier base.Notifier) {
+	go notifier.Run()
+	notifiers = append(notifiers, notifier)
+}
+
+func init() {
+	RegisterNotifier(ui.NewNotifier())
+}
+
+// NotifyCreateIssueComment notifies issue comment related message to notifiers
+func NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
+	issue *models.Issue, comment *models.Comment) {
+	for _, notifier := range notifiers {
+		notifier.NotifyCreateIssueComment(doer, repo, issue, comment)
 	}
+}
 
-	issueNotificationOpts struct {
+// NotifyNewIssue notifies new issue to notifiers
-		issue                *models.Issue
+func NotifyNewIssue(issue *models.Issue) {
-		notificationAuthorID int64
+	for _, notifier := range notifiers {
+		notifier.NotifyNewIssue(issue)
 	}
-)
+}
 
-var (
+// NotifyIssueChangeStatus notifies close or reopen issue to notifiers
-	// Service is the notification service
+func NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, closeOrReopen bool) {
-	Service = &notificationService{
+	for _, notifier := range notifiers {
-		issueQueue: make(chan issueNotificationOpts, 100),
+		notifier.NotifyIssueChangeStatus(doer, issue, closeOrReopen)
 	}
-)
+}
 
-func init() {
+// NotifyMergePullRequest notifies merge pull request to notifiers
-	go Service.Run()
+func NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repository) {
+	for _, notifier := range notifiers {
+		notifier.NotifyMergePullRequest(pr, doer, baseGitRepo)
+	}
+}
+
+// NotifyNewPullRequest notifies new pull request to notifiers
+func NotifyNewPullRequest(pr *models.PullRequest) {
+	for _, notifier := range notifiers {
+		notifier.NotifyNewPullRequest(pr)
+	}
+}
+
+// NotifyPullRequestReview notifies new pull request review
+func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
+	for _, notifier := range notifiers {
+		notifier.NotifyPullRequestReview(pr, review, comment)
+	}
+}
+
+// NotifyUpdateComment notifies update comment to notifiers
+func NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) {
+	for _, notifier := range notifiers {
+		notifier.NotifyUpdateComment(doer, c, oldContent)
+	}
+}
+
+// NotifyDeleteComment notifies delete comment to notifiers
+func NotifyDeleteComment(doer *models.User, c *models.Comment) {
+	for _, notifier := range notifiers {
+		notifier.NotifyDeleteComment(doer, c)
+	}
+}
+
+// NotifyDeleteRepository notifies delete repository to notifiers
+func NotifyDeleteRepository(doer *models.User, repo *models.Repository) {
+	for _, notifier := range notifiers {
+		notifier.NotifyDeleteRepository(doer, repo)
+	}
+}
+
+// NotifyForkRepository notifies fork repository to notifiers
+func NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) {
+	for _, notifier := range notifiers {
+		notifier.NotifyForkRepository(doer, oldRepo, repo)
+	}
+}
+
+// NotifyNewRelease notifies new release to notifiers
+func NotifyNewRelease(rel *models.Release) {
+	for _, notifier := range notifiers {
+		notifier.NotifyNewRelease(rel)
+	}
+}
+
+// NotifyUpdateRelease notifies update release to notifiers
+func NotifyUpdateRelease(doer *models.User, rel *models.Release) {
+	for _, notifier := range notifiers {
+		notifier.NotifyUpdateRelease(doer, rel)
+	}
+}
+
+// NotifyDeleteRelease notifies delete release to notifiers
+func NotifyDeleteRelease(doer *models.User, rel *models.Release) {
+	for _, notifier := range notifiers {
+		notifier.NotifyDeleteRelease(doer, rel)
+	}
+}
+
+// NotifyIssueChangeMilestone notifies change milestone to notifiers
+func NotifyIssueChangeMilestone(doer *models.User, issue *models.Issue) {
+	for _, notifier := range notifiers {
+		notifier.NotifyIssueChangeMilestone(doer, issue)
+	}
+}
+
+// NotifyIssueChangeContent notifies change content to notifiers
+func NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) {
+	for _, notifier := range notifiers {
+		notifier.NotifyIssueChangeContent(doer, issue, oldContent)
+	}
+}
+
+// NotifyIssueChangeAssignee notifies change content to notifiers
+func NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, removed bool) {
+	for _, notifier := range notifiers {
+		notifier.NotifyIssueChangeAssignee(doer, issue, removed)
+	}
+}
+
+// NotifyIssueClearLabels notifies clear labels to notifiers
+func NotifyIssueClearLabels(doer *models.User, issue *models.Issue) {
+	for _, notifier := range notifiers {
+		notifier.NotifyIssueClearLabels(doer, issue)
+	}
+}
+
+// NotifyIssueChangeTitle notifies change title to notifiers
+func NotifyIssueChangeTitle(doer *models.User, issue *models.Issue, oldTitle string) {
+	for _, notifier := range notifiers {
+		notifier.NotifyIssueChangeTitle(doer, issue, oldTitle)
+	}
+}
+
+// NotifyIssueChangeLabels notifies change labels to notifiers
+func NotifyIssueChangeLabels(doer *models.User, issue *models.Issue,
+	addedLabels []*models.Label, removedLabels []*models.Label) {
+	for _, notifier := range notifiers {
+		notifier.NotifyIssueChangeLabels(doer, issue, addedLabels, removedLabels)
+	}
 }
 
-func (ns *notificationService) Run() {
+// NotifyCreateRepository notifies create repository to notifiers
-	for {
+func NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) {
-		select {
+	for _, notifier := range notifiers {
-		case opts := <-ns.issueQueue:
+		notifier.NotifyCreateRepository(doer, u, repo)
-			if err := models.CreateOrUpdateIssueNotifications(opts.issue, opts.notificationAuthorID); err != nil {
-				log.Error(4, "Was unable to create issue notification: %v", err)
-			}
-		}
 	}
 }
 
-func (ns *notificationService) NotifyIssue(issue *models.Issue, notificationAuthorID int64) {
+// NotifyMigrateRepository notifies create repository to notifiers
-	ns.issueQueue <- issueNotificationOpts{
+func NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository) {
-		issue,
+	for _, notifier := range notifiers {
-		notificationAuthorID,
+		notifier.NotifyMigrateRepository(doer, u, repo)
 	}
 }

modules/notification/ui/ui.go

@@ -0,0 +1,134 @@
+// Copyright 2018 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 ui
+
+import (
+	"code.gitea.io/git"
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/notification/base"
+)
+
+type (
+	notificationService struct {
+		issueQueue chan issueNotificationOpts
+	}
+
+	issueNotificationOpts struct {
+		issue                *models.Issue
+		notificationAuthorID int64
+	}
+)
+
+var (
+	_ base.Notifier = &notificationService{}
+)
+
+// NewNotifier create a new notificationService notifier
+func NewNotifier() base.Notifier {
+	return &notificationService{
+		issueQueue: make(chan issueNotificationOpts, 100),
+	}
+}
+
+func (ns *notificationService) Run() {
+	for {
+		select {
+		case opts := <-ns.issueQueue:
+			if err := models.CreateOrUpdateIssueNotifications(opts.issue, opts.notificationAuthorID); err != nil {
+				log.Error(4, "Was unable to create issue notification: %v", err)
+			}
+		}
+	}
+}
+
+func (ns *notificationService) NotifyCreateIssueComment(doer *models.User, repo *models.Repository,
+	issue *models.Issue, comment *models.Comment) {
+	ns.issueQueue <- issueNotificationOpts{
+		issue,
+		doer.ID,
+	}
+}
+
+func (ns *notificationService) NotifyNewIssue(issue *models.Issue) {
+	ns.issueQueue <- issueNotificationOpts{
+		issue,
+		issue.Poster.ID,
+	}
+}
+
+func (ns *notificationService) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, isClosed bool) {
+	ns.issueQueue <- issueNotificationOpts{
+		issue,
+		doer.ID,
+	}
+}
+
+func (ns *notificationService) NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, gitRepo *git.Repository) {
+	ns.issueQueue <- issueNotificationOpts{
+		pr.Issue,
+		doer.ID,
+	}
+}
+
+func (ns *notificationService) NotifyNewPullRequest(pr *models.PullRequest) {
+	ns.issueQueue <- issueNotificationOpts{
+		pr.Issue,
+		pr.Issue.PosterID,
+	}
+}
+
+func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r *models.Review, c *models.Comment) {
+	ns.issueQueue <- issueNotificationOpts{
+		pr.Issue,
+		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) {
+}

routers/api/v1/repo/issue.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/indexer"
+	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
@@ -207,6 +208,8 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) {
 		return
 	}
 
+	notification.NotifyNewIssue(issue)
+
 	if form.Closed {
 		if err := issue.ChangeStatus(ctx.User, ctx.Repo.Repository, true); err != nil {
 			if models.IsErrDependenciesLeft(err) {
@@ -337,6 +340,8 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 			ctx.Error(500, "ChangeStatus", err)
 			return
 		}
+
+		notification.NotifyIssueChangeStatus(ctx.User, issue, api.StateClosed == api.StateType(*form.State))
 	}
 
 	// Refetch from database to assign some automatic values

routers/api/v1/repo/issue_comment.go

@@ -9,6 +9,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/notification"
 
 	api "code.gitea.io/sdk/gitea"
 )
@@ -163,6 +164,8 @@ func CreateIssueComment(ctx *context.APIContext, form api.CreateIssueCommentOpti
 		return
 	}
 
+	notification.NotifyCreateIssueComment(ctx.User, ctx.Repo.Repository, issue, comment)
+
 	ctx.JSON(201, comment.APIFormat())
 }
 

routers/api/v1/repo/pull.go

@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/util"
 
 	api "code.gitea.io/sdk/gitea"
@@ -270,6 +271,8 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 		return
 	}
 
+	notification.NotifyNewPullRequest(pr)
+
 	log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID)
 	ctx.JSON(201, pr.APIFormat())
 }
@@ -386,6 +389,8 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
 			ctx.Error(500, "ChangeStatus", err)
 			return
 		}
+
+		notification.NotifyIssueChangeStatus(ctx.User, issue, api.StateClosed == api.StateType(*form.State))
 	}
 
 	// Refetch from database

routers/repo/issue.go

@@ -490,7 +490,7 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
 		return
 	}
 
-	notification.Service.NotifyIssue(issue, ctx.User.ID)
+	notification.NotifyNewIssue(issue)
 
 	log.Trace("Issue created: %d/%d", repo.ID, issue.ID)
 	ctx.Redirect(ctx.Repo.RepoLink + "/issues/" + com.ToStr(issue.Index))
@@ -1004,15 +1004,19 @@ func UpdateIssueStatus(ctx *context.Context) {
 		return
 	}
 	for _, issue := range issues {
-		if err := issue.ChangeStatus(ctx.User, issue.Repo, isClosed); err != nil {
+		if issue.IsClosed != isClosed {
-			if models.IsErrDependenciesLeft(err) {
+			if err := issue.ChangeStatus(ctx.User, issue.Repo, isClosed); err != nil {
-				ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{
+				if models.IsErrDependenciesLeft(err) {
-					"error": "cannot close this issue because it still has open dependencies",
+					ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{
-				})
+						"error": "cannot close this issue because it still has open dependencies",
+					})
+					return
+				}
+				ctx.ServerError("ChangeStatus", err)
 				return
 			}
-			ctx.ServerError("ChangeStatus", err)
+
-			return
+			notification.NotifyIssueChangeStatus(ctx.User, issue, isClosed)
 		}
 	}
 	ctx.JSON(200, map[string]interface{}{
@@ -1072,7 +1076,8 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 			if pr != nil {
 				ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
 			} else {
-				if err := issue.ChangeStatus(ctx.User, ctx.Repo.Repository, form.Status == "close"); err != nil {
+				isClosed := form.Status == "close"
+				if err := issue.ChangeStatus(ctx.User, ctx.Repo.Repository, isClosed); err != nil {
 					log.Error(4, "ChangeStatus: %v", err)
 
 					if models.IsErrDependenciesLeft(err) {
@@ -1088,7 +1093,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 				} else {
 					log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
 
-					notification.Service.NotifyIssue(issue, ctx.User.ID)
+					notification.NotifyIssueChangeStatus(ctx.User, issue, isClosed)
 				}
 			}
 		}
@@ -1116,7 +1121,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 		return
 	}
 
-	notification.Service.NotifyIssue(issue, ctx.User.ID)
+	notification.NotifyCreateIssueComment(ctx.User, ctx.Repo.Repository, issue, comment)
 
 	log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
 }

routers/repo/pull.go

@@ -580,7 +580,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
 		return
 	}
 
-	notification.Service.NotifyIssue(pr.Issue, ctx.User.ID)
+	notification.NotifyMergePullRequest(pr, ctx.User, ctx.Repo.GitRepo)
 
 	log.Trace("Pull request merged: %d", pr.ID)
 	ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
@@ -888,7 +888,7 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 		return
 	}
 
-	notification.Service.NotifyIssue(pullIssue, ctx.User.ID)
+	notification.NotifyNewPullRequest(pullRequest)
 
 	log.Trace("Pull request created: %d/%d", repo.ID, pullIssue.ID)
 	ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))

routers/repo/pull_review.go

@@ -79,7 +79,7 @@ func CreateCodeComment(ctx *context.Context, form auth.CodeCommentForm) {
 	}
 	// Send no notification if comment is pending
 	if !form.IsReview {
-		notification.Service.NotifyIssue(issue, ctx.User.ID)
+		notification.NotifyCreateIssueComment(ctx.User, issue.Repo, issue, comment)
 	}
 
 	log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
@@ -184,5 +184,13 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
 		ctx.ServerError("Publish", err)
 		return
 	}
+
+	pr, err := issue.GetPullRequest()
+	if err != nil {
+		ctx.ServerError("GetPullRequest", err)
+		return
+	}
+	notification.NotifyPullRequestReview(pr, review, comm)
+
 	ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()))
 }