From a1451655eb272ac60da2a862bd1b0649b5da084f Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Mon, 11 Aug 2025 06:56:31 +0200 Subject: [PATCH] fix: standardize truncation of user-entered comment text in activity feed (#8854) Currently long-form comments in the activity feed are truncated in three different ways... - Comment on issue/PR: to 200 characters, with an ellipsis, potentially splitting the middle of multi-line markdown blocks - PR review: first line of text in the review comment - PR review dismissed: no truncation on comment text... - Although this feed entry doesn't work currently (#8853) and the UI doesn't really lend itself to long comments anyway. For the sake of consistency, and to fix #8781, this PR fixes the implementation so that all truncation occurs by grabbing the first line of text and then truncating it to 200 characters if necessary. This fixes #8781 by not ever truncating in the middle of a markdown *block*; it's still possible to trigger some unexpected behavior such as: - Truncate in the middle of markdown structures like a link, causing raw markdown to render in the feed - Provide an image embed on the first line of a comment, causing an image to appear in the feed; if it's a large image it could disrupt browsing a nice brief activity feed But these behaviors seem acceptable edge cases until they're identified to cause any significant user impact. ## 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... - [x] in their respective `*_test.go` for unit tests. - [ ] 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/.md` to be be used for the release notes instead of the title. ## Release notes - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/8854): standardize truncation of user-entered comment text in activity feed Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8854 Reviewed-by: Earl Warren Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- services/feed/action.go | 34 +++++++++++++++--------- services/feed/action_test.go | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/services/feed/action.go b/services/feed/action.go index e945e0905e..d1d899a28e 100644 --- a/services/feed/action.go +++ b/services/feed/action.go @@ -125,18 +125,9 @@ func (a *actionNotifier) CreateIssueComment(ctx context.Context, doer *user_mode Comment: comment, CommentID: comment.ID, IsPrivate: issue.Repo.IsPrivate, + Content: encodeContent(fmt.Sprintf("%d", issue.Index), abbreviatedComment(comment.Content)), } - truncatedContent, truncatedRight := util.SplitStringAtByteN(comment.Content, 200) - if truncatedRight != "" { - // in case the content is in a Latin family language, we remove the last broken word. - lastSpaceIdx := strings.LastIndex(truncatedContent, " ") - if lastSpaceIdx != -1 && (len(truncatedContent)-lastSpaceIdx < 15) { - truncatedContent = truncatedContent[:lastSpaceIdx] + "…" - } - } - act.Content = encodeContent(fmt.Sprintf("%d", issue.Index), truncatedContent) - if issue.IsPull { act.OpType = activities_model.ActionCommentPull } else { @@ -247,7 +238,7 @@ func (a *actionNotifier) PullRequestReview(ctx context.Context, pr *issues_model actions = append(actions, &activities_model.Action{ ActUserID: review.Reviewer.ID, ActUser: review.Reviewer, - Content: encodeContent(fmt.Sprintf("%d", review.Issue.Index), strings.Split(comm.Content, "\n")[0]), + Content: encodeContent(fmt.Sprintf("%d", review.Issue.Index), abbreviatedComment(comm.Content)), OpType: activities_model.ActionCommentPull, RepoID: review.Issue.RepoID, Repo: review.Issue.Repo, @@ -263,7 +254,7 @@ func (a *actionNotifier) PullRequestReview(ctx context.Context, pr *issues_model action := &activities_model.Action{ ActUserID: review.Reviewer.ID, ActUser: review.Reviewer, - Content: encodeContent(fmt.Sprintf("%d", review.Issue.Index), strings.Split(comment.Content, "\n")[0]), + Content: encodeContent(fmt.Sprintf("%d", review.Issue.Index), abbreviatedComment(comment.Content)), RepoID: review.Issue.RepoID, Repo: review.Issue.Repo, IsPrivate: review.Issue.Repo.IsPrivate, @@ -325,7 +316,7 @@ func (*actionNotifier) NotifyPullRevieweDismiss(ctx context.Context, doer *user_ ActUserID: doer.ID, ActUser: doer, OpType: activities_model.ActionPullReviewDismissed, - Content: encodeContent(fmt.Sprintf("%d", review.Issue.Index), reviewerName, comment.Content), + Content: encodeContent(fmt.Sprintf("%d", review.Issue.Index), reviewerName, abbreviatedComment(comment.Content)), RepoID: review.Issue.Repo.ID, Repo: review.Issue.Repo, IsPrivate: review.Issue.Repo.IsPrivate, @@ -491,3 +482,20 @@ func encodeContent(params ...string) string { } return string(contentEncoded) } + +// Given a comment of arbitrary-length Markdown text, create an abbreviated Markdown text appropriate for the +// activity feed. +func abbreviatedComment(comment string) string { + firstLine := strings.Split(comment, "\n")[0] + + truncatedContent, truncatedRight := util.SplitStringAtByteN(firstLine, 200) + if truncatedRight != "" { + // in case the content is in a Latin family language, we remove the last broken word. + lastSpaceIdx := strings.LastIndex(truncatedContent, " ") + if lastSpaceIdx != -1 && (len(truncatedContent)-lastSpaceIdx < 15) { + truncatedContent = truncatedContent[:lastSpaceIdx] + "…" + } + } + + return truncatedContent +} diff --git a/services/feed/action_test.go b/services/feed/action_test.go index 93ca543a1a..35b170e95c 100644 --- a/services/feed/action_test.go +++ b/services/feed/action_test.go @@ -143,3 +143,54 @@ func TestPushCommits(t *testing.T) { assert.JSONEq(t, `{"Commits":[{"Sha1":"69554a6","Message":"not signed commit","AuthorEmail":"user2@example.com","AuthorName":"User2","CommitterEmail":"user2@example.com","CommitterName":"User2","Signature":null,"Verification":null,"Timestamp":"0001-01-01T00:00:00Z"}],"HeadCommit":{"Sha1":"69554a6","Message":"","AuthorEmail":"","AuthorName":"","CommitterEmail":"","CommitterName":"","Signature":null,"Verification":null,"Timestamp":"0001-01-01T00:00:00Z"},"CompareURL":"","Len":0}`, newNotification.Content) }) } + +func TestAbbreviatedComment(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "short single line comment", + input: "This is a short comment", + expected: "This is a short comment", + }, + { + name: "empty comment", + input: "", + expected: "", + }, + { + name: "multiline comment - only first line", + input: "First line of comment\nSecond line\nThird line", + expected: "First line of comment", + }, + { + name: "before clip boundry", + input: strings.Repeat("abc ", 50), + expected: strings.Repeat("abc ", 50), + }, + { + name: "after clip boundry", + input: strings.Repeat("abc ", 51), + expected: "abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc…", + }, + { + name: "byte-split would land in middle of a rune", + input: strings.Repeat("🎉", 200), + expected: "🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉…", + }, + { + name: "mermaid block", + input: "Interesting point, here's a digram with my thoughts:\n```mermaid\ngraph LR\n a -->|some text| b\n```", + expected: "Interesting point, here's a digram with my thoughts:", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := abbreviatedComment(tt.input) + assert.Equal(t, tt.expected, result, "abbreviatedComment(%q)", tt.input) + }) + } +}