fix(issues): clear stale ReviewTypeRequest when submitting pending re… (#37809)

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)

<!--
Before submitting:
- Target the `main` branch; release branches are for backports only.
- Use a Conventional Commits title, e.g. `fix(repo): handle empty branch
names`.
- Read the contributing guidelines:
https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md
- Documentation changes go to https://gitea.com/gitea/docs

Describe your change below and link any issue it fixes.
-->

---------

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
Co-authored-by: Nicolas <bircni@icloud.com>
This commit is contained in:
Alaa Abdelwahab
2026-05-22 16:18:32 +02:00
committed by GitHub
parent bf1b54c3e3
commit bc9d53a5a8
2 changed files with 48 additions and 0 deletions

View File

@@ -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{

View File

@@ -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())