Skip to content

Commit

Permalink
Properly migrate target branch change GitLab comment (#29340)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
invliD authored Feb 24, 2024
1 parent b79c304 commit 6e59665
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 8 deletions.
10 changes: 8 additions & 2 deletions services/migrations/gitea_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Expand Down
10 changes: 9 additions & 1 deletion services/migrations/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"net/url"
"path"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand Down
19 changes: 18 additions & 1 deletion services/migrations/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
19 changes: 15 additions & 4 deletions templates/repo/issue/view_content/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,7 @@
{{else if eq .Type 22}}
<div class="timeline-item-group" id="{{.HashTag}}">
<div class="timeline-item event">
{{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 */}}
Expand Down Expand Up @@ -495,9 +494,21 @@
{{else if eq .Type 25}}
<div class="timeline-item event">
<span class="badge">{{svg "octicon-git-branch"}}</span>
{{template "shared/user/avatarlink" dict "user" .Poster}}
{{if not .OriginalAuthor}}
{{template "shared/user/avatarlink" dict "user" .Poster}}
{{end}}
<span class="text grey muted-links">
<a{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>{{.Poster.Name}}</a>
{{if .OriginalAuthor}}
<span class="text black">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
{{if $.Repository.OriginalURL}}
<span class="migrate">({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}})</span>
{{end}}
{{else}}
{{template "shared/user/authorlink" .Poster}}
{{end}}
{{ctx.Locale.Tr "repo.pulls.change_target_branch_at" (.OldRef|Escape) (.NewRef|Escape) $createdStr}}
</span>
</div>
Expand Down

0 comments on commit 6e59665

Please sign in to comment.