API: Allow COMMENT reviews to not specify a body (#16229)

* Allow COMMENT reviews to not specify a body

when using web ui there is no need to specify a body.
so we don't need to specify a body if adding a COMMENT-review
via our api.

* Ensure comments or Body is provided

and add some integration tests for reviewtype COMMENT.

Signed-off-by: Sebastian Sauer <[email protected]>

Author: sebastian-sauer

integrations/api_pull_review_test.go

@@ -11,6 +11,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	api "code.gitea.io/gitea/modules/structs"
+	jsoniter "github.com/json-iterator/go"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -139,6 +140,59 @@ func TestAPIPullReview(t *testing.T) {
 	req = NewRequestf(t, http.MethodDelete, "/api/v1/repos/%s/%s/pulls/%d/reviews/%d?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, review.ID, token)
 	resp = session.MakeRequest(t, req, http.StatusNoContent)
 
+	// test CreatePullReview Comment without body but with comments
+	req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.CreatePullReviewOptions{
+		// Body:  "",
+		Event: "COMMENT",
+		Comments: []api.CreatePullReviewComment{{
+			Path:       "README.md",
+			Body:       "first new line",
+			OldLineNum: 0,
+			NewLineNum: 1,
+		}, {
+			Path:       "README.md",
+			Body:       "first old line",
+			OldLineNum: 1,
+			NewLineNum: 0,
+		},
+		},
+	})
+	var commentReview api.PullReview
+
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &commentReview)
+	assert.EqualValues(t, "COMMENT", commentReview.State)
+	assert.EqualValues(t, 2, commentReview.CodeCommentsCount)
+	assert.EqualValues(t, "", commentReview.Body)
+	assert.EqualValues(t, false, commentReview.Dismissed)
+
+	// test CreatePullReview Comment with body but without comments
+	commentBody := "This is a body of the comment."
+	req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.CreatePullReviewOptions{
+		Body:     commentBody,
+		Event:    "COMMENT",
+		Comments: []api.CreatePullReviewComment{},
+	})
+
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &commentReview)
+	assert.EqualValues(t, "COMMENT", commentReview.State)
+	assert.EqualValues(t, 0, commentReview.CodeCommentsCount)
+	assert.EqualValues(t, commentBody, commentReview.Body)
+	assert.EqualValues(t, false, commentReview.Dismissed)
+
+	// test CreatePullReview Comment without body and no comments
+	req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.CreatePullReviewOptions{
+		Body:     "",
+		Event:    "COMMENT",
+		Comments: []api.CreatePullReviewComment{},
+	})
+	resp = session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+	errMap := make(map[string]interface{})
+	json := jsoniter.ConfigCompatibleWithStandardLibrary
+	json.Unmarshal(resp.Body.Bytes(), &errMap)
+	assert.EqualValues(t, "review event COMMENT requires a body or a comment", errMap["message"].(string))
+
 	// test get review requests
 	// to make it simple, use same api with get review
 	pullIssue12 := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 12}).(*models.Issue)

routers/api/v1/repo/pull_review.go

@@ -307,7 +307,7 @@ func CreatePullReview(ctx *context.APIContext) {
 	}
 
 	// determine review type
-	reviewType, isWrong := preparePullReviewType(ctx, pr, opts.Event, opts.Body)
+	reviewType, isWrong := preparePullReviewType(ctx, pr, opts.Event, opts.Body, len(opts.Comments) > 0)
 	if isWrong {
 		return
 	}
@@ -429,7 +429,7 @@ func SubmitPullReview(ctx *context.APIContext) {
 	}
 
 	// determine review type
-	reviewType, isWrong := preparePullReviewType(ctx, pr, opts.Event, opts.Body)
+	reviewType, isWrong := preparePullReviewType(ctx, pr, opts.Event, opts.Body, len(review.Comments) > 0)
 	if isWrong {
 		return
 	}
@@ -463,12 +463,15 @@ func SubmitPullReview(ctx *context.APIContext) {
 }
 
 // preparePullReviewType return ReviewType and false or nil and true if an error happen
-func preparePullReviewType(ctx *context.APIContext, pr *models.PullRequest, event api.ReviewStateType, body string) (models.ReviewType, bool) {
+func preparePullReviewType(ctx *context.APIContext, pr *models.PullRequest, event api.ReviewStateType, body string, hasComments bool) (models.ReviewType, bool) {
 	if err := pr.LoadIssue(); err != nil {
 		ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
 		return -1, true
 	}
 
+	needsBody := true
+	hasBody := len(strings.TrimSpace(body)) > 0
+
 	var reviewType models.ReviewType
 	switch event {
 	case api.ReviewStateApproved:
@@ -478,6 +481,7 @@ func preparePullReviewType(ctx *context.APIContext, pr *models.PullRequest, even
 			return -1, true
 		}
 		reviewType = models.ReviewTypeApprove
+		needsBody = false
 
 	case api.ReviewStateRequestChanges:
 		// can not reject your own PR
@@ -489,13 +493,19 @@ func preparePullReviewType(ctx *context.APIContext, pr *models.PullRequest, even
 
 	case api.ReviewStateComment:
 		reviewType = models.ReviewTypeComment
+		needsBody = false
+		// if there is no body we need to ensure that there are comments
+		if !hasBody && !hasComments {
+			ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("review event %s requires a body or a comment", event))
+			return -1, true
+		}
 	default:
 		reviewType = models.ReviewTypePending
 	}
 
-	// reject reviews with empty body if not approve type
-	if reviewType != models.ReviewTypeApprove && len(strings.TrimSpace(body)) == 0 {
-		ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("review event %s need body", event))
+	// reject reviews with empty body if a body is required for this call
+	if needsBody && !hasBody {
+		ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("review event %s requires a body", event))
 		return -1, true
 	}