Restore previous official review when an official review is deleted (#22449)

Fix #22406

Co-authored-by: Lauris BH <[email protected]>

Author: jpraet

models/issues/review.go

@@ -733,17 +733,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 := CreateComment(ctx, &CreateCommentOptions{
@@ -761,6 +753,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(db.DefaultContext)
@@ -979,6 +987,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()
 }
 

models/issues/review_test.go

@@ -200,3 +200,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)
+}