From aaeef295bb07b281fe0554aa6e97f24596d073aa Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Tue, 5 Nov 2019 19:04:08 +0800
Subject: [PATCH] Move pull webhook to notification (#8805)

* Move pull webhook to notification

* fix fmt
---
 modules/notification/base/notifier.go   |  1 +
 modules/notification/base/null.go       |  4 ++
 modules/notification/notification.go    |  7 +++
 modules/notification/webhook/webhook.go | 62 +++++++++++++++++++++++++
 services/pull/merge.go                  | 14 +-----
 services/pull/pull.go                   | 18 +------
 services/pull/review.go                 | 49 +++----------------
 7 files changed, 84 insertions(+), 71 deletions(-)

diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go
index ff865f19cf..286ebe5d69 100644
--- a/modules/notification/base/notifier.go
+++ b/modules/notification/base/notifier.go
@@ -30,6 +30,7 @@ type Notifier interface {
 
 	NotifyNewPullRequest(*models.PullRequest)
 	NotifyMergePullRequest(*models.PullRequest, *models.User, *git.Repository)
+	NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest)
 	NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment)
 
 	NotifyCreateIssueComment(*models.User, *models.Repository,
diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go
index c10e1b6340..5b6359cbd5 100644
--- a/modules/notification/base/null.go
+++ b/modules/notification/base/null.go
@@ -46,6 +46,10 @@ func (*NullNotifier) NotifyPullRequestReview(pr *models.PullRequest, r *models.R
 func (*NullNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *models.User, baseRepo *git.Repository) {
 }
 
+// NotifyPullRequestSynchronized places a place holder function
+func (*NullNotifier) NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest) {
+}
+
 // NotifyUpdateComment places a place holder function
 func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) {
 }
diff --git a/modules/notification/notification.go b/modules/notification/notification.go
index 1fd3022940..a5e450ee66 100644
--- a/modules/notification/notification.go
+++ b/modules/notification/notification.go
@@ -73,6 +73,13 @@ func NotifyNewPullRequest(pr *models.PullRequest) {
 	}
 }
 
+// NotifyPullRequestSynchronized notifies Synchronized pull request
+func NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest) {
+	for _, notifier := range notifiers {
+		notifier.NotifyPullRequestSynchronized(doer, pr)
+	}
+}
+
 // NotifyPullRequestReview notifies new pull request review
 func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
 	for _, notifier := range notifiers {
diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go
index 7d28c1c8b9..39c63edb05 100644
--- a/modules/notification/webhook/webhook.go
+++ b/modules/notification/webhook/webhook.go
@@ -520,3 +520,65 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Re
 		log.Error("PrepareWebhooks: %v", err)
 	}
 }
+
+func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
+	var reviewHookType models.HookEventType
+
+	switch review.Type {
+	case models.ReviewTypeApprove:
+		reviewHookType = models.HookEventPullRequestApproved
+	case models.ReviewTypeComment:
+		reviewHookType = models.HookEventPullRequestComment
+	case models.ReviewTypeReject:
+		reviewHookType = models.HookEventPullRequestRejected
+	default:
+		// unsupported review webhook type here
+		log.Error("Unsupported review webhook type")
+		return
+	}
+
+	if err := pr.LoadIssue(); err != nil {
+		log.Error("pr.LoadIssue: %v", err)
+		return
+	}
+
+	mode, err := models.AccessLevel(review.Issue.Poster, review.Issue.Repo)
+	if err != nil {
+		log.Error("models.AccessLevel: %v", err)
+		return
+	}
+	if err := webhook.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{
+		Action:      api.HookIssueSynchronized,
+		Index:       review.Issue.Index,
+		PullRequest: pr.APIFormat(),
+		Repository:  review.Issue.Repo.APIFormat(mode),
+		Sender:      review.Reviewer.APIFormat(),
+		Review: &api.ReviewPayload{
+			Type:    string(reviewHookType),
+			Content: review.Content,
+		},
+	}); err != nil {
+		log.Error("PrepareWebhooks: %v", err)
+	}
+}
+
+func (m *webhookNotifier) NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest) {
+	if err := pr.LoadIssue(); err != nil {
+		log.Error("pr.LoadIssue: %v", err)
+		return
+	}
+	if err := pr.Issue.LoadAttributes(); err != nil {
+		log.Error("LoadAttributes: %v", err)
+		return
+	}
+
+	if err := webhook.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
+		Action:      api.HookIssueSynchronized,
+		Index:       pr.Issue.Index,
+		PullRequest: pr.Issue.PullRequest.APIFormat(),
+		Repository:  pr.Issue.Repo.APIFormat(models.AccessModeNone),
+		Sender:      doer.APIFormat(),
+	}); err != nil {
+		log.Error("PrepareWebhooks [pull_id: %v]: %v", pr.ID, err)
+	}
+}
diff --git a/services/pull/merge.go b/services/pull/merge.go
index 4a2f4511c4..c6607910a2 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -20,10 +20,9 @@ import (
 	"code.gitea.io/gitea/modules/cache"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/notification"
 	"code.gitea.io/gitea/modules/setting"
-	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
-	"code.gitea.io/gitea/modules/webhook"
 
 	"github.com/mcuadros/go-version"
 )
@@ -360,16 +359,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
 		return nil
 	}
 
-	mode, _ := models.AccessLevel(doer, pr.Issue.Repo)
-	if err = webhook.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
-		Action:      api.HookIssueClosed,
-		Index:       pr.Index,
-		PullRequest: pr.APIFormat(),
-		Repository:  pr.Issue.Repo.APIFormat(mode),
-		Sender:      doer.APIFormat(),
-	}); err != nil {
-		log.Error("PrepareWebhooks: %v", err)
-	}
+	notification.NotifyIssueChangeStatus(doer, pr.Issue, true)
 
 	return nil
 }
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 20939c397f..4e981b2b26 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -11,8 +11,6 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/notification"
-	api "code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/webhook"
 	issue_service "code.gitea.io/gitea/services/issue"
 )
 
@@ -90,23 +88,9 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy
 		if err == nil {
 			for _, pr := range prs {
 				pr.Issue.PullRequest = pr
-				if err = pr.Issue.LoadAttributes(); err != nil {
-					log.Error("LoadAttributes: %v", err)
-					continue
-				}
-				if err = webhook.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
-					Action:      api.HookIssueSynchronized,
-					Index:       pr.Issue.Index,
-					PullRequest: pr.Issue.PullRequest.APIFormat(),
-					Repository:  pr.Issue.Repo.APIFormat(models.AccessModeNone),
-					Sender:      doer.APIFormat(),
-				}); err != nil {
-					log.Error("PrepareWebhooks [pull_id: %v]: %v", pr.ID, err)
-					continue
-				}
+				notification.NotifyPullRequestSynchronized(doer, pr)
 			}
 		}
-
 	}
 
 	addHeadRepoTasks(prs)
diff --git a/services/pull/review.go b/services/pull/review.go
index ffb7be82b2..e4aae3c0d5 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -7,8 +7,7 @@ package pull
 
 import (
 	"code.gitea.io/gitea/models"
-	api "code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/webhook"
+	"code.gitea.io/gitea/modules/notification"
 )
 
 // CreateReview creates a new review based on opts
@@ -18,7 +17,9 @@ func CreateReview(opts models.CreateReviewOptions) (*models.Review, error) {
 		return nil, err
 	}
 
-	return review, reviewHook(review)
+	notification.NotifyPullRequestReview(review.Issue.PullRequest, review, nil)
+
+	return review, nil
 }
 
 // UpdateReview updates a review
@@ -28,43 +29,7 @@ func UpdateReview(review *models.Review) error {
 		return err
 	}
 
-	return reviewHook(review)
-}
-
-func reviewHook(review *models.Review) error {
-	var reviewHookType models.HookEventType
-
-	switch review.Type {
-	case models.ReviewTypeApprove:
-		reviewHookType = models.HookEventPullRequestApproved
-	case models.ReviewTypeComment:
-		reviewHookType = models.HookEventPullRequestComment
-	case models.ReviewTypeReject:
-		reviewHookType = models.HookEventPullRequestRejected
-	default:
-		// unsupported review webhook type here
-		return nil
-	}
-
-	pr := review.Issue.PullRequest
-
-	if err := pr.LoadIssue(); err != nil {
-		return err
-	}
-
-	mode, err := models.AccessLevel(review.Issue.Poster, review.Issue.Repo)
-	if err != nil {
-		return err
-	}
-	return webhook.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{
-		Action:      api.HookIssueSynchronized,
-		Index:       review.Issue.Index,
-		PullRequest: pr.APIFormat(),
-		Repository:  review.Issue.Repo.APIFormat(mode),
-		Sender:      review.Reviewer.APIFormat(),
-		Review: &api.ReviewPayload{
-			Type:    string(reviewHookType),
-			Content: review.Content,
-		},
-	})
+	notification.NotifyPullRequestReview(review.Issue.PullRequest, review, nil)
+
+	return nil
 }