From 510c811574f035470ef0acf455e015e76a716f58 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Sun, 15 Jan 2023 20:47:54 +0100 Subject: [PATCH] Restore previous official review when an official review is deleted (#22449) (#22460) Backport #22449 Co-authored-by: Lauris BH --- models/issues/review.go | 32 +++++++++++++++++++++++--------- models/issues/review_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/models/issues/review.go b/models/issues/review.go index 3d2fceda2..40781befe 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -742,17 +742,9 @@ func RemoveReviewRequest(issue *Issue, reviewer, doer *user_model.User) (*Commen if err != nil { return nil, err } else if official { - // recalculate the latest official review for reviewer - review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { + if err := restoreLatestOfficialReview(ctx, issue.ID, reviewer.ID); err != nil { return nil, err } - - if review != nil { - if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE id=?", true, review.ID); err != nil { - return nil, err - } - } } comment, err := CreateCommentCtx(ctx, &CreateCommentOptions{ @@ -770,6 +762,22 @@ func RemoveReviewRequest(issue *Issue, reviewer, doer *user_model.User) (*Commen return comment, committer.Commit() } +// Recalculate the latest official review for reviewer +func restoreLatestOfficialReview(ctx context.Context, issueID, reviewerID int64) error { + review, err := GetReviewByIssueIDAndUserID(ctx, issueID, reviewerID) + if err != nil && !IsErrReviewNotExist(err) { + return err + } + + if review != nil { + if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE id=?", true, review.ID); err != nil { + return err + } + } + + return nil +} + // AddTeamReviewRequest add a review request from one team func AddTeamReviewRequest(issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { ctx, committer, err := db.TxContext() @@ -988,6 +996,12 @@ func DeleteReview(r *Review) error { return err } + if r.Official { + if err := restoreLatestOfficialReview(ctx, r.IssueID, r.ReviewerID); err != nil { + return err + } + } + return committer.Commit() } diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 46d1cc777..e1f3b1a80 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -201,3 +201,38 @@ func TestDismissReview(t *testing.T) { assert.False(t, requestReviewExample.Dismissed) assert.True(t, approveReviewExample.Dismissed) } + +func TestDeleteReview(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + review1, err := issues_model.CreateReview(db.DefaultContext, issues_model.CreateReviewOptions{ + Content: "Official rejection", + Type: issues_model.ReviewTypeReject, + Official: false, + Issue: issue, + Reviewer: user, + }) + assert.NoError(t, err) + + review2, err := issues_model.CreateReview(db.DefaultContext, issues_model.CreateReviewOptions{ + Content: "Official approval", + Type: issues_model.ReviewTypeApprove, + Official: true, + Issue: issue, + Reviewer: user, + }) + assert.NoError(t, err) + + assert.NoError(t, issues_model.DeleteReview(review2)) + + _, err = issues_model.GetReviewByID(db.DefaultContext, review2.ID) + assert.Error(t, err) + assert.True(t, issues_model.IsErrReviewNotExist(err), "IsErrReviewNotExist") + + review1, err = issues_model.GetReviewByID(db.DefaultContext, review1.ID) + assert.NoError(t, err) + assert.True(t, review1.Official) +}