From 83ba882bab7e1545fe02cd41f554ae41b83a6040 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Mar 2024 03:46:15 +0800 Subject: [PATCH] Fix possible NPE in ToPullReviewList (#29759) Co-authored-by: wxiaoguang --- services/convert/pull_review.go | 2 +- services/convert/pull_review_test.go | 52 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 services/convert/pull_review_test.go diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index aa7ad68a4785..29a5ab7466b1 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_review_test.go b/services/convert/pull_review_test.go new file mode 100644 index 000000000000..68869502802b --- /dev/null +++ b/services/convert/pull_review_test.go @@ -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) + }) +}