Skip to content

Commit c5f6f8f

Browse files
lunnylafriks
lunny
and
lafriks
authored
Refactor combine label comments with tests (#13619)
Co-authored-by: Lauris BH <[email protected]>
1 parent f915161 commit c5f6f8f

File tree

2 files changed

+275
-56
lines changed

2 files changed

+275
-56
lines changed

‎routers/repo/issue.go

Lines changed: 23 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2392,67 +2392,34 @@ func attachmentsHTML(ctx *context.Context, attachments []*models.Attachment) str
23922392
}
23932393

23942394
func combineLabelComments(issue *models.Issue) {
2395-
for i := 0; i < len(issue.Comments); {
2396-
c := issue.Comments[i]
2397-
var shouldMerge bool
2398-
var removingCur bool
2399-
var prev *models.Comment
2400-
2401-
if i == 0 {
2402-
shouldMerge = false
2403-
} else {
2395+
for i := 0; i < len(issue.Comments); i++ {
2396+
var (
2397+
prev *models.Comment
2398+
cur = issue.Comments[i]
2399+
)
2400+
if i > 0 {
24042401
prev = issue.Comments[i-1]
2405-
removingCur = c.Content != "1"
2406-
2407-
shouldMerge = prev.PosterID == c.PosterID && c.CreatedUnix-prev.CreatedUnix < 60 &&
2408-
c.Type == prev.Type
24092402
}
2410-
2411-
if c.Type == models.CommentTypeLabel {
2412-
if !shouldMerge {
2413-
if removingCur {
2414-
c.RemovedLabels = make([]*models.Label, 1)
2415-
c.AddedLabels = make([]*models.Label, 0)
2416-
c.RemovedLabels[0] = c.Label
2403+
if i == 0 || cur.Type != models.CommentTypeLabel ||
2404+
(prev != nil && prev.PosterID != cur.PosterID) ||
2405+
(prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) {
2406+
if cur.Type == models.CommentTypeLabel {
2407+
if cur.Content != "1" {
2408+
cur.RemovedLabels = append(cur.RemovedLabels, cur.Label)
24172409
} else {
2418-
c.RemovedLabels = make([]*models.Label, 0)
2419-
c.AddedLabels = make([]*models.Label, 1)
2420-
c.AddedLabels[0] = c.Label
2410+
cur.AddedLabels = append(cur.AddedLabels, cur.Label)
24212411
}
2422-
} else {
2423-
// Remove duplicated "added" and "removed" labels
2424-
// This way, adding and immediately removing a label won't generate a comment.
2425-
var appendingTo *[]*models.Label
2426-
var other *[]*models.Label
2427-
2428-
if removingCur {
2429-
appendingTo = &prev.RemovedLabels
2430-
other = &prev.AddedLabels
2431-
} else {
2432-
appendingTo = &prev.AddedLabels
2433-
other = &prev.RemovedLabels
2434-
}
2435-
2436-
appending := true
2437-
2438-
for i := 0; i < len(*other); i++ {
2439-
l := (*other)[i]
2440-
if l.ID == c.Label.ID {
2441-
*other = append((*other)[:i], (*other)[i+1:]...)
2442-
appending = false
2443-
break
2444-
}
2445-
}
2446-
2447-
if appending {
2448-
*appendingTo = append(*appendingTo, c.Label)
2449-
}
2450-
2451-
prev.CreatedUnix = c.CreatedUnix
2452-
issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...)
2453-
continue
24542412
}
2413+
continue
24552414
}
2456-
i++
2415+
2416+
if cur.Content != "1" {
2417+
prev.RemovedLabels = append(prev.RemovedLabels, cur.Label)
2418+
} else {
2419+
prev.AddedLabels = append(prev.AddedLabels, cur.Label)
2420+
}
2421+
prev.CreatedUnix = cur.CreatedUnix
2422+
issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...)
2423+
i--
24572424
}
24582425
}

‎routers/repo/issue_test.go

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package repo
6+
7+
import (
8+
"testing"
9+
10+
"code.gitea.io/gitea/models"
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func TestCombineLabelComments(t *testing.T) {
15+
var kases = []struct {
16+
beforeCombined []*models.Comment
17+
afterCombined []*models.Comment
18+
}{
19+
{
20+
beforeCombined: []*models.Comment{
21+
{
22+
Type: models.CommentTypeLabel,
23+
PosterID: 1,
24+
Content: "1",
25+
Label: &models.Label{
26+
Name: "kind/bug",
27+
},
28+
CreatedUnix: 0,
29+
},
30+
{
31+
Type: models.CommentTypeLabel,
32+
PosterID: 1,
33+
Content: "",
34+
Label: &models.Label{
35+
Name: "kind/bug",
36+
},
37+
CreatedUnix: 0,
38+
},
39+
{
40+
Type: models.CommentTypeComment,
41+
PosterID: 1,
42+
Content: "test",
43+
CreatedUnix: 0,
44+
},
45+
},
46+
afterCombined: []*models.Comment{
47+
{
48+
Type: models.CommentTypeLabel,
49+
PosterID: 1,
50+
Content: "1",
51+
CreatedUnix: 0,
52+
AddedLabels: []*models.Label{
53+
{
54+
Name: "kind/bug",
55+
},
56+
},
57+
RemovedLabels: []*models.Label{
58+
{
59+
Name: "kind/bug",
60+
},
61+
},
62+
Label: &models.Label{
63+
Name: "kind/bug",
64+
},
65+
},
66+
{
67+
Type: models.CommentTypeComment,
68+
PosterID: 1,
69+
Content: "test",
70+
CreatedUnix: 0,
71+
},
72+
},
73+
},
74+
{
75+
beforeCombined: []*models.Comment{
76+
{
77+
Type: models.CommentTypeLabel,
78+
PosterID: 1,
79+
Content: "1",
80+
Label: &models.Label{
81+
Name: "kind/bug",
82+
},
83+
CreatedUnix: 0,
84+
},
85+
{
86+
Type: models.CommentTypeLabel,
87+
PosterID: 1,
88+
Content: "",
89+
Label: &models.Label{
90+
Name: "kind/bug",
91+
},
92+
CreatedUnix: 70,
93+
},
94+
{
95+
Type: models.CommentTypeComment,
96+
PosterID: 1,
97+
Content: "test",
98+
CreatedUnix: 0,
99+
},
100+
},
101+
afterCombined: []*models.Comment{
102+
{
103+
Type: models.CommentTypeLabel,
104+
PosterID: 1,
105+
Content: "1",
106+
CreatedUnix: 0,
107+
AddedLabels: []*models.Label{
108+
{
109+
Name: "kind/bug",
110+
},
111+
},
112+
Label: &models.Label{
113+
Name: "kind/bug",
114+
},
115+
},
116+
{
117+
Type: models.CommentTypeLabel,
118+
PosterID: 1,
119+
Content: "",
120+
CreatedUnix: 70,
121+
RemovedLabels: []*models.Label{
122+
{
123+
Name: "kind/bug",
124+
},
125+
},
126+
Label: &models.Label{
127+
Name: "kind/bug",
128+
},
129+
},
130+
{
131+
Type: models.CommentTypeComment,
132+
PosterID: 1,
133+
Content: "test",
134+
CreatedUnix: 0,
135+
},
136+
},
137+
},
138+
{
139+
beforeCombined: []*models.Comment{
140+
{
141+
Type: models.CommentTypeLabel,
142+
PosterID: 1,
143+
Content: "1",
144+
Label: &models.Label{
145+
Name: "kind/bug",
146+
},
147+
CreatedUnix: 0,
148+
},
149+
{
150+
Type: models.CommentTypeLabel,
151+
PosterID: 2,
152+
Content: "",
153+
Label: &models.Label{
154+
Name: "kind/bug",
155+
},
156+
CreatedUnix: 0,
157+
},
158+
{
159+
Type: models.CommentTypeComment,
160+
PosterID: 1,
161+
Content: "test",
162+
CreatedUnix: 0,
163+
},
164+
},
165+
afterCombined: []*models.Comment{
166+
{
167+
Type: models.CommentTypeLabel,
168+
PosterID: 1,
169+
Content: "1",
170+
CreatedUnix: 0,
171+
AddedLabels: []*models.Label{
172+
{
173+
Name: "kind/bug",
174+
},
175+
},
176+
Label: &models.Label{
177+
Name: "kind/bug",
178+
},
179+
},
180+
{
181+
Type: models.CommentTypeLabel,
182+
PosterID: 2,
183+
Content: "",
184+
CreatedUnix: 0,
185+
RemovedLabels: []*models.Label{
186+
{
187+
Name: "kind/bug",
188+
},
189+
},
190+
Label: &models.Label{
191+
Name: "kind/bug",
192+
},
193+
},
194+
{
195+
Type: models.CommentTypeComment,
196+
PosterID: 1,
197+
Content: "test",
198+
CreatedUnix: 0,
199+
},
200+
},
201+
},
202+
{
203+
beforeCombined: []*models.Comment{
204+
{
205+
Type: models.CommentTypeLabel,
206+
PosterID: 1,
207+
Content: "1",
208+
Label: &models.Label{
209+
Name: "kind/bug",
210+
},
211+
CreatedUnix: 0,
212+
},
213+
{
214+
Type: models.CommentTypeLabel,
215+
PosterID: 1,
216+
Content: "1",
217+
Label: &models.Label{
218+
Name: "kind/backport",
219+
},
220+
CreatedUnix: 10,
221+
},
222+
},
223+
afterCombined: []*models.Comment{
224+
{
225+
Type: models.CommentTypeLabel,
226+
PosterID: 1,
227+
Content: "1",
228+
CreatedUnix: 10,
229+
AddedLabels: []*models.Label{
230+
{
231+
Name: "kind/bug",
232+
},
233+
{
234+
Name: "kind/backport",
235+
},
236+
},
237+
Label: &models.Label{
238+
Name: "kind/bug",
239+
},
240+
},
241+
},
242+
},
243+
}
244+
245+
for _, kase := range kases {
246+
var issue = models.Issue{
247+
Comments: kase.beforeCombined,
248+
}
249+
combineLabelComments(&issue)
250+
assert.EqualValues(t, kase.afterCombined, issue.Comments)
251+
}
252+
}

0 commit comments

Comments
 (0)