diff --git a/models/issues/pull.go b/models/issues/pull.go
index 853e2a69e6..319ead5dbd 100644
--- a/models/issues/pull.go
+++ b/models/issues/pull.go
@@ -301,7 +301,7 @@ func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error {
return nil
}
- reviews, err := GetReviewsByIssueID(ctx, pr.Issue.ID)
+ reviews, _, err := GetReviewsByIssueID(ctx, pr.Issue.ID)
if err != nil {
return err
}
@@ -320,7 +320,7 @@ func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error {
// LoadRequestedReviewersTeams loads the requested reviewers teams.
func (pr *PullRequest) LoadRequestedReviewersTeams(ctx context.Context) error {
- reviews, err := GetReviewsByIssueID(ctx, pr.Issue.ID)
+ reviews, _, err := GetReviewsByIssueID(ctx, pr.Issue.ID)
if err != nil {
return err
}
diff --git a/models/issues/review_list.go b/models/issues/review_list.go
index bc7d7ec0f0..928f24fb2d 100644
--- a/models/issues/review_list.go
+++ b/models/issues/review_list.go
@@ -5,6 +5,8 @@ package issues
import (
"context"
+ "slices"
+ "sort"
"code.gitea.io/gitea/models/db"
organization_model "code.gitea.io/gitea/models/organization"
@@ -153,43 +155,60 @@ func CountReviews(ctx context.Context, opts FindReviewOptions) (int64, error) {
return db.GetEngine(ctx).Where(opts.toCond()).Count(&Review{})
}
-// GetReviewersFromOriginalAuthorsByIssueID gets the latest review of each original authors for a pull request
-func GetReviewersFromOriginalAuthorsByIssueID(ctx context.Context, issueID int64) (ReviewList, error) {
- reviews := make([]*Review, 0, 10)
-
- // Get latest review of each reviewer, sorted in order they were made
- if err := db.GetEngine(ctx).SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND reviewer_team_id = 0 AND type in (?, ?, ?) AND original_author_id <> 0 GROUP BY issue_id, original_author_id) ORDER BY review.updated_unix ASC",
- issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest).
- Find(&reviews); err != nil {
- return nil, err
- }
-
- return reviews, nil
-}
-
// GetReviewsByIssueID gets the latest review of each reviewer for a pull request
-func GetReviewsByIssueID(ctx context.Context, issueID int64) (ReviewList, error) {
+// The first returned parameter is the latest review of each individual reviewer or team
+// The second returned parameter is the latest review of each original author which is migrated from other systems
+// The reviews are sorted by updated time
+func GetReviewsByIssueID(ctx context.Context, issueID int64) (latestReviews, migratedOriginalReviews ReviewList, err error) {
reviews := make([]*Review, 0, 10)
- sess := db.GetEngine(ctx)
-
- // Get latest review of each reviewer, sorted in order they were made
- if err := sess.SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND reviewer_team_id = 0 AND type in (?, ?, ?) AND dismissed = ? AND original_author_id = 0 GROUP BY issue_id, reviewer_id) ORDER BY review.updated_unix ASC",
- issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, false).
- Find(&reviews); err != nil {
- return nil, err
+ // Get all reviews for the issue id
+ if err := db.GetEngine(ctx).Where("issue_id=?", issueID).OrderBy("updated_unix ASC").Find(&reviews); err != nil {
+ return nil, nil, err
}
+ // filter them in memory to get the latest review of each reviewer
+ // Since the reviews should not be too many for one issue, less than 100 commonly, it's acceptable to do this in memory
+ // And since there are too less indexes in review table, it will be very slow to filter in the database
+ reviewersMap := make(map[int64][]*Review) // key is reviewer id
+ originalReviewersMap := make(map[int64][]*Review) // key is original author id
+ reviewTeamsMap := make(map[int64][]*Review) // key is reviewer team id
+ countedReivewTypes := []ReviewType{ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest}
+ for _, review := range reviews {
+ if review.ReviewerTeamID == 0 && slices.Contains(countedReivewTypes, review.Type) && !review.Dismissed {
+ if review.OriginalAuthorID != 0 {
+ originalReviewersMap[review.OriginalAuthorID] = append(originalReviewersMap[review.OriginalAuthorID], review)
+ } else {
+ reviewersMap[review.ReviewerID] = append(reviewersMap[review.ReviewerID], review)
+ }
+ } else if review.ReviewerTeamID != 0 && review.OriginalAuthorID == 0 {
+ reviewTeamsMap[review.ReviewerTeamID] = append(reviewTeamsMap[review.ReviewerTeamID], review)
+ }
+ }
+
+ individualReviews := make([]*Review, 0, 10)
+ for _, reviews := range reviewersMap {
+ individualReviews = append(individualReviews, reviews[len(reviews)-1])
+ }
+ sort.Slice(individualReviews, func(i, j int) bool {
+ return individualReviews[i].UpdatedUnix < individualReviews[j].UpdatedUnix
+ })
+
+ originalReviews := make([]*Review, 0, 10)
+ for _, reviews := range originalReviewersMap {
+ originalReviews = append(originalReviews, reviews[len(reviews)-1])
+ }
+ sort.Slice(originalReviews, func(i, j int) bool {
+ return originalReviews[i].UpdatedUnix < originalReviews[j].UpdatedUnix
+ })
+
teamReviewRequests := make([]*Review, 0, 5)
- if err := sess.SQL("SELECT * FROM review WHERE id IN (SELECT max(id) as id FROM review WHERE issue_id = ? AND reviewer_team_id <> 0 AND original_author_id = 0 GROUP BY issue_id, reviewer_team_id) ORDER BY review.updated_unix ASC",
- issueID).
- Find(&teamReviewRequests); err != nil {
- return nil, err
+ for _, reviews := range reviewTeamsMap {
+ teamReviewRequests = append(teamReviewRequests, reviews[len(reviews)-1])
}
+ sort.Slice(teamReviewRequests, func(i, j int) bool {
+ return teamReviewRequests[i].UpdatedUnix < teamReviewRequests[j].UpdatedUnix
+ })
- if len(teamReviewRequests) > 0 {
- reviews = append(reviews, teamReviewRequests...)
- }
-
- return reviews, nil
+ return append(individualReviews, teamReviewRequests...), originalReviews, nil
}
diff --git a/models/issues/review_test.go b/models/issues/review_test.go
index 50330e3ff2..2588b8ba41 100644
--- a/models/issues/review_test.go
+++ b/models/issues/review_test.go
@@ -162,8 +162,9 @@ func TestGetReviewersByIssueID(t *testing.T) {
},
)
- allReviews, err := issues_model.GetReviewsByIssueID(db.DefaultContext, issue.ID)
+ allReviews, migratedReviews, err := issues_model.GetReviewsByIssueID(db.DefaultContext, issue.ID)
assert.NoError(t, err)
+ assert.Empty(t, migratedReviews)
for _, review := range allReviews {
assert.NoError(t, review.LoadReviewer(db.DefaultContext))
}
diff --git a/routers/web/repo/issue_page_meta.go b/routers/web/repo/issue_page_meta.go
index b536b04d7c..272343f460 100644
--- a/routers/web/repo/issue_page_meta.go
+++ b/routers/web/repo/issue_page_meta.go
@@ -193,6 +193,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
var posterID int64
var isClosed bool
var reviews issues_model.ReviewList
+ var err error
if d.Issue == nil {
if ctx.Doer != nil {
@@ -206,14 +207,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
isClosed = d.Issue.IsClosed || d.Issue.PullRequest.HasMerged
- originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, d.Issue.ID)
- if err != nil {
- ctx.ServerError("GetReviewersFromOriginalAuthorsByIssueID", err)
- return
- }
- data.OriginalReviews = originalAuthorReviews
-
- reviews, err = issues_model.GetReviewsByIssueID(ctx, d.Issue.ID)
+ reviews, data.OriginalReviews, err = issues_model.GetReviewsByIssueID(ctx, d.Issue.ID)
if err != nil {
ctx.ServerError("GetReviewersByIssueID", err)
return