From 8b4ba3dce7fc99fa328444ef27383dccca49c237 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 9 Jan 2024 12:49:18 +0100 Subject: [PATCH] [GITEA] POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/comments Refs: https://codeberg.org/forgejo/forgejo/issues/2109 --- modules/structs/pull_review.go | 3 + routers/api/v1/api.go | 3 +- routers/api/v1/repo/pull_review.go | 91 +++++++++++++++++++ routers/api/v1/swagger/options.go | 3 + services/convert/pull_review.go | 50 ++++++----- templates/swagger/v1_json.tmpl | 65 ++++++++++++++ tests/integration/api_pull_review_test.go | 104 ++++++++++++++++++++++ 7 files changed, 298 insertions(+), 21 deletions(-) diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index 810be8f521..c77ebea07d 100644 --- a/modules/structs/pull_review.go +++ b/modules/structs/pull_review.go @@ -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"` diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 51c54abf5b..d5e738dd9c 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1217,7 +1217,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) }) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 7b9445be4c..adedf90630 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -208,6 +208,97 @@ 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.CreateCodeComment(ctx, + ctx.Doer, + ctx.Repo.GitRepo, + pr.Issue, + line, + opts.Body, + opts.Path, + // as of e522e774cae2240279fc48c349fc513c9d3353ee + // isPending is not needed because review.ID is always available + // and does not need to be discovered implicitly + false, + review.ID, + // as of e522e774cae2240279fc48c349fc513c9d3353ee + // latestCommitID is not needed because it is only used to + // create a new review in case it does not already exist + "", + ) + 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 diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index cca6d2d572..2886b865e8 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -161,6 +161,9 @@ type swaggerParameterBodies struct { // in:body CreatePullReviewComment api.CreatePullReviewComment + // in:body + CreatePullReviewCommentOptions api.CreatePullReviewCommentOptions + // in:body SubmitPullReviewOptions api.SubmitPullReviewOptions diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index 85f2d703d2..b06bf537f0 100644 --- a/services/convert/pull_review.go +++ b/services/convert/pull_review.go @@ -76,6 +76,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 { @@ -90,26 +117,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) } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 52c19a06d2..8d7de9abd9 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -11539,6 +11539,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": { @@ -18517,6 +18578,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", diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go index daa136b21e..98a26161c8 100644 --- a/tests/integration/api_pull_review_test.go +++ b/tests/integration/api_pull_review_test.go @@ -18,8 +18,112 @@ 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 + existingCommentBody := "existing comment body" + 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, + Comments: []api.CreatePullReviewComment{ + { + Path: path, + Body: existingCommentBody, + OldLineNum: reviewLine, + }, + }, + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &review) + require.EqualValues(t, string(event), review.State) + require.EqualValues(t, 1, 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) + } + + { + 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) + require.Len(t, reviewComments, 1) + assert.EqualValues(t, username, reviewComments[0].Poster.UserName) + assert.EqualValues(t, existingCommentBody, reviewComments[0].Body) + } + + 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})