From c5f6f8f2f19d6893d5fe32e61214fc3d457a1a39 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 21 Nov 2020 06:29:09 +0800 Subject: [PATCH] Refactor combine label comments with tests (#13619) Co-authored-by: Lauris BH --- routers/repo/issue.go | 79 ++++-------- routers/repo/issue_test.go | 252 +++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 56 deletions(-) create mode 100644 routers/repo/issue_test.go diff --git a/routers/repo/issue.go b/routers/repo/issue.go index c55a0ce693..159cc5b9f0 100644 --- a/routers/repo/issue.go +++ b/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); { - c := issue.Comments[i] - var shouldMerge bool - var removingCur bool - var prev *models.Comment - - if i == 0 { - shouldMerge = false - } else { + for i := 0; i < len(issue.Comments); i++ { + var ( + prev *models.Comment + cur = issue.Comments[i] + ) + if i > 0 { prev = issue.Comments[i-1] - removingCur = c.Content != "1" - - shouldMerge = prev.PosterID == c.PosterID && c.CreatedUnix-prev.CreatedUnix < 60 && - c.Type == prev.Type } - - if c.Type == models.CommentTypeLabel { - if !shouldMerge { - if removingCur { - c.RemovedLabels = make([]*models.Label, 1) - c.AddedLabels = make([]*models.Label, 0) - c.RemovedLabels[0] = c.Label + if i == 0 || cur.Type != models.CommentTypeLabel || + (prev != nil && prev.PosterID != cur.PosterID) || + (prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) { + if cur.Type == models.CommentTypeLabel { + if cur.Content != "1" { + cur.RemovedLabels = append(cur.RemovedLabels, cur.Label) } else { - c.RemovedLabels = make([]*models.Label, 0) - c.AddedLabels = make([]*models.Label, 1) - c.AddedLabels[0] = c.Label + cur.AddedLabels = append(cur.AddedLabels, cur.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 } + continue } - i++ + + if cur.Content != "1" { + prev.RemovedLabels = append(prev.RemovedLabels, cur.Label) + } else { + prev.AddedLabels = append(prev.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 new file mode 100644 index 0000000000..d6efef4252 --- /dev/null +++ b/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) + } +}