diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2023-07-20 15:18:52 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-20 15:18:52 +0800 |
commit | 01c04607c76391e09620c6f2475b722207d2ee73 (patch) | |
tree | ff1cdb3ef3b37f6e8ab3108999e1def1215414fe /models | |
parent | cf467119ffa82177258226fd04946a2105ee7aa0 (diff) | |
download | gitea-01c04607c76391e09620c6f2475b722207d2ee73.tar.gz gitea-01c04607c76391e09620c6f2475b722207d2ee73.zip |
Fix bug when pushing to a pull request which enabled dismiss approval automatically (#25882)
Fix #25858
The option `dissmiss stale approvals` was listed on protected branch but
never implemented. This PR fixes that.
<img width="1006" alt="图片"
src="https://github.com/go-gitea/gitea/assets/81045/60bfa968-4db7-4c24-b8be-2e5978f91bb9">
<img width="1021" alt="图片"
src="https://github.com/go-gitea/gitea/assets/81045/8dabc14d-2dfe-40c2-94ed-24fcbf6e0e8f">
Diffstat (limited to 'models')
-rw-r--r-- | models/issues/issue.go | 2 | ||||
-rw-r--r-- | models/issues/pull.go | 14 | ||||
-rw-r--r-- | models/issues/review.go | 160 | ||||
-rw-r--r-- | models/issues/review_list.go | 172 | ||||
-rw-r--r-- | models/issues/review_test.go | 16 |
5 files changed, 189 insertions, 175 deletions
diff --git a/models/issues/issue.go b/models/issues/issue.go index d0c5ad2bf8..c1a802c792 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -551,7 +551,7 @@ func GetIssueWithAttrsByID(id int64) (*Issue, error) { // GetIssuesByIDs return issues with the given IDs. func GetIssuesByIDs(ctx context.Context, issueIDs []int64) (IssueList, error) { - issues := make([]*Issue, 0, 10) + issues := make([]*Issue, 0, len(issueIDs)) return issues, db.GetEngine(ctx).In("id", issueIDs).Find(&issues) } diff --git a/models/issues/pull.go b/models/issues/pull.go index 2acc2b4226..cedbb62c3c 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -316,15 +316,13 @@ func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error { return err } - if len(reviews) > 0 { - err = LoadReviewers(ctx, reviews) - if err != nil { - return err - } - for _, review := range reviews { - pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer) - } + if err = reviews.LoadReviewers(ctx); err != nil { + return err } + for _, review := range reviews { + pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer) + } + return nil } diff --git a/models/issues/review.go b/models/issues/review.go index 3ec2c00fe9..b2736044fc 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -162,27 +162,6 @@ func (r *Review) LoadReviewer(ctx context.Context) (err error) { return err } -// LoadReviewers loads reviewers -func LoadReviewers(ctx context.Context, reviews []*Review) (err error) { - reviewerIds := make([]int64, len(reviews)) - for i := 0; i < len(reviews); i++ { - reviewerIds[i] = reviews[i].ReviewerID - } - reviewers, err := user_model.GetPossibleUserByIDs(ctx, reviewerIds) - if err != nil { - return err - } - - userMap := make(map[int64]*user_model.User, len(reviewers)) - for _, reviewer := range reviewers { - userMap[reviewer.ID] = reviewer - } - for _, review := range reviews { - review.Reviewer = userMap[review.ReviewerID] - } - return nil -} - // LoadReviewerTeam loads reviewer team func (r *Review) LoadReviewerTeam(ctx context.Context) (err error) { if r.ReviewerTeamID == 0 || r.ReviewerTeam != nil { @@ -236,71 +215,6 @@ func GetReviewByID(ctx context.Context, id int64) (*Review, error) { } } -// FindReviewOptions represent possible filters to find reviews -type FindReviewOptions struct { - db.ListOptions - Type ReviewType - IssueID int64 - ReviewerID int64 - OfficialOnly bool -} - -func (opts *FindReviewOptions) toCond() builder.Cond { - cond := builder.NewCond() - if opts.IssueID > 0 { - cond = cond.And(builder.Eq{"issue_id": opts.IssueID}) - } - 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 opts.OfficialOnly { - cond = cond.And(builder.Eq{"official": true}) - } - return cond -} - -// FindReviews returns reviews passing FindReviewOptions -func FindReviews(ctx context.Context, opts FindReviewOptions) ([]*Review, error) { - reviews := make([]*Review, 0, 10) - sess := db.GetEngine(ctx).Where(opts.toCond()) - if opts.Page > 0 { - sess = db.SetSessionPagination(sess, &opts) - } - return reviews, sess. - Asc("created_unix"). - Asc("id"). - Find(&reviews) -} - -// FindLatestReviews returns only latest reviews per user, passing FindReviewOptions -func FindLatestReviews(ctx context.Context, opts FindReviewOptions) ([]*Review, error) { - reviews := make([]*Review, 0, 10) - cond := opts.toCond() - sess := db.GetEngine(ctx).Where(cond) - if opts.Page > 0 { - sess = db.SetSessionPagination(sess, &opts) - } - - sess.In("id", builder. - Select("max ( id ) "). - From("review"). - Where(cond). - GroupBy("reviewer_id")) - - return reviews, sess. - Asc("created_unix"). - Asc("id"). - Find(&reviews) -} - -// CountReviews returns count of reviews passing FindReviewOptions -func CountReviews(opts FindReviewOptions) (int64, error) { - return db.GetEngine(db.DefaultContext).Where(opts.toCond()).Count(&Review{}) -} - // CreateReviewOptions represent the options to create a review. Type, Issue and Reviewer are required. type CreateReviewOptions struct { Content string @@ -533,76 +447,6 @@ func SubmitReview(doer *user_model.User, issue *Issue, reviewType ReviewType, co return review, comm, committer.Commit() } -// GetReviewOptions represent filter options for GetReviews -type GetReviewOptions struct { - IssueID int64 - ReviewerID int64 - Dismissed util.OptionalBool -} - -// GetReviews return reviews based on GetReviewOptions -func GetReviews(ctx context.Context, opts *GetReviewOptions) ([]*Review, error) { - if opts == nil { - return nil, fmt.Errorf("opts are nil") - } - - sess := db.GetEngine(ctx) - - 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()) - } - - reviews := make([]*Review, 0, 4) - return reviews, sess.Find(&reviews) -} - -// GetReviewsByIssueID gets the latest review of each reviewer for a pull request -func GetReviewsByIssueID(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 - } - - 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 len(teamReviewRequests) > 0 { - reviews = append(reviews, teamReviewRequests...) - } - - return reviews, nil -} - -// GetReviewersFromOriginalAuthorsByIssueID gets the latest review of each original authors for a pull request -func GetReviewersFromOriginalAuthorsByIssueID(issueID int64) ([]*Review, error) { - reviews := make([]*Review, 0, 10) - - // Get latest review of each reviewer, sorted in order they were made - if err := db.GetEngine(db.DefaultContext).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 -} - // GetReviewByIssueIDAndUserID get the latest review of reviewer for a pull request func GetReviewByIssueIDAndUserID(ctx context.Context, issueID, userID int64) (*Review, error) { review := new(Review) @@ -654,7 +498,7 @@ func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) { } // DismissReview change the dismiss status of a review -func DismissReview(review *Review, isDismiss bool) (err error) { +func DismissReview(ctx context.Context, review *Review, isDismiss bool) (err error) { if review.Dismissed == isDismiss || (review.Type != ReviewTypeApprove && review.Type != ReviewTypeReject) { return nil } @@ -665,7 +509,7 @@ func DismissReview(review *Review, isDismiss bool) (err error) { return ErrReviewNotExist{} } - _, err = db.GetEngine(db.DefaultContext).ID(review.ID).Cols("dismissed").Update(review) + _, err = db.GetEngine(ctx).ID(review.ID).Cols("dismissed").Update(review) return err } diff --git a/models/issues/review_list.go b/models/issues/review_list.go new file mode 100644 index 0000000000..c044fe915a --- /dev/null +++ b/models/issues/review_list.go @@ -0,0 +1,172 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import ( + "context" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" + "code.gitea.io/gitea/modules/util" + + "xorm.io/builder" +) + +type ReviewList []*Review + +// LoadReviewers loads reviewers +func (reviews ReviewList) LoadReviewers(ctx context.Context) error { + reviewerIds := make([]int64, len(reviews)) + for i := 0; i < len(reviews); i++ { + reviewerIds[i] = reviews[i].ReviewerID + } + reviewers, err := user_model.GetPossibleUserByIDs(ctx, reviewerIds) + if err != nil { + return err + } + + userMap := make(map[int64]*user_model.User, len(reviewers)) + for _, reviewer := range reviewers { + userMap[reviewer.ID] = reviewer + } + for _, review := range reviews { + review.Reviewer = userMap[review.ReviewerID] + } + return nil +} + +func (reviews ReviewList) LoadIssues(ctx context.Context) error { + issueIds := container.Set[int64]{} + for i := 0; i < len(reviews); i++ { + issueIds.Add(reviews[i].IssueID) + } + + issues, err := GetIssuesByIDs(ctx, issueIds.Values()) + if err != nil { + return err + } + if _, err := issues.LoadRepositories(ctx); err != nil { + return err + } + issueMap := make(map[int64]*Issue, len(issues)) + for _, issue := range issues { + issueMap[issue.ID] = issue + } + + for _, review := range reviews { + review.Issue = issueMap[review.IssueID] + } + return nil +} + +// FindReviewOptions represent possible filters to find reviews +type FindReviewOptions struct { + db.ListOptions + Type ReviewType + IssueID int64 + ReviewerID int64 + OfficialOnly bool + Dismissed util.OptionalBool +} + +func (opts *FindReviewOptions) toCond() builder.Cond { + cond := builder.NewCond() + if opts.IssueID > 0 { + cond = cond.And(builder.Eq{"issue_id": opts.IssueID}) + } + 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 opts.OfficialOnly { + cond = cond.And(builder.Eq{"official": true}) + } + if !opts.Dismissed.IsNone() { + cond = cond.And(builder.Eq{"dismissed": opts.Dismissed.IsTrue()}) + } + return cond +} + +// FindReviews returns reviews passing FindReviewOptions +func FindReviews(ctx context.Context, opts FindReviewOptions) (ReviewList, error) { + reviews := make([]*Review, 0, 10) + sess := db.GetEngine(ctx).Where(opts.toCond()) + if opts.Page > 0 && !opts.IsListAll() { + sess = db.SetSessionPagination(sess, &opts) + } + return reviews, sess. + Asc("created_unix"). + Asc("id"). + Find(&reviews) +} + +// FindLatestReviews returns only latest reviews per user, passing FindReviewOptions +func FindLatestReviews(ctx context.Context, opts FindReviewOptions) (ReviewList, error) { + reviews := make([]*Review, 0, 10) + cond := opts.toCond() + sess := db.GetEngine(ctx).Where(cond) + if opts.Page > 0 { + sess = db.SetSessionPagination(sess, &opts) + } + + sess.In("id", builder. + Select("max ( id ) "). + From("review"). + Where(cond). + GroupBy("reviewer_id")) + + return reviews, sess. + Asc("created_unix"). + Asc("id"). + Find(&reviews) +} + +// CountReviews returns count of reviews passing FindReviewOptions +func CountReviews(opts FindReviewOptions) (int64, error) { + return db.GetEngine(db.DefaultContext).Where(opts.toCond()).Count(&Review{}) +} + +// GetReviewersFromOriginalAuthorsByIssueID gets the latest review of each original authors for a pull request +func GetReviewersFromOriginalAuthorsByIssueID(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(db.DefaultContext).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(issueID int64) (ReviewList, 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 + } + + 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 len(teamReviewRequests) > 0 { + reviews = append(reviews, teamReviewRequests...) + } + + return reviews, nil +} diff --git a/models/issues/review_test.go b/models/issues/review_test.go index de7b6b2902..19816e864b 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -159,7 +159,7 @@ func TestGetReviewersByIssueID(t *testing.T) { allReviews, err = issues_model.GetReviewsByIssueID(issue.ID) assert.NoError(t, err) - assert.NoError(t, issues_model.LoadReviewers(db.DefaultContext, allReviews)) + assert.NoError(t, allReviews.LoadReviewers(db.DefaultContext)) if assert.Len(t, allReviews, 3) { for i, review := range allReviews { assert.Equal(t, expectedReviews[i].Reviewer, review.Reviewer) @@ -179,46 +179,46 @@ func TestDismissReview(t *testing.T) { assert.False(t, requestReviewExample.Dismissed) assert.False(t, approveReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(rejectReviewExample, true)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, rejectReviewExample, true)) rejectReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 9}) requestReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 11}) assert.True(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(requestReviewExample, true)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, requestReviewExample, true)) rejectReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 9}) requestReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 11}) assert.True(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) assert.False(t, approveReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(requestReviewExample, true)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, requestReviewExample, true)) rejectReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 9}) requestReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 11}) assert.True(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) assert.False(t, approveReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(requestReviewExample, false)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, requestReviewExample, false)) rejectReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 9}) requestReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 11}) assert.True(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) assert.False(t, approveReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(requestReviewExample, false)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, requestReviewExample, false)) rejectReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 9}) requestReviewExample = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 11}) assert.True(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) assert.False(t, approveReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(rejectReviewExample, false)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, rejectReviewExample, false)) assert.False(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) assert.False(t, approveReviewExample.Dismissed) - assert.NoError(t, issues_model.DismissReview(approveReviewExample, true)) + assert.NoError(t, issues_model.DismissReview(db.DefaultContext, approveReviewExample, true)) assert.False(t, rejectReviewExample.Dismissed) assert.False(t, requestReviewExample.Dismissed) assert.True(t, approveReviewExample.Dismissed) |