Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround to clean up old reviews on creating a new one #28554

Merged
merged 25 commits into from
Feb 19, 2024
Merged
Changes from 3 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
cf0f77d
Clean up old reviews on creating a new one
6543 Dec 20, 2023
05db236
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Dec 20, 2023
9245e65
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Dec 20, 2023
9a71752
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Dec 21, 2023
a6adb7e
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Dec 21, 2023
e08e306
else if
6543 Dec 21, 2023
00abbf6
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Dec 23, 2023
8a06ccb
handle ReviewTypePending
6543 Dec 23, 2023
9fcea4c
Update models/issues/review.go
6543 Dec 23, 2023
b03faf4
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Jan 13, 2024
0df942a
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Jan 19, 2024
84024c1
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 12, 2024
f1b38c0
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 12, 2024
f4fbcf5
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 14, 2024
d4049d9
return result for unittest.AssertCount* helper func
6543 Feb 15, 2024
2db7e7c
Add TestAPIPullReviewStayDismissed test
6543 Feb 15, 2024
18dc9b3
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 15, 2024
758dd63
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 15, 2024
ab2ef38
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 16, 2024
01dafcc
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 16, 2024
36a2152
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 19, 2024
68c4de2
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 19, 2024
d23ace7
Merge branch 'main' into cleanup_old-reviews-on-review
lafriks Feb 19, 2024
b3d062d
test against rerequest and add some codecomments
6543 Feb 19, 2024
48fa3ad
Merge branch 'main' into cleanup_old-reviews-on-review
6543 Feb 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions models/issues/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,14 @@ func IsOfficialReviewerTeam(ctx context.Context, issue *Issue, team *organizatio

// CreateReview creates a new review based on opts
func CreateReview(ctx context.Context, opts CreateReviewOptions) (*Review, error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return nil, err
}
defer committer.Close()
sess := db.GetEngine(ctx)

review := &Review{
Type: opts.Type,
Issue: opts.Issue,
IssueID: opts.Issue.ID,
Reviewer: opts.Reviewer,
Expand All @@ -295,15 +301,37 @@ func CreateReview(ctx context.Context, opts CreateReviewOptions) (*Review, error
CommitID: opts.CommitID,
Stale: opts.Stale,
}
if opts.Reviewer != nil {

switch {
case opts.Reviewer != nil:
6543 marked this conversation as resolved.
Show resolved Hide resolved
review.Type = opts.Type
review.ReviewerID = opts.Reviewer.ID
} else {
if review.Type != ReviewTypeRequest {
review.Type = ReviewTypeRequest

reviewCond := builder.Eq{"reviewer_id": opts.Reviewer.ID, "issue_id": opts.Issue.ID}
// make sure user review requests are cleared
if _, err := sess.Where(reviewCond.And(builder.Eq{"type": ReviewTypeRequest})).Delete(new(Review)); err != nil {
return nil, err
}
// make sure if the created review gets dismissed no old review surface
if opts.Type == ReviewTypeApprove || opts.Type == ReviewTypeReject {
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
if _, err := sess.Where(reviewCond.And(builder.In("type", ReviewTypeApprove, ReviewTypeReject))).
6543 marked this conversation as resolved.
Show resolved Hide resolved
Cols("dismissed").Update(&Review{Dismissed: true}); err != nil {
return nil, err
}
}

case opts.ReviewerTeam != nil:
review.Type = ReviewTypeRequest
review.ReviewerTeamID = opts.ReviewerTeam.ID

default:
return nil, fmt.Errorf("provide either reviewer or reviewer team")
}

if _, err := sess.Insert(review); err != nil {
return nil, err
}
return review, db.Insert(ctx, review)
return review, committer.Commit()
}

// GetCurrentReview returns the current pending review of reviewer for given issue
Expand Down