Skip to content

Commit 83ba882

Browse files
lunnywxiaoguang
andauthored
Fix possible NPE in ToPullReviewList (#29759)
Co-authored-by: wxiaoguang <[email protected]>
1 parent 712e19f commit 83ba882

File tree

2 files changed

+53
-1
lines changed

2 files changed

+53
-1
lines changed

services/convert/pull_review.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func ToPullReviewList(ctx context.Context, rl []*issues_model.Review, doer *user
6666
result := make([]*api.PullReview, 0, len(rl))
6767
for i := range rl {
6868
// show pending reviews only for the user who created them
69-
if rl[i].Type == issues_model.ReviewTypePending && !(doer.IsAdmin || doer.ID == rl[i].ReviewerID) {
69+
if rl[i].Type == issues_model.ReviewTypePending && (doer == nil || (!doer.IsAdmin && doer.ID != rl[i].ReviewerID)) {
7070
continue
7171
}
7272
r, err := ToPullReview(ctx, rl[i], doer)

services/convert/pull_review_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package convert
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/db"
10+
issues_model "code.gitea.io/gitea/models/issues"
11+
"code.gitea.io/gitea/models/unittest"
12+
user_model "code.gitea.io/gitea/models/user"
13+
14+
"github.com/stretchr/testify/assert"
15+
)
16+
17+
func Test_ToPullReview(t *testing.T) {
18+
assert.NoError(t, unittest.PrepareTestDatabase())
19+
20+
reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
21+
review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 6})
22+
assert.EqualValues(t, reviewer.ID, review.ReviewerID)
23+
assert.EqualValues(t, issues_model.ReviewTypePending, review.Type)
24+
25+
reviewList := []*issues_model.Review{review}
26+
27+
t.Run("Anonymous User", func(t *testing.T) {
28+
prList, err := ToPullReviewList(db.DefaultContext, reviewList, nil)
29+
assert.NoError(t, err)
30+
assert.Empty(t, prList)
31+
})
32+
33+
t.Run("Reviewer Himself", func(t *testing.T) {
34+
prList, err := ToPullReviewList(db.DefaultContext, reviewList, reviewer)
35+
assert.NoError(t, err)
36+
assert.Len(t, prList, 1)
37+
})
38+
39+
t.Run("Other User", func(t *testing.T) {
40+
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
41+
prList, err := ToPullReviewList(db.DefaultContext, reviewList, user4)
42+
assert.NoError(t, err)
43+
assert.Len(t, prList, 0)
44+
})
45+
46+
t.Run("Admin User", func(t *testing.T) {
47+
adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
48+
prList, err := ToPullReviewList(db.DefaultContext, reviewList, adminUser)
49+
assert.NoError(t, err)
50+
assert.Len(t, prList, 1)
51+
})
52+
}

0 commit comments

Comments
 (0)