From 7c4c01c0fda90d94cf3785f27b505d9fc502fee6 Mon Sep 17 00:00:00 2001
From: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Date: Fri, 18 Oct 2019 03:33:19 -0500
Subject: [PATCH] Fix review webhooks (#8570)

Signed-off-by: jolheiser <john.olheiser@gmail.com>
---
 models/webhook_discord.go   | 12 ++++++++++-
 routers/repo/pull_review.go |  4 ++--
 services/pull/review.go     | 40 +++++++++++++++++++++++++------------
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/models/webhook_discord.go b/models/webhook_discord.go
index d7a2de0d11..0b190495f2 100644
--- a/models/webhook_discord.go
+++ b/models/webhook_discord.go
@@ -413,7 +413,17 @@ func getDiscordPullRequestApprovalPayload(p *api.PullRequestPayload, meta *Disco
 
 		title = fmt.Sprintf("[%s] Pull request review %s: #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title)
 		text = p.PullRequest.Body
-		color = warnColor
+
+		switch event {
+		case HookEventPullRequestApproved:
+			color = successColor
+		case HookEventPullRequestRejected:
+			color = failedColor
+		case HookEventPullRequestComment:
+			fallthrough
+		default:
+			color = warnColor
+		}
 	}
 
 	return &DiscordPayload{
diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go
index 5eb0dfe9a7..7d030988fd 100644
--- a/routers/repo/pull_review.go
+++ b/routers/repo/pull_review.go
@@ -157,7 +157,7 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
 			return
 		}
 		// No current review. Create a new one!
-		if review, err = models.CreateReview(models.CreateReviewOptions{
+		if review, err = pull_service.CreateReview(models.CreateReviewOptions{
 			Type:     reviewType,
 			Issue:    issue,
 			Reviewer: ctx.User,
@@ -169,7 +169,7 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
 	} else {
 		review.Content = form.Content
 		review.Type = reviewType
-		if err = models.UpdateReview(review); err != nil {
+		if err = pull_service.UpdateReview(review); err != nil {
 			ctx.ServerError("UpdateReview", err)
 			return
 		}
diff --git a/services/pull/review.go b/services/pull/review.go
index 3fdfaaff84..261c2d32d2 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -17,9 +17,23 @@ func CreateReview(opts models.CreateReviewOptions) (*models.Review, error) {
 		return nil, err
 	}
 
+	return review, reviewHook(review)
+}
+
+// UpdateReview updates a review
+func UpdateReview(review *models.Review) error {
+	err := models.UpdateReview(review)
+	if err != nil {
+		return err
+	}
+
+	return reviewHook(review)
+}
+
+func reviewHook(review *models.Review) error {
 	var reviewHookType models.HookEventType
 
-	switch opts.Type {
+	switch review.Type {
 	case models.ReviewTypeApprove:
 		reviewHookType = models.HookEventPullRequestApproved
 	case models.ReviewTypeComment:
@@ -28,30 +42,30 @@ func CreateReview(opts models.CreateReviewOptions) (*models.Review, error) {
 		reviewHookType = models.HookEventPullRequestRejected
 	default:
 		// unsupported review webhook type here
-		return review, nil
+		return nil
 	}
 
-	pr := opts.Issue.PullRequest
+	pr := review.Issue.PullRequest
 
 	if err := pr.LoadIssue(); err != nil {
-		return nil, err
+		return err
 	}
 
-	mode, err := models.AccessLevel(opts.Issue.Poster, opts.Issue.Repo)
+	mode, err := models.AccessLevel(review.Issue.Poster, review.Issue.Repo)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	if err := models.PrepareWebhooks(opts.Issue.Repo, reviewHookType, &api.PullRequestPayload{
+	if err := models.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{
 		Action:      api.HookIssueSynchronized,
-		Index:       opts.Issue.Index,
+		Index:       review.Issue.Index,
 		PullRequest: pr.APIFormat(),
-		Repository:  opts.Issue.Repo.APIFormat(mode),
-		Sender:      opts.Reviewer.APIFormat(),
+		Repository:  review.Issue.Repo.APIFormat(mode),
+		Sender:      review.Reviewer.APIFormat(),
 	}); err != nil {
-		return nil, err
+		return err
 	}
-	go models.HookQueue.Add(opts.Issue.Repo.ID)
+	go models.HookQueue.Add(review.Issue.Repo.ID)
 
-	return review, nil
+	return nil
 }