Skip to content

Commit

Permalink
Fix possible NPE in ToPullReviewList (#29759) (#29775)
Browse files Browse the repository at this point in the history
Backport #29759 by @lunny

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
  • Loading branch information
3 people committed Mar 13, 2024
1 parent bb98603 commit 5e3581f
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
2 changes: 1 addition & 1 deletion services/convert/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func ToPullReviewList(ctx context.Context, rl []*issues_model.Review, doer *user
result := make([]*api.PullReview, 0, len(rl))
for i := range rl {
// show pending reviews only for the user who created them
if rl[i].Type == issues_model.ReviewTypePending && !(doer.IsAdmin || doer.ID == rl[i].ReviewerID) {
if rl[i].Type == issues_model.ReviewTypePending && (doer == nil || (!doer.IsAdmin && doer.ID != rl[i].ReviewerID)) {
continue
}
r, err := ToPullReview(ctx, rl[i], doer)
Expand Down
52 changes: 52 additions & 0 deletions services/convert/pull_review_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package convert

import (
"testing"

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"

"github.com/stretchr/testify/assert"
)

func Test_ToPullReview(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 6})
assert.EqualValues(t, reviewer.ID, review.ReviewerID)
assert.EqualValues(t, issues_model.ReviewTypePending, review.Type)

reviewList := []*issues_model.Review{review}

t.Run("Anonymous User", func(t *testing.T) {
prList, err := ToPullReviewList(db.DefaultContext, reviewList, nil)
assert.NoError(t, err)
assert.Empty(t, prList)
})

t.Run("Reviewer Himself", func(t *testing.T) {
prList, err := ToPullReviewList(db.DefaultContext, reviewList, reviewer)
assert.NoError(t, err)
assert.Len(t, prList, 1)
})

t.Run("Other User", func(t *testing.T) {
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
prList, err := ToPullReviewList(db.DefaultContext, reviewList, user4)
assert.NoError(t, err)
assert.Len(t, prList, 0)
})

t.Run("Admin User", func(t *testing.T) {
adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
prList, err := ToPullReviewList(db.DefaultContext, reviewList, adminUser)
assert.NoError(t, err)
assert.Len(t, prList, 1)
})
}

0 comments on commit 5e3581f

Please sign in to comment.