Skip to content

Commit bf32895

Browse files
committed
Fix bug when pushing to a pull request which enabled dismiss approval automatically
1 parent 61c9268 commit bf32895

File tree

7 files changed

+249
-175
lines changed

7 files changed

+249
-175
lines changed

models/issues/issue.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,13 @@ func GetIssueWithAttrsByID(id int64) (*Issue, error) {
551551

552552
// GetIssuesByIDs return issues with the given IDs.
553553
func GetIssuesByIDs(ctx context.Context, issueIDs []int64) (IssueList, error) {
554-
issues := make([]*Issue, 0, 10)
554+
issues := make([]*Issue, 0, len(issueIDs))
555+
return issues, db.GetEngine(ctx).In("id", issueIDs).Find(&issues)
556+
}
557+
558+
// GetIssuesMapByIDs return issues with the given IDs.
559+
func GetIssuesMapByIDs(ctx context.Context, issueIDs []int64) (map[int64]*Issue, error) {
560+
issues := make(map[int64]*Issue, len(issueIDs))
555561
return issues, db.GetEngine(ctx).In("id", issueIDs).Find(&issues)
556562
}
557563

models/issues/pull.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -316,15 +316,13 @@ func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error {
316316
return err
317317
}
318318

319-
if len(reviews) > 0 {
320-
err = LoadReviewers(ctx, reviews)
321-
if err != nil {
322-
return err
323-
}
324-
for _, review := range reviews {
325-
pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer)
326-
}
319+
if err = reviews.LoadReviewers(ctx); err != nil {
320+
return err
327321
}
322+
for _, review := range reviews {
323+
pr.RequestedReviewers = append(pr.RequestedReviewers, review.Reviewer)
324+
}
325+
328326
return nil
329327
}
330328

models/issues/review.go

Lines changed: 2 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -162,27 +162,6 @@ func (r *Review) LoadReviewer(ctx context.Context) (err error) {
162162
return err
163163
}
164164

165-
// LoadReviewers loads reviewers
166-
func LoadReviewers(ctx context.Context, reviews []*Review) (err error) {
167-
reviewerIds := make([]int64, len(reviews))
168-
for i := 0; i < len(reviews); i++ {
169-
reviewerIds[i] = reviews[i].ReviewerID
170-
}
171-
reviewers, err := user_model.GetPossibleUserByIDs(ctx, reviewerIds)
172-
if err != nil {
173-
return err
174-
}
175-
176-
userMap := make(map[int64]*user_model.User, len(reviewers))
177-
for _, reviewer := range reviewers {
178-
userMap[reviewer.ID] = reviewer
179-
}
180-
for _, review := range reviews {
181-
review.Reviewer = userMap[review.ReviewerID]
182-
}
183-
return nil
184-
}
185-
186165
// LoadReviewerTeam loads reviewer team
187166
func (r *Review) LoadReviewerTeam(ctx context.Context) (err error) {
188167
if r.ReviewerTeamID == 0 || r.ReviewerTeam != nil {
@@ -236,71 +215,6 @@ func GetReviewByID(ctx context.Context, id int64) (*Review, error) {
236215
}
237216
}
238217

239-
// FindReviewOptions represent possible filters to find reviews
240-
type FindReviewOptions struct {
241-
db.ListOptions
242-
Type ReviewType
243-
IssueID int64
244-
ReviewerID int64
245-
OfficialOnly bool
246-
}
247-
248-
func (opts *FindReviewOptions) toCond() builder.Cond {
249-
cond := builder.NewCond()
250-
if opts.IssueID > 0 {
251-
cond = cond.And(builder.Eq{"issue_id": opts.IssueID})
252-
}
253-
if opts.ReviewerID > 0 {
254-
cond = cond.And(builder.Eq{"reviewer_id": opts.ReviewerID})
255-
}
256-
if opts.Type != ReviewTypeUnknown {
257-
cond = cond.And(builder.Eq{"type": opts.Type})
258-
}
259-
if opts.OfficialOnly {
260-
cond = cond.And(builder.Eq{"official": true})
261-
}
262-
return cond
263-
}
264-
265-
// FindReviews returns reviews passing FindReviewOptions
266-
func FindReviews(ctx context.Context, opts FindReviewOptions) ([]*Review, error) {
267-
reviews := make([]*Review, 0, 10)
268-
sess := db.GetEngine(ctx).Where(opts.toCond())
269-
if opts.Page > 0 {
270-
sess = db.SetSessionPagination(sess, &opts)
271-
}
272-
return reviews, sess.
273-
Asc("created_unix").
274-
Asc("id").
275-
Find(&reviews)
276-
}
277-
278-
// FindLatestReviews returns only latest reviews per user, passing FindReviewOptions
279-
func FindLatestReviews(ctx context.Context, opts FindReviewOptions) ([]*Review, error) {
280-
reviews := make([]*Review, 0, 10)
281-
cond := opts.toCond()
282-
sess := db.GetEngine(ctx).Where(cond)
283-
if opts.Page > 0 {
284-
sess = db.SetSessionPagination(sess, &opts)
285-
}
286-
287-
sess.In("id", builder.
288-
Select("max ( id ) ").
289-
From("review").
290-
Where(cond).
291-
GroupBy("reviewer_id"))
292-
293-
return reviews, sess.
294-
Asc("created_unix").
295-
Asc("id").
296-
Find(&reviews)
297-
}
298-
299-
// CountReviews returns count of reviews passing FindReviewOptions
300-
func CountReviews(opts FindReviewOptions) (int64, error) {
301-
return db.GetEngine(db.DefaultContext).Where(opts.toCond()).Count(&Review{})
302-
}
303-
304218
// CreateReviewOptions represent the options to create a review. Type, Issue and Reviewer are required.
305219
type CreateReviewOptions struct {
306220
Content string
@@ -533,76 +447,6 @@ func SubmitReview(doer *user_model.User, issue *Issue, reviewType ReviewType, co
533447
return review, comm, committer.Commit()
534448
}
535449

536-
// GetReviewOptions represent filter options for GetReviews
537-
type GetReviewOptions struct {
538-
IssueID int64
539-
ReviewerID int64
540-
Dismissed util.OptionalBool
541-
}
542-
543-
// GetReviews return reviews based on GetReviewOptions
544-
func GetReviews(ctx context.Context, opts *GetReviewOptions) ([]*Review, error) {
545-
if opts == nil {
546-
return nil, fmt.Errorf("opts are nil")
547-
}
548-
549-
sess := db.GetEngine(ctx)
550-
551-
if opts.IssueID != 0 {
552-
sess = sess.Where("issue_id=?", opts.IssueID)
553-
}
554-
if opts.ReviewerID != 0 {
555-
sess = sess.Where("reviewer_id=?", opts.ReviewerID)
556-
}
557-
if !opts.Dismissed.IsNone() {
558-
sess = sess.Where("dismissed=?", opts.Dismissed.IsTrue())
559-
}
560-
561-
reviews := make([]*Review, 0, 4)
562-
return reviews, sess.Find(&reviews)
563-
}
564-
565-
// GetReviewsByIssueID gets the latest review of each reviewer for a pull request
566-
func GetReviewsByIssueID(issueID int64) ([]*Review, error) {
567-
reviews := make([]*Review, 0, 10)
568-
569-
sess := db.GetEngine(db.DefaultContext)
570-
571-
// Get latest review of each reviewer, sorted in order they were made
572-
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",
573-
issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, false).
574-
Find(&reviews); err != nil {
575-
return nil, err
576-
}
577-
578-
teamReviewRequests := make([]*Review, 0, 5)
579-
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",
580-
issueID).
581-
Find(&teamReviewRequests); err != nil {
582-
return nil, err
583-
}
584-
585-
if len(teamReviewRequests) > 0 {
586-
reviews = append(reviews, teamReviewRequests...)
587-
}
588-
589-
return reviews, nil
590-
}
591-
592-
// GetReviewersFromOriginalAuthorsByIssueID gets the latest review of each original authors for a pull request
593-
func GetReviewersFromOriginalAuthorsByIssueID(issueID int64) ([]*Review, error) {
594-
reviews := make([]*Review, 0, 10)
595-
596-
// Get latest review of each reviewer, sorted in order they were made
597-
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",
598-
issueID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest).
599-
Find(&reviews); err != nil {
600-
return nil, err
601-
}
602-
603-
return reviews, nil
604-
}
605-
606450
// GetReviewByIssueIDAndUserID get the latest review of reviewer for a pull request
607451
func GetReviewByIssueIDAndUserID(ctx context.Context, issueID, userID int64) (*Review, error) {
608452
review := new(Review)
@@ -654,7 +498,7 @@ func MarkReviewsAsNotStale(issueID int64, commitID string) (err error) {
654498
}
655499

656500
// DismissReview change the dismiss status of a review
657-
func DismissReview(review *Review, isDismiss bool) (err error) {
501+
func DismissReview(ctx context.Context, review *Review, isDismiss bool) (err error) {
658502
if review.Dismissed == isDismiss || (review.Type != ReviewTypeApprove && review.Type != ReviewTypeReject) {
659503
return nil
660504
}
@@ -665,7 +509,7 @@ func DismissReview(review *Review, isDismiss bool) (err error) {
665509
return ErrReviewNotExist{}
666510
}
667511

668-
_, err = db.GetEngine(db.DefaultContext).ID(review.ID).Cols("dismissed").Update(review)
512+
_, err = db.GetEngine(ctx).ID(review.ID).Cols("dismissed").Update(review)
669513

670514
return err
671515
}

0 commit comments

Comments
 (0)