From 6e5966597c2d498d1a8540dad965461d44ff8e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Br=C3=BCckner?= Date: Sat, 24 Feb 2024 07:49:16 +0000 Subject: [PATCH] Properly migrate target branch change GitLab comment (#29340) 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 a new specific kind of event: Changing the target branch of a PR. When detected, it is downloaded using Gitea's type for this event, 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 this type. ref: https://gitlab.com/gitlab-org/gitlab/-/blob/11bd6dc826e0bea2832324a1d7356949a9398884/app/services/system_notes/merge_requests_service.rb#L102 --- services/migrations/gitea_uploader.go | 10 ++++++++-- services/migrations/gitlab.go | 10 +++++++++- services/migrations/gitlab_test.go | 19 ++++++++++++++++++- .../repo/issue/view_content/comments.tmpl | 19 +++++++++++++++---- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 468be6c9d..8bcf48394 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -492,10 +492,16 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error { } case issues_model.CommentTypeChangeTitle: if comment.Meta["OldTitle"] != nil { - cm.OldTitle = fmt.Sprintf("%s", comment.Meta["OldTitle"]) + cm.OldTitle = fmt.Sprint(comment.Meta["OldTitle"]) } if comment.Meta["NewTitle"] != nil { - cm.NewTitle = fmt.Sprintf("%s", comment.Meta["NewTitle"]) + cm.NewTitle = fmt.Sprint(comment.Meta["NewTitle"]) + } + case issues_model.CommentTypeChangeTargetBranch: + if comment.Meta["OldRef"] != nil && comment.Meta["NewRef"] != nil { + cm.OldRef = fmt.Sprint(comment.Meta["OldRef"]) + cm.NewRef = fmt.Sprint(comment.Meta["NewRef"]) + cm.Content = "" } case issues_model.CommentTypePRScheduledToAutoMerge, issues_model.CommentTypePRUnScheduledToAutoMerge: cm.Content = "" diff --git a/services/migrations/gitlab.go b/services/migrations/gitlab.go index d08eaf0f8..5e49ae6d5 100644 --- a/services/migrations/gitlab.go +++ b/services/migrations/gitlab.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "path" + "regexp" "strings" "time" @@ -519,6 +520,8 @@ func (g *GitlabDownloader) GetComments(commentable base.Commentable) ([]*base.Co return allComments, true, nil } +var targetBranchChangeRegexp = regexp.MustCompile("^changed target branch from `(.*?)` to `(.*?)`$") + func (g *GitlabDownloader) convertNoteToComment(localIndex int64, note *gitlab.Note) *base.Comment { comment := &base.Comment{ IssueIndex: localIndex, @@ -528,11 +531,16 @@ func (g *GitlabDownloader) convertNoteToComment(localIndex int64, note *gitlab.N PosterEmail: note.Author.Email, Content: note.Body, Created: *note.CreatedAt, + Meta: map[string]any{}, } // Try to find the underlying event of system notes. if note.System { - if strings.HasPrefix(note.Body, "enabled an automatic merge") { + if match := targetBranchChangeRegexp.FindStringSubmatch(note.Body); match != nil { + comment.CommentType = issues_model.CommentTypeChangeTargetBranch.String() + comment.Meta["OldRef"] = match[1] + comment.Meta["NewRef"] = match[2] + } else 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() diff --git a/services/migrations/gitlab_test.go b/services/migrations/gitlab_test.go index 2b87a1dfe..0b9eeaed5 100644 --- a/services/migrations/gitlab_test.go +++ b/services/migrations/gitlab_test.go @@ -545,7 +545,8 @@ func TestNoteToComment(t *testing.T) { 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), + makeTestNote(3, "changed target branch from `master` to `main`", true), + makeTestNote(4, "canceled the automatic merge", true), } comments := []base.Comment{{ IssueIndex: 17, @@ -556,6 +557,7 @@ func TestNoteToComment(t *testing.T) { CommentType: "", Content: "This is a regular comment", Created: now, + Meta: map[string]any{}, }, { IssueIndex: 17, Index: 2, @@ -565,15 +567,30 @@ func TestNoteToComment(t *testing.T) { CommentType: "pull_scheduled_merge", Content: "enabled an automatic merge for abcd1234", Created: now, + Meta: map[string]any{}, }, { IssueIndex: 17, Index: 3, PosterID: 72, PosterName: "test", PosterEmail: "test@example.com", + CommentType: "change_target_branch", + Content: "changed target branch from `master` to `main`", + Created: now, + Meta: map[string]any{ + "OldRef": "master", + "NewRef": "main", + }, + }, { + IssueIndex: 17, + Index: 4, + PosterID: 72, + PosterName: "test", + PosterEmail: "test@example.com", CommentType: "pull_cancel_scheduled_merge", Content: "canceled the automatic merge", Created: now, + Meta: map[string]any{}, }} for i, note := range notes { diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 597f02547..7bd7e8c35 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -365,8 +365,7 @@ {{else if eq .Type 22}}
- {{if .OriginalAuthor}} - {{else}} + {{if not .OriginalAuthor}} {{/* Some timeline avatars need a offset to correctly align with their speech bubble. The condition depends on review type and for positive reviews whether there is a comment element or not */}} @@ -495,9 +494,21 @@ {{else if eq .Type 25}}
{{svg "octicon-git-branch"}} - {{template "shared/user/avatarlink" dict "user" .Poster}} + {{if not .OriginalAuthor}} + {{template "shared/user/avatarlink" dict "user" .Poster}} + {{end}} - {{.Poster.Name}} + {{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}} {{ctx.Locale.Tr "repo.pulls.change_target_branch_at" (.OldRef|Escape) (.NewRef|Escape) $createdStr}}