diff --git a/models/issues/action_aggregator.go b/models/issues/action_aggregator.go new file mode 100644 index 0000000000..722696fbb2 --- /dev/null +++ b/models/issues/action_aggregator.go @@ -0,0 +1,375 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package issues + +import ( + "slices" + + "code.gitea.io/gitea/models/organization" + user_model "code.gitea.io/gitea/models/user" +) + +type ActionAggregator struct { + StartUnix int64 + AggAge int64 + PosterID int64 + StartInd int + EndInd int + + PrevClosed bool + IsClosed bool + + AddedLabels []*Label + RemovedLabels []*Label + + AddedRequestReview []RequestReviewTarget + RemovedRequestReview []RequestReviewTarget +} + +// Get the time threshold for aggregation of multiple actions together +func (agg *ActionAggregator) timeThreshold() int64 { + if agg.AggAge > (60 * 60 * 24 * 30) { // Age > 1 month, aggregate by day + return 60 * 60 * 24 + } else if agg.AggAge > (60 * 60 * 24) { // Age > 1 day, aggregate by hour + return 60 * 60 + } else if agg.AggAge > (60 * 60) { // Age > 1 hour, aggregate by 10 mins + return 60 * 10 + } + // Else, aggregate by minute + return 60 +} + +// TODO Aggregate also +// - Dependency added / removed +// - Added / Removed due date +// - Milestone Added / Removed +func (agg *ActionAggregator) aggregateAction(c *Comment, index int) { + if agg.StartInd == -1 { + agg.StartInd = index + } + agg.EndInd = 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 *ActionAggregator) Merge(next *ActionAggregator) { + 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 *ActionAggregator) IsAggregated(t *CommentType) bool { + switch *t { + case CommentTypeAggregator, CommentTypeClose, CommentTypeReopen, CommentTypeLabel, CommentTypeReviewRequest: + { + return true + } + default: + { + return false + } + } +} + +// Add a label to the aggregated list +func (agg *ActionAggregator) addLabel(lbl *Label) { + for l, agglbl := range agg.RemovedLabels { + if agglbl.ID == lbl.ID { + agg.RemovedLabels = slices.Delete(agg.RemovedLabels, l, 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 *ActionAggregator) delLabel(lbl *Label) { + for l, agglbl := range agg.AddedLabels { + if agglbl.ID == lbl.ID { + agg.AddedLabels = slices.Delete(agg.AddedLabels, l, 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 *ActionAggregator) addReviewRequest(req RequestReviewTarget) { + reqid := req.ID() + reqty := req.Type() + for r, aggreq := range agg.RemovedRequestReview { + if (aggreq.ID() == reqid) && (aggreq.Type() == reqty) { + agg.RemovedRequestReview = slices.Delete(agg.RemovedRequestReview, r, 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 *ActionAggregator) delReviewRequest(req RequestReviewTarget) { + reqid := req.ID() + reqty := req.Type() + for r, aggreq := range agg.AddedRequestReview { + if (aggreq.ID() == reqid) && (aggreq.Type() == reqty) { + agg.AddedRequestReview = slices.Delete(agg.AddedRequestReview, r, 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 *ActionAggregator) Changed() bool { + return (agg.IsClosed != agg.PrevClosed) || + (len(agg.AddedLabels) > 0) || + (len(agg.RemovedLabels) > 0) || + (len(agg.AddedRequestReview) > 0) || + (len(agg.RemovedRequestReview) > 0) +} + +func (agg *ActionAggregator) OnlyLabelsChanged() bool { + return ((len(agg.AddedLabels) > 0) || (len(agg.RemovedLabels) > 0)) && + (len(agg.AddedRequestReview) == 0) && (len(agg.RemovedRequestReview) == 0) && + (agg.PrevClosed == agg.IsClosed) +} + +func (agg *ActionAggregator) OnlyRequestReview() bool { + return ((len(agg.AddedRequestReview) > 0) || (len(agg.RemovedRequestReview) > 0)) && + (len(agg.AddedLabels) == 0) && (len(agg.RemovedLabels) == 0) && + (agg.PrevClosed == agg.IsClosed) +} + +func (agg *ActionAggregator) OnlyClosedReopened() bool { + return (agg.IsClosed != agg.PrevClosed) && + (len(agg.AddedLabels) == 0) && (len(agg.RemovedLabels) == 0) && + (len(agg.AddedRequestReview) == 0) && (len(agg.RemovedRequestReview) == 0) +} + +// Reset the aggregator to start a new aggregating context +func (agg *ActionAggregator) Reset(cur *Comment, now int64) { + agg.StartUnix = int64(cur.CreatedUnix) + agg.AggAge = now - agg.StartUnix + agg.PosterID = cur.PosterID + + agg.PrevClosed = agg.IsClosed + + agg.StartInd = -1 + agg.EndInd = -1 + 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 *ActionAggregator) createAggregatedComment(issue *Issue, final bool) int { + // If the aggregation of comments make the whole thing null, erase all the comments + if !agg.Changed() { + if final { + issue.Comments = issue.Comments[:agg.StartInd] + } else { + issue.Comments = slices.Replace(issue.Comments, agg.StartInd, agg.EndInd+1) + } + return (agg.EndInd - agg.StartInd) + 1 + } + + newAgg := *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[agg.StartInd] + comment.Content = "" + comment.Label = nil + comment.Aggregator = nil + comment.Assignee = nil + comment.AssigneeID = 0 + comment.AssigneeTeam = nil + comment.AssigneeTeamID = 0 + 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 = &newAgg + } + + if len(newAgg.AddedLabels) > 0 { + comment.AddedLabels = newAgg.AddedLabels + } + + if len(newAgg.RemovedLabels) > 0 { + comment.RemovedLabels = newAgg.RemovedLabels + } + + if len(newAgg.AddedRequestReview) > 0 { + comment.AddedRequestReview = newAgg.AddedRequestReview + } + + if len(newAgg.RemovedRequestReview) > 0 { + comment.RemovedRequestReview = newAgg.RemovedRequestReview + } + + if final { + issue.Comments = append(issue.Comments[:agg.StartInd], comment) + } else { + issue.Comments = slices.Replace(issue.Comments, agg.StartInd, agg.EndInd+1, comment) + } + return agg.EndInd - agg.StartInd +} + +// combineCommentsHistory combines nearby elements in the history as one +func CombineCommentsHistory(issue *Issue, now int64) { + if len(issue.Comments) < 1 { + return + } + + // Initialise a new empty aggregator, ready to combine comments + var agg ActionAggregator + agg.Reset(issue.Comments[0], now) + + for i := 0; i < len(issue.Comments); i++ { + cur := issue.Comments[i] + // 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 agg.StartInd != -1 { + i -= agg.createAggregatedComment(issue, false) + } + + agg.StartInd = -1 + if i+1 < len(issue.Comments) { + agg.Reset(issue.Comments[i+1], now) + } + + // 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 + threshold := agg.timeThreshold() + if ((int64(cur.CreatedUnix) - agg.StartUnix) > threshold) || (cur.PosterID != agg.PosterID) { + // First, create the aggregated comment if there's data in it + if agg.StartInd != -1 { + i -= agg.createAggregatedComment(issue, false) + } + agg.Reset(cur, now) + } + + agg.aggregateAction(cur, i) + } + + // Create the aggregated comment if there's data in it + if agg.StartInd != -1 { + agg.createAggregatedComment(issue, true) + } +} + +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" +} diff --git a/models/issues/comment.go b/models/issues/comment.go index 0bf53bb4dd..f77b2bcc8d 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{ @@ -155,6 +157,7 @@ var commentStrings = []string{ "pull_cancel_scheduled_merge", "pin", "unpin", + "action_aggregator", } func (t CommentType) String() string { @@ -236,12 +239,6 @@ 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 -} - // Comment represents a comment in commit and issue page. type Comment struct { ID int64 `xorm:"pk autoincr"` @@ -254,6 +251,7 @@ type Comment struct { Issue *Issue `xorm:"-"` LabelID int64 Label *Label `xorm:"-"` + Aggregator *ActionAggregator `xorm:"-"` AddedLabels []*Label `xorm:"-"` RemovedLabels []*Label `xorm:"-"` AddedRequestReview []RequestReviewTarget `xorm:"-"` diff --git a/release-notes/6523.md b/release-notes/6523.md new file mode 100644 index 0000000000..96a1fe0cf3 --- /dev/null +++ b/release-notes/6523.md @@ -0,0 +1 @@ +Reduce noise in the timeline of issues and pull requests. If certain timeline events are performed within a certain timeframe of each other with no other events in between, they will be combined into a single timeline event, and any contradictory actions will be canceled and not displayed. The older the events, the wider the timeframe will become. diff --git a/routers/web/repo/action_aggregator_test.go b/routers/web/repo/action_aggregator_test.go new file mode 100644 index 0000000000..181c1120db --- /dev/null +++ b/routers/web/repo/action_aggregator_test.go @@ -0,0 +1,800 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package repo + +import ( + "strings" + "testing" + + issue_model "code.gitea.io/gitea/models/issues" + "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) *issue_model.Comment { + return &issue_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) issue_model.RequestReviewTarget { + if strings.HasSuffix(name, "-team") { + team := createTeam(name) + return issue_model.RequestReviewTarget{Team: &team} + } + user := createUser(name) + return issue_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) issue_model.Label { + return issue_model.Label{Name: name, ID: nameToID(name)} +} + +func addLabel(t int64, name string) *issue_model.Comment { + c := testComment(t) + c.Type = issue_model.CommentTypeLabel + c.Content = "1" + lbl := createLabel(name) + c.Label = &lbl + c.AddedLabels = []*issue_model.Label{&lbl} + return c +} + +func delLabel(t int64, name string) *issue_model.Comment { + c := addLabel(t, name) + c.Content = "" + c.RemovedLabels = c.AddedLabels + c.AddedLabels = nil + return c +} + +func openOrClose(t int64, close bool) *issue_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) *issue_model.Comment { + c := testComment(t) + c.Type = issue_model.CommentTypeReviewRequest + if strings.HasSuffix(name, "-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) *issue_model.Comment { + req := []issue_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 []*issue_model.Label, + delLabels []*issue_model.Label, + addReqReview []issue_model.RequestReviewTarget, + delReqReview []issue_model.RequestReviewTarget, +) *issue_model.Comment { + cmnt := testComment(t) + cmnt.Type = issue_model.CommentTypeAggregator + cmnt.Aggregator = &issue_model.ActionAggregator{ + 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) *issue_model.Comment { + c := testComment(t) + c.Type = issue_model.CommentTypeComment + c.Content = text + return c +} + +// **************************************************************** + +type testCase struct { + name string + beforeCombined []*issue_model.Comment + afterCombined []*issue_model.Comment + sameAfter bool + timestampCombination int64 +} + +func (kase *testCase) doTest(t *testing.T) { + issue := issue_model.Issue{Comments: kase.beforeCombined} + + var now int64 = -9223372036854775808 + for c := 0; c < len(kase.beforeCombined); c++ { + assert.Greater(t, int64(kase.beforeCombined[c].CreatedUnix), now) + now = int64(kase.beforeCombined[c].CreatedUnix) + } + + if kase.timestampCombination != 0 { + now = kase.timestampCombination + } + + issue_model.CombineCommentsHistory(&issue, now) + + 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.PrevClosed = false + r.Aggregator.PosterID = 0 + r.Aggregator.StartInd = 0 + r.Aggregator.EndInd = 0 + r.Aggregator.AggAge = 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 = 0 + l.AssigneeTeam = nil + l.AssigneeTeamID = 0 + } + + assert.EqualValues(t, (after)[c], issue.Comments[c], + "Comment %v is not equal", c, + ) + } +} + +// **************** Start of the tests ****************** + +func TestCombineLabelComments(t *testing.T) { + var tmon int64 = 60 * 60 * 24 * 30 + var tday int64 = 60 * 60 * 24 + var thour int64 = 60 * 60 + kases := []testCase{ + // ADD single = normal label comment + { + name: "add_single_label", + beforeCombined: []*issue_model.Comment{ + addLabel(0, "a"), + commentText(10, "I'm a salmon"), + }, + sameAfter: true, + }, + + // ADD then REMOVE = Nothing + { + name: "add_label_then_remove", + beforeCombined: []*issue_model.Comment{ + addLabel(0, "a"), + delLabel(1, "a"), + commentText(65, "I'm a salmon"), + }, + afterCombined: []*issue_model.Comment{ + commentText(65, "I'm a salmon"), + }, + }, + + // ADD 1 then comment then REMOVE = separate comments + { + name: "add_label_then_comment_then_remove", + beforeCombined: []*issue_model.Comment{ + addLabel(0, "a"), + commentText(10, "I'm a salmon"), + delLabel(20, "a"), + }, + sameAfter: true, + }, + + // ADD 2 = Combined labels + { + name: "combine_labels", + beforeCombined: []*issue_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: []*issue_model.Comment{ + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(0), + AddedLabels: []*issue_model.Label{ + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, + }, + }, + commentText(20, "I'm a salmon"), + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(30), + AddedLabels: []*issue_model.Label{ + {Name: "d", ID: nameToID("d")}, + {Name: "e", ID: nameToID("e")}, + }, + }, + }, + }, + + // ADD 1, then 1 later = 2 separate comments + { + name: "add_then_later_label", + beforeCombined: []*issue_model.Comment{ + addLabel(0, "a"), + addLabel(60, "b"), + addLabel(121, "c"), + }, + afterCombined: []*issue_model.Comment{ + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(0), + AddedLabels: []*issue_model.Label{ + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, + }, + }, + addLabel(121, "c"), + }, + }, + + // ADD 2 then REMOVE 1 = label + { + name: "add_2_remove_1", + beforeCombined: []*issue_model.Comment{ + addLabel(0, "a"), + addLabel(10, "b"), + delLabel(20, "a"), + }, + afterCombined: []*issue_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: []*issue_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: []*issue_model.Comment{ + addLabel(0, "a"), + addLabel(1, "b"), + delLabel(120, "a"), + delLabel(121, "b"), + }, + afterCombined: []*issue_model.Comment{ + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(0), + AddedLabels: []*issue_model.Label{ + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, + }, + }, + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(120), + RemovedLabels: []*issue_model.Label{ + {Name: "a", ID: nameToID("a")}, + {Name: "b", ID: nameToID("b")}, + }, + }, + }, + }, + + // Regression check on edge case + { + name: "regression_edgecase_finalagg", + beforeCombined: []*issue_model.Comment{ + commentText(0, "hey"), + commentText(1, "ho"), + addLabel(2, "a"), + addLabel(3, "b"), + delLabel(4, "a"), + delLabel(5, "b"), + + addLabel(120, "a"), + + addLabel(220, "c"), + addLabel(221, "d"), + addLabel(222, "e"), + delLabel(223, "d"), + + delLabel(400, "a"), + }, + afterCombined: []*issue_model.Comment{ + commentText(0, "hey"), + commentText(1, "ho"), + addLabel(120, "a"), + { + PosterID: 1, + Type: issue_model.CommentTypeLabel, + CreatedUnix: timeutil.TimeStamp(220), + AddedLabels: []*issue_model.Label{ + {Name: "c", ID: nameToID("c")}, + {Name: "e", ID: nameToID("e")}, + }, + }, + delLabel(400, "a"), + }, + }, + + { + name: "combine_label_high_timestamp_separated", + timestampCombination: tmon + 1, + beforeCombined: []*issue_model.Comment{ + // 1 month old, comments separated by 1 Day + 1 sec (not agg) + addLabel(0, "d"), + delLabel(tday+1, "d"), + + // 1 day old, comments separated by 1 hour + 1 sec (not agg) + addLabel((tmon-tday)-thour, "c"), + delLabel((tmon-tday)+1, "c"), + + // 1 hour old, comments separated by 10 mins + 1 sec (not agg) + addLabel(tmon-thour, "b"), + delLabel((tmon-(50*60))+1, "b"), + + // Else, aggregate by minute + addLabel(tmon-61, "a"), + delLabel(tmon, "a"), + }, + sameAfter: true, + }, + + // Test higher timestamp diff + { + name: "combine_label_high_timestamp_merged", + timestampCombination: tmon + 1, + beforeCombined: []*issue_model.Comment{ + // 1 month old, comments separated by 1 Day (aggregated) + addLabel(0, "d"), + delLabel(tday, "d"), + + // 1 day old, comments separated by 1 hour (aggregated) + addLabel((tmon-tday)-thour, "c"), + delLabel(tmon-tday, "c"), + + // 1 hour old, comments separated by 10 mins (aggregated) + addLabel(tmon-thour, "b"), + delLabel(tmon-(50*60), "b"), + + addLabel(tmon-60, "a"), + delLabel(tmon, "a"), + }, + }, + } + + for _, kase := range kases { + t.Run(kase.name, kase.doTest) + } +} + +func TestCombineReviewRequests(t *testing.T) { + kases := []testCase{ + // ADD single = normal request review comment + { + name: "add_single_review", + beforeCombined: []*issue_model.Comment{ + reqReview(0, "toto", false), + commentText(10, "I'm a salmon"), + reqReview(20, "toto-team", false), + }, + sameAfter: true, + }, + + // ADD then REMOVE = Nothing + { + name: "add_then_remove_review", + beforeCombined: []*issue_model.Comment{ + reqReview(0, "toto", false), + reqReview(5, "toto", true), + commentText(10, "I'm a salmon"), + }, + afterCombined: []*issue_model.Comment{ + commentText(10, "I'm a salmon"), + }, + }, + + // ADD 1 then comment then REMOVE = separate comments + { + name: "add_comment_del_review", + beforeCombined: []*issue_model.Comment{ + 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: []*issue_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: []*issue_model.Comment{ + 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: "add_then_later_review", + beforeCombined: []*issue_model.Comment{ + reqReview(0, "titi", false), + reqReview(60, "toto-team", false), + reqReview(121, "tutu", false), + }, + afterCombined: []*issue_model.Comment{ + reqReviewList(0, false, "titi", "toto-team"), + reqReviewList(121, false, "tutu"), + }, + }, + + // ADD 2 then REMOVE 1 = single request review + { + name: "add_2_then_remove_review", + beforeCombined: []*issue_model.Comment{ + reqReview(0, "titi-team", false), + reqReview(59, "toto", false), + reqReview(60, "titi-team", true), + }, + afterCombined: []*issue_model.Comment{ + reqReviewList(0, false, "toto"), + }, + }, + + // ADD then REMOVE multiple = nothing + { + name: "add_multiple_then_remove_all_review", + beforeCombined: []*issue_model.Comment{ + 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: "add2_wait_rm2_requests", + beforeCombined: []*issue_model.Comment{ + reqReview(1, "titi", false), + reqReview(2, "toto-team", false), + reqReview(121, "titi", true), + reqReview(122, "toto-team", true), + }, + afterCombined: []*issue_model.Comment{ + reqReviewList(1, false, "titi", "toto-team"), + reqReviewList(121, true, "titi", "toto-team"), + }, + }, + } + + 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: []*issue_model.Comment{ + openOrClose(0, true), + openOrClose(10, false), + }, + afterCombined: nil, + }, + + // Close then open later = separate comments + { + name: "close_open_later", + beforeCombined: []*issue_model.Comment{ + openOrClose(0, true), + openOrClose(61, false), + }, + sameAfter: true, + }, + + // Close then comment then open = separate comments + { + name: "close_comment_open", + beforeCombined: []*issue_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) { + lblA := createLabel("a") + kases := []testCase{ + // Add Label + Close + ReqReview = Combined + { + name: "label_close_reqreview_combined", + beforeCombined: []*issue_model.Comment{ + reqReview(1, "toto", false), + addLabel(2, "a"), + openOrClose(3, true), + + reqReview(101, "toto", true), + openOrClose(102, false), + delLabel(103, "a"), + }, + afterCombined: []*issue_model.Comment{ + aggregatedComment(1, + true, + []*issue_model.Label{&lblA}, + []*issue_model.Label{}, + []issue_model.RequestReviewTarget{createReqReviewTarget("toto")}, + []issue_model.RequestReviewTarget{}, + ), + aggregatedComment(101, + false, + []*issue_model.Label{}, + []*issue_model.Label{&lblA}, + []issue_model.RequestReviewTarget{}, + []issue_model.RequestReviewTarget{createReqReviewTarget("toto")}, + ), + }, + }, + + // Add Req + Add Label + Close + Del Req + Del Label = Close only + { + name: "req_label_close_dellabel_delreq", + beforeCombined: []*issue_model.Comment{ + addLabel(2, "a"), + reqReview(3, "titi", false), + openOrClose(4, true), + delLabel(5, "a"), + reqReview(6, "titi", true), + }, + afterCombined: []*issue_model.Comment{ + openOrClose(2, true), + }, + }, + + // Close + Add Req + Add Label + Del Req + Open = Label only + { + name: "close_req_label_open_delreq", + beforeCombined: []*issue_model.Comment{ + openOrClose(2, true), + reqReview(4, "titi", false), + addLabel(5, "a"), + reqReview(6, "titi", true), + openOrClose(8, false), + }, + afterCombined: []*issue_model.Comment{ + addLabel(2, "a"), + }, + }, + + // Add Label + Close + Add ReqReview + Del Label + Open = ReqReview only + { + name: "label_close_req_dellabel_open", + beforeCombined: []*issue_model.Comment{ + addLabel(1, "a"), + openOrClose(2, true), + reqReview(4, "titi", false), + openOrClose(7, false), + delLabel(8, "a"), + }, + afterCombined: []*issue_model.Comment{ + reqReviewList(1, false, "titi"), + }, + }, + + // Add Label + Close + ReqReview, then delete everything = nothing + { + name: "add_multiple_delete_everything", + beforeCombined: []*issue_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: []*issue_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: []*issue_model.Comment{ + aggregatedComment(1, + true, + []*issue_model.Label{&lblA}, + []*issue_model.Label{}, + []issue_model.RequestReviewTarget{createReqReviewTarget("titi")}, + []issue_model.RequestReviewTarget{}, + ), + commentText(6, "I'm a salmon"), + aggregatedComment(7, + false, + []*issue_model.Label{}, + []*issue_model.Label{&lblA}, + []issue_model.RequestReviewTarget{}, + []issue_model.RequestReviewTarget{createReqReviewTarget("titi")}, + ), + }, + }, + + { + name: "regression_edgecase_finalagg", + beforeCombined: []*issue_model.Comment{ + commentText(0, "hey"), + commentText(1, "ho"), + addLabel(2, "a"), + reqReview(3, "titi", false), + delLabel(4, "a"), + reqReview(5, "titi", true), + + addLabel(120, "a"), + + openOrClose(220, true), + addLabel(221, "d"), + reqReview(222, "toto-team", false), + delLabel(223, "d"), + + delLabel(400, "a"), + }, + afterCombined: []*issue_model.Comment{ + commentText(0, "hey"), + commentText(1, "ho"), + addLabel(120, "a"), + aggregatedComment(220, + true, + []*issue_model.Label{}, + []*issue_model.Label{}, + []issue_model.RequestReviewTarget{createReqReviewTarget("toto-team")}, + []issue_model.RequestReviewTarget{}, + ), + delLabel(400, "a"), + }, + }, + } + + for _, kase := range kases { + t.Run(kase.name, kase.doTest) + } +} diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index a86d043497..e45abd3952 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1834,8 +1834,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, time.Now().Unix()) getBranchData(ctx, issue) if issue.IsPull { @@ -3710,194 +3709,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 deleted file mode 100644 index d642c14b5f..0000000000 --- a/routers/web/repo/issue_test.go +++ /dev/null @@ -1,806 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repo - -import ( - "testing" - - issues_model "code.gitea.io/gitea/models/issues" - org_model "code.gitea.io/gitea/models/organization" - user_model "code.gitea.io/gitea/models/user" - - "github.com/stretchr/testify/assert" -) - -func TestCombineLabelComments(t *testing.T) { - kases := []struct { - name string - beforeCombined []*issues_model.Comment - afterCombined []*issues_model.Comment - }{ - { - name: "kase 1", - 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, - }, - }, - 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: issues_model.CommentTypeComment, - PosterID: 1, - Content: "test", - CreatedUnix: 0, - }, - }, - }, - { - name: "kase 2", - 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, - }, - }, - 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: 1, - Content: "", - CreatedUnix: 70, - 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 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) - }) - } -} - -func TestCombineReviewRequests(t *testing.T) { - testCases := []struct { - name string - beforeCombined []*issues_model.Comment - afterCombined []*issues_model.Comment - }{ - { - name: "case 1", - 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, - }, - }, - 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, - }, - }, - }, - { - name: "case 2", - 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, - }, - }, - 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", - }, - }, - }, - }, - { - name: "case 3", - 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, - }, - }, - 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", - }, - }, - }, - }, - { - name: "case 4", - 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, - }, - }, - 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", - }, - }, - }, - }, - { - name: "case 5", - 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", - }, - }, - }, - }, - { - name: "case 6", - 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, - }, - }, - 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", - }, - }, - }, - }, - } - - 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]) - }) - } -} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 604efed737..bea517dfa2 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -12,7 +12,7 @@ 26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST, 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_BOARD_CHANGED 32 = DISMISSED_REVIEW, 33 = COMMENT_TYPE_CHANGE_ISSUE_REF, 34 = PR_SCHEDULE_TO_AUTO_MERGE, - 35 = CANCEL_SCHEDULED_AUTO_MERGE_PR, 36 = PIN_ISSUE, 37 = UNPIN_ISSUE --> + 35 = CANCEL_SCHEDULED_AUTO_MERGE_PR, 36 = PIN_ISSUE, 37 = UNPIN_ISSUE, 38 = ACTION_AGGREGATOR --> {{if eq .Type 0}}
{{if .OriginalAuthor}} @@ -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 46c6b70c29..1b962aacb6 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -759,7 +759,7 @@ td .commit-summary { .repository.view.issue .comment-list .timeline-item, .repository.view.issue .comment-list .timeline-item-group { - padding: 16px 0; + padding: 0.65rem 0; } .repository.view.issue .comment-list .timeline-item-group .timeline-item { @@ -842,6 +842,18 @@ td .commit-summary { margin-right: 0.25em; } +.repository.view.issue .comment-list .timeline-item .aggregated-actions .badge { + width: 20px; + height: 20px; + margin-top: 5px; + padding: 12px; +} + +.repository.view.issue .comment-list .timeline-item .aggregated-actions .badge .svg { + width: 20px; + height: 20px; +} + .singular-commit { display: flex; align-items: center;