Skip to content

Commit

Permalink
Properly migrate automatic merge GitLab comments (#27873)
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 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 <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
  • Loading branch information
@invliD @silverwind @wxiaoguang
3 people authored Feb 22, 2024
1 parent e9b1373 commit a70c00b
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 30 deletions.
2 changes: 2 additions & 0 deletions services/migrations/gitea_uploader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
}

Expand Down
50 changes: 26 additions & 24 deletions services/migrations/gitlab.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
65 changes: 65 additions & 0 deletions services/migrations/gitlab_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,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, &note)
assert.EqualValues(t, actualComment, comments[i])
}
}

func TestGitlabIIDResolver(t *testing.T) {
r := gitlabIIDResolver{}
r.recordIssueIID(1)
Expand Down
17 changes: 14 additions & 3 deletions templates/repo/issue/view_content/comments.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,9 @@
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
<span class="text grey muted-links"> {{if $.Repository.OriginalURL}}</span>
<span class="text migrate">({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}}</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}}
Expand Down Expand Up @@ -663,7 +664,17 @@
<div class="timeline-item event" id="{{.HashTag}}">
<span class="badge">{{svg "octicon-git-merge" 16}}</span>
<span class="text grey muted-links">
{{template "shared/user/authorlink" .Poster}}
{{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}}
{{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}}
</span>
Expand Down
7 changes: 4 additions & 3 deletions templates/repo/issue/view_content/conversation.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,13 @@
{{end}}
<span class="text grey muted-links">
{{if .OriginalAuthor}}
<span class="text black gt-font-semibold">
<span class="text black">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
<span class="text grey muted-links"> {{if $.Repository.OriginalURL}}</span>
<span class="text migrate">({{ctx.Locale.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}}){{end}}</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}}
Expand Down

0 comments on commit a70c00b

Please sign in to comment.