From 8fdc0a7a6ced463eed481070c782c1bd405c0cfe Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 25 Oct 2024 09:24:36 +0200 Subject: [PATCH] feat: combine review requests comments - Combine review requests comments similairy how labels comments are combined. If review requests comments were made within 60 seconds of each other they will be grouped. - Integration and unit test added. - Resolves #2774 --- models/issues/comment.go | 78 ++-- modules/templates/helper.go | 1 + modules/templates/util_render.go | 12 + options/locale/locale_en-US.ini | 9 +- routers/web/repo/issue.go | 122 +++++ routers/web/repo/issue_test.go | 431 ++++++++++++++++++ .../repo/issue/view_content/comments.tmpl | 42 +- .../TestPullCombinedReviewRequest/review.yml | 21 + tests/integration/pull_test.go | 34 ++ 9 files changed, 685 insertions(+), 65 deletions(-) create mode 100644 tests/integration/fixtures/TestPullCombinedReviewRequest/review.yml diff --git a/models/issues/comment.go b/models/issues/comment.go index d53e5f5949..90c7122174 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -222,43 +222,51 @@ 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"` - Type CommentType `xorm:"INDEX"` - PosterID int64 `xorm:"INDEX"` - Poster *user_model.User `xorm:"-"` - OriginalAuthor string - OriginalAuthorID int64 - IssueID int64 `xorm:"INDEX"` - Issue *Issue `xorm:"-"` - LabelID int64 - Label *Label `xorm:"-"` - AddedLabels []*Label `xorm:"-"` - RemovedLabels []*Label `xorm:"-"` - OldProjectID int64 - ProjectID int64 - OldProject *project_model.Project `xorm:"-"` - Project *project_model.Project `xorm:"-"` - OldMilestoneID int64 - MilestoneID int64 - OldMilestone *Milestone `xorm:"-"` - Milestone *Milestone `xorm:"-"` - TimeID int64 - Time *TrackedTime `xorm:"-"` - AssigneeID int64 - RemovedAssignee bool - Assignee *user_model.User `xorm:"-"` - AssigneeTeamID int64 `xorm:"NOT NULL DEFAULT 0"` - AssigneeTeam *organization.Team `xorm:"-"` - ResolveDoerID int64 - ResolveDoer *user_model.User `xorm:"-"` - OldTitle string - NewTitle string - OldRef string - NewRef string - DependentIssueID int64 `xorm:"index"` // This is used by issue_service.deleteIssue - DependentIssue *Issue `xorm:"-"` + ID int64 `xorm:"pk autoincr"` + Type CommentType `xorm:"INDEX"` + PosterID int64 `xorm:"INDEX"` + Poster *user_model.User `xorm:"-"` + OriginalAuthor string + OriginalAuthorID int64 + IssueID int64 `xorm:"INDEX"` + Issue *Issue `xorm:"-"` + LabelID int64 + Label *Label `xorm:"-"` + AddedLabels []*Label `xorm:"-"` + RemovedLabels []*Label `xorm:"-"` + AddedRequestReview []RequestReviewTarget `xorm:"-"` + RemovedRequestReview []RequestReviewTarget `xorm:"-"` + OldProjectID int64 + ProjectID int64 + OldProject *project_model.Project `xorm:"-"` + Project *project_model.Project `xorm:"-"` + OldMilestoneID int64 + MilestoneID int64 + OldMilestone *Milestone `xorm:"-"` + Milestone *Milestone `xorm:"-"` + TimeID int64 + Time *TrackedTime `xorm:"-"` + AssigneeID int64 + RemovedAssignee bool + Assignee *user_model.User `xorm:"-"` + AssigneeTeamID int64 `xorm:"NOT NULL DEFAULT 0"` + AssigneeTeam *organization.Team `xorm:"-"` + ResolveDoerID int64 + ResolveDoer *user_model.User `xorm:"-"` + OldTitle string + NewTitle string + OldRef string + NewRef string + DependentIssueID int64 `xorm:"index"` // This is used by issue_service.deleteIssue + DependentIssue *Issue `xorm:"-"` CommitID int64 Line int64 // - previous line / + proposed line diff --git a/modules/templates/helper.go b/modules/templates/helper.go index aeae8204ad..5b5470b87e 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -182,6 +182,7 @@ func NewFuncMap() template.FuncMap { "RenderMarkdownToHtml": RenderMarkdownToHtml, "RenderLabel": RenderLabel, "RenderLabels": RenderLabels, + "RenderReviewRequest": RenderReviewRequest, // ----------------------------------------------------------------- // misc diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index c53bdd876f..3c337ae895 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -262,3 +262,15 @@ func RenderLabels(ctx context.Context, locale translation.Locale, labels []*issu htmlCode += "" return template.HTML(htmlCode) } + +func RenderReviewRequest(users []issues_model.RequestReviewTarget) template.HTML { + usernames := make([]string, 0, len(users)) + for _, user := range users { + usernames = append(usernames, template.HTMLEscapeString(user.Name())) + } + + htmlCode := `` + htmlCode += strings.Join(usernames, ", ") + htmlCode += "" + return template.HTML(htmlCode) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b87fd2a69b..cdfd409d96 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1791,9 +1791,12 @@ issues.review.left_comment = left a comment issues.review.content.empty = You need to leave a comment indicating the requested change(s). issues.review.reject = requested changes %s issues.review.wait = was requested for review %s -issues.review.add_review_request = requested review from %s %s -issues.review.remove_review_request = removed review request for %s %s -issues.review.remove_review_request_self = refused to review %s +issues.review.add_review_request = requested review from %[1]s %[2]s +issues.review.add_review_requests = requested reviews from %[1]s %[2]s +issues.review.remove_review_request = removed review request for %[1]s %[2]s +issues.review.remove_review_requests = removed review requests for %[1]s %[2]s +issues.review.remove_review_request_self = refused to review %[1]s %[2]s +issues.review.add_remove_review_requests = requested reviews from %[1]s and removed review requests for %[2]s %[3]s issues.review.pending = Pending issues.review.pending.tooltip = This comment is not currently visible to other users. To submit your pending comments, select "%s" -> "%s/%s/%s" at the top of the page. issues.review.review = Review diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 2f077572ac..a9351c6a55 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1825,6 +1825,7 @@ func ViewIssue(ctx *context.Context) { // Combine multiple label assignments into a single comment combineLabelComments(issue) + combineRequestReviewComments(issue) getBranchData(ctx, issue) if issue.IsPull { @@ -3665,6 +3666,127 @@ 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) + } + } + + // Propoagate 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 diff --git a/routers/web/repo/issue_test.go b/routers/web/repo/issue_test.go index f1d0aac72f..d642c14b5f 100644 --- a/routers/web/repo/issue_test.go +++ b/routers/web/repo/issue_test.go @@ -7,6 +7,8 @@ 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" ) @@ -373,3 +375,432 @@ func TestCombineLabelComments(t *testing.T) { }) } } + +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 08c83c07d7..78fa854dbe 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -522,35 +522,23 @@ {{else if eq .Type 27}} -
- {{svg "octicon-eye"}} - {{template "shared/user/avatarlink" dict "user" .Poster}} - - {{template "shared/user/authorlink" .Poster}} - {{if (gt .AssigneeID 0)}} - {{if .RemovedAssignee}} - {{if eq .PosterID .AssigneeID}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}} - {{else}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" .Assignee.GetDisplayName $createdStr}} - {{end}} + {{if or .AddedRequestReview .RemovedRequestReview}} +
+ {{svg "octicon-tag"}} + {{template "shared/user/avatarlink" dict "user" .Poster}} + + {{if and (eq (len .RemovedRequestReview) 1) (eq (len .AddedRequestReview) 0) (eq ((index .RemovedRequestReview 0).ID) .PosterID) (eq ((index .RemovedRequestReview 0).Type) "user")}} + {{ctx.Locale.Tr "repo.issues.review.remove_review_request_self" $createdStr}} + {{else if and .AddedRequestReview (not .RemovedRequestReview)}} + {{ctx.Locale.TrN (len .AddedRequestReview) "repo.issues.review.add_review_request" "repo.issues.review.add_review_requests" (RenderReviewRequest .AddedRequestReview) $createdStr}} + {{else if and (not .AddedRequestReview) .RemovedRequestReview}} + {{ctx.Locale.TrN (len .RemovedRequestReview) "repo.issues.review.remove_review_request" "repo.issues.review.remove_review_requests" (RenderReviewRequest .RemovedRequestReview) $createdStr}} {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request" .Assignee.GetDisplayName $createdStr}} + {{ctx.Locale.Tr "repo.issues.review.add_remove_review_requests" (RenderReviewRequest .AddedRequestReview) (RenderReviewRequest .RemovedRequestReview) $createdStr}} {{end}} - {{else}} - - {{$teamName := "Ghost Team"}} - {{if .AssigneeTeam}} - {{$teamName = .AssigneeTeam.Name}} - {{end}} - {{if .RemovedAssignee}} - {{ctx.Locale.Tr "repo.issues.review.remove_review_request" $teamName $createdStr}} - {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_review_request" $teamName $createdStr}} - {{end}} - {{end}} - -
+ +
+ {{end}} {{else if and (eq .Type 29) (or (gt .CommitsNum 0) .IsForcePush)}} {{if and .Issue.IsClosed (gt .ID $.LatestCloseCommentID)}} diff --git a/tests/integration/fixtures/TestPullCombinedReviewRequest/review.yml b/tests/integration/fixtures/TestPullCombinedReviewRequest/review.yml new file mode 100644 index 0000000000..deea4492e9 --- /dev/null +++ b/tests/integration/fixtures/TestPullCombinedReviewRequest/review.yml @@ -0,0 +1,21 @@ +- + id: 1001 + type: 4 + reviewer_id: 2 + issue_id: 3 + official: true + stale: false + original_author_id: 0 + updated_unix: 1000000000 + created_unix: 1000000000 + +- + id: 1002 + type: 4 + reviewer_id: 11 + issue_id: 3 + official: true + stale: false + original_author_id: 0 + updated_unix: 1000000000 + created_unix: 1000000000 diff --git a/tests/integration/pull_test.go b/tests/integration/pull_test.go index 10dbad2228..8f951f05ab 100644 --- a/tests/integration/pull_test.go +++ b/tests/integration/pull_test.go @@ -65,3 +65,37 @@ func TestPullManuallyMergeWarning(t *testing.T) { assert.NotContains(t, mergeInstructions, warningMessage) }) } + +func TestPullCombinedReviewRequest(t *testing.T) { + defer tests.AddFixtures("tests/integration/fixtures/TestPullCombinedReviewRequest/")() + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user2") + + helper := func(t *testing.T, action, userID, expectedText string) { + t.Helper() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/pulls/request_review", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/pulls/3"), + "issue_ids": "3", + "action": action, + "id": userID, + }) + session.MakeRequest(t, req, http.StatusOK) + + req = NewRequest(t, "GET", "/user2/repo1/pulls/3") + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + assert.Contains(t, htmlDoc.Find(".timeline-item:has(.review-request-list)").Last().Text(), expectedText) + } + + helper(t, "detach", "2", "refused to review") + helper(t, "attach", "4", "requested reviews from user4 and removed review requests for user2") + helper(t, "attach", "9", "requested reviews from user4, user9 and removed review requests for user2") + helper(t, "attach", "2", "requested reviews from user4, user9") + helper(t, "detach", "4", "requested review from user9") + helper(t, "detach", "11", "requested reviews from user9 and removed review requests for user11") + helper(t, "detach", "9", "removed review request for user11") + helper(t, "detach", "2", "removed review requests for user11, user2") +}