From 0df7cab4fb0c84444ceb4c9b5f28f410a38025f5 Mon Sep 17 00:00:00 2001
From: Lanre Adelowo <adelowomailbox@gmail.com>
Date: Tue, 7 Aug 2018 18:15:41 +0100
Subject: [PATCH] prevent empty review comment (#4632)

* prevent empty review comment

This would only require a comment for rejection and comment

* add tests

* add comment
---
 modules/auth/repo_form.go       |  8 +++++++
 modules/auth/repo_form_test.go  | 41 +++++++++++++++++++++++++++++++++
 options/locale/locale_en-US.ini |  1 +
 routers/repo/pull_review.go     |  7 ++++++
 4 files changed, 57 insertions(+)
 create mode 100644 modules/auth/repo_form_test.go

diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go
index d205a6fd5..3c7940afc 100644
--- a/modules/auth/repo_form.go
+++ b/modules/auth/repo_form.go
@@ -402,6 +402,14 @@ func (f SubmitReviewForm) ReviewType() models.ReviewType {
 	}
 }
 
+// HasEmptyContent checks if the content of the review form is empty.
+func (f SubmitReviewForm) HasEmptyContent() bool {
+	reviewType := f.ReviewType()
+
+	return (reviewType == models.ReviewTypeComment || reviewType == models.ReviewTypeReject) &&
+		len(strings.TrimSpace(f.Content)) == 0
+}
+
 // __________       .__
 // \______   \ ____ |  |   ____ _____    ______ ____
 //  |       _// __ \|  | _/ __ \\__  \  /  ___// __ \
diff --git a/modules/auth/repo_form_test.go b/modules/auth/repo_form_test.go
new file mode 100644
index 000000000..f6223d6c8
--- /dev/null
+++ b/modules/auth/repo_form_test.go
@@ -0,0 +1,41 @@
+// 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 auth
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSubmitReviewForm_IsEmpty(t *testing.T) {
+
+	cases := []struct {
+		form     SubmitReviewForm
+		expected bool
+	}{
+		// Approved PR with a comment shouldn't count as empty
+		{SubmitReviewForm{Type: "approve", Content: "Awesome"}, false},
+
+		// Approved PR without a comment shouldn't count as empty
+		{SubmitReviewForm{Type: "approve", Content: ""}, false},
+
+		// Rejected PR without a comment should count as empty
+		{SubmitReviewForm{Type: "reject", Content: ""}, true},
+
+		// Rejected PR with a comment shouldn't count as empty
+		{SubmitReviewForm{Type: "reject", Content: "Awesome"}, false},
+
+		// Comment review on a PR with a comment shouldn't count as empty
+		{SubmitReviewForm{Type: "comment", Content: "Awesome"}, false},
+
+		// Comment review on a PR without a comment should count as empty
+		{SubmitReviewForm{Type: "comment", Content: ""}, true},
+	}
+
+	for _, v := range cases {
+		assert.Equal(t, v.expected, v.form.HasEmptyContent())
+	}
+}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 85178d341..145dfe803 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -815,6 +815,7 @@ issues.dependency.add_error_cannot_create_circular = You cannot create a depende
 issues.dependency.add_error_dep_not_same_repo = Both issues must be in the same repository.
 issues.review.approve = "approved these changes %s"
 issues.review.comment = "reviewed %s"
+issues.review.content.empty = You need to leave a comment indicating the requested change(s).
 issues.review.reject = "rejected these changes %s"
 issues.review.pending = Pending
 issues.review.review = Review
diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go
index fa13cacfd..7ca02ac80 100644
--- a/routers/repo/pull_review.go
+++ b/routers/repo/pull_review.go
@@ -107,6 +107,13 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) {
 		ctx.ServerError("GetCurrentReview", fmt.Errorf("unknown ReviewType: %s", form.Type))
 		return
 	}
+
+	if form.HasEmptyContent() {
+		ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty"))
+		ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index))
+		return
+	}
+
 	review, err = models.GetCurrentReview(ctx.User, issue)
 	if err != nil {
 		if !models.IsErrReviewNotExist(err) {