diff --git a/models/issues/issue.go b/models/issues/issue.go index 064f0d22abd01..91a1ab7edc64f 100644 --- a/models/issues/issue.go +++ b/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 } diff --git a/models/issues/review.go b/models/issues/review.go index 5835900801778..115c45372194a 100644 --- a/models/issues/review.go +++ b/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,6 +494,10 @@ 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) @@ -496,39 +505,50 @@ func GetReviews(ctx context.Context, opts *GetReviewOptions) ([]*Review, error) 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 diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 3506604b46dbe..08bd396927e4f 100644 --- a/models/issues/review_test.go +++ b/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()) } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index e6f9529e31e81..a5267ffcd445a 100644 --- a/routers/web/repo/issue.go +++ b/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 diff --git a/services/pull/review.go b/services/pull/review.go index 8d8903c6a9521..6a392bad08952 100644 --- a/services/pull/review.go +++ b/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{