Fix panic when comment is nil (#34257) (#34277)

Fix #34254
Backport #34257
This commit is contained in:
Lunny Xiao 2025-04-24 18:28:02 -07:00 committed by GitHub
parent 030ed9462d
commit 4e5aca62ee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 10 additions and 22 deletions

View File

@ -663,7 +663,7 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo
}
if review != nil {
// skip it when reviewer hase been request to review
// skip it when reviewer has been request to review
if review.Type == ReviewTypeRequest {
return nil, committer.Commit() // still commit the transaction, or committer.Close() will rollback it, even if it's a reused transaction.
}

View File

@ -145,6 +145,9 @@ func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_m
log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err)
return nil, err
}
if comment == nil { // comment maybe nil if review type is ReviewTypeRequest
continue
}
notifiers = append(notifiers, &ReviewRequestNotifier{
Comment: comment,
IsAdd: true,
@ -158,6 +161,9 @@ func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_m
log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err)
return nil, err
}
if comment == nil { // comment maybe nil if review type is ReviewTypeRequest
continue
}
notifiers = append(notifiers, &ReviewRequestNotifier{
Comment: comment,
IsAdd: true,

View File

@ -8,7 +8,6 @@ import (
"net/http/httptest"
"net/url"
"path"
"sort"
"strings"
"testing"
@ -76,7 +75,7 @@ func TestPullView_CodeOwner(t *testing.T) {
t.Run("First Pull Request", func(t *testing.T) {
// create a new branch to prepare for pull request
resp1, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
_, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
NewBranch: "codeowner-basebranch",
Files: []*files_service.ChangeRepoFile{
{
@ -96,13 +95,8 @@ func TestPullView_CodeOwner(t *testing.T) {
unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
assert.NoError(t, pr.LoadIssue(db.DefaultContext))
reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 1)
assert.EqualValues(t, 5, reviewNotifiers[0].Reviewer.ID)
// update the file on the pr branch
resp2, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
OldBranch: "codeowner-basebranch",
Files: []*files_service.ChangeRepoFile{
{
@ -114,14 +108,7 @@ func TestPullView_CodeOwner(t *testing.T) {
})
assert.NoError(t, err)
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 2)
reviewerIDs := []int64{reviewNotifiers[0].Reviewer.ID, reviewNotifiers[1].Reviewer.ID}
sort.Slice(reviewerIDs, func(i, j int) bool { return reviewerIDs[i] < reviewerIDs[j] })
assert.EqualValues(t, []int64{5, 8}, reviewerIDs)
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(db.DefaultContext, pr, resp1.Commit.SHA, resp2.Commit.SHA)
reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 1)
assert.EqualValues(t, 8, reviewNotifiers[0].Reviewer.ID)
@ -171,11 +158,6 @@ func TestPullView_CodeOwner(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch2"})
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(db.DefaultContext, pr)
assert.NoError(t, err)
assert.Len(t, reviewNotifiers, 1)
assert.EqualValues(t, 8, reviewNotifiers[0].Reviewer.ID)
})
t.Run("Forked Repo Pull Request", func(t *testing.T) {