Skip to content

Commit a70c00b

Browse files
invliDsilverwindwxiaoguang
authored
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 <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent e9b1373 commit a70c00b

File tree

5 files changed

+111
-30
lines changed

5 files changed

+111
-30
lines changed

Diff for: ‎services/migrations/gitea_uploader.go

+2
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,8 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error {
487487
if comment.Meta["NewTitle"] != nil {
488488
cm.NewTitle = fmt.Sprintf("%s", comment.Meta["NewTitle"])
489489
}
490+
case issues_model.CommentTypePRScheduledToAutoMerge, issues_model.CommentTypePRUnScheduledToAutoMerge:
491+
cm.Content = ""
490492
default:
491493
}
492494

Diff for: ‎services/migrations/gitlab.go

+26-24
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"strings"
1515
"time"
1616

17+
issues_model "code.gitea.io/gitea/models/issues"
1718
"code.gitea.io/gitea/modules/container"
1819
"code.gitea.io/gitea/modules/log"
1920
base "code.gitea.io/gitea/modules/migration"
@@ -506,30 +507,8 @@ func (g *GitlabDownloader) GetComments(commentable base.Commentable) ([]*base.Co
506507
return nil, false, fmt.Errorf("error while listing comments: %v %w", g.repoID, err)
507508
}
508509
for _, comment := range comments {
509-
// Flatten comment threads
510-
if !comment.IndividualNote {
511-
for _, note := range comment.Notes {
512-
allComments = append(allComments, &base.Comment{
513-
IssueIndex: commentable.GetLocalIndex(),
514-
Index: int64(note.ID),
515-
PosterID: int64(note.Author.ID),
516-
PosterName: note.Author.Username,
517-
PosterEmail: note.Author.Email,
518-
Content: note.Body,
519-
Created: *note.CreatedAt,
520-
})
521-
}
522-
} else {
523-
c := comment.Notes[0]
524-
allComments = append(allComments, &base.Comment{
525-
IssueIndex: commentable.GetLocalIndex(),
526-
Index: int64(c.ID),
527-
PosterID: int64(c.Author.ID),
528-
PosterName: c.Author.Username,
529-
PosterEmail: c.Author.Email,
530-
Content: c.Body,
531-
Created: *c.CreatedAt,
532-
})
510+
for _, note := range comment.Notes {
511+
allComments = append(allComments, g.convertNoteToComment(commentable.GetLocalIndex(), note))
533512
}
534513
}
535514
if resp.NextPage == 0 {
@@ -540,6 +519,29 @@ func (g *GitlabDownloader) GetComments(commentable base.Commentable) ([]*base.Co
540519
return allComments, true, nil
541520
}
542521

522+
func (g *GitlabDownloader) convertNoteToComment(localIndex int64, note *gitlab.Note) *base.Comment {
523+
comment := &base.Comment{
524+
IssueIndex: localIndex,
525+
Index: int64(note.ID),
526+
PosterID: int64(note.Author.ID),
527+
PosterName: note.Author.Username,
528+
PosterEmail: note.Author.Email,
529+
Content: note.Body,
530+
Created: *note.CreatedAt,
531+
}
532+
533+
// Try to find the underlying event of system notes.
534+
if note.System {
535+
if strings.HasPrefix(note.Body, "enabled an automatic merge") {
536+
comment.CommentType = issues_model.CommentTypePRScheduledToAutoMerge.String()
537+
} else if note.Body == "canceled the automatic merge" {
538+
comment.CommentType = issues_model.CommentTypePRUnScheduledToAutoMerge.String()
539+
}
540+
}
541+
542+
return comment
543+
}
544+
543545
// GetPullRequests returns pull requests according page and perPage
544546
func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullRequest, bool, error) {
545547
if perPage > g.maxPerPage {

Diff for: ‎services/migrations/gitlab_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,71 @@ func TestAwardsToReactions(t *testing.T) {
517517
}, reactions)
518518
}
519519

520+
func TestNoteToComment(t *testing.T) {
521+
downloader := &GitlabDownloader{}
522+
523+
now := time.Now()
524+
makeTestNote := func(id int, body string, system bool) gitlab.Note {
525+
return gitlab.Note{
526+
ID: id,
527+
Author: struct {
528+
ID int `json:"id"`
529+
Username string `json:"username"`
530+
Email string `json:"email"`
531+
Name string `json:"name"`
532+
State string `json:"state"`
533+
AvatarURL string `json:"avatar_url"`
534+
WebURL string `json:"web_url"`
535+
}{
536+
ID: 72,
537+
Email: "[email protected]",
538+
Username: "test",
539+
},
540+
Body: body,
541+
CreatedAt: &now,
542+
System: system,
543+
}
544+
}
545+
notes := []gitlab.Note{
546+
makeTestNote(1, "This is a regular comment", false),
547+
makeTestNote(2, "enabled an automatic merge for abcd1234", true),
548+
makeTestNote(3, "canceled the automatic merge", true),
549+
}
550+
comments := []base.Comment{{
551+
IssueIndex: 17,
552+
Index: 1,
553+
PosterID: 72,
554+
PosterName: "test",
555+
PosterEmail: "[email protected]",
556+
CommentType: "",
557+
Content: "This is a regular comment",
558+
Created: now,
559+
}, {
560+
IssueIndex: 17,
561+
Index: 2,
562+
PosterID: 72,
563+
PosterName: "test",
564+
PosterEmail: "[email protected]",
565+
CommentType: "pull_scheduled_merge",
566+
Content: "enabled an automatic merge for abcd1234",
567+
Created: now,
568+
}, {
569+
IssueIndex: 17,
570+
Index: 3,
571+
PosterID: 72,
572+
PosterName: "test",
573+
PosterEmail: "[email protected]",
574+
CommentType: "pull_cancel_scheduled_merge",
575+
Content: "canceled the automatic merge",
576+
Created: now,
577+
}}
578+
579+
for i, note := range notes {
580+
actualComment := *downloader.convertNoteToComment(17, &note)
581+
assert.EqualValues(t, actualComment, comments[i])
582+
}
583+
}
584+
520585
func TestGitlabIIDResolver(t *testing.T) {
521586
r := gitlabIIDResolver{}
522587
r.recordIssueIID(1)

Diff for: ‎templates/repo/issue/view_content/comments.tmpl

+14-3
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,9 @@
381381
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
382382
{{.OriginalAuthor}}
383383
</span>
384-
<span class="text grey muted-links"> {{if $.Repository.OriginalURL}}</span>
385-
<span class="text migrate">({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}}</span>
384+
{{if $.Repository.OriginalURL}}
385+
<span class="migrate">({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}})</span>
386+
{{end}}
386387
{{else}}
387388
{{template "shared/user/authorlink" .Poster}}
388389
{{end}}
@@ -663,7 +664,17 @@
663664
<div class="timeline-item event" id="{{.HashTag}}">
664665
<span class="badge">{{svg "octicon-git-merge" 16}}</span>
665666
<span class="text grey muted-links">
666-
{{template "shared/user/authorlink" .Poster}}
667+
{{if .OriginalAuthor}}
668+
<span class="text black">
669+
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
670+
{{.OriginalAuthor}}
671+
</span>
672+
{{if $.Repository.OriginalURL}}
673+
<span class="migrate">({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}})</span>
674+
{{end}}
675+
{{else}}
676+
{{template "shared/user/authorlink" .Poster}}
677+
{{end}}
667678
{{if eq .Type 34}}{{ctx.Locale.Tr "repo.pulls.auto_merge_newly_scheduled_comment" $createdStr | Safe}}
668679
{{else}}{{ctx.Locale.Tr "repo.pulls.auto_merge_canceled_schedule_comment" $createdStr | Safe}}{{end}}
669680
</span>

Diff for: ‎templates/repo/issue/view_content/conversation.tmpl

+4-3
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,13 @@
6363
{{end}}
6464
<span class="text grey muted-links">
6565
{{if .OriginalAuthor}}
66-
<span class="text black gt-font-semibold">
66+
<span class="text black">
6767
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
6868
{{.OriginalAuthor}}
6969
</span>
70-
<span class="text grey muted-links"> {{if $.Repository.OriginalURL}}</span>
71-
<span class="text migrate">({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}}</span>
70+
{{if $.Repository.OriginalURL}}
71+
<span class="migrate">({{ctx.Locale.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname}})</span>
72+
{{end}}
7273
{{else}}
7374
{{template "shared/user/authorlink" .Poster}}
7475
{{end}}

0 commit comments

Comments
 (0)