From 542badbb76408c17ce6692e99fff680bee69face Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Br=C3=BCckner?= Date: Thu, 22 Feb 2024 07:29:03 +0000 Subject: [PATCH] Properly migrate automatic merge GitLab comments (#27873) GitLab generates "system notes" whenever an event happens within the platform. Unlike Gitea, those events are stored and retrieved as text comments with no semantic details. The only way to tell whether a comment was generated in this manner is the `system` flag on the note type. This PR adds detection for two specific kinds of events: Scheduling and un-scheduling of automatic merges on a PR. When detected, they are downloaded using Gitea's type for these events, and eventually uploaded into Gitea in the expected format, i.e. with no text content in the comment. This PR also updates the template used to render comments to add support for migrated comments of these two types. ref: https://gitlab.com/gitlab-org/gitlab/-/blob/11bd6dc826e0bea2832324a1d7356949a9398884/app/services/system_notes/merge_requests_service.rb#L6-L17 --------- Co-authored-by: silverwind Co-authored-by: wxiaoguang (cherry picked from commit a70c00b80bcb5de8479e407f1b8f08dcf756019d) --- services/migrations/gitea_uploader.go | 2 + services/migrations/gitlab.go | 50 +++++++------- services/migrations/gitlab_test.go | 65 +++++++++++++++++++ .../repo/issue/view_content/comments.tmpl | 17 ++++- .../repo/issue/view_content/conversation.tmpl | 7 +- 5 files changed, 111 insertions(+), 30 deletions(-) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 0f0ca7e0f5..ebb1313c2b 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -487,6 +487,8 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error { if comment.Meta["NewTitle"] != nil { cm.NewTitle = fmt.Sprintf("%s", comment.Meta["NewTitle"]) } + case issues_model.CommentTypePRScheduledToAutoMerge, issues_model.CommentTypePRUnScheduledToAutoMerge: + cm.Content = "" default: } diff --git a/services/migrations/gitlab.go b/services/migrations/gitlab.go index 3db10465fc..d08eaf0f84 100644 --- a/services/migrations/gitlab.go +++ b/services/migrations/gitlab.go @@ -14,6 +14,7 @@ import ( "strings" "time" + issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" base "code.gitea.io/gitea/modules/migration" @@ -506,30 +507,8 @@ func (g *GitlabDownloader) GetComments(commentable base.Commentable) ([]*base.Co return nil, false, fmt.Errorf("error while listing comments: %v %w", g.repoID, err) } for _, comment := range comments { - // Flatten comment threads - if !comment.IndividualNote { - for _, note := range comment.Notes { - allComments = append(allComments, &base.Comment{ - IssueIndex: commentable.GetLocalIndex(), - Index: int64(note.ID), - PosterID: int64(note.Author.ID), - PosterName: note.Author.Username, - PosterEmail: note.Author.Email, - Content: note.Body, - Created: *note.CreatedAt, - }) - } - } else { - c := comment.Notes[0] - allComments = append(allComments, &base.Comment{ - IssueIndex: commentable.GetLocalIndex(), - Index: int64(c.ID), - PosterID: int64(c.Author.ID), - PosterName: c.Author.Username, - PosterEmail: c.Author.Email, - Content: c.Body, - Created: *c.CreatedAt, - }) + for _, note := range comment.Notes { + allComments = append(allComments, g.convertNoteToComment(commentable.GetLocalIndex(), note)) } } if resp.NextPage == 0 { @@ -540,6 +519,29 @@ func (g *GitlabDownloader) GetComments(commentable base.Commentable) ([]*base.Co return allComments, true, nil } +func (g *GitlabDownloader) convertNoteToComment(localIndex int64, note *gitlab.Note) *base.Comment { + comment := &base.Comment{ + IssueIndex: localIndex, + Index: int64(note.ID), + PosterID: int64(note.Author.ID), + PosterName: note.Author.Username, + PosterEmail: note.Author.Email, + Content: note.Body, + Created: *note.CreatedAt, + } + + // Try to find the underlying event of system notes. + if note.System { + if strings.HasPrefix(note.Body, "enabled an automatic merge") { + comment.CommentType = issues_model.CommentTypePRScheduledToAutoMerge.String() + } else if note.Body == "canceled the automatic merge" { + comment.CommentType = issues_model.CommentTypePRUnScheduledToAutoMerge.String() + } + } + + return comment +} + // GetPullRequests returns pull requests according page and perPage func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullRequest, bool, error) { if perPage > g.maxPerPage { diff --git a/services/migrations/gitlab_test.go b/services/migrations/gitlab_test.go index 25324a087f..d941cebd2c 100644 --- a/services/migrations/gitlab_test.go +++ b/services/migrations/gitlab_test.go @@ -558,6 +558,71 @@ func TestAwardsToReactions(t *testing.T) { }, reactions) } +func TestNoteToComment(t *testing.T) { + downloader := &GitlabDownloader{} + + now := time.Now() + makeTestNote := func(id int, body string, system bool) gitlab.Note { + return gitlab.Note{ + ID: id, + Author: struct { + ID int `json:"id"` + Username string `json:"username"` + Email string `json:"email"` + Name string `json:"name"` + State string `json:"state"` + AvatarURL string `json:"avatar_url"` + WebURL string `json:"web_url"` + }{ + ID: 72, + Email: "test@example.com", + Username: "test", + }, + Body: body, + CreatedAt: &now, + System: system, + } + } + notes := []gitlab.Note{ + makeTestNote(1, "This is a regular comment", false), + makeTestNote(2, "enabled an automatic merge for abcd1234", true), + makeTestNote(3, "canceled the automatic merge", true), + } + comments := []base.Comment{{ + IssueIndex: 17, + Index: 1, + PosterID: 72, + PosterName: "test", + PosterEmail: "test@example.com", + CommentType: "", + Content: "This is a regular comment", + Created: now, + }, { + IssueIndex: 17, + Index: 2, + PosterID: 72, + PosterName: "test", + PosterEmail: "test@example.com", + CommentType: "pull_scheduled_merge", + Content: "enabled an automatic merge for abcd1234", + Created: now, + }, { + IssueIndex: 17, + Index: 3, + PosterID: 72, + PosterName: "test", + PosterEmail: "test@example.com", + CommentType: "pull_cancel_scheduled_merge", + Content: "canceled the automatic merge", + Created: now, + }} + + for i, note := range notes { + actualComment := *downloader.convertNoteToComment(17, ¬e) + assert.EqualValues(t, actualComment, comments[i]) + } +} + func TestGitlabIIDResolver(t *testing.T) { r := gitlabIIDResolver{} r.recordIssueIID(1) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index a4fd97297f..43e059d6c1 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -385,8 +385,9 @@ {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} {{.OriginalAuthor}} - {{if $.Repository.OriginalURL}} - ({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}} + {{if $.Repository.OriginalURL}} + ({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}}) + {{end}} {{else}} {{template "shared/user/authorlink" .Poster}} {{end}} @@ -667,7 +668,17 @@
{{svg "octicon-git-merge" 16}} - {{template "shared/user/authorlink" .Poster}} + {{if .OriginalAuthor}} + + {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} + {{.OriginalAuthor}} + + {{if $.Repository.OriginalURL}} + ({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}}) + {{end}} + {{else}} + {{template "shared/user/authorlink" .Poster}} + {{end}} {{if eq .Type 34}}{{ctx.Locale.Tr "repo.pulls.auto_merge_newly_scheduled_comment" $createdStr | Safe}} {{else}}{{ctx.Locale.Tr "repo.pulls.auto_merge_canceled_schedule_comment" $createdStr | Safe}}{{end}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index c9e5ee6275..fc1d9865f5 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -63,12 +63,13 @@ {{end}} {{if .OriginalAuthor}} - + {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} {{.OriginalAuthor}} - {{if $.Repository.OriginalURL}} - ({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}} + {{if $.Repository.OriginalURL}} + ({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}}) + {{end}} {{else}} {{template "shared/user/authorlink" .Poster}} {{end}}