From 70623e8da1eb6c7e13a3cef04f1db9d479ffd7a4 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 30 Dec 2023 15:06:17 +0100 Subject: [PATCH] [GITEA] Fix NPE in `ToPullReviewList` - Add condition to ensure doer isn't nil when using it. - Added unit test. - Resolves #2055 (cherry picked from commit 8f1a74fb2944c2a1cf3824c2c6f233d6df2df593) (cherry picked from commit 60ac881776c750bc25e1d142e201e78e48e3ac23) (cherry picked from commit 5fdc461ac53ec486e609ad6ac40cde8e701c0fb8) --- services/convert/pull_review.go | 2 +- services/convert/pull_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index aa7ad68a47..05638c2c9b 100644 --- a/services/convert/pull_review.go +++ b/services/convert/pull_review.go @@ -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) diff --git a/services/convert/pull_test.go b/services/convert/pull_test.go index e069fa4a68..66c7313f7d 100644 --- a/services/convert/pull_test.go +++ b/services/convert/pull_test.go @@ -12,6 +12,7 @@ import ( access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/structs" @@ -47,3 +48,30 @@ func TestPullRequest_APIFormat(t *testing.T) { assert.Nil(t, apiPullRequest.Head.Repository) assert.EqualValues(t, -1, apiPullRequest.Head.RepoID) } + +func TestPullReviewList(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Pending review", func(t *testing.T) { + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 6, ReviewerID: reviewer.ID}) + rl := []*issues_model.Review{review} + + t.Run("Anonymous", func(t *testing.T) { + prList, err := ToPullReviewList(db.DefaultContext, rl, nil) + assert.NoError(t, err) + assert.Empty(t, prList) + }) + t.Run("Reviewer", func(t *testing.T) { + prList, err := ToPullReviewList(db.DefaultContext, rl, reviewer) + assert.NoError(t, err) + assert.Len(t, prList, 1) + }) + t.Run("Admin", func(t *testing.T) { + adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{IsAdmin: true}, unittest.Cond("id != ?", reviewer.ID)) + prList, err := ToPullReviewList(db.DefaultContext, rl, adminUser) + assert.NoError(t, err) + assert.Len(t, prList, 1) + }) + }) +}