Refactor combine label comments with tests (#13619)

Co-authored-by: Lauris BH <lauris@nix.lv>

routers/repo/issue.go

@@ -2392,67 +2392,34 @@ func attachmentsHTML(ctx *context.Context, attachments []*models.Attachment) str
 }
 
 func combineLabelComments(issue *models.Issue) {
-	for i := 0; i < len(issue.Comments); {
+	for i := 0; i < len(issue.Comments); i++ {
-		c := issue.Comments[i]
+		var (
-		var shouldMerge bool
+			prev *models.Comment
-		var removingCur bool
+			cur  = issue.Comments[i]
-		var prev *models.Comment
+		)
-
+		if i > 0 {
-		if i == 0 {
-			shouldMerge = false
-		} else {
 			prev = issue.Comments[i-1]
-			removingCur = c.Content != "1"
-
-			shouldMerge = prev.PosterID == c.PosterID && c.CreatedUnix-prev.CreatedUnix < 60 &&
-				c.Type == prev.Type
 		}
-
+		if i == 0 || cur.Type != models.CommentTypeLabel ||
-		if c.Type == models.CommentTypeLabel {
+			(prev != nil && prev.PosterID != cur.PosterID) ||
-			if !shouldMerge {
+			(prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) {
-				if removingCur {
+			if cur.Type == models.CommentTypeLabel {
-					c.RemovedLabels = make([]*models.Label, 1)
+				if cur.Content != "1" {
-					c.AddedLabels = make([]*models.Label, 0)
+					cur.RemovedLabels = append(cur.RemovedLabels, cur.Label)
-					c.RemovedLabels[0] = c.Label
 				} else {
-					c.RemovedLabels = make([]*models.Label, 0)
+					cur.AddedLabels = append(cur.AddedLabels, cur.Label)
-					c.AddedLabels = make([]*models.Label, 1)
-					c.AddedLabels[0] = c.Label
-				}
-			} else {
-				// Remove duplicated "added" and "removed" labels
-				// This way, adding and immediately removing a label won't generate a comment.
-				var appendingTo *[]*models.Label
-				var other *[]*models.Label
-
-				if removingCur {
-					appendingTo = &prev.RemovedLabels
-					other = &prev.AddedLabels
-				} else {
-					appendingTo = &prev.AddedLabels
-					other = &prev.RemovedLabels
-				}
-
-				appending := true
-
-				for i := 0; i < len(*other); i++ {
-					l := (*other)[i]
-					if l.ID == c.Label.ID {
-						*other = append((*other)[:i], (*other)[i+1:]...)
-						appending = false
-						break
 				}
 			}
-
-				if appending {
-					*appendingTo = append(*appendingTo, c.Label)
-				}
-
-				prev.CreatedUnix = c.CreatedUnix
-				issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...)
 			continue
 		}
+
+		if cur.Content != "1" {
+			prev.RemovedLabels = append(prev.RemovedLabels, cur.Label)
+		} else {
+			prev.AddedLabels = append(prev.AddedLabels, cur.Label)
 		}
-		i++
+		prev.CreatedUnix = cur.CreatedUnix
+		issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...)
+		i--
 	}
 }

routers/repo/issue_test.go

@@ -0,0 +1,252 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package repo
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCombineLabelComments(t *testing.T) {
+	var kases = []struct {
+		beforeCombined []*models.Comment
+		afterCombined  []*models.Comment
+	}{
+		{
+			beforeCombined: []*models.Comment{
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "1",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 0,
+				},
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 0,
+				},
+				{
+					Type:        models.CommentTypeComment,
+					PosterID:    1,
+					Content:     "test",
+					CreatedUnix: 0,
+				},
+			},
+			afterCombined: []*models.Comment{
+				{
+					Type:        models.CommentTypeLabel,
+					PosterID:    1,
+					Content:     "1",
+					CreatedUnix: 0,
+					AddedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+					},
+					RemovedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+					},
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+				},
+				{
+					Type:        models.CommentTypeComment,
+					PosterID:    1,
+					Content:     "test",
+					CreatedUnix: 0,
+				},
+			},
+		},
+		{
+			beforeCombined: []*models.Comment{
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "1",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 0,
+				},
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 70,
+				},
+				{
+					Type:        models.CommentTypeComment,
+					PosterID:    1,
+					Content:     "test",
+					CreatedUnix: 0,
+				},
+			},
+			afterCombined: []*models.Comment{
+				{
+					Type:        models.CommentTypeLabel,
+					PosterID:    1,
+					Content:     "1",
+					CreatedUnix: 0,
+					AddedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+					},
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+				},
+				{
+					Type:        models.CommentTypeLabel,
+					PosterID:    1,
+					Content:     "",
+					CreatedUnix: 70,
+					RemovedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+					},
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+				},
+				{
+					Type:        models.CommentTypeComment,
+					PosterID:    1,
+					Content:     "test",
+					CreatedUnix: 0,
+				},
+			},
+		},
+		{
+			beforeCombined: []*models.Comment{
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "1",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 0,
+				},
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 2,
+					Content:  "",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 0,
+				},
+				{
+					Type:        models.CommentTypeComment,
+					PosterID:    1,
+					Content:     "test",
+					CreatedUnix: 0,
+				},
+			},
+			afterCombined: []*models.Comment{
+				{
+					Type:        models.CommentTypeLabel,
+					PosterID:    1,
+					Content:     "1",
+					CreatedUnix: 0,
+					AddedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+					},
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+				},
+				{
+					Type:        models.CommentTypeLabel,
+					PosterID:    2,
+					Content:     "",
+					CreatedUnix: 0,
+					RemovedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+					},
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+				},
+				{
+					Type:        models.CommentTypeComment,
+					PosterID:    1,
+					Content:     "test",
+					CreatedUnix: 0,
+				},
+			},
+		},
+		{
+			beforeCombined: []*models.Comment{
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "1",
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+					CreatedUnix: 0,
+				},
+				{
+					Type:     models.CommentTypeLabel,
+					PosterID: 1,
+					Content:  "1",
+					Label: &models.Label{
+						Name: "kind/backport",
+					},
+					CreatedUnix: 10,
+				},
+			},
+			afterCombined: []*models.Comment{
+				{
+					Type:        models.CommentTypeLabel,
+					PosterID:    1,
+					Content:     "1",
+					CreatedUnix: 10,
+					AddedLabels: []*models.Label{
+						{
+							Name: "kind/bug",
+						},
+						{
+							Name: "kind/backport",
+						},
+					},
+					Label: &models.Label{
+						Name: "kind/bug",
+					},
+				},
+			},
+		},
+	}
+
+	for _, kase := range kases {
+		var issue = models.Issue{
+			Comments: kase.beforeCombined,
+		}
+		combineLabelComments(&issue)
+		assert.EqualValues(t, kase.afterCombined, issue.Comments)
+	}
+}