Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions modules/structs/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ type CreatePullReviewComment struct {
OldLineNum int64 `json:"old_position"`
// if comment to new file line or 0
NewLineNum int64 `json:"new_position"`
// if comment is a reply to an existing review thread, the review ID (pull_request_review_id) to reply to;
// the new comment will be threaded under that review
InReplyTo int64 `json:"in_reply_to"`
}

// SubmitPullReviewOptions are options to submit a pending pull request review
Expand Down
54 changes: 50 additions & 4 deletions routers/api/v1/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,8 @@ func CreatePullReview(ctx *context.APIContext) {
// responses:
// "200":
// "$ref": "#/responses/PullReview"
// "204":
// description: no content (reply-only request with no review body)
// "404":
// "$ref": "#/responses/notFound"
// "422":
Expand All @@ -436,8 +438,20 @@ func CreatePullReview(ctx *context.APIContext) {
return
}

// determine review type
reviewType, isWrong := preparePullReviewType(ctx, pr, opts.Event, opts.Body, len(opts.Comments) > 0)
// separate reply comments from pending comments
var pendingComments []api.CreatePullReviewComment
var replyComments []api.CreatePullReviewComment
for _, c := range opts.Comments {
if c.InReplyTo > 0 {
replyComments = append(replyComments, c)
} else {
pendingComments = append(pendingComments, c)
}
}

// determine review type (reply comments count as "having comments" for validation)
hasComments := len(pendingComments) > 0 || len(replyComments) > 0
reviewType, isWrong := preparePullReviewType(ctx, pr, opts.Event, opts.Body, hasComments)
if isWrong {
return
}
Expand Down Expand Up @@ -465,8 +479,40 @@ func CreatePullReview(ctx *context.APIContext) {
opts.CommitID = headCommitID
}

// create review comments
for _, c := range opts.Comments {
// create reply comments (posted immediately, not part of the pending review)
for _, c := range replyComments {
line := c.NewLineNum
if c.OldLineNum > 0 {
line = c.OldLineNum * -1
}

if _, err := pull_service.CreateCodeComment(ctx,
ctx.Doer,
ctx.Repo.GitRepo,
pr.Issue,
line,
c.Body,
c.Path,
false, // not a pending review
c.InReplyTo, // review ID to reply to
opts.CommitID,
nil,
); err != nil {
ctx.APIErrorInternal(err)
return
}
}

// if there are no pending comments and no review body, skip creating a review
// unless the user wants to approve or request changes (which are meaningful without comments)
if len(pendingComments) == 0 && len(opts.Body) == 0 &&
reviewType != issues_model.ReviewTypeApprove && reviewType != issues_model.ReviewTypeReject {
ctx.Status(http.StatusNoContent)
return
}

// create pending review comments
for _, c := range pendingComments {
line := c.NewLineNum
if c.OldLineNum > 0 {
line = c.OldLineNum * -1
Expand Down
9 changes: 9 additions & 0 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

93 changes: 93 additions & 0 deletions tests/integration/api_pull_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,99 @@ func TestAPIPullReviewCommentResolveEndpoints(t *testing.T) {
MakeRequest(t, req, http.StatusForbidden)
}

func TestAPIPullReviewCommentReply(t *testing.T) {
defer tests.PrepareTestEnv(t)()

ctx := t.Context()
pullIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
require.NoError(t, pullIssue.LoadAttributes(ctx))
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: pullIssue.RepoID})

doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
require.NoError(t, pullIssue.LoadPullRequest(ctx))
gitRepo, err := gitrepo.OpenRepository(ctx, repo)
require.NoError(t, err)
defer gitRepo.Close()

latestCommitID, err := gitRepo.GetRefCommitID(pullIssue.PullRequest.GetGitHeadRefName())
require.NoError(t, err)

// Create a code comment to reply to (this creates a review)
comment, err := pull_service.CreateCodeComment(ctx, doer, gitRepo, pullIssue, 1, "original comment", "README.md", false, 0, latestCommitID, nil)
require.NoError(t, err)
require.NotNil(t, comment)
originalReviewID := comment.ReviewID

session := loginUser(t, doer.Name)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)

// Test: reply to an existing review comment using in_reply_to
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews", repo.OwnerName, repo.Name, pullIssue.Index), &api.CreatePullReviewOptions{
Event: "COMMENT",
Comments: []api.CreatePullReviewComment{
{
Path: "README.md",
Body: "reply to original comment",
NewLineNum: 1,
InReplyTo: originalReviewID,
},
},
}).AddTokenAuth(token)
// Reply-only reviews return 204 No Content (no new review is created)
MakeRequest(t, req, http.StatusNoContent)

// Verify the reply comment was created under the original review
reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
IssueID: pullIssue.ID,
ReviewerID: doer.ID,
})
require.NoError(t, err)

// Find the original review and check it has the reply
var foundReply bool
for _, review := range reviews {
if review.ID == originalReviewID {
require.NoError(t, review.LoadCodeComments(ctx))
for _, commentsPerLine := range review.CodeComments {
for _, comments := range commentsPerLine {
for _, c := range comments {
if c.Content == "reply to original comment" {
foundReply = true
}
}
}
}
}
}
assert.True(t, foundReply, "reply comment should be threaded under the original review")

// Test: mix of reply and non-reply comments creates a review
req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews", repo.OwnerName, repo.Name, pullIssue.Index), &api.CreatePullReviewOptions{
Body: "mixed review",
Event: "COMMENT",
Comments: []api.CreatePullReviewComment{
{
Path: "README.md",
Body: "another reply",
NewLineNum: 1,
InReplyTo: originalReviewID,
},
{
Path: "README.md",
Body: "new standalone comment",
NewLineNum: 1,
},
},
}).AddTokenAuth(token)
resp := MakeRequest(t, req, http.StatusOK)
var review api.PullReview
DecodeJSON(t, resp, &review)
assert.EqualValues(t, "COMMENT", review.State)
assert.Equal(t, "mixed review", review.Body)
// Only the non-reply comment should be in the new review
assert.Equal(t, 1, review.CodeCommentsCount)
}

func TestAPIPullReviewStayDismissed(t *testing.T) {
// This test against issue https://github.com/go-gitea/gitea/issues/28542
// where old reviews surface after a review request got dismissed.
Expand Down
Loading