Prevent NPE whilst migrating if there is a team request review (go-gi…

…tea#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 <art27@cantab.net>
kitspace · Aug 23, 2022 · c60f5c2 · c60f5c2
1 parent 36d5e78
commit c60f5c2
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 8 deletions.
diff --git a/go.mod b/go.mod
@@ -43,7 +43,7 @@ require (
 	github.com/gogs/cron v0.0.0-20171120032916-9f6c956d3e14
 	github.com/gogs/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85
 	github.com/golang-jwt/jwt/v4 v4.4.1
-	github.com/google/go-github/v39 v39.2.0
+	github.com/google/go-github/v45 v45.0.0
 	github.com/google/pprof v0.0.0-20220509035851-59ca7ad80af3
 	github.com/google/uuid v1.3.0
 	github.com/gorilla/feeds v1.1.1

diff --git a/go.sum b/go.sum
@@ -735,11 +735,11 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o=
 github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
+github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
 github.com/google/go-github/v28 v28.1.1/go.mod h1:bsqJWQX05omyWVmc00nEUql9mhQyv38lDZ8kPZcQVoM=
-github.com/google/go-github/v39 v39.2.0 h1:rNNM311XtPOz5rDdsJXAp2o8F67X9FnROXTvto3aSnQ=
-github.com/google/go-github/v39 v39.2.0/go.mod h1:C1s8C5aCC9L+JXIYpJM5GYytdX52vC1bLvHEF1IhBrE=
+github.com/google/go-github/v45 v45.0.0 h1:LU0WBjYidxIVyx7PZeWb+FP4JZJ3Wh3FQgdumnGqiLs=
+github.com/google/go-github/v45 v45.0.0/go.mod h1:FObaZJEDSTa/WGCzZ2Z3eoCDXWJKMenWWTrd8jrta28=
 github.com/google/go-licenses v0.0.0-20210329231322-ce1d9163b77d/go.mod h1:+TYOmkVoJOpwnS0wfdsJCV9CoD5nJYsHoFk/0CrTK4M=
 github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=

diff --git a/modules/migration/review.go b/modules/migration/review.go
@@ -18,6 +18,7 @@ const (
 	ReviewStateApproved         = "APPROVED"
 	ReviewStateChangesRequested = "CHANGES_REQUESTED"
 	ReviewStateCommented        = "COMMENTED"
+	ReviewStateRequestReview    = "REQUEST_REVIEW"
 )
 
 // Review is a standard review information

diff --git a/services/migrations/error.go b/services/migrations/error.go
@@ -8,7 +8,7 @@ package migrations
 import (
 	"errors"
 
-	"github.com/google/go-github/v39/github"
+	"github.com/google/go-github/v45/github"
 )
 
 // ErrRepoNotCreated returns the error that repository not created

diff --git a/services/migrations/gitea_downloader.go b/services/migrations/gitea_downloader.go
@@ -639,6 +639,11 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review
 		}
 
 		for _, pr := range prl {
+			if pr.Reviewer == nil {
+				// 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.
+				// TODO: handle team reviews
+				continue
+			}
 
 			rcl, _, err := g.client.ListPullReviewComments(g.repoOwner, g.repoName, reviewable.GetForeignIndex(), pr.ID)
 			if err != nil {
@@ -664,7 +669,7 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review
 				})
 			}
 
-			allReviews = append(allReviews, &base.Review{
+			review := &base.Review{
 				ID:           pr.ID,
 				IssueIndex:   reviewable.GetLocalIndex(),
 				ReviewerID:   pr.Reviewer.ID,
@@ -675,7 +680,9 @@ func (g *GiteaDownloader) GetReviews(reviewable base.Reviewable) ([]*base.Review
 				CreatedAt:    pr.Submitted,
 				State:        string(pr.State),
 				Comments:     reviewComments,
-			})
+			}
+
+			allReviews = append(allReviews, review)
 		}
 
 		if len(prl) < g.maxPerPage {

diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go
@@ -696,6 +696,8 @@ func convertReviewState(state string) models.ReviewType {
 		return models.ReviewTypeReject
 	case base.ReviewStateCommented:
 		return models.ReviewTypeComment
+	case base.ReviewStateRequestReview:
+		return models.ReviewTypeRequest
 	default:
 		return models.ReviewTypePending
 	}

diff --git a/services/migrations/github.go b/services/migrations/github.go
@@ -21,7 +21,7 @@ import (
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
 
-	"github.com/google/go-github/v39/github"
+	"github.com/google/go-github/v45/github"
 	"golang.org/x/oauth2"
 )
 
@@ -778,6 +778,7 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev
 	opt := &github.ListOptions{
 		PerPage: g.maxPerPage,
 	}
+	// Get approve/request change reviews
 	for {
 		g.waitAndPickClient()
 		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
 		}
 		opt.Page = resp.NextPage
 	}
+	// Get requested reviews
+	for {
+		g.waitAndPickClient()
+		reviewers, resp, err := g.getClient().PullRequests.ListReviewers(g.ctx, g.repoOwner, g.repoName, int(reviewable.GetForeignIndex()), opt)
+		if err != nil {
+			return nil, fmt.Errorf("error while listing repos: %v", err)
+		}
+		g.setRate(&resp.Rate)
+		for _, user := range reviewers.Users {
+			r := &base.Review{
+				ReviewerID:   user.GetID(),
+				ReviewerName: user.GetLogin(),
+				State:        base.ReviewStateRequestReview,
+				IssueIndex:   reviewable.GetLocalIndex(),
+			}
+			allReviews = append(allReviews, r)
+		}
+		// TODO: Handle Team requests
+		if resp.NextPage == 0 {
+			break
+		}
+		opt.Page = resp.NextPage
+	}
 	return allReviews, nil
 }