Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent NPE whilst migrating if there is a team request review #19855

Merged
Merged
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
1 change: 1 addition & 0 deletions modules/migration/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
ReviewStateApproved = "APPROVED"
ReviewStateChangesRequested = "CHANGES_REQUESTED"
ReviewStateCommented = "COMMENTED"
ReviewStateRequestReview = "REQUEST_REVIEW"
zeripath marked this conversation as resolved.
Show resolved Hide resolved
)

// Review is a standard review information
Expand Down
2 changes: 1 addition & 1 deletion services/migrations/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions services/migrations/gitea_downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions services/migrations/gitea_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
26 changes: 25 additions & 1 deletion services/migrations/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -817,5 +818,28 @@ func (g *GithubDownloaderV3) GetReviews(reviewable base.Reviewable) ([]*base.Rev
}
opt.Page = resp.NextPage
}
// Get requested reviews
for {
zeripath marked this conversation as resolved.
Show resolved Hide resolved
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
}