From 9db590f2ee198fa260c16387a8baa73f107db533 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 5 Mar 2021 23:17:32 +0800 Subject: [PATCH] Fix bug when combine label comments (#14894) * Fix bug when combine label comments * Added some code comments * More comments --- routers/repo/issue.go | 32 +++++++++------ routers/repo/issue_test.go | 82 +++++++++++++++++++++++++++++++++++--- 2 files changed, 97 insertions(+), 17 deletions(-) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 99df9db18..9f04c5481 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -2481,12 +2481,11 @@ func attachmentsHTML(ctx *context.Context, attachments []*models.Attachment, con return attachHTML } +// combineLabelComments combine the nearby label comments as one. func combineLabelComments(issue *models.Issue) { + var prev, cur *models.Comment for i := 0; i < len(issue.Comments); i++ { - var ( - prev *models.Comment - cur = issue.Comments[i] - ) + cur = issue.Comments[i] if i > 0 { prev = issue.Comments[i-1] } @@ -2503,16 +2502,25 @@ func combineLabelComments(issue *models.Issue) { continue } - if cur.Label != nil { - if cur.Content != "1" { - prev.RemovedLabels = append(prev.RemovedLabels, cur.Label) - } else { - prev.AddedLabels = append(prev.AddedLabels, cur.Label) + if cur.Label != nil { // now cur MUST be label comment + if prev.Type == models.CommentTypeLabel { // we can combine them only prev is a label comment + if cur.Content != "1" { + prev.RemovedLabels = append(prev.RemovedLabels, cur.Label) + } else { + prev.AddedLabels = append(prev.AddedLabels, cur.Label) + } + prev.CreatedUnix = cur.CreatedUnix + // remove the current comment since it has been combined to prev comment + issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...) + i-- + } else { // if prev is not a label comment, start a new group + if cur.Content != "1" { + cur.RemovedLabels = append(cur.RemovedLabels, cur.Label) + } else { + cur.AddedLabels = append(cur.AddedLabels, cur.Label) + } } } - prev.CreatedUnix = cur.CreatedUnix - issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...) - i-- } } diff --git a/routers/repo/issue_test.go b/routers/repo/issue_test.go index d6efef425..7fb837fa1 100644 --- a/routers/repo/issue_test.go +++ b/routers/repo/issue_test.go @@ -13,10 +13,12 @@ import ( func TestCombineLabelComments(t *testing.T) { var kases = []struct { + name string beforeCombined []*models.Comment afterCombined []*models.Comment }{ { + name: "kase 1", beforeCombined: []*models.Comment{ { Type: models.CommentTypeLabel, @@ -72,6 +74,7 @@ func TestCombineLabelComments(t *testing.T) { }, }, { + name: "kase 2", beforeCombined: []*models.Comment{ { Type: models.CommentTypeLabel, @@ -136,6 +139,7 @@ func TestCombineLabelComments(t *testing.T) { }, }, { + name: "kase 3", beforeCombined: []*models.Comment{ { Type: models.CommentTypeLabel, @@ -200,6 +204,7 @@ func TestCombineLabelComments(t *testing.T) { }, }, { + name: "kase 4", beforeCombined: []*models.Comment{ { Type: models.CommentTypeLabel, @@ -240,13 +245,80 @@ func TestCombineLabelComments(t *testing.T) { }, }, }, + { + name: "kase 5", + beforeCombined: []*models.Comment{ + { + Type: models.CommentTypeLabel, + PosterID: 1, + Content: "1", + Label: &models.Label{ + Name: "kind/bug", + }, + CreatedUnix: 0, + }, + { + Type: models.CommentTypeComment, + PosterID: 2, + Content: "testtest", + CreatedUnix: 0, + }, + { + Type: models.CommentTypeLabel, + PosterID: 1, + Content: "", + Label: &models.Label{ + Name: "kind/bug", + }, + CreatedUnix: 0, + }, + }, + afterCombined: []*models.Comment{ + { + Type: models.CommentTypeLabel, + PosterID: 1, + Content: "1", + Label: &models.Label{ + Name: "kind/bug", + }, + AddedLabels: []*models.Label{ + { + Name: "kind/bug", + }, + }, + CreatedUnix: 0, + }, + { + Type: models.CommentTypeComment, + PosterID: 2, + Content: "testtest", + CreatedUnix: 0, + }, + { + Type: models.CommentTypeLabel, + PosterID: 1, + Content: "", + RemovedLabels: []*models.Label{ + { + Name: "kind/bug", + }, + }, + Label: &models.Label{ + Name: "kind/bug", + }, + CreatedUnix: 0, + }, + }, + }, } for _, kase := range kases { - var issue = models.Issue{ - Comments: kase.beforeCombined, - } - combineLabelComments(&issue) - assert.EqualValues(t, kase.afterCombined, issue.Comments) + t.Run(kase.name, func(t *testing.T) { + var issue = models.Issue{ + Comments: kase.beforeCombined, + } + combineLabelComments(&issue) + assert.EqualValues(t, kase.afterCombined, issue.Comments) + }) } }