Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Add pull review endpoints #11224

Merged
merged 40 commits into from
May 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
bbdc5a0
API: Added pull review read only endpoints
tbe Jan 26, 2020
44d32f0
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 26, 2020
70dfe71
Update Structs, move Conversion, Refactor
6543 Apr 27, 2020
644aa1c
refactor
6543 Apr 27, 2020
79eca0b
lint & co
6543 Apr 27, 2020
0ce765c
fix lint + refactor
6543 Apr 27, 2020
d860be8
add new Review state, rm unessesary, refacotr loadAttributes, convert…
6543 Apr 27, 2020
a4b998c
add DeletePullReview
6543 Apr 27, 2020
582c6c0
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 27, 2020
e1c13b0
add paggination
6543 Apr 27, 2020
4b60f40
draft1: Create & submit review
6543 Apr 27, 2020
4f95b62
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 27, 2020
2f2c400
fix lint
6543 Apr 27, 2020
aeda695
fix lint
6543 Apr 27, 2020
ed15f52
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 28, 2020
63c7b4e
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 28, 2020
ef2047d
impruve test
6543 Apr 28, 2020
42a6b89
DONT use GhostUser for loadReviewer
6543 Apr 28, 2020
5a6f383
expose comments_count of a PullReview
6543 Apr 28, 2020
eedd1f2
infent GetCodeCommentsCount()
6543 Apr 28, 2020
ac86f43
fixes
6543 Apr 28, 2020
75f6c1b
fix+impruve
6543 Apr 28, 2020
efd8bb9
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 28, 2020
c7cd256
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 29, 2020
627b9d6
some nits
6543 Apr 29, 2020
0cf99dc
Handle Ghosts :ghost:
6543 Apr 29, 2020
d9e1480
add TEST for GET apis
6543 Apr 29, 2020
cbca510
complete TESTS
6543 Apr 29, 2020
9b0bbc1
add HTMLURL to PullReview responce
6543 Apr 29, 2020
ac9a31f
code format as per @lafriks
6543 Apr 29, 2020
c7722ae
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 29, 2020
28d4f34
update swagger definition
6543 Apr 29, 2020
ab3edb3
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 30, 2020
55f37fd
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 30, 2020
65dc905
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 30, 2020
ff661a9
Merge branch 'master' into api-review-endpoints_5733
6543 Apr 30, 2020
5af6fc5
Merge branch 'master' into api-review-endpoints_5733
6543 May 1, 2020
d1ae269
Update routers/api/v1/repo/pull_review.go
6543 May 1, 2020
e66b139
add comments
6543 May 1, 2020
42dfbe7
Merge branch 'master' into api-review-endpoints_5733
6543 May 1, 2020
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
120 changes: 120 additions & 0 deletions integrations/api_pull_review_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
4 changes: 2 additions & 2 deletions models/fixtures/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -22,7 +22,7 @@
base_repo_id: 1
head_branch: branch2
base_branch: master
merge_base: fedcba9876543210
merge_base: 4a357436d925b5c974181ff12a994538ddc5a269
has_merged: false

-
Expand Down
5 changes: 4 additions & 1 deletion models/fixtures/review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
-
Expand All @@ -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
98 changes: 93 additions & 5 deletions models/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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").
Expand Down Expand Up @@ -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()
}
10 changes: 6 additions & 4 deletions models/review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Loading