Skip to content

Commit 7948cb3

Browse files
authored
Prevent NPE whilst migrating if there is a team request review (#19855)
A pr.Reviewer may be nil when migrating from Gitea if this is a team request review. We do not migrate teams therefore we cannot map these requests, but we can migrate user requests. Signed-off-by: Andrew Thornton <[email protected]>
1 parent d087554 commit 7948cb3

File tree

7 files changed

+42
-8
lines changed

7 files changed

+42
-8
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ require (
4343
github.com/gogs/cron v0.0.0-20171120032916-9f6c956d3e14
4444
github.com/gogs/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85
4545
github.com/golang-jwt/jwt/v4 v4.4.1
46-
github.com/google/go-github/v39 v39.2.0
46+
github.com/google/go-github/v45 v45.0.0
4747
github.com/google/pprof v0.0.0-20220509035851-59ca7ad80af3
4848
github.com/google/uuid v1.3.0
4949
github.com/gorilla/feeds v1.1.1

go.sum

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -735,11 +735,11 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
735735
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
736736
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
737737
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
738-
github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o=
739738
github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
739+
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
740740
github.com/google/go-github/v28 v28.1.1/go.mod h1:bsqJWQX05omyWVmc00nEUql9mhQyv38lDZ8kPZcQVoM=
741-
github.com/google/go-github/v39 v39.2.0 h1:rNNM311XtPOz5rDdsJXAp2o8F67X9FnROXTvto3aSnQ=
742-
github.com/google/go-github/v39 v39.2.0/go.mod h1:C1s8C5aCC9L+JXIYpJM5GYytdX52vC1bLvHEF1IhBrE=
741+
github.com/google/go-github/v45 v45.0.0 h1:LU0WBjYidxIVyx7PZeWb+FP4JZJ3Wh3FQgdumnGqiLs=
742+
github.com/google/go-github/v45 v45.0.0/go.mod h1:FObaZJEDSTa/WGCzZ2Z3eoCDXWJKMenWWTrd8jrta28=
743743
github.com/google/go-licenses v0.0.0-20210329231322-ce1d9163b77d/go.mod h1:+TYOmkVoJOpwnS0wfdsJCV9CoD5nJYsHoFk/0CrTK4M=
744744
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
745745
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=

modules/migration/review.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const (
1818
ReviewStateApproved = "APPROVED"
1919
ReviewStateChangesRequested = "CHANGES_REQUESTED"
2020
ReviewStateCommented = "COMMENTED"
21+
ReviewStateRequestReview = "REQUEST_REVIEW"
2122
)
2223

2324
// Review is a standard review information

services/migrations/error.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ package migrations
88
import (
99
"errors"
1010

11-
"github.com/google/go-github/v39/github"
11+
"github.com/google/go-github/v45/github"
1212
)
1313

1414
// ErrRepoNotCreated returns the error that repository not created

services/migrations/gitea_downloader.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,11 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review
639639
}
640640

641641
for _, pr := range prl {
642+
if pr.Reviewer == nil {
643+
// Presumably this is a team review which we cannot migrate at present but we have to skip this review as otherwise the review will be mapped on to an incorrect user.
644+
// TODO: handle team reviews
645+
continue
646+
}
642647

643648
rcl, _, err := g.client.ListPullReviewComments(g.repoOwner, g.repoName, reviewable.GetForeignIndex(), pr.ID)
644649
if err != nil {
@@ -664,7 +669,7 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review
664669
})
665670
}
666671

667-
allReviews = append(allReviews, &base.Review{
672+
review := &base.Review{
668673
ID: pr.ID,
669674
IssueIndex: reviewable.GetLocalIndex(),
670675
ReviewerID: pr.Reviewer.ID,
@@ -675,7 +680,9 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review
675680
CreatedAt: pr.Submitted,
676681
State: string(pr.State),
677682
Comments: reviewComments,
678-
})
683+
}
684+
685+
allReviews = append(allReviews, review)
679686
}
680687

681688
if len(prl) < g.maxPerPage {

services/migrations/gitea_uploader.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,8 @@ func convertReviewState(state string) models.ReviewType {
696696
return models.ReviewTypeReject
697697
case base.ReviewStateCommented:
698698
return models.ReviewTypeComment
699+
case base.ReviewStateRequestReview:
700+
return models.ReviewTypeRequest
699701
default:
700702
return models.ReviewTypePending
701703
}

services/migrations/github.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
"code.gitea.io/gitea/modules/structs"
2222
"code.gitea.io/gitea/modules/util"
2323

24-
"github.com/google/go-github/v39/github"
24+
"github.com/google/go-github/v45/github"
2525
"golang.org/x/oauth2"
2626
)
2727

@@ -778,6 +778,7 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev
778778
opt := &github.ListOptions{
779779
PerPage: g.maxPerPage,
780780
}
781+
// Get approve/request change reviews
781782
for {
782783
g.waitAndPickClient()
783784
reviews, resp, err := g.getClient().PullRequests.ListReviews(g.ctx, g.repoOwner, g.repoName, int(reviewable.GetForeignIndex()), opt)
@@ -817,5 +818,28 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev
817818
}
818819
opt.Page = resp.NextPage
819820
}
821+
// Get requested reviews
822+
for {
823+
g.waitAndPickClient()
824+
reviewers, resp, err := g.getClient().PullRequests.ListReviewers(g.ctx, g.repoOwner, g.repoName, int(reviewable.GetForeignIndex()), opt)
825+
if err != nil {
826+
return nil, fmt.Errorf("error while listing repos: %v", err)
827+
}
828+
g.setRate(&resp.Rate)
829+
for _, user := range reviewers.Users {
830+
r := &base.Review{
831+
ReviewerID: user.GetID(),
832+
ReviewerName: user.GetLogin(),
833+
State: base.ReviewStateRequestReview,
834+
IssueIndex: reviewable.GetLocalIndex(),
835+
}
836+
allReviews = append(allReviews, r)
837+
}
838+
// TODO: Handle Team requests
839+
if resp.NextPage == 0 {
840+
break
841+
}
842+
opt.Page = resp.NextPage
843+
}
820844
return allReviews, nil
821845
}

0 commit comments

Comments
 (0)