diff --git a/models/issues/comment.go b/models/issues/comment.go index 0bf53bb4dd..5b79506041 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -114,6 +114,8 @@ const ( CommentTypePin // 36 pin Issue CommentTypeUnpin // 37 unpin Issue + + CommentTypeAggregator // 38 Aggregator of comments ) var commentStrings = []string{ @@ -236,10 +238,30 @@ func (r RoleInRepo) LocaleHelper(lang translation.Locale) string { return lang.TrString("repo.issues.role." + string(r) + "_helper") } -type RequestReviewTarget interface { - ID() int64 - Name() string - Type() string +type RequestReviewTarget struct { + User *user_model.User + Team *organization.Team +} + +func (t *RequestReviewTarget) ID() int64 { + if t.User != nil { + return t.User.ID + } + return t.Team.ID +} + +func (t *RequestReviewTarget) Name() string { + if t.User != nil { + return t.User.GetDisplayName() + } + return t.Team.Name +} + +func (t *RequestReviewTarget) Type() string { + if t.User != nil { + return "user" + } + return "team" } // Comment represents a comment in commit and issue page. @@ -254,6 +276,7 @@ type Comment struct { Issue *Issue `xorm:"-"` LabelID int64 Label *Label `xorm:"-"` + Aggregator *CommentAggregator `xorm:"-"` AddedLabels []*Label `xorm:"-"` RemovedLabels []*Label `xorm:"-"` AddedRequestReview []RequestReviewTarget `xorm:"-"` diff --git a/models/issues/comment_aggregator.go b/models/issues/comment_aggregator.go new file mode 100644 index 0000000000..f419a9ebce --- /dev/null +++ b/models/issues/comment_aggregator.go @@ -0,0 +1,337 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package issues + +import ( + "slices" +) + +type CommentAggregator struct { + StartUnix int64 + Indexes []int + PosterID int64 + + PrevClosed bool + IsClosed bool + + AddedLabels []*Label + RemovedLabels []*Label + + AddedRequestReview []RequestReviewTarget + RemovedRequestReview []RequestReviewTarget +} + +// TODO Aggregate also +// - Dependency added / removed +// - Added / Removed due date +// - Milestone Added / Removed +func (agg *CommentAggregator) aggregateComment(c *Comment, index int) { + agg.Indexes = append(agg.Indexes, index) + + if c.Type == CommentTypeClose { + agg.IsClosed = true + + } else if c.Type == CommentTypeReopen { + agg.IsClosed = false + + } else if c.Type == CommentTypeReviewRequest { + if c.AssigneeID > 0 { + req := RequestReviewTarget{User: c.Assignee} + if c.RemovedAssignee { + agg.delReviewRequest(req) + } else { + agg.addReviewRequest(req) + } + + } else if c.AssigneeTeamID > 0 { + req := RequestReviewTarget{Team: c.AssigneeTeam} + if c.RemovedAssignee { + agg.delReviewRequest(req) + } else { + agg.addReviewRequest(req) + } + } + + for _, r := range c.RemovedRequestReview { + agg.delReviewRequest(r) + } + for _, r := range c.AddedRequestReview { + agg.addReviewRequest(r) + } + + } else if c.Type == CommentTypeLabel { + if c.Content == "1" { + agg.addLabel(c.Label) + } else { + agg.delLabel(c.Label) + } + + } else if c.Type == CommentTypeAggregator { + agg.Merge(c.Aggregator) + } +} + +// Merge a past CommentAggregator with the next one in the issue comments list +func (agg *CommentAggregator) Merge(next *CommentAggregator) { + agg.IsClosed = next.IsClosed + + for _, l := range next.AddedLabels { + agg.addLabel(l) + } + + for _, l := range next.RemovedLabels { + agg.delLabel(l) + } + + for _, r := range next.AddedRequestReview { + agg.addReviewRequest(r) + } + + for _, r := range next.RemovedRequestReview { + agg.delReviewRequest(r) + } +} + +// Check if a comment can be aggregated or not depending on its type +func (agg *CommentAggregator) IsAggregated(t *CommentType) bool { + a := false + a = a || (*t == CommentTypeAggregator) + a = a || (*t == CommentTypeClose) + a = a || (*t == CommentTypeReopen) + a = a || (*t == CommentTypeLabel) + a = a || (*t == CommentTypeReviewRequest) + return a +} + +// Add a label to the aggregated list +func (agg *CommentAggregator) addLabel(lbl *Label) { + for l, agglbl := range agg.RemovedLabels { + if agglbl.ID == lbl.ID { + agg.RemovedLabels = append(agg.RemovedLabels[:l], agg.RemovedLabels[l+1:]...) + return + } + } + + if !slices.ContainsFunc(agg.AddedLabels, func(l *Label) bool { return l.ID == lbl.ID }) { + agg.AddedLabels = append(agg.AddedLabels, lbl) + } +} + +// Remove a label from the aggregated list +func (agg *CommentAggregator) delLabel(lbl *Label) { + for l, agglbl := range agg.AddedLabels { + if agglbl.ID == lbl.ID { + agg.AddedLabels = append(agg.AddedLabels[:l], agg.AddedLabels[l+1:]...) + return + } + } + + if !slices.ContainsFunc(agg.RemovedLabels, func(l *Label) bool { return l.ID == lbl.ID }) { + agg.RemovedLabels = append(agg.RemovedLabels, lbl) + } +} + +// Add a review request to the aggregated list +func (agg *CommentAggregator) addReviewRequest(req RequestReviewTarget) { + reqid := req.ID() + reqty := req.Type() + for r, aggreq := range agg.RemovedRequestReview { + if (aggreq.ID() == reqid) && (aggreq.Type() == reqty) { + agg.RemovedRequestReview = append(agg.RemovedRequestReview[:r], agg.RemovedRequestReview[r+1:]...) + return + } + } + + if !slices.ContainsFunc(agg.AddedRequestReview, func(r RequestReviewTarget) bool { return (r.ID() == reqid) && (r.Type() == reqty) }) { + agg.AddedRequestReview = append(agg.AddedRequestReview, req) + } +} + +// Delete a review request from the aggregated list +func (agg *CommentAggregator) delReviewRequest(req RequestReviewTarget) { + reqid := req.ID() + reqty := req.Type() + for r, aggreq := range agg.AddedRequestReview { + if (aggreq.ID() == reqid) && (aggreq.Type() == reqty) { + agg.AddedRequestReview = append(agg.AddedRequestReview[:r], agg.AddedRequestReview[r+1:]...) + return + } + } + + if !slices.ContainsFunc(agg.RemovedRequestReview, func(r RequestReviewTarget) bool { return (r.ID() == reqid) && (r.Type() == reqty) }) { + agg.RemovedRequestReview = append(agg.RemovedRequestReview, req) + } +} + +// Check if anything has changed with this aggregated list of comments +func (agg *CommentAggregator) Changed() bool { + changed := false + changed = changed || (agg.IsClosed != agg.PrevClosed) + changed = changed || (len(agg.AddedLabels) > 0) + changed = changed || (len(agg.RemovedLabels) > 0) + changed = changed || (len(agg.AddedRequestReview) > 0) + changed = changed || (len(agg.RemovedRequestReview) > 0) + return changed +} + +func (agg *CommentAggregator) OnlyLabelsChanged() bool { + l := (len(agg.AddedLabels) > 0) || (len(agg.RemovedLabels) > 0) + l = l && (len(agg.AddedRequestReview) == 0) && (len(agg.RemovedRequestReview) == 0) + l = l && (agg.PrevClosed == agg.IsClosed) + return l +} + +func (agg *CommentAggregator) OnlyRequestReview() bool { + l := (len(agg.AddedRequestReview) > 0) || (len(agg.RemovedRequestReview) > 0) + l = l && (len(agg.AddedLabels) == 0) && (len(agg.RemovedLabels) == 0) + l = l && (agg.PrevClosed == agg.IsClosed) + return l +} + +func (agg *CommentAggregator) OnlyClosedReopened() bool { + l := (agg.IsClosed != agg.PrevClosed) + l = l && (len(agg.AddedLabels) == 0) && (len(agg.RemovedLabels) == 0) + l = l && (len(agg.AddedRequestReview) == 0) && (len(agg.RemovedRequestReview) == 0) + return l +} + +// Reset the aggregator to start a new aggregating context +func (agg *CommentAggregator) Reset(cur *Comment) { + agg.StartUnix = int64(cur.CreatedUnix) + agg.PosterID = cur.PosterID + + agg.PrevClosed = agg.IsClosed + + agg.Indexes = []int{} + agg.AddedLabels = []*Label{} + agg.RemovedLabels = []*Label{} + agg.AddedRequestReview = []RequestReviewTarget{} + agg.RemovedRequestReview = []RequestReviewTarget{} +} + +// Function that replaces all the comments aggregated with a single one +// Its CommentType depend on whether multiple type of comments are been aggregated or not +// If nothing has changed, we remove all the comments that get nullified +// +// The function returns how many comments has been removed, in order for the "for" loop +// of the main algorithm to change its index +func (agg *CommentAggregator) createAggregatedComment(issue *Issue, final bool) int { + endind := agg.Indexes[len(agg.Indexes)-1] + startind := agg.Indexes[0] + + // If the aggregation of comments make the whole thing null, erase all the comments + if !agg.Changed() { + if final { + issue.Comments = issue.Comments[:startind] + } else { + issue.Comments = append(issue.Comments[:startind], issue.Comments[endind+1:]...) + } + return endind - startind + } + + new_agg := *agg // Trigger a memory allocation, get a COPY of the aggregator + + // Keep the same author, time, etc... But reset the parts we may want to use + comment := issue.Comments[startind] + comment.Content = "" + comment.Label = nil + comment.Aggregator = nil + comment.Assignee = nil + comment.AssigneeID = -1 + comment.AssigneeTeam = nil + comment.AssigneeTeamID = -1 + comment.RemovedAssignee = false + comment.AddedLabels = nil + comment.RemovedLabels = nil + + // In case there's only a single change, create a comment of this type + // instead of an aggregator + if agg.OnlyLabelsChanged() { + comment.Type = CommentTypeLabel + + } else if agg.OnlyClosedReopened() { + if agg.IsClosed { + comment.Type = CommentTypeClose + } else { + comment.Type = CommentTypeReopen + } + + } else if agg.OnlyRequestReview() { + comment.Type = CommentTypeReviewRequest + + } else { + comment.Type = CommentTypeAggregator + comment.Aggregator = &new_agg + } + + if len(new_agg.AddedLabels) > 0 { + comment.AddedLabels = new_agg.AddedLabels + } + + if len(new_agg.RemovedLabels) > 0 { + comment.RemovedLabels = new_agg.RemovedLabels + } + + if len(new_agg.AddedRequestReview) > 0 { + comment.AddedRequestReview = new_agg.AddedRequestReview + } + + if len(new_agg.RemovedRequestReview) > 0 { + comment.RemovedRequestReview = new_agg.RemovedRequestReview + } + + if final { + issue.Comments = append(issue.Comments[:startind], comment) + } else { + issue.Comments = append(append(issue.Comments[:startind], comment), issue.Comments[endind+1:]...) + } + return endind - startind +} + +// combineCommentsHistory combines nearby elements in the history as one +func CombineCommentsHistory(issue *Issue) { + // Initialise a new empty aggregator, ready to combine comments + var agg CommentAggregator + agg.StartUnix = 0 + agg.PosterID = -1 + req_reset := false + + for i := 0; i < len(issue.Comments); i++ { + cur := issue.Comments[i] + if req_reset { + agg.Reset(cur) + req_reset = false + } + + // If the comment we encounter is not accepted inside an aggregator + if !agg.IsAggregated(&cur.Type) { + + // If we aggregated some data, create the resulting comment for it + if len(agg.Indexes) > 0 { + i -= agg.createAggregatedComment(issue, false) + } + req_reset = true + + // Do not need to continue the aggregation loop, skip to next comment + continue + } + + // If the comment we encounter cannot be aggregated with the current aggregator, + // we create a new empty aggregator + if ((int64(cur.CreatedUnix) - agg.StartUnix) > 60) || (cur.PosterID != agg.PosterID) { + // First, create the aggregated comment if there's data in it + if len(agg.Indexes) > 0 { + i -= agg.createAggregatedComment(issue, false) + } + agg.Reset(cur) + } + + agg.aggregateComment(cur, i) + } + + // Create the aggregated comment if there's data in it + if !req_reset && len(agg.Indexes) > 0 { + agg.createAggregatedComment(issue, true) + } +} diff --git a/release-notes/6523.md b/release-notes/6523.md new file mode 100644 index 0000000000..60cef0b4d8 --- /dev/null +++ b/release-notes/6523.md @@ -0,0 +1 @@ +Reduce noise in issue / PR comments. If performed in under 60 secs, cancel out contradictory actions, aggregate many actions under the same comment diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 61711095b9..e108a4c3dc 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1831,8 +1831,7 @@ func ViewIssue(ctx *context.Context) { ctx.Data["LatestCloseCommentID"] = latestCloseCommentID // Combine multiple label assignments into a single comment - combineLabelComments(issue) - combineRequestReviewComments(issue) + issues_model.CombineCommentsHistory(issue) getBranchData(ctx, issue) if issue.IsPull { @@ -3693,194 +3692,6 @@ func attachmentsHTML(ctx *context.Context, attachments []*repo_model.Attachment, return attachHTML } -type RequestReviewTarget struct { - user *user_model.User - team *organization.Team -} - -func (t *RequestReviewTarget) ID() int64 { - if t.user != nil { - return t.user.ID - } - return t.team.ID -} - -func (t *RequestReviewTarget) Name() string { - if t.user != nil { - return t.user.GetDisplayName() - } - return t.team.Name -} - -func (t *RequestReviewTarget) Type() string { - if t.user != nil { - return "user" - } - return "team" -} - -// combineRequestReviewComments combine the nearby request review comments as one. -func combineRequestReviewComments(issue *issues_model.Issue) { - var prev, cur *issues_model.Comment - for i := 0; i < len(issue.Comments); i++ { - cur = issue.Comments[i] - if i > 0 { - prev = issue.Comments[i-1] - } - if i == 0 || cur.Type != issues_model.CommentTypeReviewRequest || - (prev != nil && prev.PosterID != cur.PosterID) || - (prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) { - if cur.Type == issues_model.CommentTypeReviewRequest && (cur.Assignee != nil || cur.AssigneeTeam != nil) { - if cur.RemovedAssignee { - if cur.AssigneeTeam != nil { - cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) - } else { - cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{user: cur.Assignee}) - } - } else { - if cur.AssigneeTeam != nil { - cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) - } else { - cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{user: cur.Assignee}) - } - } - } - continue - } - - // Previous comment is not a review request, so cannot group. Start a new group. - if prev.Type != issues_model.CommentTypeReviewRequest { - if cur.RemovedAssignee { - if cur.AssigneeTeam != nil { - cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) - } else { - cur.RemovedRequestReview = append(cur.RemovedRequestReview, &RequestReviewTarget{user: cur.Assignee}) - } - } else { - if cur.AssigneeTeam != nil { - cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) - } else { - cur.AddedRequestReview = append(cur.AddedRequestReview, &RequestReviewTarget{user: cur.Assignee}) - } - } - continue - } - - // Start grouping. - if cur.RemovedAssignee { - addedIndex := slices.IndexFunc(prev.AddedRequestReview, func(t issues_model.RequestReviewTarget) bool { - if cur.AssigneeTeam != nil { - return cur.AssigneeTeam.ID == t.ID() && t.Type() == "team" - } - return cur.Assignee.ID == t.ID() && t.Type() == "user" - }) - - // If for this target a AddedRequestReview, then we remove that entry. If it's not found, then add it to the RemovedRequestReview. - if addedIndex == -1 { - if cur.AssigneeTeam != nil { - prev.RemovedRequestReview = append(prev.RemovedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) - } else { - prev.RemovedRequestReview = append(prev.RemovedRequestReview, &RequestReviewTarget{user: cur.Assignee}) - } - } else { - prev.AddedRequestReview = slices.Delete(prev.AddedRequestReview, addedIndex, addedIndex+1) - } - } else { - removedIndex := slices.IndexFunc(prev.RemovedRequestReview, func(t issues_model.RequestReviewTarget) bool { - if cur.AssigneeTeam != nil { - return cur.AssigneeTeam.ID == t.ID() && t.Type() == "team" - } - return cur.Assignee.ID == t.ID() && t.Type() == "user" - }) - - // If for this target a RemovedRequestReview, then we remove that entry. If it's not found, then add it to the AddedRequestReview. - if removedIndex == -1 { - if cur.AssigneeTeam != nil { - prev.AddedRequestReview = append(prev.AddedRequestReview, &RequestReviewTarget{team: cur.AssigneeTeam}) - } else { - prev.AddedRequestReview = append(prev.AddedRequestReview, &RequestReviewTarget{user: cur.Assignee}) - } - } else { - prev.RemovedRequestReview = slices.Delete(prev.RemovedRequestReview, removedIndex, removedIndex+1) - } - } - - // Propagate creation time. - 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-- - } -} - -// combineLabelComments combine the nearby label comments as one. -func combineLabelComments(issue *issues_model.Issue) { - var prev, cur *issues_model.Comment - for i := 0; i < len(issue.Comments); i++ { - cur = issue.Comments[i] - if i > 0 { - prev = issue.Comments[i-1] - } - if i == 0 || cur.Type != issues_model.CommentTypeLabel || - (prev != nil && prev.PosterID != cur.PosterID) || - (prev != nil && cur.CreatedUnix-prev.CreatedUnix >= 60) { - if cur.Type == issues_model.CommentTypeLabel && cur.Label != nil { - if cur.Content != "1" { - cur.RemovedLabels = append(cur.RemovedLabels, cur.Label) - } else { - cur.AddedLabels = append(cur.AddedLabels, cur.Label) - } - } - continue - } - - if cur.Label != nil { // now cur MUST be label comment - if prev.Type == issues_model.CommentTypeLabel { // we can combine them only prev is a label comment - if cur.Content != "1" { - // remove labels from the AddedLabels list if the label that was removed is already - // in this list, and if it's not in this list, add the label to RemovedLabels - addedAndRemoved := false - for i, label := range prev.AddedLabels { - if cur.Label.ID == label.ID { - prev.AddedLabels = append(prev.AddedLabels[:i], prev.AddedLabels[i+1:]...) - addedAndRemoved = true - break - } - } - if !addedAndRemoved { - prev.RemovedLabels = append(prev.RemovedLabels, cur.Label) - } - } else { - // remove labels from the RemovedLabels list if the label that was added is already - // in this list, and if it's not in this list, add the label to AddedLabels - removedAndAdded := false - for i, label := range prev.RemovedLabels { - if cur.Label.ID == label.ID { - prev.RemovedLabels = append(prev.RemovedLabels[:i], prev.RemovedLabels[i+1:]...) - removedAndAdded = true - break - } - } - if !removedAndAdded { - 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) - } - } - } - } -} - // get all teams that current user can mention func handleTeamMentions(ctx *context.Context) { if ctx.Doer == nil || !ctx.Repo.Owner.IsOrganization() { diff --git a/routers/web/repo/issue_test.go b/routers/web/repo/issue_test.go index d642c14b5f..81923b67e5 100644 --- a/routers/web/repo/issue_test.go +++ b/routers/web/repo/issue_test.go @@ -1,806 +1,668 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later package repo import ( + "bytes" "testing" + issue_model "code.gitea.io/gitea/models/issues" issues_model "code.gitea.io/gitea/models/issues" - org_model "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/organization" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" ) +// *************** Helper functions for the tests *************** + +func testComment(t int64) *issues_model.Comment { + return &issues_model.Comment{PosterID: 1, CreatedUnix: timeutil.TimeStamp(t)} +} + +func nameToID(name string) int64 { + var id int64 + for c, letter := range name { + id += int64((c+1)*1000) * int64(letter) + } + return id +} + +func createReqReviewTarget(name string) issues_model.RequestReviewTarget { + if bytes.HasSuffix([]byte(name), []byte("-team")) { + team := createTeam(name) + return issues_model.RequestReviewTarget{Team: &team} + + } else { + user := createUser(name) + return issues_model.RequestReviewTarget{User: &user} + } +} + +func createUser(name string) user_model.User { + return user_model.User{Name: name, ID: nameToID(name)} +} + +func createTeam(name string) organization.Team { + return organization.Team{Name: name, ID: nameToID(name)} +} + +func createLabel(name string) issues_model.Label { + return issues_model.Label{Name: name, ID: nameToID(name)} +} + +func addLabel(t int64, name string) *issues_model.Comment { + c := testComment(t) + c.Type = issue_model.CommentTypeLabel + c.Content = "1" + lbl := createLabel(name) + c.Label = &lbl + c.AddedLabels = []*issues_model.Label{&lbl} + return c +} + +func delLabel(t int64, name string) *issues_model.Comment { + c := addLabel(t, name) + c.Content = "" + return c +} + +func openOrClose(t int64, close bool) *issues_model.Comment { + c := testComment(t) + if close { + c.Type = issue_model.CommentTypeClose + } else { + c.Type = issue_model.CommentTypeReopen + } + return c +} + +func reqReview(t int64, name string, delReq bool) *issues_model.Comment { + c := testComment(t) + c.Type = issue_model.CommentTypeReviewRequest + if bytes.HasSuffix([]byte(name), []byte("-team")) { + team := createTeam(name) + c.AssigneeTeam = &team + c.AssigneeTeamID = team.ID + } else { + user := createUser(name) + c.Assignee = &user + c.AssigneeID = user.ID + } + c.RemovedAssignee = delReq + return c +} + +func reqReviewList(t int64, del bool, names ...string) *issues_model.Comment { + req := []issues_model.RequestReviewTarget{} + for _, name := range names { + req = append(req, createReqReviewTarget(name)) + } + cmnt := testComment(t) + cmnt.Type = issue_model.CommentTypeReviewRequest + if del { + cmnt.RemovedRequestReview = req + } else { + cmnt.AddedRequestReview = req + } + return cmnt +} + +func aggregatedComment(t int64, + closed bool, + addLabels []*issues_model.Label, + delLabels []*issues_model.Label, + addReqReview []issues_model.RequestReviewTarget, + delReqReview []issues_model.RequestReviewTarget, +) *issues_model.Comment { + cmnt := testComment(t) + cmnt.Type = issues_model.CommentTypeAggregator + cmnt.Aggregator = &issues_model.CommentAggregator{ + IsClosed: closed, + AddedLabels: addLabels, + RemovedLabels: delLabels, + AddedRequestReview: addReqReview, + RemovedRequestReview: delReqReview, + } + if len(addLabels) > 0 { + cmnt.AddedLabels = addLabels + } + if len(delLabels) > 0 { + cmnt.RemovedLabels = delLabels + } + if len(addReqReview) > 0 { + cmnt.AddedRequestReview = addReqReview + } + if len(delReqReview) > 0 { + cmnt.RemovedRequestReview = delReqReview + } + return cmnt +} + +func commentText(t int64, text string) *issues_model.Comment { + c := testComment(t) + c.Type = issue_model.CommentTypeComment + c.Content = text + return c +} + +// **************************************************************** + +type testCase struct { + name string + beforeCombined []*issues_model.Comment + afterCombined []*issues_model.Comment + sameAfter bool +} + +func (kase *testCase) doTest(t *testing.T) { + issue := issues_model.Issue{Comments: kase.beforeCombined} + issues_model.CombineCommentsHistory(&issue) + + after := &kase.afterCombined + if kase.sameAfter { + after = &kase.beforeCombined + } + + if len(*after) != len(issue.Comments) { + t.Logf("Expected %v comments, got %v", len(*after), len(issue.Comments)) + t.Logf("Comments got after combination:") + for c := 0; c < len(issue.Comments); c++ { + cmt := issue.Comments[c] + t.Logf("%v %v %v\n", cmt.Type, cmt.CreatedUnix, cmt.Content) + } + assert.EqualValues(t, len(*after), len(issue.Comments)) + t.Fail() + return + } + + for c := 0; c < len(*after); c++ { + l := (*after)[c] + r := issue.Comments[c] + + // Ignore some inner data of the aggregator to facilitate testing + if l.Type == issue_model.CommentTypeAggregator { + r.Aggregator.StartUnix = 0 + r.Aggregator.Indexes = nil + r.Aggregator.PrevClosed = false + r.Aggregator.PosterID = 0 + } + + // We can safely ignore this if the rest matches + if l.Type == issue_model.CommentTypeLabel { + l.Label = nil + l.Content = "" + } else if l.Type == issue_model.CommentTypeReviewRequest { + l.Assignee = nil + l.AssigneeID = -1 + l.AssigneeTeam = nil + l.AssigneeTeamID = -1 + } + + assert.EqualValues(t, (*after)[c], issue.Comments[c], + "Comment %v is not equal", c, + ) + } +} + +// **************** Start of the tests ****************** + func TestCombineLabelComments(t *testing.T) { - kases := []struct { - name string - beforeCombined []*issues_model.Comment - afterCombined []*issues_model.Comment - }{ + kases := []testCase{ + // ADD single = normal label comment { - name: "kase 1", + name: "add_single_label", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, + addLabel(0, "a"), + commentText(10, "I'm a salmon"), + }, + sameAfter: true, + }, + + // ADD then REMOVE = Nothing + { + name: "add_label_then_remove", + beforeCombined: []*issues_model.Comment{ + addLabel(0, "a"), + delLabel(1, "a"), + commentText(65, "I'm a salmon"), + }, + afterCombined: []*issues_model.Comment{ + commentText(65, "I'm a salmon"), + }, + }, + + // ADD 1 then comment then REMOVE = separate comments + { + name: "add_label_then_comment_then_remove", + beforeCombined: []*issues_model.Comment{ + addLabel(0, "a"), + commentText(10, "I'm a salmon"), + delLabel(20, "a"), + }, + sameAfter: true, + }, + + // ADD 2 = Combined labels + { + name: "combine_labels", + beforeCombined: []*issues_model.Comment{ + addLabel(0, "a"), + addLabel(10, "b"), + commentText(20, "I'm a salmon"), + addLabel(30, "c"), + addLabel(80, "d"), + addLabel(85, "e"), + delLabel(90, "c"), }, afterCombined: []*issues_model.Comment{ { - Type: issues_model.CommentTypeLabel, PosterID: 1, - Content: "1", - CreatedUnix: 0, - AddedLabels: []*issues_model.Label{}, - Label: &issues_model.Label{ - Name: "kind/bug", + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(0), + AddedLabels: []*issues_model.Label{ + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, }, }, + commentText(20, "I'm a salmon"), { - Type: issues_model.CommentTypeComment, PosterID: 1, - Content: "test", - CreatedUnix: 0, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(30), + AddedLabels: []*issues_model.Label{ + {Name: "d", ID: nameToID("d")}, + {Name: "e", ID: nameToID("e")}, + }, }, }, }, + + // ADD 1, then 1 later = 2 separate comments { - name: "kase 2", + name: "add_then_later_label", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 70, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, + addLabel(0, "a"), + addLabel(60, "b"), + addLabel(121, "c"), }, afterCombined: []*issues_model.Comment{ { - Type: issues_model.CommentTypeLabel, PosterID: 1, - Content: "1", - CreatedUnix: 0, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(0), AddedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, }, - Label: &issues_model.Label{ - Name: "kind/bug", + }, + addLabel(121, "c"), + }, + }, + + // ADD 2 then REMOVE 1 = label + { + name: "add_2_remove_1", + beforeCombined: []*issues_model.Comment{ + addLabel(0, "a"), + addLabel(10, "b"), + delLabel(20, "a"), + }, + afterCombined: []*issues_model.Comment{ + // The timestamp will be the one of the first aggregated comment + addLabel(0, "b"), + }, + }, + + // ADD then REMOVE multiple = nothing + { + name: "add_multiple_remove_all", + beforeCombined: []*issues_model.Comment{ + addLabel(0, "a"), + addLabel(1, "b"), + addLabel(2, "c"), + addLabel(3, "d"), + addLabel(4, "e"), + delLabel(5, "d"), + delLabel(6, "a"), + delLabel(7, "e"), + delLabel(8, "c"), + delLabel(9, "b"), + }, + afterCombined: nil, + }, + + // ADD 2, wait, REMOVE 2 = +2 then -2 comments + { + name: "add2_wait_rm2_labels", + beforeCombined: []*issues_model.Comment{ + addLabel(0, "a"), + addLabel(1, "b"), + delLabel(120, "a"), + delLabel(121, "b"), + }, + afterCombined: []*issues_model.Comment{ + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(0), + AddedLabels: []*issues_model.Label{ + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, }, }, { - Type: issues_model.CommentTypeLabel, PosterID: 1, - Content: "", - CreatedUnix: 70, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(120), RemovedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, }, - Label: &issues_model.Label{ - Name: "kind/bug", - }, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, - }, - }, - { - name: "kase 3", - beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 2, - Content: "", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, - }, - afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - CreatedUnix: 0, - AddedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, - }, - Label: &issues_model.Label{ - Name: "kind/bug", - }, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 2, - Content: "", - CreatedUnix: 0, - RemovedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, - }, - Label: &issues_model.Label{ - Name: "kind/bug", - }, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, - }, - }, - { - name: "kase 4", - beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/backport", - }, - CreatedUnix: 10, - }, - }, - afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - CreatedUnix: 10, - AddedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, - { - Name: "kind/backport", - }, - }, - Label: &issues_model.Label{ - Name: "kind/bug", - }, - }, - }, - }, - { - name: "kase 5", - beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 2, - Content: "testtest", - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - }, - afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - AddedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 2, - Content: "testtest", - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "", - RemovedLabels: []*issues_model.Label{ - { - Name: "kind/bug", - }, - }, - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - }, - }, - { - name: "kase 6", - beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "reviewed/confirmed", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/feature", - }, - CreatedUnix: 0, - }, - }, - afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeLabel, - PosterID: 1, - Content: "1", - Label: &issues_model.Label{ - Name: "kind/bug", - }, - AddedLabels: []*issues_model.Label{ - { - Name: "reviewed/confirmed", - }, - { - Name: "kind/feature", - }, - }, - CreatedUnix: 0, }, }, }, } for _, kase := range kases { - t.Run(kase.name, func(t *testing.T) { - issue := issues_model.Issue{ - Comments: kase.beforeCombined, - } - combineLabelComments(&issue) - assert.EqualValues(t, kase.afterCombined, issue.Comments) - }) + t.Run(kase.name, kase.doTest) } } func TestCombineReviewRequests(t *testing.T) { - testCases := []struct { - name string - beforeCombined []*issues_model.Comment - afterCombined []*issues_model.Comment - }{ + kases := []testCase{ + // ADD single = normal request review comment { - name: "case 1", + name: "add_single_review", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, + reqReview(0, "toto", false), + commentText(10, "I'm a salmon"), + reqReview(0, "toto-team", false), + }, + sameAfter: true, + }, + + // ADD then REMOVE = Nothing + { + name: "add_then_remove_review", + beforeCombined: []*issues_model.Comment{ + reqReview(0, "toto", false), + reqReview(5, "toto", true), + commentText(10, "I'm a salmon"), }, afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{}, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, + commentText(10, "I'm a salmon"), }, }, + + // ADD 1 then comment then REMOVE = separate comments { - name: "case 2", + name: "add_comment_del_review", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 2, - Name: "Ghost 2", - }, - CreatedUnix: 0, - }, + reqReview(0, "toto", false), + commentText(5, "I'm a salmon"), + reqReview(10, "toto", true), + }, + sameAfter: true, + }, + + // ADD 2 = Combined request reviews + { + name: "combine_reviews", + beforeCombined: []*issues_model.Comment{ + reqReview(0, "toto", false), + reqReview(10, "tutu-team", false), + commentText(20, "I'm a salmon"), + reqReview(30, "titi", false), + reqReview(80, "tata", false), + reqReview(85, "tyty-team", false), + reqReview(90, "titi", true), }, afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{ - &RequestReviewTarget{ - user: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, - &RequestReviewTarget{ - user: &user_model.User{ - ID: 2, - Name: "Ghost 2", - }, - }, - }, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, + reqReviewList(0, false, "toto", "tutu-team"), + commentText(20, "I'm a salmon"), + reqReviewList(30, false, "tata", "tyty-team"), }, }, + + // ADD 1, then 1 later = 2 separate comments { - name: "case 3", + name: "add_then_later_review", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, + reqReview(0, "titi", false), + reqReview(60, "toto-team", false), + reqReview(121, "tutu", false), }, afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{ - &RequestReviewTarget{ - user: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, - }, - RemovedRequestReview: []issues_model.RequestReviewTarget{ - &RequestReviewTarget{ - team: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - }, - }, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, + reqReviewList(0, false, "titi", "toto-team"), + reqReviewList(121, false, "tutu"), }, }, + + // ADD 2 then REMOVE 1 = single request review { - name: "case 4", + name: "add_2_then_remove_review", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, + reqReview(0, "titi-team", false), + reqReview(59, "toto", false), + reqReview(60, "titi-team", true), }, afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{ - &RequestReviewTarget{ - user: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, - }, - RemovedRequestReview: []issues_model.RequestReviewTarget{}, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, + reqReviewList(0, false, "toto"), }, }, + + // ADD then REMOVE multiple = nothing { - name: "case 5", + name: "add_multiple_then_remove_all_review", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - }, - afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{}, - RemovedRequestReview: []issues_model.RequestReviewTarget{}, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, + reqReview(0, "titi0-team", false), + reqReview(1, "toto1", false), + reqReview(2, "titi2", false), + reqReview(3, "titi3-team", false), + reqReview(4, "titi4", false), + reqReview(5, "titi5", false), + reqReview(6, "titi6-team", false), + reqReview(10, "titi0-team", true), + reqReview(11, "toto1", true), + reqReview(12, "titi2", true), + reqReview(13, "titi3-team", true), + reqReview(14, "titi4", true), + reqReview(15, "titi5", true), + reqReview(16, "titi6-team", true), }, + afterCombined: nil, }, + + // ADD 2, wait, REMOVE 2 = +2 then -2 comments { - name: "case 6", + name: "add2_wait_rm2_requests", beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - RemovedAssignee: true, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, + reqReview(1, "titi", false), + reqReview(2, "toto-team", false), + reqReview(121, "titi", true), + reqReview(122, "toto-team", true), }, afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - RemovedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ - team: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - }}, - AddedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ - user: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }}, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, - { - Type: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ - team: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - }}, - RemovedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ - user: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }}, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - }, - }, - }, - { - name: "case 7", - beforeCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - CreatedUnix: 0, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - CreatedUnix: 61, - }, - }, - afterCombined: []*issues_model.Comment{ - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - AddedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ - user: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }}, - Assignee: &user_model.User{ - ID: 1, - Name: "Ghost", - }, - }, - { - Type: issues_model.CommentTypeReviewRequest, - PosterID: 1, - CreatedUnix: 0, - RemovedRequestReview: []issues_model.RequestReviewTarget{&RequestReviewTarget{ - team: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - }}, - AssigneeTeam: &org_model.Team{ - ID: 1, - Name: "Team 1", - }, - }, + reqReviewList(1, false, "titi", "toto-team"), + reqReviewList(121, true, "titi", "toto-team"), }, }, } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - issue := issues_model.Issue{ - Comments: testCase.beforeCombined, - } - combineRequestReviewComments(&issue) - assert.EqualValues(t, testCase.afterCombined[0], issue.Comments[0]) - }) + for _, kase := range kases { + t.Run(kase.name, kase.doTest) + } +} + +func TestCombineOpenClose(t *testing.T) { + kases := []testCase{ + // Close then open = nullified + { + name: "close_open_nullified", + beforeCombined: []*issues_model.Comment{ + openOrClose(0, true), + openOrClose(10, false), + }, + afterCombined: nil, + }, + + // Close then open later = separate comments + { + name: "close_open_later", + beforeCombined: []*issues_model.Comment{ + openOrClose(0, true), + openOrClose(61, false), + }, + sameAfter: true, + }, + + // Close then comment then open = separate comments + { + name: "close_comment_open", + beforeCombined: []*issues_model.Comment{ + openOrClose(0, true), + commentText(1, "I'm a salmon"), + openOrClose(2, false), + }, + sameAfter: true, + }, + } + + for _, kase := range kases { + t.Run(kase.name, kase.doTest) + } +} + +func TestCombineMultipleDifferentComments(t *testing.T) { + lbl_a := createLabel("a") + kases := []testCase{ + // Add Label + Close + ReqReview = Combined + { + name: "label_close_reqreview_combined", + beforeCombined: []*issues_model.Comment{ + reqReview(1, "toto", false), + addLabel(2, "a"), + openOrClose(3, true), + + reqReview(101, "toto", true), + openOrClose(102, false), + delLabel(103, "a"), + }, + afterCombined: []*issues_model.Comment{ + aggregatedComment(1, + true, + []*issues_model.Label{&lbl_a}, + []*issues_model.Label{}, + []issues_model.RequestReviewTarget{createReqReviewTarget("toto")}, + []issues_model.RequestReviewTarget{}, + ), + aggregatedComment(101, + false, + []*issues_model.Label{}, + []*issues_model.Label{&lbl_a}, + []issues_model.RequestReviewTarget{}, + []issues_model.RequestReviewTarget{createReqReviewTarget("toto")}, + ), + }, + }, + + // Add Req + Add Label + Close + Del Req + Del Label = Close only + { + name: "req_label_close_dellabel_delreq", + beforeCombined: []*issues_model.Comment{ + addLabel(2, "a"), + reqReview(3, "titi", false), + openOrClose(4, true), + delLabel(5, "a"), + reqReview(6, "titi", true), + }, + afterCombined: []*issues_model.Comment{ + openOrClose(2, true), + }, + }, + + // Close + Add Req + Add Label + Del Req + Open = Label only + { + name: "close_req_label_open_delreq", + beforeCombined: []*issues_model.Comment{ + openOrClose(2, true), + reqReview(4, "titi", false), + addLabel(5, "a"), + reqReview(6, "titi", true), + openOrClose(8, false), + }, + afterCombined: []*issues_model.Comment{ + addLabel(2, "a"), + }, + }, + + // Add Label + Close + Add ReqReview + Del Label + Open = ReqReview only + { + name: "label_close_req_dellabel_open", + beforeCombined: []*issues_model.Comment{ + addLabel(1, "a"), + openOrClose(2, true), + reqReview(4, "titi", false), + openOrClose(7, false), + delLabel(8, "a"), + }, + afterCombined: []*issues_model.Comment{ + reqReview(1, "titi", false), + }, + }, + + // Add Label + Close + ReqReview, then delete everything = nothing + { + name: "add_multiple_delete_everything", + beforeCombined: []*issues_model.Comment{ + addLabel(1, "a"), + openOrClose(2, true), + reqReview(4, "titi", false), + openOrClose(7, false), + delLabel(8, "a"), + reqReview(10, "titi", true), + }, + afterCombined: nil, + }, + + // Add multiple, then comment, then delete everything = separate aggregation + { + name: "add_multiple_comment_delete_everything", + beforeCombined: []*issues_model.Comment{ + addLabel(1, "a"), + openOrClose(2, true), + reqReview(4, "titi", false), + + commentText(6, "I'm a salmon"), + + openOrClose(7, false), + delLabel(8, "a"), + reqReview(10, "titi", true), + }, + afterCombined: []*issues_model.Comment{ + aggregatedComment(1, + true, + []*issues_model.Label{&lbl_a}, + []*issues_model.Label{}, + []issues_model.RequestReviewTarget{createReqReviewTarget("titi")}, + []issues_model.RequestReviewTarget{}, + ), + commentText(6, "I'm a salmon"), + aggregatedComment(7, + false, + []*issues_model.Label{}, + []*issues_model.Label{&lbl_a}, + []issues_model.RequestReviewTarget{}, + []issues_model.RequestReviewTarget{createReqReviewTarget("titi")}, + ), + }, + }, + } + + for _, kase := range kases { + t.Run(kase.name, kase.doTest) } } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index c8eabb9345..f02e7195c8 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -524,7 +524,7 @@ {{else if eq .Type 27}} - {{if or .AddedRequestReview .RemovedRequestReview}} + {{if or .AddedRequestReview .RemovedRequestReview}}
{{svg "octicon-tag"}} {{template "shared/user/avatarlink" dict "user" .Poster}} @@ -540,7 +540,7 @@ {{end}}
- {{end}} + {{end}} {{else if and (eq .Type 29) (or (gt .CommitsNum 0) .IsForcePush)}} {{if and .Issue.IsClosed (gt .ID $.LatestCloseCommentID)}} @@ -676,6 +676,73 @@ {{else}}{{ctx.Locale.Tr "repo.issues.unpin_comment" $createdStr}}{{end}} + {{else if eq .Type 38}} +
+ {{svg "octicon-list-unordered" 16}} + {{template "shared/user/avatarlink" dict "user" .Poster}} + + + {{template "shared/user/authorlink" .Poster}} + {{ $createdStr }} + + + +
{{end}} {{end}} {{end}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index e9cfc1ddde..34c36e56b3 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -840,6 +840,22 @@ td .commit-summary { margin-right: 0.25em; } +.comment-aggregator { + list-style-type: none; +} + +.repository.view.issue .comment-list .timeline-item .comment-aggregator .badge { + width: 20px; + height: 20px; + margin-top: 5px; + padding: 12px; +} + +.repository.view.issue .comment-list .timeline-item .comment-aggregator .badge .svg { + width: 20px; + height: 20px; +} + .singular-commit { display: flex; align-items: center;