Fix bug when combine label comments (#14894)

* Fix bug when combine label comments

* Added some code comments

* More comments

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 (
+		cur = issue.Comments[i]
-			prev *models.Comment
-			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.Label != nil { // now cur MUST be label comment
-			if cur.Content != "1" {
+			if prev.Type == models.CommentTypeLabel { // we can combine them only prev is a label comment
-				prev.RemovedLabels = append(prev.RemovedLabels, cur.Label)
+				if cur.Content != "1" {
-			} else {
+					prev.RemovedLabels = append(prev.RemovedLabels, cur.Label)
-				prev.AddedLabels = append(prev.AddedLabels, 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--
 	}
 }
 

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{
+		t.Run(kase.name, func(t *testing.T) {
-			Comments: kase.beforeCombined,
+			var issue = models.Issue{
-		}
+				Comments: kase.beforeCombined,
-		combineLabelComments(&issue)
+			}
-		assert.EqualValues(t, kase.afterCombined, issue.Comments)
+			combineLabelComments(&issue)
+			assert.EqualValues(t, kase.afterCombined, issue.Comments)
+		})
 	}
 }