Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly migrate automatic merge GitLab comments #27873

Merged
merged 22 commits into from
Feb 22, 2024
Merged

Properly migrate automatic merge GitLab comments #27873

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
bc87e20
Properly migrate automatic merge GitLab comments
invliD Nov 2, 2023
202a7f5
Bring back text class
invliD Nov 3, 2023
53a5385
Add unit test
invliD Nov 7, 2023
a078154
Merge branch 'main' into gitlab-schedule-merge
invliD Dec 2, 2023
aa2804a
Merge branch 'main' into gitlab-schedule-merge
invliD Dec 9, 2023
d0dc369
Merge branch 'main' into gitlab-schedule-merge
invliD Jan 7, 2024
fb7cd14
Merge branch 'main' into gitlab-schedule-merge
invliD Jan 13, 2024
884bbf1
Merge branch 'main' into gitlab-schedule-merge
invliD Jan 20, 2024
6c8675c
Merge branch 'main' into gitlab-schedule-merge
invliD Feb 10, 2024
80bc42a
Merge branch 'main' into gitlab-schedule-merge
invliD Feb 18, 2024
5678c1c
Update templates/repo/issue/view_content/comments.tmpl
silverwind Feb 21, 2024
b5c16b9
Update templates/repo/issue/view_content/comments.tmpl
silverwind Feb 21, 2024
9302751
Update templates/repo/issue/view_content/conversation.tmpl
silverwind Feb 21, 2024
ad93eb0
Merge branch 'main' into gitlab-schedule-merge
GiteaBot Feb 21, 2024
e46a86b
Update templates/repo/issue/view_content/comments.tmpl
silverwind Feb 21, 2024
1061071
Update templates/repo/issue/view_content/comments.tmpl
silverwind Feb 21, 2024
d2f18dd
Update templates/repo/issue/view_content/conversation.tmpl
silverwind Feb 21, 2024
29835ae
Merge branch 'main' into gitlab-schedule-merge
GiteaBot Feb 22, 2024
fb38b8d
Apply suggestions from code review
invliD Feb 22, 2024
60edb71
Merge branch 'main' into gitlab-schedule-merge
invliD Feb 22, 2024
e02245d
Update templates/repo/issue/view_content/comments.tmpl
wxiaoguang Feb 22, 2024
c54487e
Merge branch 'main' into gitlab-schedule-merge
GiteaBot Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions services/migrations/gitea_uploader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,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
Open in desktop
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 @@ -488,30 +489,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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line now will be ignored?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both branches here were exactly the same, except only the first note was converted if IndividualNote is true. That isn't really necessary, because IndividualNote means that this is not a discussion (=thread) but just a single comment that cannot be responded to. There will always be exactly one note in such a "discussion", so there is no reason to explicitly ignore any further notes.

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 @@ -522,6 +501,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,3 +516,68 @@ 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])
}
}
24 changes: 18 additions & 6 deletions templates/repo/issue/view_content/comments.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,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|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}})</span>
silverwind marked this conversation as resolved.
Show resolved Hide resolved
invliD marked this conversation as resolved.
Show resolved Hide resolved
{{end}}
{{else}}
{{template "shared/user/authorlink" .Poster}}
{{end}}
Expand Down Expand Up @@ -526,12 +527,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|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}})</span>
{{end}}
{{else}}
{{template "shared/user/authorlink" .Poster}}
{{end}}
Expand Down Expand Up @@ -796,7 +798,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|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe}})</span>
silverwind marked this conversation as resolved.
Show resolved Hide resolved
invliD marked this conversation as resolved.
Show resolved Hide resolved
{{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