API: Add pull review endpoints (#11224)

* API: Added pull review read only endpoints

* Update Structs, move Conversion, Refactor

* refactor

* lint & co

* fix lint + refactor

* add new Review state, rm unessesary, refacotr loadAttributes, convert patch to diff

* add DeletePullReview

* add paggination

* draft1: Create & submit review

* fix lint

* fix lint

* impruve test

* DONT use GhostUser for loadReviewer

* expose comments_count of a PullReview

* infent GetCodeCommentsCount()

* fixes

* fix+impruve

* some nits

* Handle Ghosts 👻

* add TEST for GET apis

* complete TESTS

* add HTMLURL to PullReview responce

* code format as per @lafriks

* update swagger definition

* Update routers/api/v1/repo/pull_review.go

Co-authored-by: David Svantesson <davidsvantesson@gmail.com>

* add comments

Co-authored-by: Thomas Berger <loki@lokis-chaos.de>
Co-authored-by: David Svantesson <davidsvantesson@gmail.com>
This commit is contained in:
6543
2020-05-02 02:20:51 +02:00
committed by GitHub
parent 4ed7d2a2bb
commit c97494a4f4
12 changed files with 1580 additions and 26 deletions
+120
View File
@@ -0,0 +1,120 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package integrations
import (
"fmt"
"net/http"
"testing"
"code.gitea.io/gitea/models"
api "code.gitea.io/gitea/modules/structs"
"github.com/stretchr/testify/assert"
)
func TestAPIPullReview(t *testing.T) {
defer prepareTestEnv(t)()
pullIssue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 3}).(*models.Issue)
assert.NoError(t, pullIssue.LoadAttributes())
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: pullIssue.RepoID}).(*models.Repository)
// test ListPullReviews
session := loginUser(t, "user2")
token := getTokenForLoggedInUser(t, session)
req := NewRequestf(t, http.MethodGet, "/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token)
resp := session.MakeRequest(t, req, http.StatusOK)
var reviews []*api.PullReview
DecodeJSON(t, resp, &reviews)
if !assert.Len(t, reviews, 6) {
return
}
for _, r := range reviews {
assert.EqualValues(t, pullIssue.HTMLURL(), r.HTMLPullURL)
}
assert.EqualValues(t, 8, reviews[3].ID)
assert.EqualValues(t, "APPROVED", reviews[3].State)
assert.EqualValues(t, 0, reviews[3].CodeCommentsCount)
assert.EqualValues(t, true, reviews[3].Stale)
assert.EqualValues(t, false, reviews[3].Official)
assert.EqualValues(t, 10, reviews[5].ID)
assert.EqualValues(t, "REQUEST_CHANGES", reviews[5].State)
assert.EqualValues(t, 1, reviews[5].CodeCommentsCount)
assert.EqualValues(t, 0, reviews[5].Reviewer.ID) // ghost user
assert.EqualValues(t, false, reviews[5].Stale)
assert.EqualValues(t, true, reviews[5].Official)
// test GetPullReview
req = NewRequestf(t, http.MethodGet, "/api/v1/repos/%s/%s/pulls/%d/reviews/%d?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, reviews[3].ID, token)
resp = session.MakeRequest(t, req, http.StatusOK)
var review api.PullReview
DecodeJSON(t, resp, &review)
assert.EqualValues(t, *reviews[3], review)
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/pulls/%d/reviews/%d?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, reviews[5].ID, token)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &review)
assert.EqualValues(t, *reviews[5], review)
// test GetPullReviewComments
comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 7}).(*models.Comment)
req = NewRequestf(t, http.MethodGet, "/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, 10, token)
resp = session.MakeRequest(t, req, http.StatusOK)
var reviewComments []*api.PullReviewComment
DecodeJSON(t, resp, &reviewComments)
assert.Len(t, reviewComments, 1)
assert.EqualValues(t, "Ghost", reviewComments[0].Reviewer.UserName)
assert.EqualValues(t, "a review from a deleted user", reviewComments[0].Body)
assert.EqualValues(t, comment.ID, reviewComments[0].ID)
assert.EqualValues(t, comment.UpdatedUnix, reviewComments[0].Updated.Unix())
assert.EqualValues(t, comment.HTMLURL(), reviewComments[0].HTMLURL)
// test CreatePullReview
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: "body1",
// Event: "" # will result in PENDING
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,
},
},
})
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &review)
assert.EqualValues(t, 6, review.ID)
assert.EqualValues(t, "PENDING", review.State)
assert.EqualValues(t, 2, review.CodeCommentsCount)
// test SubmitPullReview
req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/reviews/%d?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, review.ID, token), &api.SubmitPullReviewOptions{
Event: "APPROVED",
Body: "just two nits",
})
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &review)
assert.EqualValues(t, 6, review.ID)
assert.EqualValues(t, "APPROVED", review.State)
assert.EqualValues(t, 2, review.CodeCommentsCount)
// test DeletePullReview
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: "just a comment",
Event: "COMMENT",
})
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &review)
assert.EqualValues(t, "COMMENT", review.State)
assert.EqualValues(t, 0, review.CodeCommentsCount)
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)
}
+2 -2
View File
@@ -8,7 +8,7 @@
base_repo_id: 1
head_branch: branch1
base_branch: master
merge_base: 1234567890abcdef
merge_base: 4a357436d925b5c974181ff12a994538ddc5a269
has_merged: true
merger_id: 2
@@ -22,7 +22,7 @@
base_repo_id: 1
head_branch: branch2
base_branch: master
merge_base: fedcba9876543210
merge_base: 4a357436d925b5c974181ff12a994538ddc5a269
has_merged: false
-
+4 -1
View File
@@ -60,6 +60,8 @@
reviewer_id: 4
issue_id: 3
content: "New review 5"
commit_id: 8091a55037cd59e47293aca02981b5a67076b364
stale: true
updated_unix: 946684813
created_unix: 946684813
-
@@ -77,5 +79,6 @@
reviewer_id: 100
issue_id: 3
content: "a deleted user's review"
official: true
updated_unix: 946684815
created_unix: 946684815
created_unix: 946684815
+93 -5
View File
@@ -74,9 +74,13 @@ type Review struct {
}
func (r *Review) loadCodeComments(e Engine) (err error) {
if r.CodeComments == nil {
r.CodeComments, err = fetchCodeCommentsByReview(e, r.Issue, nil, r)
if r.CodeComments != nil {
return
}
if err = r.loadIssue(e); err != nil {
return
}
r.CodeComments, err = fetchCodeCommentsByReview(e, r.Issue, nil, r)
return
}
@@ -86,12 +90,15 @@ func (r *Review) LoadCodeComments() error {
}
func (r *Review) loadIssue(e Engine) (err error) {
if r.Issue != nil {
return
}
r.Issue, err = getIssueByID(e, r.IssueID)
return
}
func (r *Review) loadReviewer(e Engine) (err error) {
if r.ReviewerID == 0 {
if r.Reviewer != nil || r.ReviewerID == 0 {
return nil
}
r.Reviewer, err = getUserByID(e, r.ReviewerID)
@@ -104,10 +111,13 @@ func (r *Review) LoadReviewer() error {
}
func (r *Review) loadAttributes(e Engine) (err error) {
if err = r.loadReviewer(e); err != nil {
if err = r.loadIssue(e); err != nil {
return
}
if err = r.loadIssue(e); err != nil {
if err = r.loadCodeComments(e); err != nil {
return
}
if err = r.loadReviewer(e); err != nil {
return
}
return
@@ -136,6 +146,7 @@ func GetReviewByID(id int64) (*Review, error) {
// FindReviewOptions represent possible filters to find reviews
type FindReviewOptions struct {
ListOptions
Type ReviewType
IssueID int64
ReviewerID int64
@@ -162,6 +173,9 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
func findReviews(e Engine, opts FindReviewOptions) ([]*Review, error) {
reviews := make([]*Review, 0, 10)
sess := e.Where(opts.toCond())
if opts.Page > 0 {
sess = opts.ListOptions.setSessionPagination(sess)
}
return reviews, sess.
Asc("created_unix").
Asc("id").
@@ -656,3 +670,77 @@ func CanMarkConversation(issue *Issue, doer *User) (permResult bool, err error)
return true, nil
}
// DeleteReview delete a review and it's code comments
func DeleteReview(r *Review) error {
sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}
if r.ID == 0 {
return fmt.Errorf("review is not allowed to be 0")
}
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: r.IssueID,
ReviewID: r.ID,
}
if _, err := sess.Where(opts.toConds()).Delete(new(Comment)); err != nil {
return err
}
opts = FindCommentsOptions{
Type: CommentTypeReview,
IssueID: r.IssueID,
ReviewID: r.ID,
}
if _, err := sess.Where(opts.toConds()).Delete(new(Comment)); err != nil {
return err
}
if _, err := sess.ID(r.ID).Delete(new(Review)); err != nil {
return err
}
return sess.Commit()
}
// GetCodeCommentsCount return count of CodeComments a Review has
func (r *Review) GetCodeCommentsCount() int {
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: r.IssueID,
ReviewID: r.ID,
}
conds := opts.toConds()
if r.ID == 0 {
conds = conds.And(builder.Eq{"invalidated": false})
}
count, err := x.Where(conds).Count(new(Comment))
if err != nil {
return 0
}
return int(count)
}
// HTMLURL formats a URL-string to the related review issue-comment
func (r *Review) HTMLURL() string {
opts := FindCommentsOptions{
Type: CommentTypeReview,
IssueID: r.IssueID,
ReviewID: r.ID,
}
comment := new(Comment)
has, err := x.Where(opts.toConds()).Get(comment)
if err != nil || !has {
return ""
}
return comment.HTMLURL()
}
+6 -4
View File
@@ -131,9 +131,11 @@ func TestGetReviewersByIssueID(t *testing.T) {
allReviews, err := GetReviewersByIssueID(issue.ID)
assert.NoError(t, err)
for i, review := range allReviews {
assert.Equal(t, expectedReviews[i].Reviewer, review.Reviewer)
assert.Equal(t, expectedReviews[i].Type, review.Type)
assert.Equal(t, expectedReviews[i].UpdatedUnix, review.UpdatedUnix)
if assert.Len(t, allReviews, 3) {
for i, review := range allReviews {
assert.Equal(t, expectedReviews[i].Reviewer, review.Reviewer)
assert.Equal(t, expectedReviews[i].Type, review.Type)
assert.Equal(t, expectedReviews[i].UpdatedUnix, review.UpdatedUnix)
}
}
}
+127
View File
@@ -0,0 +1,127 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package convert
import (
"strings"
"code.gitea.io/gitea/models"
api "code.gitea.io/gitea/modules/structs"
)
// ToPullReview convert a review to api format
func ToPullReview(r *models.Review, doer *models.User) (*api.PullReview, error) {
if err := r.LoadAttributes(); err != nil {
if !models.IsErrUserNotExist(err) {
return nil, err
}
r.Reviewer = models.NewGhostUser()
}
auth := false
if doer != nil {
auth = doer.IsAdmin || doer.ID == r.ReviewerID
}
result := &api.PullReview{
ID: r.ID,
Reviewer: ToUser(r.Reviewer, doer != nil, auth),
State: api.ReviewStateUnknown,
Body: r.Content,
CommitID: r.CommitID,
Stale: r.Stale,
Official: r.Official,
CodeCommentsCount: r.GetCodeCommentsCount(),
Submitted: r.CreatedUnix.AsTime(),
HTMLURL: r.HTMLURL(),
HTMLPullURL: r.Issue.HTMLURL(),
}
switch r.Type {
case models.ReviewTypeApprove:
result.State = api.ReviewStateApproved
case models.ReviewTypeReject:
result.State = api.ReviewStateRequestChanges
case models.ReviewTypeComment:
result.State = api.ReviewStateComment
case models.ReviewTypePending:
result.State = api.ReviewStatePending
case models.ReviewTypeRequest:
result.State = api.ReviewStateRequestReview
}
return result, nil
}
// ToPullReviewList convert a list of review to it's api format
func ToPullReviewList(rl []*models.Review, doer *models.User) ([]*api.PullReview, error) {
result := make([]*api.PullReview, 0, len(rl))
for i := range rl {
// show pending reviews only for the user who created them
if rl[i].Type == models.ReviewTypePending && !(doer.IsAdmin || doer.ID == rl[i].ReviewerID) {
continue
}
r, err := ToPullReview(rl[i], doer)
if err != nil {
return nil, err
}
result = append(result, r)
}
return result, nil
}
// ToPullReviewCommentList convert the CodeComments of an review to it's api format
func ToPullReviewCommentList(review *models.Review, doer *models.User) ([]*api.PullReviewComment, error) {
if err := review.LoadAttributes(); err != nil {
if !models.IsErrUserNotExist(err) {
return nil, err
}
review.Reviewer = models.NewGhostUser()
}
apiComments := make([]*api.PullReviewComment, 0, len(review.CodeComments))
auth := false
if doer != nil {
auth = doer.IsAdmin || doer.ID == review.ReviewerID
}
for _, lines := range review.CodeComments {
for _, comments := range lines {
for _, comment := range comments {
apiComment := &api.PullReviewComment{
ID: comment.ID,
Body: comment.Content,
Reviewer: ToUser(review.Reviewer, doer != nil, auth),
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(),
HTMLPullURL: review.Issue.APIURL(),
}
if comment.Line < 0 {
apiComment.OldLineNum = comment.UnsignedLine()
} else {
apiComment.LineNum = comment.UnsignedLine()
}
apiComments = append(apiComments, apiComment)
}
}
}
return apiComments, nil
}
func patch2diff(patch string) string {
split := strings.Split(patch, "\n@@")
if len(split) == 2 {
return "@@" + split[1]
}
return ""
}
+92
View File
@@ -0,0 +1,92 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package structs
import (
"time"
)
// ReviewStateType review state type
type ReviewStateType string
const (
// ReviewStateApproved pr is approved
ReviewStateApproved ReviewStateType = "APPROVED"
// ReviewStatePending pr state is pending
ReviewStatePending ReviewStateType = "PENDING"
// ReviewStateComment is a comment review
ReviewStateComment ReviewStateType = "COMMENT"
// ReviewStateRequestChanges changes for pr are requested
ReviewStateRequestChanges ReviewStateType = "REQUEST_CHANGES"
// ReviewStateRequestReview review is requested from user
ReviewStateRequestReview ReviewStateType = "REQUEST_REVIEW"
// ReviewStateUnknown state of pr is unknown
ReviewStateUnknown ReviewStateType = ""
)
// PullReview represents a pull request review
type PullReview struct {
ID int64 `json:"id"`
Reviewer *User `json:"user"`
State ReviewStateType `json:"state"`
Body string `json:"body"`
CommitID string `json:"commit_id"`
Stale bool `json:"stale"`
Official bool `json:"official"`
CodeCommentsCount int `json:"comments_count"`
// swagger:strfmt date-time
Submitted time.Time `json:"submitted_at"`
HTMLURL string `json:"html_url"`
HTMLPullURL string `json:"pull_request_url"`
}
// PullReviewComment represents a comment on a pull request review
type PullReviewComment struct {
ID int64 `json:"id"`
Body string `json:"body"`
Reviewer *User `json:"user"`
ReviewID int64 `json:"pull_request_review_id"`
// swagger:strfmt date-time
Created time.Time `json:"created_at"`
// swagger:strfmt date-time
Updated time.Time `json:"updated_at"`
Path string `json:"path"`
CommitID string `json:"commit_id"`
OrigCommitID string `json:"original_commit_id"`
DiffHunk string `json:"diff_hunk"`
LineNum uint64 `json:"position"`
OldLineNum uint64 `json:"original_position"`
HTMLURL string `json:"html_url"`
HTMLPullURL string `json:"pull_request_url"`
}
// CreatePullReviewOptions are options to create a pull review
type CreatePullReviewOptions struct {
Event ReviewStateType `json:"event"`
Body string `json:"body"`
CommitID string `json:"commit_id"`
Comments []CreatePullReviewComment `json:"comments"`
}
// CreatePullReviewComment represent a review comment for creation api
type CreatePullReviewComment struct {
// the tree path
Path string `json:"path"`
Body string `json:"body"`
// if comment to old file line or 0
OldLineNum int64 `json:"old_position"`
// if comment to new file line or 0
NewLineNum int64 `json:"new_position"`
}
// SubmitPullReviewOptions are options to submit a pending pull review
type SubmitPullReviewOptions struct {
Event ReviewStateType `json:"event"`
Body string `json:"body"`
}
+15 -1
View File
@@ -500,7 +500,7 @@ func RegisterRoutes(m *macaron.Macaron) {
bind := binding.Bind
if setting.API.EnableSwagger {
m.Get("/swagger", misc.Swagger) //Render V1 by default
m.Get("/swagger", misc.Swagger) // Render V1 by default
}
m.Group("/v1", func() {
@@ -794,6 +794,20 @@ func RegisterRoutes(m *macaron.Macaron) {
Patch(reqToken(), reqRepoWriter(models.UnitTypePullRequests), bind(api.EditPullRequestOption{}), repo.EditPullRequest)
m.Combo("/merge").Get(repo.IsPullRequestMerged).
Post(reqToken(), mustNotBeArchived, bind(auth.MergePullRequestForm{}), repo.MergePullRequest)
m.Group("/reviews", func() {
m.Combo("").
Get(repo.ListPullReviews).
Post(reqToken(), bind(api.CreatePullReviewOptions{}), repo.CreatePullReview)
m.Group("/:id", func() {
m.Combo("").
Get(repo.GetPullReview).
Delete(reqToken(), repo.DeletePullReview).
Post(reqToken(), bind(api.SubmitPullReviewOptions{}), repo.SubmitPullReview)
m.Combo("/comments").
Get(repo.GetPullReviewComments)
})
})
})
}, mustAllowPulls, reqRepoReader(models.UnitTypeCode), context.ReferencesGitRepo(false))
m.Group("/statuses", func() {
File diff suppressed because it is too large Load Diff
+9
View File
@@ -137,4 +137,13 @@ type swaggerParameterBodies struct {
// in:body
CreateOAuth2ApplicationOptions api.CreateOAuth2ApplicationOptions
// in:body
CreatePullReviewOptions api.CreatePullReviewOptions
// in:body
CreatePullReviewComment api.CreatePullReviewComment
// in:body
SubmitPullReviewOptions api.SubmitPullReviewOptions
}
+40 -12
View File
@@ -141,6 +141,34 @@ type swaggerResponsePullRequestList struct {
Body []api.PullRequest `json:"body"`
}
// PullReview
// swagger:response PullReview
type swaggerResponsePullReview struct {
// in:body
Body api.PullReview `json:"body"`
}
// PullReviewList
// swagger:response PullReviewList
type swaggerResponsePullReviewList struct {
// in:body
Body []api.PullReview `json:"body"`
}
// PullComment
// swagger:response PullReviewComment
type swaggerPullReviewComment struct {
// in:body
Body api.PullReviewComment `json:"body"`
}
// PullCommentList
// swagger:response PullReviewCommentList
type swaggerResponsePullReviewCommentList struct {
// in:body
Body []api.PullReviewComment `json:"body"`
}
// Status
// swagger:response Status
type swaggerResponseStatus struct {
@@ -172,35 +200,35 @@ type swaggerResponseSearchResults struct {
// AttachmentList
// swagger:response AttachmentList
type swaggerResponseAttachmentList struct {
//in: body
// in: body
Body []api.Attachment `json:"body"`
}
// Attachment
// swagger:response Attachment
type swaggerResponseAttachment struct {
//in: body
// in: body
Body api.Attachment `json:"body"`
}
// GitTreeResponse
// swagger:response GitTreeResponse
type swaggerGitTreeResponse struct {
//in: body
// in: body
Body api.GitTreeResponse `json:"body"`
}
// GitBlobResponse
// swagger:response GitBlobResponse
type swaggerGitBlobResponse struct {
//in: body
// in: body
Body api.GitBlobResponse `json:"body"`
}
// Commit
// swagger:response Commit
type swaggerCommit struct {
//in: body
// in: body
Body api.Commit `json:"body"`
}
@@ -222,28 +250,28 @@ type swaggerCommitList struct {
// True if there is another page
HasMore bool `json:"X-HasMore"`
//in: body
// in: body
Body []api.Commit `json:"body"`
}
// EmptyRepository
// swagger:response EmptyRepository
type swaggerEmptyRepository struct {
//in: body
// in: body
Body api.APIError `json:"body"`
}
// FileResponse
// swagger:response FileResponse
type swaggerFileResponse struct {
//in: body
// in: body
Body api.FileResponse `json:"body"`
}
// ContentsResponse
// swagger:response ContentsResponse
type swaggerContentsResponse struct {
//in: body
// in: body
Body api.ContentsResponse `json:"body"`
}
@@ -257,20 +285,20 @@ type swaggerContentsListResponse struct {
// FileDeleteResponse
// swagger:response FileDeleteResponse
type swaggerFileDeleteResponse struct {
//in: body
// in: body
Body api.FileDeleteResponse `json:"body"`
}
// TopicListResponse
// swagger:response TopicListResponse
type swaggerTopicListResponse struct {
//in: body
// in: body
Body []api.TopicResponse `json:"body"`
}
// TopicNames
// swagger:response TopicNames
type swaggerTopicNames struct {
//in: body
// in: body
Body api.TopicName `json:"body"`
}
File diff suppressed because it is too large Load Diff