From ea619b39b2f2a3c1fb5ad28ebd4a269b2f822111 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 18 Oct 2018 19:23:05 +0800 Subject: [PATCH] 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 | 4 + models/review.go | 2 + modules/notification/base/base.go | 43 +++++++ modules/notification/notification.go | 185 ++++++++++++++++++++++----- modules/notification/ui/ui.go | 134 +++++++++++++++++++ routers/api/v1/repo/issue.go | 5 + routers/api/v1/repo/issue_comment.go | 3 + routers/api/v1/repo/pull.go | 5 + routers/repo/issue.go | 27 ++-- routers/repo/pull.go | 4 +- routers/repo/pull_review.go | 10 +- 11 files changed, 378 insertions(+), 44 deletions(-) create mode 100644 modules/notification/base/base.go create mode 100644 modules/notification/ui/ui.go diff --git a/models/issue.go b/models/issue.go index a32741043..8dc046675 100644 --- a/models/issue.go +++ b/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 } diff --git a/models/review.go b/models/review.go index 3326ea054..dd8743586 100644 --- a/models/review.go +++ b/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 } diff --git a/modules/notification/base/base.go b/modules/notification/base/base.go new file mode 100644 index 000000000..bac90f5bb --- /dev/null +++ b/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) +} diff --git a/modules/notification/notification.go b/modules/notification/notification.go index ffe885240..3f3579394 100644 --- a/modules/notification/notification.go +++ b/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" -) - -type ( - notificationService struct { - issueQueue chan issueNotificationOpts - } - - issueNotificationOpts struct { - issue *models.Issue - notificationAuthorID int64 - } + "code.gitea.io/gitea/modules/notification/base" + "code.gitea.io/gitea/modules/notification/ui" ) var ( - // Service is the notification service - Service = ¬ificationService{ - issueQueue: make(chan issueNotificationOpts, 100), - } + notifiers []base.Notifier ) +// RegisterNotifier providers method to receive notify messages +func RegisterNotifier(notifier base.Notifier) { + go notifier.Run() + notifiers = append(notifiers, notifier) +} + func init() { - go Service.Run() + RegisterNotifier(ui.NewNotifier()) } -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) - } - } +// 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) } } -func (ns *notificationService) NotifyIssue(issue *models.Issue, notificationAuthorID int64) { - ns.issueQueue <- issueNotificationOpts{ - issue, - notificationAuthorID, +// NotifyNewIssue notifies new issue to notifiers +func NotifyNewIssue(issue *models.Issue) { + for _, notifier := range notifiers { + notifier.NotifyNewIssue(issue) + } +} + +// NotifyIssueChangeStatus notifies close or reopen issue to notifiers +func NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, closeOrReopen bool) { + for _, notifier := range notifiers { + notifier.NotifyIssueChangeStatus(doer, issue, closeOrReopen) + } +} + +// NotifyMergePullRequest notifies merge pull request to notifiers +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) + } +} + +// NotifyCreateRepository notifies create repository to notifiers +func NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) { + for _, notifier := range notifiers { + notifier.NotifyCreateRepository(doer, u, repo) + } +} + +// NotifyMigrateRepository notifies create repository to notifiers +func NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository) { + for _, notifier := range notifiers { + notifier.NotifyMigrateRepository(doer, u, repo) } } diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go new file mode 100644 index 000000000..f0d708eb5 --- /dev/null +++ b/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 = ¬ificationService{} +) + +// NewNotifier create a new notificationService notifier +func NewNotifier() base.Notifier { + return ¬ificationService{ + 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) { +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index f8ef0fe3d..4b634c9ca 100644 --- a/routers/api/v1/repo/issue.go +++ b/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 diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index ba627bb8a..f95892291 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/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()) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 1527b8e8c..0ec2d3687 100644 --- a/routers/api/v1/repo/pull.go +++ b/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 diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 3cce48306..3bcfdf1b0 100644 --- a/routers/repo/issue.go +++ b/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 models.IsErrDependenciesLeft(err) { - ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{ - "error": "cannot close this issue because it still has open dependencies", - }) + if issue.IsClosed != isClosed { + if err := issue.ChangeStatus(ctx.User, issue.Repo, isClosed); err != nil { + if models.IsErrDependenciesLeft(err) { + 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) } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 57fe33f85..4ec1c27ce 100644 --- a/routers/repo/pull.go +++ b/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)) diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go index 9d1db3ff4..91257fea3 100644 --- a/routers/repo/pull_review.go +++ b/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())) }