From 6c3433151fdb84a9dc1214442573da2d7cc76e3e Mon Sep 17 00:00:00 2001
From: sebastian-sauer <sauer.sebastian@gmail.com>
Date: Fri, 25 Jun 2021 00:05:51 +0200
Subject: [PATCH] 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 <sauer.sebastian@gmail.com>
---
integrations/api_pull_review_test.go | 54 ++++++++++++++++++++++++++++
routers/api/v1/repo/pull_review.go | 22 ++++++++----
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/integrations/api_pull_review_test.go b/integrations/api_pull_review_test.go
index ebe8539a82..bcc0cbffcb 100644
--- a/integrations/api_pull_review_test.go
+++ b/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)
diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go
index 35414e0a80..323904f45c 100644
--- a/routers/api/v1/repo/pull_review.go
+++ b/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
}