From d7ac9727bb5046118915cbb26b2dac1b7b27c9d4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 22 Nov 2019 01:08:42 +0800 Subject: [PATCH] Move merge actions to notification (#9024) * Move merge actions to notification * Add missing mail notification --- models/pull.go | 3 ++ modules/notification/action/action.go | 15 ++++++++++ modules/notification/mail/mail.go | 12 ++++++++ modules/notification/webhook/webhook.go | 38 +++++++++++++++++++++++++ routers/repo/pull.go | 2 -- services/pull/merge.go | 20 +------------ 6 files changed, 69 insertions(+), 21 deletions(-) diff --git a/models/pull.go b/models/pull.go index b25c52571d..174faee97a 100644 --- a/models/pull.go +++ b/models/pull.go @@ -152,6 +152,9 @@ func (pr *PullRequest) loadIssue(e Engine) (err error) { } pr.Issue, err = getIssueByID(e, pr.IssueID) + if err == nil { + pr.Issue.PullRequest = pr + } return err } diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index dd4dc0ae32..70ab9975b2 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification/base" ) @@ -191,3 +192,17 @@ func (a *actionNotifier) NotifyPullRequestReview(pr *models.PullRequest, review log.Error("notify watchers '%d/%d': %v", review.Reviewer.ID, review.Issue.RepoID, err) } } + +func (*actionNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, baseRepo *git.Repository) { + if err := models.NotifyWatchers(&models.Action{ + ActUserID: doer.ID, + ActUser: doer, + OpType: models.ActionMergePullRequest, + Content: fmt.Sprintf("%d|%s", pr.Issue.Index, pr.Issue.Title), + RepoID: pr.Issue.Repo.ID, + Repo: pr.Issue.Repo, + IsPrivate: pr.Issue.Repo.IsPrivate, + }); err != nil { + log.Error("NotifyWatchers [%d]: %v", pr.ID, err) + } +} diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 24f68bd642..558c9a6243 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -8,6 +8,7 @@ import ( "fmt" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification/base" "code.gitea.io/gitea/services/mailer" @@ -98,3 +99,14 @@ func (m *mailNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *model mailer.SendIssueAssignedMail(issue, doer, ct, comment, []string{assignee.Email}) } } + +func (m *mailNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, baseRepo *git.Repository) { + if err := pr.LoadIssue(); err != nil { + log.Error("pr.LoadIssue: %v", err) + return + } + + if err := mailer.MailParticipants(pr.Issue, doer, models.ActionClosePullRequest); err != nil { + log.Error("MailParticipants: %v", err) + } +} diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 43be0d2e1c..213e33c096 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -521,6 +521,44 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Re } } +func (*webhookNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, baseRepo *git.Repository) { + // Reload pull request information. + if err := pr.LoadAttributes(); err != nil { + log.Error("LoadAttributes: %v", err) + return + } + + if err := pr.LoadIssue(); err != nil { + log.Error("LoadAttributes: %v", err) + return + } + + if err := pr.Issue.LoadRepo(); err != nil { + log.Error("pr.Issue.LoadRepo: %v", err) + return + } + + mode, err := models.AccessLevel(doer, pr.Issue.Repo) + if err != nil { + log.Error("models.AccessLevel: %v", err) + return + } + + // Merge pull request calls issue.changeStatus so we need to handle separately. + apiPullRequest := &api.PullRequestPayload{ + Index: pr.Issue.Index, + PullRequest: pr.APIFormat(), + Repository: pr.Issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + Action: api.HookIssueClosed, + } + + err = webhook_module.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequest, apiPullRequest) + if err != nil { + log.Error("PrepareWebhooks: %v", err) + } +} + func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) { var reviewHookType models.HookEventType diff --git a/routers/repo/pull.go b/routers/repo/pull.go index a1a4e5e2bb..0ff077b462 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -716,8 +716,6 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { return } - 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)) } diff --git a/services/pull/merge.go b/services/pull/merge.go index 8ae5f029e8..e5563a89b9 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -426,29 +426,11 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor log.Error("setMerged [%d]: %v", pr.ID, err) } - if err := models.NotifyWatchers(&models.Action{ - ActUserID: doer.ID, - ActUser: doer, - OpType: models.ActionMergePullRequest, - Content: fmt.Sprintf("%d|%s", pr.Issue.Index, pr.Issue.Title), - RepoID: pr.Issue.Repo.ID, - Repo: pr.Issue.Repo, - IsPrivate: pr.Issue.Repo.IsPrivate, - }); err != nil { - log.Error("NotifyWatchers [%d]: %v", pr.ID, err) - } + notification.NotifyMergePullRequest(pr, doer, baseGitRepo) // Reset cached commit count cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) - // Reload pull request information. - if err = pr.LoadAttributes(); err != nil { - log.Error("LoadAttributes: %v", err) - return nil - } - - notification.NotifyIssueChangeStatus(doer, pr.Issue, true) - // Resolve cross references refs, err := pr.ResolveCrossReferences() if err != nil {