From bc9d53a5a89d9e1debf08ee52374c6d4e8cac038 Mon Sep 17 00:00:00 2001 From: Alaa Abdelwahab <82750565+Powerscore@users.noreply.github.com> Date: Fri, 22 May 2026 16:18:32 +0200 Subject: [PATCH] =?UTF-8?q?fix(issues):=20clear=20stale=20ReviewTypeReques?= =?UTF-8?q?t=20when=20submitting=20pending=20re=E2=80=A6=20(#37809)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When SubmitReview updates an existing pending review in-place, it was not deleting the reviewer's ReviewTypeRequest row, unlike the CreateReview path. That leftover row causes AddReviewRequest to bail out silently, making the re-request icon in the PR sidebar a no-op. Fixes #37808 (Claude Opus 4.7) --------- Co-authored-by: silverwind Co-authored-by: Claude (Opus 4.7) Co-authored-by: Nicolas --- models/issues/review.go | 8 ++++++++ models/issues/review_test.go | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/models/issues/review.go b/models/issues/review.go index 78ef0d20c23..99366222198 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -483,6 +483,14 @@ func SubmitReview(ctx context.Context, doer *user_model.User, issue *Issue, revi if _, err := sess.ID(review.ID).Cols("content, type, official, commit_id, stale").Update(review); err != nil { return nil, nil, err } + + // make sure the leftover review request is cleared, consistent with CreateReview + if reviewType != ReviewTypePending { + if _, err := sess.Where(builder.Eq{"reviewer_id": doer.ID, "issue_id": issue.ID, "type": ReviewTypeRequest}). + Delete(new(Review)); err != nil { + return nil, nil, err + } + } } comm, err := CreateComment(ctx, &CreateCommentOptions{ diff --git a/models/issues/review_test.go b/models/issues/review_test.go index 092d88d1749..a384dbd30fe 100644 --- a/models/issues/review_test.go +++ b/models/issues/review_test.go @@ -303,6 +303,46 @@ func TestDeleteDismissedReview(t *testing.T) { unittest.AssertNotExistsBean(t, &issues_model.Comment{ID: comment.ID}) } +func TestSubmitReviewClearsStaleReviewRequest(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3}) + assert.NoError(t, issue.LoadRepo(t.Context())) + assert.NoError(t, issue.Repo.LoadOwner(t.Context())) + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // the reviewer is requested to review the pull request + requestReview, err := issues_model.CreateReview(t.Context(), issues_model.CreateReviewOptions{ + Type: issues_model.ReviewTypeRequest, + Issue: issue, + Reviewer: reviewer, + }) + assert.NoError(t, err) + + // the reviewer starts a pending review (e.g. by adding code comments) + pendingReview, err := issues_model.CreateReview(t.Context(), issues_model.CreateReviewOptions{ + Type: issues_model.ReviewTypePending, + Issue: issue, + Reviewer: reviewer, + }) + assert.NoError(t, err) + + // submitting the pending review must clear the leftover review request, + // otherwise the reviewer can no longer be re-requested afterwards + review, _, err := issues_model.SubmitReview(t.Context(), reviewer, issue, issues_model.ReviewTypeComment, "looks good", "", false, nil) + assert.NoError(t, err) + assert.Equal(t, pendingReview.ID, review.ID) + assert.Equal(t, issues_model.ReviewTypeComment, review.Type) + + unittest.AssertNotExistsBean(t, &issues_model.Review{ID: requestReview.ID}) + + // the reviewer can be re-requested afterwards (no-op before the fix) + comment, err := issues_model.AddReviewRequest(t.Context(), issue, reviewer, doer, false) + assert.NoError(t, err) + assert.NotNil(t, comment) +} + func TestAddReviewRequest(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase())