From 5bfb9bc2b6a2e10d8f0fcdd2cc1e93de39a58555 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 20 Apr 2020 14:30:46 +0200 Subject: [PATCH] When migrating from Gitlab map Approvals to approving Reviews (#11147) When migrating from Gitlab map Gitlab Approvals to approving Reviews Co-Authored-By: zeripath --- modules/migrations/base/pullrequest.go | 1 + modules/migrations/gitlab.go | 34 +++++++++++++++++++++++--- modules/migrations/gitlab_test.go | 24 +++++++++++++++--- modules/migrations/migrate.go | 16 ++++++++++-- 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/modules/migrations/base/pullrequest.go b/modules/migrations/base/pullrequest.go index 3a1e0f25b..964512e13 100644 --- a/modules/migrations/base/pullrequest.go +++ b/modules/migrations/base/pullrequest.go @@ -13,6 +13,7 @@ import ( // PullRequest defines a standard pull request information type PullRequest struct { Number int64 + OriginalNumber int64 Title string PosterName string PosterID int64 diff --git a/modules/migrations/gitlab.go b/modules/migrations/gitlab.go index 02891d504..8e1c7d0a8 100644 --- a/modules/migrations/gitlab.go +++ b/modules/migrations/gitlab.go @@ -32,7 +32,7 @@ func init() { type GitlabDownloaderFactory struct { } -// Match returns ture if the migration remote URL matched this downloader factory +// Match returns true if the migration remote URL matched this downloader factory func (f *GitlabDownloaderFactory) Match(opts base.MigrateOptions) (bool, error) { var matched bool @@ -492,11 +492,12 @@ func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullReque } // Add the PR ID to the Issue Count because PR and Issues share ID space in Gitea - newPRnumber := g.issueCount + int64(pr.IID) + newPRNumber := g.issueCount + int64(pr.IID) allPRs = append(allPRs, &base.PullRequest{ Title: pr.Title, - Number: int64(newPRnumber), + Number: newPRNumber, + OriginalNumber: int64(pr.IID), PosterName: pr.Author.Username, PosterID: int64(pr.Author.ID), Content: pr.Description, @@ -532,5 +533,30 @@ func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullReque // GetReviews returns pull requests review func (g *GitlabDownloader) GetReviews(pullRequestNumber int64) ([]*base.Review, error) { - return nil, nil + state, _, err := g.client.MergeRequestApprovals.GetApprovalState(g.repoID, int(pullRequestNumber)) + if err != nil { + return nil, err + } + + // GitLab's Approvals are equivalent to Gitea's approve reviews + approvers := make(map[int]string) + for i := range state.Rules { + for u := range state.Rules[i].ApprovedBy { + approvers[state.Rules[i].ApprovedBy[u].ID] = state.Rules[i].ApprovedBy[u].Username + } + } + + var reviews = make([]*base.Review, 0, len(approvers)) + for id, name := range approvers { + reviews = append(reviews, &base.Review{ + ReviewerID: int64(id), + ReviewerName: name, + // GitLab API doesn't return a creation date + CreatedAt: time.Now(), + // All we get are approvals + State: base.ReviewStateApproved, + }) + } + + return reviews, nil } diff --git a/modules/migrations/gitlab_test.go b/modules/migrations/gitlab_test.go index 8525d7e9c..003da5bbd 100644 --- a/modules/migrations/gitlab_test.go +++ b/modules/migrations/gitlab_test.go @@ -110,7 +110,6 @@ func TestGitlabDownloadRepo(t *testing.T) { }, }, releases[len(releases)-1:]) - // downloader.GetIssues() issues, isEnd, err := downloader.GetIssues(1, 2) assert.NoError(t, err) assert.EqualValues(t, 2, len(issues)) @@ -162,7 +161,6 @@ func TestGitlabDownloadRepo(t *testing.T) { }, }, issues) - // downloader.GetComments() comments, err := downloader.GetComments(2) assert.NoError(t, err) assert.EqualValues(t, 4, len(comments)) @@ -202,10 +200,9 @@ func TestGitlabDownloadRepo(t *testing.T) { }, }, comments[:4]) - // downloader.GetPullRequests() prs, err := downloader.GetPullRequests(1, 1) assert.NoError(t, err) - assert.EqualValues(t, 1, len(prs)) + assert.Len(t, prs, 1) assert.EqualValues(t, []*base.PullRequest{ { @@ -243,4 +240,23 @@ func TestGitlabDownloadRepo(t *testing.T) { MergeCommitSHA: "", }, }, prs) + + rvs, err := downloader.GetReviews(1) + assert.NoError(t, err) + if assert.Len(t, prs, 2) { + assert.EqualValues(t, 527793, rvs[0].ReviewerID) + assert.EqualValues(t, "axifive", rvs[0].ReviewerName) + assert.EqualValues(t, "APPROVED", rvs[0].State) + assert.EqualValues(t, 4102996, rvs[1].ReviewerID) + assert.EqualValues(t, "zeripath", rvs[1].ReviewerName) + assert.EqualValues(t, "APPROVED", rvs[1].State) + } + rvs, err = downloader.GetReviews(2) + assert.NoError(t, err) + if assert.Len(t, prs, 1) { + assert.EqualValues(t, 4575606, rvs[0].ReviewerID) + assert.EqualValues(t, "real6543", rvs[0].ReviewerName) + assert.EqualValues(t, "APPROVED", rvs[0].State) + } + } diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index 3b3e318f6..c970ba692 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -86,7 +86,7 @@ func MigrateRepository(ctx context.Context, doer *models.User, ownerName string, return uploader.repo, nil } -// migrateRepository will download informations and upload to Uploader, this is a simple +// migrateRepository will download information and then upload it to Uploader, this is a simple // process for small repository. For a big repository, save all the data to disk // before upload is better func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts base.MigrateOptions) error { @@ -277,7 +277,19 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts // migrate reviews var allReviews = make([]*base.Review, 0, reviewBatchSize) for _, pr := range prs { - reviews, err := downloader.GetReviews(pr.Number) + number := pr.Number + + // on gitlab migrations pull number change + if pr.OriginalNumber > 0 { + number = pr.OriginalNumber + } + + reviews, err := downloader.GetReviews(number) + if pr.OriginalNumber > 0 { + for i := range reviews { + reviews[i].IssueIndex = pr.Number + } + } if err != nil { return err }