Skip to content

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

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

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
@@ -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=
1 change: 1 addition & 0 deletions modules/migration/review.go
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ const (
ReviewStateApproved = "APPROVED"
ReviewStateChangesRequested = "CHANGES_REQUESTED"
ReviewStateCommented = "COMMENTED"
ReviewStateRequestReview = "REQUEST_REVIEW"
)

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