Merge branch 'release/v1.22' into backport-32130-v1.22

GiteaBot · web-flow · commit 8903d394018a · 2024-10-01T12:58:10.000+08:00
diff --git a/models/activities/action.go b/models/activities/action.go
@@ -450,9 +450,7 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err
 		return nil, 0, err
 	}
 
-	sess := db.GetEngine(ctx).Where(cond).
-		Select("`action`.*"). // this line will avoid select other joined table's columns
-		Join("INNER", "repository", "`repository`.id = `action`.repo_id")
+	sess := db.GetEngine(ctx).Where(cond)
 
 	opts.SetDefaultValues()
 	sess = db.SetSessionPagination(sess, &opts)
diff --git a/models/activities/action_test.go b/models/activities/action_test.go
@@ -226,6 +226,8 @@ func TestNotifyWatchers(t *testing.T) {
 }
 
 func TestGetFeedsCorrupted(t *testing.T) {
+	// Now we will not check for corrupted data in the feeds
+	// users should run doctor to fix their data
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 	unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
@@ -239,8 +241,8 @@ func TestGetFeedsCorrupted(t *testing.T) {
 		IncludePrivate: true,
 	})
 	assert.NoError(t, err)
-	assert.Len(t, actions, 0)
-	assert.Equal(t, int64(0), count)
+	assert.Len(t, actions, 1)
+	assert.Equal(t, int64(1), count)
 }
 
 func TestConsistencyUpdateAction(t *testing.T) {
diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml
@@ -83,3 +83,22 @@
   issue_id: 2 # in repo_id 1
   review_id: 20
   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
diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml
@@ -179,3 +179,22 @@
   content: "Review Comment"
   updated_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
diff --git a/models/issues/pull.go b/models/issues/pull.go
@@ -387,7 +387,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
 	reviews, err := FindLatestReviews(ctx, FindReviewOptions{
-		Type:         ReviewTypeApprove,
+		Types:        []ReviewType{ReviewTypeApprove},
 		IssueID:      pr.IssueID,
 		OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
 	})
diff --git a/models/issues/review.go b/models/issues/review.go
@@ -364,7 +364,7 @@ func GetCurrentReview(ctx context.Context, reviewer *user_model.User, issue *Iss
 		return nil, nil
 	}
 	reviews, err := FindReviews(ctx, FindReviewOptions{
-		Type:       ReviewTypePending,
+		Types:      []ReviewType{ReviewTypePending},
 		IssueID:    issue.ID,
 		ReviewerID: reviewer.ID,
 	})
diff --git a/models/issues/review_list.go b/models/issues/review_list.go
@@ -63,7 +63,7 @@ func (reviews ReviewList) LoadIssues(ctx context.Context) error {
 // FindReviewOptions represent possible filters to find reviews
 type FindReviewOptions struct {
 	db.ListOptions
-	Type         ReviewType
+	Types        []ReviewType
 	IssueID      int64
 	ReviewerID   int64
 	OfficialOnly bool
@@ -78,8 +78,8 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
 	if opts.ReviewerID > 0 {
 		cond = cond.And(builder.Eq{"reviewer_id": opts.ReviewerID})
 	}
-	if opts.Type != ReviewTypeUnknown {
-		cond = cond.And(builder.Eq{"type": opts.Type})
+	if len(opts.Types) > 0 {
+		cond = cond.And(builder.In("type", opts.Types))
 	}
 	if opts.OfficialOnly {
 		cond = cond.And(builder.Eq{"official": true})
diff --git a/models/issues/review_test.go b/models/issues/review_test.go
@@ -63,7 +63,7 @@ func TestReviewType_Icon(t *testing.T) {
 func TestFindReviews(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	reviews, err := issues_model.FindReviews(db.DefaultContext, issues_model.FindReviewOptions{
-		Type:       issues_model.ReviewTypeApprove,
+		Types:      []issues_model.ReviewType{issues_model.ReviewTypeApprove},
 		IssueID:    2,
 		ReviewerID: 1,
 	})
@@ -75,7 +75,7 @@ func TestFindReviews(t *testing.T) {
 func TestFindLatestReviews(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	reviews, err := issues_model.FindLatestReviews(db.DefaultContext, issues_model.FindReviewOptions{
-		Type:    issues_model.ReviewTypeApprove,
+		Types:   []issues_model.ReviewType{issues_model.ReviewTypeApprove},
 		IssueID: 11,
 	})
 	assert.NoError(t, err)
diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go
@@ -83,7 +83,6 @@ func ListPullReviews(ctx *context.APIContext) {
 
 	opts := issues_model.FindReviewOptions{
 		ListOptions: utils.GetListOptions(ctx),
-		Type:        issues_model.ReviewTypeUnknown,
 		IssueID:     pr.IssueID,
 	}
 
diff --git a/services/pull/pull.go b/services/pull/pull.go
@@ -990,6 +990,8 @@ type CommitInfo struct {
 }
 
 // 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) {
 	pull := issue.PullRequest
 
@@ -1035,7 +1037,11 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co
 		lastreview, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{
 			IssueID:    issue.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) {
diff --git a/services/pull/review.go b/services/pull/review.go
@@ -348,7 +348,7 @@ func DismissApprovalReviews(ctx context.Context, doer *user_model.User, pull *is
 	reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
 		ListOptions: db.ListOptionsAll,
 		IssueID:     pull.IssueID,
-		Type:        issues_model.ReviewTypeApprove,
+		Types:       []issues_model.ReviewType{issues_model.ReviewTypeApprove},
 		Dismissed:   optional.Some(false),
 	})
 	if err != nil {
diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go
@@ -38,7 +38,7 @@ func TestAPIPullReview(t *testing.T) {
 
 	var reviews []*api.PullReview
 	DecodeJSON(t, resp, &reviews)
-	if !assert.Len(t, reviews, 6) {
+	if !assert.Len(t, reviews, 8) {
 		return
 	}
 	for _, r := range reviews {
diff --git a/tests/integration/pull_commit_test.go b/tests/integration/pull_commit_test.go
@@ -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)
+	})
+}

Original file line number	Diff line number	Diff line change
`@@ -450,9 +450,7 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err`
`450`	`450`	`return nil, 0, err`
`451`	`451`	`}`
`452`	`452`
`453`		`- sess := db.GetEngine(ctx).Where(cond).`
`454`		- Select("`action`.*"). // this line will avoid select other joined table's columns
`455`		- Join("INNER", "repository", "`repository`.id = `action`.repo_id")
	`453`	`+ sess := db.GetEngine(ctx).Where(cond)`
`456`	`454`
`457`	`455`	`opts.SetDefaultValues()`
`458`	`456`	`sess = db.SetSessionPagination(sess, &opts)`
Original file line number	Diff line number	Diff line change
`@@ -364,7 +364,7 @@ func GetCurrentReview(ctx context.Context, reviewer user_model.User, issue Iss`
`364`	`364`	`return nil, nil`
`365`	`365`	`}`
`366`	`366`	`reviews, err := FindReviews(ctx, FindReviewOptions{`
`367`		`- Type: ReviewTypePending,`
	`367`	`+ Types: []ReviewType{ReviewTypePending},`
`368`	`368`	`IssueID: issue.ID,`
`369`	`369`	`ReviewerID: reviewer.ID,`
`370`	`370`	`})`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,6 @@ func ListPullReviews(ctx *context.APIContext) {`
`83`	`83`
`84`	`84`	`opts := issues_model.FindReviewOptions{`
`85`	`85`	`ListOptions: utils.GetListOptions(ctx),`
`86`		`- Type: issues_model.ReviewTypeUnknown,`
`87`	`86`	`IssueID: pr.IssueID,`
`88`	`87`	`}`
`89`	`88`