mirror of
https://github.com/go-gitea/gitea.git
synced 2024-08-26 21:54:19 -04:00
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>
This commit is contained in:
parent
fd6b1be1b6
commit
6c3433151f
@ -11,6 +11,7 @@ import (
|
|||||||
|
|
||||||
"code.gitea.io/gitea/models"
|
"code.gitea.io/gitea/models"
|
||||||
api "code.gitea.io/gitea/modules/structs"
|
api "code.gitea.io/gitea/modules/structs"
|
||||||
|
jsoniter "github.com/json-iterator/go"
|
||||||
"github.com/stretchr/testify/assert"
|
"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)
|
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)
|
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
|
// test get review requests
|
||||||
// to make it simple, use same api with get review
|
// to make it simple, use same api with get review
|
||||||
pullIssue12 := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 12}).(*models.Issue)
|
pullIssue12 := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 12}).(*models.Issue)
|
||||||
|
@ -307,7 +307,7 @@ func CreatePullReview(ctx *context.APIContext) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// determine review type
|
// 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 {
|
if isWrong {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@ -429,7 +429,7 @@ func SubmitPullReview(ctx *context.APIContext) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// determine review type
|
// 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 {
|
if isWrong {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@ -463,12 +463,15 @@ func SubmitPullReview(ctx *context.APIContext) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// preparePullReviewType return ReviewType and false or nil and true if an error happen
|
// 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 {
|
if err := pr.LoadIssue(); err != nil {
|
||||||
ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
|
ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
|
||||||
return -1, true
|
return -1, true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
needsBody := true
|
||||||
|
hasBody := len(strings.TrimSpace(body)) > 0
|
||||||
|
|
||||||
var reviewType models.ReviewType
|
var reviewType models.ReviewType
|
||||||
switch event {
|
switch event {
|
||||||
case api.ReviewStateApproved:
|
case api.ReviewStateApproved:
|
||||||
@ -478,6 +481,7 @@ func preparePullReviewType(ctx *context.APIContext, pr *models.PullRequest, even
|
|||||||
return -1, true
|
return -1, true
|
||||||
}
|
}
|
||||||
reviewType = models.ReviewTypeApprove
|
reviewType = models.ReviewTypeApprove
|
||||||
|
needsBody = false
|
||||||
|
|
||||||
case api.ReviewStateRequestChanges:
|
case api.ReviewStateRequestChanges:
|
||||||
// can not reject your own PR
|
// can not reject your own PR
|
||||||
@ -489,13 +493,19 @@ func preparePullReviewType(ctx *context.APIContext, pr *models.PullRequest, even
|
|||||||
|
|
||||||
case api.ReviewStateComment:
|
case api.ReviewStateComment:
|
||||||
reviewType = models.ReviewTypeComment
|
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:
|
default:
|
||||||
reviewType = models.ReviewTypePending
|
reviewType = models.ReviewTypePending
|
||||||
}
|
}
|
||||||
|
|
||||||
// reject reviews with empty body if not approve type
|
// reject reviews with empty body if a body is required for this call
|
||||||
if reviewType != models.ReviewTypeApprove && len(strings.TrimSpace(body)) == 0 {
|
if needsBody && !hasBody {
|
||||||
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("review event %s need body", event))
|
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("review event %s requires a body", event))
|
||||||
return -1, true
|
return -1, true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user