From 5624b9f094f57a74614d7dbefb5ede767db9e022 Mon Sep 17 00:00:00 2001 From: Robert Wolff Date: Sun, 27 Jul 2025 10:12:28 +0200 Subject: [PATCH] fix(ui): issue comment review request targets when aggregated (#8689) regression of !8239 ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8689 Reviewed-by: Earl Warren Co-authored-by: Robert Wolff Co-committed-by: Robert Wolff --- .../repo/issue/view_content/comments.tmpl | 6 +- tests/integration/issue_comment_test.go | 105 ++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 454467a4d0..a212bb871a 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -759,11 +759,11 @@ {{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" ""}} {{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) ""}} + {{ctx.Locale.TrN (len .AddedRequestReview) "repo.issues.review.add_review_request" "repo.issues.review.add_review_requests" (RenderReviewRequest $.Context .AddedRequestReview) ""}} {{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) ""}} + {{ctx.Locale.TrN (len .RemovedRequestReview) "repo.issues.review.remove_review_request" "repo.issues.review.remove_review_requests" (RenderReviewRequest $.Context .RemovedRequestReview) ""}} {{else}} - {{ctx.Locale.Tr "repo.issues.review.add_remove_review_requests" (RenderReviewRequest .AddedRequestReview) (RenderReviewRequest .RemovedRequestReview) ""}} + {{ctx.Locale.Tr "repo.issues.review.add_remove_review_requests" (RenderReviewRequest $.Context .AddedRequestReview) (RenderReviewRequest $.Context .RemovedRequestReview) ""}} {{end}} diff --git a/tests/integration/issue_comment_test.go b/tests/integration/issue_comment_test.go index eda643fa79..a604378479 100644 --- a/tests/integration/issue_comment_test.go +++ b/tests/integration/issue_comment_test.go @@ -315,6 +315,111 @@ func TestIssueCommentChangeReviewRequest(t *testing.T) { assert.Empty(t, htmlDoc.Find("#issuecomment-"+strconv.FormatInt(comment6.ID, 10)+" .text").Text()) } +func TestIssueCommentChangeReviewRequestAggregated(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 6}) + require.NoError(t, pull.LoadIssue(db.DefaultContext)) + issue := pull.Issue + require.NoError(t, issue.LoadRepo(db.DefaultContext)) + + user1, err := user_model.GetUserByID(db.DefaultContext, 1) + require.NoError(t, err) + user2, err := user_model.GetUserByID(db.DefaultContext, 2) + require.NoError(t, err) + team1, err := org_model.GetTeamByID(db.DefaultContext, 2) + require.NoError(t, err) + assert.NotNil(t, team1) + label, err := issues_model.GetLabelByID(db.DefaultContext, 1) + require.NoError(t, err) + assert.NotNil(t, label) + + // Request from other + comment1, err := issues_model.AddReviewRequest(db.DefaultContext, issue, user2, user1) + require.NoError(t, err) + // Add label + issues_model.CreateComment(db.DefaultContext, &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypeLabel, + Doer: user1, + Issue: issue, + Label: label, + Repo: issue.Repo, + }) + + // Refuse review + comment2, err := issues_model.RemoveReviewRequest(db.DefaultContext, issue, user2, user2) + require.NoError(t, err) + // Remove label + issues_model.CreateComment(db.DefaultContext, &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypeLabel, + Doer: user2, + Issue: issue, + Label: label, + Repo: issue.Repo, + }) + + // Request from other + comment3, err := issues_model.AddReviewRequest(db.DefaultContext, issue, user2, user1) + require.NoError(t, err) + // Request from team + comment4, err := issues_model.AddTeamReviewRequest(db.DefaultContext, issue, team1, user1) + require.NoError(t, err) + // Add label + issues_model.CreateComment(db.DefaultContext, &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypeLabel, + Doer: user1, + Issue: issue, + Label: label, + Repo: issue.Repo, + }) + + // Remove request from team + comment5, err := issues_model.RemoveTeamReviewRequest(db.DefaultContext, issue, team1, user2) + require.NoError(t, err) + // Request from other + comment6, err := issues_model.AddReviewRequest(db.DefaultContext, issue, user1, user2) + require.NoError(t, err) + // Remove label + issues_model.CreateComment(db.DefaultContext, &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypeLabel, + Doer: user2, + Issue: issue, + Label: label, + Repo: issue.Repo, + }) + + session := loginUser(t, "user2") + req := NewRequest(t, "GET", "/org3/repo3/pulls/2") + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + // Request from other + testIssueCommentChangeEvent(t, htmlDoc, strconv.FormatInt(comment1.ID, 10), + "octicon-eye", "User One", "/user1", + []string{"user1 requested review from user2"}, + []string{"/user1", "#issuecomment-" + strconv.FormatInt(comment1.ID, 10), "/org3/repo3/pulls?labels=1", "/user2"}) + + // Refuse review + testIssueCommentChangeEvent(t, htmlDoc, strconv.FormatInt(comment2.ID, 10), + "octicon-eye", "< Ur Tw ><", "/user2", + []string{"user2 refused to review"}, + []string{"/user2", "#issuecomment-" + strconv.FormatInt(comment2.ID, 10), "/org3/repo3/pulls?labels=1"}) + + // Request review from other and from team + testIssueCommentChangeEvent(t, htmlDoc, strconv.FormatInt(comment3.ID, 10), + "octicon-eye", "User One", "/user1", + []string{"user1 requested reviews from user2, team1"}, + []string{"/user1", "#issuecomment-" + strconv.FormatInt(comment3.ID, 10), "/org3/repo3/pulls?labels=1", "/user2", "/org/org3/teams/team1"}) + assert.Empty(t, htmlDoc.Find("#issuecomment-"+strconv.FormatInt(comment4.ID, 10)+" .text").Text()) + + // Remove and add request + testIssueCommentChangeEvent(t, htmlDoc, strconv.FormatInt(comment5.ID, 10), + "octicon-eye", "< Ur Tw ><", "/user2", + []string{"user2 requested reviews from user1 and removed review requests for team1"}, + []string{"/user2", "#issuecomment-" + strconv.FormatInt(comment5.ID, 10), "/org3/repo3/pulls?labels=1", "/user1", "/org/org3/teams/team1"}) + assert.Empty(t, htmlDoc.Find("#issuecomment-"+strconv.FormatInt(comment6.ID, 10)+" .text").Text()) +} + func TestIssueCommentChangeLock(t *testing.T) { defer tests.PrepareTestEnv(t)()