Replace issues_model.GetReviewersByIssueID() with more generic issues_model.GetReviews()

models/issues/issue.go

@@ -258,7 +258,7 @@ func (issue *Issue) loadPoster(ctx context.Context) (err error) {
 	return err
 }
 
-func (issue *Issue) loadPullRequest(ctx context.Context) (err error) {
+func (issue *Issue) LoadPullRequestCtx(ctx context.Context) (err error) {
 	if issue.IsPull && issue.PullRequest == nil {
 		issue.PullRequest, err = GetPullRequestByIssueID(ctx, issue.ID)
 		if err != nil {
@@ -274,7 +274,7 @@ func (issue *Issue) loadPullRequest(ctx context.Context) (err error) {
 
 // LoadPullRequest loads pull request info
 func (issue *Issue) LoadPullRequest() error {
-	return issue.loadPullRequest(db.DefaultContext)
+	return issue.LoadPullRequestCtx(db.DefaultContext)
 }
 
 func (issue *Issue) loadComments(ctx context.Context) (err error) {
@@ -390,7 +390,7 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
 		return
 	}
 
-	if err = issue.loadPullRequest(ctx); err != nil && !IsErrPullRequestNotExist(err) {
+	if err = issue.LoadPullRequestCtx(ctx); err != nil && !IsErrPullRequestNotExist(err) {
 		// It is possible pull request is not yet created.
 		return err
 	}
@@ -545,7 +545,7 @@ func ClearIssueLabels(issue *Issue, doer *user_model.User) (err error) {
 
 	if err := issue.LoadRepo(ctx); err != nil {
 		return err
-	} else if err = issue.loadPullRequest(ctx); err != nil {
+	} else if err = issue.LoadPullRequestCtx(ctx); err != nil {
 		return err
 	}
 

models/issues/review.go

@@ -477,9 +477,14 @@ func SubmitReview(doer *user_model.User, issue *Issue, reviewType ReviewType, co
 
 // GetReviewOptions represent filter options for GetReviews
 type GetReviewOptions struct {
-	IssueID    int64
-	ReviewerID int64
-	Dismissed  util.OptionalBool
+	db.ListOptions
+	IssueID        int64
+	ReviewerID     int64
+	ReviewerTeamID int64
+	Official       util.OptionalBool
+	Stale          util.OptionalBool
+	Dismissed      util.OptionalBool
+	LatestOnly     bool
 }
 
 // GetReviews return reviews based on GetReviewOptions
@@ -489,46 +494,61 @@ func GetReviews(ctx context.Context, opts *GetReviewOptions) ([]*Review, error)
 	}
 
 	sess := db.GetEngine(ctx)
+	if opts.ListOptions.Page != -1 {
+		opts.SetDefaultValues()
+		sess = db.SetEnginePagination(sess, opts)
+	}
 
 	if opts.IssueID != 0 {
 		sess = sess.Where("issue_id=?", opts.IssueID)
 	}
 	if opts.ReviewerID != 0 {
 		sess = sess.Where("reviewer_id=?", opts.ReviewerID)
 	}
-	if !opts.Dismissed.IsNone() {
-		sess = sess.Where("dismissed=?", opts.Dismissed.IsTrue())
+	if opts.ReviewerTeamID != 0 {
+		sess = sess.Where("reviewerTeam_id=?", opts.ReviewerTeamID)
 	}
-
-	reviews := make([]*Review, 0, 4)
-	return reviews, sess.Find(&reviews)
-}
-
-// GetReviewersByIssueID gets the latest review of each reviewer for a pull request
-func GetReviewersByIssueID(issueID int64) ([]*Review, error) {
-	reviews := make([]*Review, 0, 10)
-
-	sess := db.GetEngine(db.DefaultContext)
-
-	// 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
+	if !opts.Official.IsNone() {
+		sess = sess.Where("official=?", opts.Official.IsTrue())
 	}
-
-	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
+	if !opts.Stale.IsNone() {
+		sess = sess.Where("stale=?", opts.Stale.IsTrue())
 	}
-
-	if len(teamReviewRequests) > 0 {
-		reviews = append(reviews, teamReviewRequests...)
+	if !opts.Dismissed.IsNone() {
+		sess = sess.Where("dismissed=?", opts.Dismissed.IsTrue())
 	}
 
-	return reviews, nil
+	if opts.LatestOnly {
+		// Get latest review of each reviewer, sorted in order they were made
+		sess = sess.Where(builder.In("review.id",
+			builder.Select("max(id)").From("review").
+				Where(builder.Eq{
+					"issue_id":           opts.IssueID,
+					"original_author_id": 0,
+					"reviewer_team_id":   0,
+					"dismissed":          opts.Dismissed.IsTrue(),
+				}.And(builder.In(
+					"type",
+					ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest,
+				))).
+				GroupBy("issue_id, reviewer_id").
+				OrderBy("review.updated_unix ASC")).
+			// Get latest review of each review-team, sorted in order they were made
+			Or(builder.In("review.id",
+				builder.Select("max(id)").From("review").
+					Where(builder.Eq{
+						"issue_id":           opts.IssueID,
+						"original_author_id": 0,
+						"reviewer_id":        0,
+					}.And(builder.Neq{
+						"reviewer_team_id": 0,
+					})).
+					GroupBy("issue_id, reviewer_team_id").
+					OrderBy("review.updated_unix ASC"))))
+	}
+
+	reviews := make([]*Review, 0, opts.PageSize)
+	return reviews, sess.Find(&reviews)
 }
 
 // GetReviewersFromOriginalAuthorsByIssueID gets the latest review of each original authors for a pull request
@@ -596,6 +616,7 @@ func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) {
 }
 
 // DismissReview change the dismiss status of a review
+// only if dismiss status changed and type is either approve or reject
 func DismissReview(review *Review, isDismiss bool) (err error) {
 	if review.Dismissed == isDismiss || (review.Type != ReviewTypeApprove && review.Type != ReviewTypeReject) {
 		return nil

models/issues/review_test.go

@@ -11,6 +11,7 @@ import (
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -133,7 +134,11 @@ func TestGetReviewersByIssueID(t *testing.T) {
 			UpdatedUnix: 946684814,
 		})
 
-	allReviews, err := issues_model.GetReviewersByIssueID(issue.ID)
+	allReviews, err := issues_model.GetReviews(db.DefaultContext, &issues_model.GetReviewOptions{
+		IssueID:    issue.ID,
+		Dismissed:  util.OptionalBoolFalse,
+		LatestOnly: true,
+	})
 	for _, reviewer := range allReviews {
 		assert.NoError(t, reviewer.LoadReviewer())
 	}

routers/web/repo/issue.go

@@ -500,7 +500,11 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 	}
 	ctx.Data["OriginalReviews"] = originalAuthorReviews
 
-	reviews, err := issues_model.GetReviewersByIssueID(issue.ID)
+	reviews, err := issues_model.GetReviews(ctx, &issues_model.GetReviewOptions{
+		IssueID:    issue.ID,
+		Dismissed:  util.OptionalBoolFalse,
+		LatestOnly: true,
+	})
 	if err != nil {
 		ctx.ServerError("GetReviewersByIssueID", err)
 		return

services/pull/review.go

@@ -275,7 +275,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos
 func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissPriors bool) (comment *issues_model.Comment, err error) {
 	review, err := issues_model.GetReviewByID(ctx, reviewID)
 	if err != nil {
-		return
+		return nil, err
 	}
 
 	if review.Type != issues_model.ReviewTypeApprove && review.Type != issues_model.ReviewTypeReject {
@@ -293,7 +293,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
 	}
 
 	if err = issues_model.DismissReview(review, isDismiss); err != nil {
-		return
+		return nil, err
 	}
 
 	if dismissPriors {
@@ -316,11 +316,11 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
 		return nil, nil
 	}
 
-	if err = review.Issue.LoadPullRequest(); err != nil {
+	if err = review.Issue.LoadPullRequestCtx(ctx); err != nil {
 		return
 	}
 	if err = review.Issue.LoadAttributes(ctx); err != nil {
-		return
+		return nil, err
 	}
 
 	comment, err = issues_model.CreateComment(&issues_model.CreateCommentOptions{