fix: PR review dismissals were not appearing in activity feed (#8853)

Discovered that `NotifyPullRevieweDismiss` was dead code while working on another issue; it should have been `PullReviewDismiss` when originally implemented.  Related fixes to the activity feed view which were incomplete as well.

Dismissing a PR review now appears on the activity feed:
![image](/attachments/99bd37c5-218d-4ecf-a74c-d01451ceae17)

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### 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

- [ ] I do not want this change to show in the release notes.
- [x] 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/<pull request number>.md` to be be used for the release notes instead of the title.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8853
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>

services/feed/action.go

@@ -307,7 +307,7 @@ func (*actionNotifier) AutoMergePullRequest(ctx context.Context, doer *user_mode
 	}
 }
 
-func (*actionNotifier) NotifyPullRevieweDismiss(ctx context.Context, doer *user_model.User, review *issues_model.Review, comment *issues_model.Comment) {
+func (*actionNotifier) PullReviewDismiss(ctx context.Context, doer *user_model.User, review *issues_model.Review, comment *issues_model.Comment) {
 	reviewerName := review.Reviewer.Name
 	if len(review.OriginalAuthor) > 0 {
 		reviewerName = review.OriginalAuthor

templates/user/dashboard/feeds.tmpl

@@ -76,7 +76,7 @@
 					{{else if .GetOpType.InActions "publish_release"}}
 						{{$linkText := .Content | RenderEmoji $.Context}}
 						{{ctx.Locale.Tr "action.publish_release" (.GetRepoLink ctx) (printf "%s/releases/tag/%s" (.GetRepoLink ctx) .GetTag) (.ShortRepoPath ctx) $linkText}}
-					{{else if .GetOpType.InActions "review_dismissed"}}
+					{{else if .GetOpType.InActions "pull_review_dismissed"}}
 						{{$index := index .GetIssueInfos 0}}
 						{{$reviewer := index .GetIssueInfos 1}}
 						{{ctx.Locale.Tr "action.review_dismissed" (printf "%s/pulls/%s" (.GetRepoLink ctx) $index) $index (.ShortRepoPath ctx) $reviewer}}
@@ -121,8 +121,8 @@
 				{{else if .GetOpType.InActions "close_issue" "reopen_issue" "close_pull_request" "reopen_pull_request"}}
 					<span class="text truncate issue title">{{RenderIssueTitle ctx (.GetIssueTitle ctx) (.Repo.ComposeMetas ctx)}}</span>
 				{{else if .GetOpType.InActions "pull_review_dismissed"}}
-				<div class="flex-item-body text black">{{ctx.Locale.Tr "action.review_dismissed_reason"}}</div>
-				<div class="flex-item-body text black">{{index .GetIssueInfos 2 | RenderEmoji $.Context}}</div>
+					<div class="flex-item-body text black">{{ctx.Locale.Tr "action.review_dismissed_reason"}}</div>
+					<div class="flex-item-body text black">{{index .GetIssueInfos 2 | RenderEmoji $.Context}}</div>
 				{{end}}
 			</div>
 			<div class="flex-item-trailing">

tests/integration/user_dashboard_test.go

@@ -10,11 +10,13 @@ import (
 	"strings"
 	"testing"
 
-	"forgejo.org/models/db"
+	"forgejo.org/models/issues"
 	unit_model "forgejo.org/models/unit"
 	"forgejo.org/models/unittest"
 	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/gitrepo"
 	issue_service "forgejo.org/services/issue"
+	pr_service "forgejo.org/services/pull"
 	files_service "forgejo.org/services/repository/files"
 	"forgejo.org/tests"
 
@@ -62,10 +64,10 @@ func TestDashboardTitleRendering(t *testing.T) {
 		issue := createIssue(t, user4, repo, "`:exclamation:` not rendered", "Hi there!")
 		pr := createPullRequest(t, user4, repo, "testing", "`:exclamation:` not rendered")
 
-		_, err := issue_service.CreateIssueComment(db.DefaultContext, user4, repo, issue, "hi", nil)
+		_, err := issue_service.CreateIssueComment(t.Context(), user4, repo, issue, "hi", nil)
 		require.NoError(t, err)
 
-		_, err = issue_service.CreateIssueComment(db.DefaultContext, user4, repo, pr.Issue, "hi", nil)
+		_, err = issue_service.CreateIssueComment(t.Context(), user4, repo, pr.Issue, "hi", nil)
 		require.NoError(t, err)
 
 		testIssueClose(t, sess, repo.OwnerName, repo.Name, strconv.Itoa(int(issue.Index)), false)
@@ -101,7 +103,7 @@ func TestDashboardActionEscaping(t *testing.T) {
 
 		issue := createIssue(t, user4, repo, "Issue with | in title", "Hey here's a | for you")
 
-		_, err := issue_service.CreateIssueComment(db.DefaultContext, user4, repo, issue, "Comment with a | in it", nil)
+		_, err := issue_service.CreateIssueComment(t.Context(), user4, repo, issue, "Comment with a | in it", nil)
 		require.NoError(t, err)
 
 		testIssueClose(t, sess, repo.OwnerName, repo.Name, strconv.Itoa(int(issue.Index)), false)
@@ -122,3 +124,43 @@ func TestDashboardActionEscaping(t *testing.T) {
 		assert.Equal(t, 4, count)
 	})
 }
+
+func TestDashboardReviewWorkflows(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+		sess := loginUser(t, user4.Name)
+
+		repo, _, f := tests.CreateDeclarativeRepo(t, user4, "",
+			[]unit_model.Type{unit_model.TypePullRequests, unit_model.TypeIssues}, nil,
+			[]*files_service.ChangeRepoFile{},
+		)
+		defer f()
+		gitRepo, err := gitrepo.OpenRepository(t.Context(), repo)
+		require.NoError(t, err)
+
+		pr := createPullRequest(t, user4, repo, "testing", "My very first PR!")
+
+		review, _, err := pr_service.SubmitReview(t.Context(), user4, gitRepo, pr.Issue, issues.ReviewTypeReject, "This isn't good enough!", "HEAD", []string{})
+		require.NoError(t, err)
+
+		_, err = pr_service.DismissReview(t.Context(), review.ID, repo.ID, "Come on, give the newbie a break!", user4, true, true)
+		require.NoError(t, err)
+
+		response := sess.MakeRequest(t, NewRequest(t, "GET", "/"), http.StatusOK)
+		htmlDoc := NewHTMLParser(t, response.Body)
+
+		count := 0
+		htmlDoc.doc.Find("#activity-feed .flex-item-main .title").Each(func(i int, s *goquery.Selection) {
+			count++
+			assert.Equal(t, "My very first PR!", s.Text())
+		})
+		htmlDoc.doc.Find("#activity-feed .flex-item-main .flex-item-body").Each(func(i int, s *goquery.Selection) {
+			count++
+			if s.Text() != "Reason:" && s.Text() != "Come on, give the newbie a break!" {
+				assert.Fail(t, "Unexpected feed text", "Expected 'Reason:' and reason explanation, but found: %q", s.Text())
+			}
+		})
+
+		assert.Equal(t, 4, count)
+	})
+}