Skip to content

Commit 510c811

Browse files
jpraetlafriks
andauthored
Restore previous official review when an official review is deleted (#22449) (#22460)
Backport #22449 Co-authored-by: Lauris BH <[email protected]>
1 parent f93522d commit 510c811

File tree

2 files changed

+58
-9
lines changed

2 files changed

+58
-9
lines changed

models/issues/review.go

+23-9
Original file line numberDiff line numberDiff line change
@@ -742,17 +742,9 @@ func RemoveReviewRequest(issue *Issue, reviewer, doer *user_model.User) (*Commen
742742
if err != nil {
743743
return nil, err
744744
} else if official {
745-
// recalculate the latest official review for reviewer
746-
review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID)
747-
if err != nil && !IsErrReviewNotExist(err) {
745+
if err := restoreLatestOfficialReview(ctx, issue.ID, reviewer.ID); err != nil {
748746
return nil, err
749747
}
750-
751-
if review != nil {
752-
if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE id=?", true, review.ID); err != nil {
753-
return nil, err
754-
}
755-
}
756748
}
757749

758750
comment, err := CreateCommentCtx(ctx, &CreateCommentOptions{
@@ -770,6 +762,22 @@ func RemoveReviewRequest(issue *Issue, reviewer, doer *user_model.User) (*Commen
770762
return comment, committer.Commit()
771763
}
772764

765+
// Recalculate the latest official review for reviewer
766+
func restoreLatestOfficialReview(ctx context.Context, issueID, reviewerID int64) error {
767+
review, err := GetReviewByIssueIDAndUserID(ctx, issueID, reviewerID)
768+
if err != nil && !IsErrReviewNotExist(err) {
769+
return err
770+
}
771+
772+
if review != nil {
773+
if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE id=?", true, review.ID); err != nil {
774+
return err
775+
}
776+
}
777+
778+
return nil
779+
}
780+
773781
// AddTeamReviewRequest add a review request from one team
774782
func AddTeamReviewRequest(issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) {
775783
ctx, committer, err := db.TxContext()
@@ -988,6 +996,12 @@ func DeleteReview(r *Review) error {
988996
return err
989997
}
990998

999+
if r.Official {
1000+
if err := restoreLatestOfficialReview(ctx, r.IssueID, r.ReviewerID); err != nil {
1001+
return err
1002+
}
1003+
}
1004+
9911005
return committer.Commit()
9921006
}
9931007

models/issues/review_test.go

+35
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,38 @@ func TestDismissReview(t *testing.T) {
201201
assert.False(t, requestReviewExample.Dismissed)
202202
assert.True(t, approveReviewExample.Dismissed)
203203
}
204+
205+
func TestDeleteReview(t *testing.T) {
206+
assert.NoError(t, unittest.PrepareTestDatabase())
207+
208+
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
209+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
210+
211+
review1, err := issues_model.CreateReview(db.DefaultContext, issues_model.CreateReviewOptions{
212+
Content: "Official rejection",
213+
Type: issues_model.ReviewTypeReject,
214+
Official: false,
215+
Issue: issue,
216+
Reviewer: user,
217+
})
218+
assert.NoError(t, err)
219+
220+
review2, err := issues_model.CreateReview(db.DefaultContext, issues_model.CreateReviewOptions{
221+
Content: "Official approval",
222+
Type: issues_model.ReviewTypeApprove,
223+
Official: true,
224+
Issue: issue,
225+
Reviewer: user,
226+
})
227+
assert.NoError(t, err)
228+
229+
assert.NoError(t, issues_model.DeleteReview(review2))
230+
231+
_, err = issues_model.GetReviewByID(db.DefaultContext, review2.ID)
232+
assert.Error(t, err)
233+
assert.True(t, issues_model.IsErrReviewNotExist(err), "IsErrReviewNotExist")
234+
235+
review1, err = issues_model.GetReviewByID(db.DefaultContext, review1.ID)
236+
assert.NoError(t, err)
237+
assert.True(t, review1.Official)
238+
}

0 commit comments

Comments
 (0)