Skip to content
Merged
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
5 changes: 5 additions & 0 deletions modules/structs/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ type CreatePullReviewComment struct {
NewLineNum int64 `json:"new_position"`
}

// CreatePullReviewCommentReplyOptions are options to reply to a pull request review comment
type CreatePullReviewCommentReplyOptions struct {
Body string `json:"body" binding:"Required"`
}

// SubmitPullReviewOptions are options to submit a pending pull request review
type SubmitPullReviewOptions struct {
Event ReviewStateType `json:"event"`
Expand Down
1 change: 1 addition & 0 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,7 @@ func Routes() *web.Router {
m.Combo("/requested_reviewers", reqToken()).
Delete(bind(api.PullReviewRequestOptions{}), repo.DeleteReviewRequests).
Post(bind(api.PullReviewRequestOptions{}), repo.CreateReviewRequests)
m.Post("/comments/{id}/replies", reqToken(), mustNotBeArchived, bind(api.CreatePullReviewCommentReplyOptions{}), repo.CreatePullReviewCommentReply)
})
m.Get("/{base}/*", repo.GetPullRequestByBaseHead)
}, mustAllowPulls, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo())
Expand Down
82 changes: 82 additions & 0 deletions routers/api/v1/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,88 @@ func GetPullReviewComments(ctx *context.APIContext) {
ctx.JSON(http.StatusOK, apiComments)
}

// CreatePullReviewCommentReply replies to a pull request review comment.
// The URL mirrors GitHub's endpoint, {index} is verified against the parent comment's pull request.
func CreatePullReviewCommentReply(ctx *context.APIContext) {
// swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/comments/{id}/replies repository repoCreatePullReviewCommentReply
// ---
// summary: Reply to a pull request review comment
// consumes:
// - application/json
// 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 comment to reply to
// type: integer
// format: int64
// required: true
// - name: body
// in: body
// required: true
// schema:
// "$ref": "#/definitions/CreatePullReviewCommentReplyOptions"
// responses:
// "201":
// "$ref": "#/responses/PullReviewComment"
// "400":
// "$ref": "#/responses/validationError"
// "404":
// "$ref": "#/responses/notFound"
// "422":
// "$ref": "#/responses/validationError"

opts := web.GetForm(ctx).(*api.CreatePullReviewCommentReplyOptions)

parent := getPullReviewCommentToResolve(ctx)
if parent == nil {
return
}
if parent.Issue.Index != ctx.PathParamInt64("index") {
Comment thread
wxiaoguang marked this conversation as resolved.
ctx.APIErrorNotFound()
return
}
if parent.ReviewID == 0 {
ctx.APIError(http.StatusBadRequest, "comment is not a review comment")
return
}

comment, err := pull_service.CreateCodeComment(ctx,
ctx.Doer, ctx.Repo.GitRepo, parent.Issue,
parent.Line, opts.Body, parent.TreePath,
false, parent.ReviewID,
"", nil,
)
if err != nil {
ctx.APIErrorInternal(err)
return
}
if err := comment.LoadPoster(ctx); err != nil {
ctx.APIErrorInternal(err)
return
}
comment.Issue = parent.Issue

ctx.JSON(http.StatusCreated, convert.ToPullReviewComment(ctx, comment, ctx.Doer))
}

// ResolvePullReviewComment resolves a review comment in a pull request
func ResolvePullReviewComment(ctx *context.APIContext) {
// swagger:operation POST /repos/{owner}/{repo}/pulls/comments/{id}/resolve repository repoResolvePullReviewComment
Expand Down
3 changes: 3 additions & 0 deletions routers/api/v1/swagger/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ type swaggerParameterBodies struct {
// in:body
CreatePullReviewComment api.CreatePullReviewComment

// in:body
CreatePullReviewCommentReplyOptions api.CreatePullReviewCommentReplyOptions

// in:body
SubmitPullReviewOptions api.SubmitPullReviewOptions

Expand Down
80 changes: 80 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.

54 changes: 54 additions & 0 deletions tests/integration/api_pull_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import (

func TestAPIPullReview(t *testing.T) {
defer tests.PrepareTestEnv(t)()
t.Run("General", testAPIPullReviewGeneral)
t.Run("CommentReply", testAPIPullReviewCommentReply)
}

func testAPIPullReviewGeneral(t *testing.T) {
pullIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
assert.NoError(t, pullIssue.LoadAttributes(t.Context()))
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: pullIssue.RepoID})
Expand Down Expand Up @@ -526,6 +531,55 @@ func TestAPIPullReviewStayDismissed(t *testing.T) {
pullIssue.ID, user8.ID, 2, 0, 3, false)
}

func testAPIPullReviewCommentReply(t *testing.T) {
pullIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
require.NoError(t, pullIssue.LoadRepo(t.Context()))
require.NoError(t, pullIssue.LoadPullRequest(t.Context()))
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
gitRepo, err := gitrepo.OpenRepository(t.Context(), pullIssue.Repo)
require.NoError(t, err)
defer gitRepo.Close()

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

parent, err := pull_service.CreateCodeComment(t.Context(), doer, gitRepo, pullIssue, 1, "parent comment", "README.md", false, 0, commitID, nil)
require.NoError(t, err)
require.NotZero(t, parent.ReviewID)

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

url := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/comments/%d/replies", repo.OwnerName, repo.Name, pullIssue.Index, parent.ID)

// happy path
req := NewRequestWithJSON(t, http.MethodPost, url, &api.CreatePullReviewCommentReplyOptions{Body: "the reply"}).AddTokenAuth(token)
resp := MakeRequest(t, req, http.StatusCreated)

var reply api.PullReviewComment
DecodeJSON(t, resp, &reply)
assert.Equal(t, "the reply", reply.Body)
assert.Equal(t, parent.ReviewID, reply.ReviewID)
assert.Equal(t, "README.md", reply.Path)

// empty body — caught by binding
req = NewRequestWithJSON(t, http.MethodPost, url, &api.CreatePullReviewCommentReplyOptions{}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusUnprocessableEntity)

// reply to a non-existent comment
bad := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/comments/%d/replies", repo.OwnerName, repo.Name, pullIssue.Index, 999999)
req = NewRequestWithJSON(t, http.MethodPost, bad, &api.CreatePullReviewCommentReplyOptions{Body: "x"}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)

// reply to a code comment that belongs to a different PR — 404
otherCodeComment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 4, Type: issues_model.CommentTypeCode})
require.NotEqual(t, pullIssue.ID, otherCodeComment.IssueID)
wrongPR := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/comments/%d/replies", repo.OwnerName, repo.Name, pullIssue.Index, otherCodeComment.ID)
req = NewRequestWithJSON(t, http.MethodPost, wrongPR, &api.CreatePullReviewCommentReplyOptions{Body: "x"}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)
}

Comment thread
silverwind marked this conversation as resolved.
func reviewsCountCheck(t *testing.T, name string, issueID, reviewerID int64, expectedDismissed, expectedRequested, expectedTotal int, expectApproval bool) {
t.Run(name, func(t *testing.T) {
unittest.AssertCountByCond(t, "review", builder.Eq{
Expand Down