mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-11-10 04:05:42 +01:00
[GITEA] POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments
Refs: https://codeberg.org/forgejo/forgejo/issues/2109 (cherry picked from commit8b4ba3dce7
) (cherry picked from commit196edea0f9
) [GITEA] POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments (squash) do not implicitly create a review If a comment already exists in a review, the comment is added. If it is the first comment added to a review, it will implicitly create a new review instead of adding to the existing one. The pull_service.CreateCodeComment function is responsibe for this behavior and it will defer to createCodeComment once the review is determined, either because it was found or because it was created. Rename createCodeComment into CreateCodeCommentKnownReviewID to expose it and change the API endpoint to use it instead. Since the review is provided by the user and verified to exist already, there is no need for the logic implemented by CreateCodeComment. The tests are modified to remove the initial comment from the fixture because it was creating the false positive. I was verified to fail without this fix. (cherry picked from commit6a555996dc
) (cherry picked from commitb173a0ccee
) (cherry picked from commit838ab9740a
)
This commit is contained in:
parent
91b5aa0e5f
commit
603a44edf0
8 changed files with 274 additions and 24 deletions
|
@ -89,6 +89,9 @@ type CreatePullReviewComment struct {
|
|||
NewLineNum int64 `json:"new_position"`
|
||||
}
|
||||
|
||||
// CreatePullReviewCommentOptions are options to create a pull review comment
|
||||
type CreatePullReviewCommentOptions CreatePullReviewComment
|
||||
|
||||
// SubmitPullReviewOptions are options to submit a pending pull review
|
||||
type SubmitPullReviewOptions struct {
|
||||
Event ReviewStateType `json:"event"`
|
||||
|
|
|
@ -1228,7 +1228,8 @@ func Routes() *web.Route {
|
|||
Delete(reqToken(), repo.DeletePullReview).
|
||||
Post(reqToken(), bind(api.SubmitPullReviewOptions{}), repo.SubmitPullReview)
|
||||
m.Combo("/comments").
|
||||
Get(repo.GetPullReviewComments)
|
||||
Get(repo.GetPullReviewComments).
|
||||
Post(reqToken(), bind(api.CreatePullReviewCommentOptions{}), repo.CreatePullReviewComment)
|
||||
m.Post("/dismissals", reqToken(), bind(api.DismissPullReviewOptions{}), repo.DismissPullReview)
|
||||
m.Post("/undismissals", reqToken(), repo.UnDismissPullReview)
|
||||
})
|
||||
|
|
|
@ -208,6 +208,89 @@ func GetPullReviewComments(ctx *context.APIContext) {
|
|||
ctx.JSON(http.StatusOK, apiComments)
|
||||
}
|
||||
|
||||
// CreatePullReviewComments add a new comment to a pull request review
|
||||
func CreatePullReviewComment(ctx *context.APIContext) {
|
||||
// swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments repository repoCreatePullReviewComment
|
||||
// ---
|
||||
// summary: Add a new comment to a pull request review
|
||||
// produces:
|
||||
// - application/json
|
||||
// parameters:
|
||||
// - name: owner
|
||||
// in: path
|
||||
// description: owner of the repo
|
||||
// type: string
|
||||
// required: true
|
||||
// - name: repo
|
||||
// in: path
|
||||
// description: name of the repo
|
||||
// type: string
|
||||
// required: true
|
||||
// - name: index
|
||||
// in: path
|
||||
// description: index of the pull request
|
||||
// type: integer
|
||||
// format: int64
|
||||
// required: true
|
||||
// - name: id
|
||||
// in: path
|
||||
// description: id of the review
|
||||
// type: integer
|
||||
// format: int64
|
||||
// required: true
|
||||
// - name: body
|
||||
// in: body
|
||||
// required: true
|
||||
// schema:
|
||||
// "$ref": "#/definitions/CreatePullReviewCommentOptions"
|
||||
// responses:
|
||||
// "200":
|
||||
// "$ref": "#/responses/PullReviewComment"
|
||||
// "404":
|
||||
// "$ref": "#/responses/notFound"
|
||||
// "422":
|
||||
// "$ref": "#/responses/validationError"
|
||||
|
||||
opts := web.GetForm(ctx).(*api.CreatePullReviewCommentOptions)
|
||||
|
||||
review, pr, statusSet := prepareSingleReview(ctx)
|
||||
if statusSet {
|
||||
return
|
||||
}
|
||||
|
||||
if err := pr.Issue.LoadRepo(ctx); err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
|
||||
line := opts.NewLineNum
|
||||
if opts.OldLineNum > 0 {
|
||||
line = opts.OldLineNum * -1
|
||||
}
|
||||
|
||||
comment, err := pull_service.CreateCodeCommentKnownReviewID(ctx,
|
||||
ctx.Doer,
|
||||
pr.Issue.Repo,
|
||||
pr.Issue,
|
||||
opts.Body,
|
||||
opts.Path,
|
||||
line,
|
||||
review.ID,
|
||||
)
|
||||
if err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
|
||||
apiComment, err := convert.ToPullReviewComment(ctx, review, comment, ctx.Doer)
|
||||
if err != nil {
|
||||
ctx.InternalServerError(err)
|
||||
return
|
||||
}
|
||||
|
||||
ctx.JSON(http.StatusOK, apiComment)
|
||||
}
|
||||
|
||||
// DeletePullReview delete a specific review from a pull request
|
||||
func DeletePullReview(ctx *context.APIContext) {
|
||||
// swagger:operation DELETE /repos/{owner}/{repo}/pulls/{index}/reviews/{id} repository repoDeletePullReview
|
||||
|
|
|
@ -161,6 +161,9 @@ type swaggerParameterBodies struct {
|
|||
// in:body
|
||||
CreatePullReviewComment api.CreatePullReviewComment
|
||||
|
||||
// in:body
|
||||
CreatePullReviewCommentOptions api.CreatePullReviewCommentOptions
|
||||
|
||||
// in:body
|
||||
SubmitPullReviewOptions api.SubmitPullReviewOptions
|
||||
|
||||
|
|
|
@ -78,6 +78,33 @@ func ToPullReviewList(ctx context.Context, rl []*issues_model.Review, doer *user
|
|||
return result, nil
|
||||
}
|
||||
|
||||
// ToPullReviewCommentList convert the CodeComments of an review to it's api format
|
||||
func ToPullReviewComment(ctx context.Context, review *issues_model.Review, comment *issues_model.Comment, doer *user_model.User) (*api.PullReviewComment, error) {
|
||||
apiComment := &api.PullReviewComment{
|
||||
ID: comment.ID,
|
||||
Body: comment.Content,
|
||||
Poster: ToUser(ctx, comment.Poster, doer),
|
||||
Resolver: ToUser(ctx, comment.ResolveDoer, doer),
|
||||
ReviewID: review.ID,
|
||||
Created: comment.CreatedUnix.AsTime(),
|
||||
Updated: comment.UpdatedUnix.AsTime(),
|
||||
Path: comment.TreePath,
|
||||
CommitID: comment.CommitSHA,
|
||||
OrigCommitID: comment.OldRef,
|
||||
DiffHunk: patch2diff(comment.Patch),
|
||||
HTMLURL: comment.HTMLURL(ctx),
|
||||
HTMLPullURL: review.Issue.HTMLURL(),
|
||||
}
|
||||
|
||||
if comment.Line < 0 {
|
||||
apiComment.OldLineNum = comment.UnsignedLine()
|
||||
} else {
|
||||
apiComment.LineNum = comment.UnsignedLine()
|
||||
}
|
||||
|
||||
return apiComment, nil
|
||||
}
|
||||
|
||||
// ToPullReviewCommentList convert the CodeComments of an review to it's api format
|
||||
func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, doer *user_model.User) ([]*api.PullReviewComment, error) {
|
||||
if err := review.LoadAttributes(ctx); err != nil {
|
||||
|
@ -92,26 +119,9 @@ func ToPullReviewCommentList(ctx context.Context, review *issues_model.Review, d
|
|||
for _, lines := range review.CodeComments {
|
||||
for _, comments := range lines {
|
||||
for _, comment := range comments {
|
||||
apiComment := &api.PullReviewComment{
|
||||
ID: comment.ID,
|
||||
Body: comment.Content,
|
||||
Poster: ToUser(ctx, comment.Poster, doer),
|
||||
Resolver: ToUser(ctx, comment.ResolveDoer, doer),
|
||||
ReviewID: review.ID,
|
||||
Created: comment.CreatedUnix.AsTime(),
|
||||
Updated: comment.UpdatedUnix.AsTime(),
|
||||
Path: comment.TreePath,
|
||||
CommitID: comment.CommitSHA,
|
||||
OrigCommitID: comment.OldRef,
|
||||
DiffHunk: patch2diff(comment.Patch),
|
||||
HTMLURL: comment.HTMLURL(ctx),
|
||||
HTMLPullURL: review.Issue.HTMLURL(),
|
||||
}
|
||||
|
||||
if comment.Line < 0 {
|
||||
apiComment.OldLineNum = comment.UnsignedLine()
|
||||
} else {
|
||||
apiComment.LineNum = comment.UnsignedLine()
|
||||
apiComment, err := ToPullReviewComment(ctx, review, comment, doer)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
apiComments = append(apiComments, apiComment)
|
||||
}
|
||||
|
|
|
@ -96,7 +96,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
|
|||
return nil, err
|
||||
}
|
||||
|
||||
comment, err := createCodeComment(ctx,
|
||||
comment, err := CreateCodeCommentKnownReviewID(ctx,
|
||||
doer,
|
||||
issue.Repo,
|
||||
issue,
|
||||
|
@ -136,7 +136,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
|
|||
}
|
||||
}
|
||||
|
||||
comment, err := createCodeComment(ctx,
|
||||
comment, err := CreateCodeCommentKnownReviewID(ctx,
|
||||
doer,
|
||||
issue.Repo,
|
||||
issue,
|
||||
|
@ -162,7 +162,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
|
|||
}
|
||||
|
||||
// createCodeComment creates a plain code comment at the specified line / path
|
||||
func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64) (*issues_model.Comment, error) {
|
||||
func CreateCodeCommentKnownReviewID(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64) (*issues_model.Comment, error) {
|
||||
var commitID, patch string
|
||||
if err := issue.LoadPullRequest(ctx); err != nil {
|
||||
return nil, fmt.Errorf("LoadPullRequest: %w", err)
|
||||
|
|
65
templates/swagger/v1_json.tmpl
generated
65
templates/swagger/v1_json.tmpl
generated
|
@ -11542,6 +11542,67 @@
|
|||
"$ref": "#/responses/notFound"
|
||||
}
|
||||
}
|
||||
},
|
||||
"post": {
|
||||
"produces": [
|
||||
"application/json"
|
||||
],
|
||||
"tags": [
|
||||
"repository"
|
||||
],
|
||||
"summary": "Add a new comment to a pull request review",
|
||||
"operationId": "repoCreatePullReviewComment",
|
||||
"parameters": [
|
||||
{
|
||||
"type": "string",
|
||||
"description": "owner of the repo",
|
||||
"name": "owner",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "string",
|
||||
"description": "name of the repo",
|
||||
"name": "repo",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "index of the pull request",
|
||||
"name": "index",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "id of the review",
|
||||
"name": "id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"name": "body",
|
||||
"in": "body",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"$ref": "#/definitions/CreatePullReviewCommentOptions"
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"$ref": "#/responses/PullReviewComment"
|
||||
},
|
||||
"404": {
|
||||
"$ref": "#/responses/notFound"
|
||||
},
|
||||
"422": {
|
||||
"$ref": "#/responses/validationError"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"/repos/{owner}/{repo}/pulls/{index}/reviews/{id}/dismissals": {
|
||||
|
@ -18528,6 +18589,10 @@
|
|||
},
|
||||
"x-go-package": "code.gitea.io/gitea/modules/structs"
|
||||
},
|
||||
"CreatePullReviewCommentOptions": {
|
||||
"description": "CreatePullReviewCommentOptions are options to create a pull review comment",
|
||||
"$ref": "#/definitions/CreatePullReviewComment"
|
||||
},
|
||||
"CreatePullReviewOptions": {
|
||||
"description": "CreatePullReviewOptions are options to create a pull review",
|
||||
"type": "object",
|
||||
|
|
|
@ -18,8 +18,93 @@ import (
|
|||
"code.gitea.io/gitea/tests"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestAPIPullReviewCreateComment(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
pullIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
|
||||
assert.NoError(t, pullIssue.LoadAttributes(db.DefaultContext))
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: pullIssue.RepoID})
|
||||
|
||||
username := "user2"
|
||||
session := loginUser(t, username)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
|
||||
// as of e522e774cae2240279fc48c349fc513c9d3353ee
|
||||
// There should be no reason for CreateComment to behave differently
|
||||
// depending on the event associated with the review. But the logic of the implementation
|
||||
// at this point in time is very involved and deserves these seemingly redundant
|
||||
// test.
|
||||
for _, event := range []api.ReviewStateType{
|
||||
api.ReviewStatePending,
|
||||
api.ReviewStateRequestChanges,
|
||||
api.ReviewStateApproved,
|
||||
api.ReviewStateComment,
|
||||
} {
|
||||
t.Run("Event_"+string(event), func(t *testing.T) {
|
||||
path := "README.md"
|
||||
var review api.PullReview
|
||||
var reviewLine int64 = 1
|
||||
|
||||
{
|
||||
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/pulls/%d/reviews", repo.FullName(), pullIssue.Index), &api.CreatePullReviewOptions{
|
||||
Body: "body1",
|
||||
Event: event,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
DecodeJSON(t, resp, &review)
|
||||
require.EqualValues(t, string(event), review.State)
|
||||
require.EqualValues(t, 0, review.CodeCommentsCount)
|
||||
}
|
||||
|
||||
{
|
||||
req := NewRequestf(t, http.MethodGet, "/api/v1/repos/%s/pulls/%d/reviews/%d", repo.FullName(), pullIssue.Index, review.ID).
|
||||
AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
var getReview api.PullReview
|
||||
DecodeJSON(t, resp, &getReview)
|
||||
require.EqualValues(t, getReview, review)
|
||||
}
|
||||
|
||||
newCommentBody := "first new line"
|
||||
|
||||
{
|
||||
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/pulls/%d/reviews/%d/comments", repo.FullName(), pullIssue.Index, review.ID), &api.CreatePullReviewCommentOptions{
|
||||
Path: path,
|
||||
Body: newCommentBody,
|
||||
OldLineNum: reviewLine,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
var reviewComment *api.PullReviewComment
|
||||
DecodeJSON(t, resp, &reviewComment)
|
||||
assert.EqualValues(t, review.ID, reviewComment.ReviewID)
|
||||
}
|
||||
|
||||
{
|
||||
req := NewRequestf(t, http.MethodGet, "/api/v1/repos/%s/pulls/%d/reviews/%d/comments", repo.FullName(), pullIssue.Index, review.ID).
|
||||
AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
var reviewComments []*api.PullReviewComment
|
||||
DecodeJSON(t, resp, &reviewComments)
|
||||
assert.Len(t, reviewComments, 2)
|
||||
assert.EqualValues(t, existingCommentBody, reviewComments[0].Body)
|
||||
assert.EqualValues(t, reviewComments[0].OldLineNum, reviewComments[1].OldLineNum)
|
||||
assert.EqualValues(t, reviewComments[0].LineNum, reviewComments[1].LineNum)
|
||||
assert.EqualValues(t, newCommentBody, reviewComments[1].Body)
|
||||
assert.EqualValues(t, path, reviewComments[1].Path)
|
||||
}
|
||||
|
||||
{
|
||||
req := NewRequestf(t, http.MethodDelete, "/api/v1/repos/%s/pulls/%d/reviews/%d", repo.FullName(), pullIssue.Index, review.ID).
|
||||
AddTokenAuth(token)
|
||||
MakeRequest(t, req, http.StatusNoContent)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPIPullReview(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
pullIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
|
||||
|
|
Loading…
Reference in a new issue