Fix the logic of finding the latest pull review commit ID (#32139)

Fix #31423
This commit is contained in:
Zettat123 2024-10-01 09:58:55 +08:00 committed by GitHub
parent 13283873e9
commit f4b8f6fc40
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 88 additions and 11 deletions

View File

@ -83,3 +83,22 @@
issue_id: 2 # in repo_id 1 issue_id: 2 # in repo_id 1
review_id: 20 review_id: 20
created_unix: 946684810 created_unix: 946684810
-
id: 10
type: 22 # review
poster_id: 5
issue_id: 3 # in repo_id 1
content: "reviewed by user5"
review_id: 21
created_unix: 946684816
-
id: 11
type: 27 # review request
poster_id: 2
issue_id: 3 # in repo_id 1
content: "review request for user5"
review_id: 22
assignee_id: 5
created_unix: 946684817

View File

@ -179,3 +179,22 @@
content: "Review Comment" content: "Review Comment"
updated_unix: 946684810 updated_unix: 946684810
created_unix: 946684810 created_unix: 946684810
-
id: 21
type: 2
reviewer_id: 5
issue_id: 3
content: "reviewed by user5"
commit_id: 4a357436d925b5c974181ff12a994538ddc5a269
updated_unix: 946684816
created_unix: 946684816
-
id: 22
type: 4
reviewer_id: 5
issue_id: 3
content: "review request for user5"
updated_unix: 946684817
created_unix: 946684817

View File

@ -414,7 +414,7 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer)
// Note: This doesn't page as we only expect a very limited number of reviews // Note: This doesn't page as we only expect a very limited number of reviews
reviews, err := FindLatestReviews(ctx, FindReviewOptions{ reviews, err := FindLatestReviews(ctx, FindReviewOptions{
Type: ReviewTypeApprove, Types: []ReviewType{ReviewTypeApprove},
IssueID: pr.IssueID, IssueID: pr.IssueID,
OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly, OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
}) })

View File

@ -389,7 +389,7 @@ func GetCurrentReview(ctx context.Context, reviewer *user_model.User, issue *Iss
return nil, nil return nil, nil
} }
reviews, err := FindReviews(ctx, FindReviewOptions{ reviews, err := FindReviews(ctx, FindReviewOptions{
Type: ReviewTypePending, Types: []ReviewType{ReviewTypePending},
IssueID: issue.ID, IssueID: issue.ID,
ReviewerID: reviewer.ID, ReviewerID: reviewer.ID,
}) })

View File

@ -92,7 +92,7 @@ func (reviews ReviewList) LoadIssues(ctx context.Context) error {
// FindReviewOptions represent possible filters to find reviews // FindReviewOptions represent possible filters to find reviews
type FindReviewOptions struct { type FindReviewOptions struct {
db.ListOptions db.ListOptions
Type ReviewType Types []ReviewType
IssueID int64 IssueID int64
ReviewerID int64 ReviewerID int64
OfficialOnly bool OfficialOnly bool
@ -107,8 +107,8 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
if opts.ReviewerID > 0 { if opts.ReviewerID > 0 {
cond = cond.And(builder.Eq{"reviewer_id": opts.ReviewerID}) cond = cond.And(builder.Eq{"reviewer_id": opts.ReviewerID})
} }
if opts.Type != ReviewTypeUnknown { if len(opts.Types) > 0 {
cond = cond.And(builder.Eq{"type": opts.Type}) cond = cond.And(builder.In("type", opts.Types))
} }
if opts.OfficialOnly { if opts.OfficialOnly {
cond = cond.And(builder.Eq{"official": true}) cond = cond.And(builder.Eq{"official": true})

View File

@ -63,7 +63,7 @@ func TestReviewType_Icon(t *testing.T) {
func TestFindReviews(t *testing.T) { func TestFindReviews(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
reviews, err := issues_model.FindReviews(db.DefaultContext, issues_model.FindReviewOptions{ reviews, err := issues_model.FindReviews(db.DefaultContext, issues_model.FindReviewOptions{
Type: issues_model.ReviewTypeApprove, Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
IssueID: 2, IssueID: 2,
ReviewerID: 1, ReviewerID: 1,
}) })
@ -75,7 +75,7 @@ func TestFindReviews(t *testing.T) {
func TestFindLatestReviews(t *testing.T) { func TestFindLatestReviews(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
reviews, err := issues_model.FindLatestReviews(db.DefaultContext, issues_model.FindReviewOptions{ reviews, err := issues_model.FindLatestReviews(db.DefaultContext, issues_model.FindReviewOptions{
Type: issues_model.ReviewTypeApprove, Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
IssueID: 11, IssueID: 11,
}) })
assert.NoError(t, err) assert.NoError(t, err)

View File

@ -83,7 +83,6 @@ func ListPullReviews(ctx *context.APIContext) {
opts := issues_model.FindReviewOptions{ opts := issues_model.FindReviewOptions{
ListOptions: utils.GetListOptions(ctx), ListOptions: utils.GetListOptions(ctx),
Type: issues_model.ReviewTypeUnknown,
IssueID: pr.IssueID, IssueID: pr.IssueID,
} }

View File

@ -995,6 +995,8 @@ type CommitInfo struct {
} }
// GetPullCommits returns all commits on given pull request and the last review commit sha // GetPullCommits returns all commits on given pull request and the last review commit sha
// Attention: The last review commit sha must be from the latest review whose commit id is not empty.
// So the type of the latest review cannot be "ReviewTypeRequest".
func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]CommitInfo, string, error) { func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]CommitInfo, string, error) {
pull := issue.PullRequest pull := issue.PullRequest
@ -1040,7 +1042,11 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co
lastreview, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{ lastreview, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{
IssueID: issue.ID, IssueID: issue.ID,
ReviewerID: ctx.Doer.ID, ReviewerID: ctx.Doer.ID,
Type: issues_model.ReviewTypeUnknown, Types: []issues_model.ReviewType{
issues_model.ReviewTypeApprove,
issues_model.ReviewTypeComment,
issues_model.ReviewTypeReject,
},
}) })
if err != nil && !issues_model.IsErrReviewNotExist(err) { if err != nil && !issues_model.IsErrReviewNotExist(err) {

View File

@ -348,7 +348,7 @@ func DismissApprovalReviews(ctx context.Context, doer *user_model.User, pull *is
reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{ reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
ListOptions: db.ListOptionsAll, ListOptions: db.ListOptionsAll,
IssueID: pull.IssueID, IssueID: pull.IssueID,
Type: issues_model.ReviewTypeApprove, Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
Dismissed: optional.Some(false), Dismissed: optional.Some(false),
}) })
if err != nil { if err != nil {

View File

@ -38,7 +38,7 @@ func TestAPIPullReview(t *testing.T) {
var reviews []*api.PullReview var reviews []*api.PullReview
DecodeJSON(t, resp, &reviews) DecodeJSON(t, resp, &reviews)
if !assert.Len(t, reviews, 6) { if !assert.Len(t, reviews, 8) {
return return
} }
for _, r := range reviews { for _, r := range reviews {

View File

@ -0,0 +1,34 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
import (
"net/http"
"net/url"
"testing"
pull_service "code.gitea.io/gitea/services/pull"
"github.com/stretchr/testify/assert"
)
func TestListPullCommits(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
session := loginUser(t, "user5")
req := NewRequest(t, "GET", "/user2/repo1/pulls/3/commits/list")
resp := session.MakeRequest(t, req, http.StatusOK)
var pullCommitList struct {
Commits []pull_service.CommitInfo `json:"commits"`
LastReviewCommitSha string `json:"last_review_commit_sha"`
}
DecodeJSON(t, resp, &pullCommitList)
if assert.Len(t, pullCommitList.Commits, 2) {
assert.Equal(t, "5f22f7d0d95d614d25a5b68592adb345a4b5c7fd", pullCommitList.Commits[0].ID)
assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.Commits[1].ID)
}
assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.LastReviewCommitSha)
})
}