Skip to content

fix: use live comment content in feed and dashboard previews#36631

Open
alessandroferra wants to merge 3 commits intogo-gitea:mainfrom
alessandroferra:fix/feed-update-comment
Open

fix: use live comment content in feed and dashboard previews#36631
alessandroferra wants to merge 3 commits intogo-gitea:mainfrom
alessandroferra:fix/feed-update-comment

Conversation

@alessandroferra
Copy link
Copy Markdown
Contributor

@alessandroferra alessandroferra commented Feb 15, 2026

Summary

Dashboard activity feeds and RSS/Atom feeds display a 200-character snapshot of comment text captured at creation time. When a user edits their comment the snapshot is never refreshed, so every feed consumer sees stale content indefinitely.

  • Add Action.GetCommentPreview() that returns the live Comment.Content (already bulk-loaded by LoadComments) and falls back to the Action.Content snapshot for old rows or deleted comments
  • Replace the unbounded MarkdownToHtml call in the dashboard template with MarkdownToHTMLWithPreviewLimit, which truncates markdown source to 5 lines / 1000 runes before rendering to bound preview size. Incomplete markdown constructs are handled gracefully by the renderer.
  • Apply the same truncation in the RSS/Atom feed builder

Affected action types: ActionCommentIssue, ActionCommentPull, ActionApprovePullRequest, ActionRejectPullRequest, ActionPullReviewDismissed.

No new database queries, GetCommentPreview() reads from the Comment struct that LoadAttributes already populates.

Test plan

  • go test -tags sqlite,sqlite_unlock_notify -run TestGetCommentPreview ./models/activities/ and 6 subtests pass
  • go test -run TestTruncateToPreviewLines ./modules/templates/ and 6 subtests pass
  • go test -run TestMarkdownToHTMLWithPreviewLimit ./modules/templates/ and 3 subtests pass
  • make lint-go with 0 issues
  • Manual: post a comment, edit it, confirm dashboard shows updated text
  • Manual: confirm .rss / .atom feed entry reflects the edit

Notes

  • Preview limits (5 lines / 1000 characters) are intentionally larger than the old 200-character snapshot. Comments often contain images, code blocks, or tables that consume multiple lines but convey little text. A tight limit would produce empty-looking previews in those cases. The current values may still be too generous, feedback welcome.
  • The <description> field in RSS/Atom entries currently mixes plain text and rendered HTML. This is a pre-existing inconsistency (noted by the TODO at convert.go) and is not changed by this PR.
  • MaxPreviewLines and MaxPreviewChars are exported constants, so they can be adjusted or made configurable later without touching the rendering logic.

Add Action.GetCommentPreview() to prefer the loaded Comment.Content
over the snapshot stored in Action.Content. Introduce
TruncateToPreviewLines and MarkdownToHTMLWithPreviewLimit so that
long comments are capped at 5 lines / 1000 characters in both the
Atom feed and the dashboard template.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 15, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Feb 15, 2026
@silverwind
Copy link
Copy Markdown
Member

Thanks, this is a welcome change. I think there must be a open issue about this topic too.

@silverwind
Copy link
Copy Markdown
Member

Comment authored by AI on behalf of @silverwind

Nice idea — showing live comment content instead of the stale 200-char snapshot is a real improvement. A few things I noticed:

1. Missing truncation for ActionPullReviewDismissed in feed (convert.go)

The comment action types (ActionCommentIssue, ActionCommentPull, etc.) apply TruncateToPreviewLines before rendering in the feed, but the ActionPullReviewDismissed case doesn't:

case activities_model.ActionPullReviewDismissed:
    desc = ctx.Locale.TrString("action.review_dismissed_reason") + "\n\n" + act.GetCommentPreview()

Since GetCommentPreview() now returns the full live comment content (not the old truncated snapshot), a long dismiss reason would be included in the RSS/Atom feed without any limit. This should probably also go through TruncateToPreviewLines for consistency.

2. Truncating markdown source before rendering can break syntax

The PR description says "truncates to 5 lines / 1000 characters before rendering so markdown constructs are never split mid-syntax", but truncating by raw line/char count can absolutely split markdown constructs. For example:

  • A fenced code block (```) could lose its closing fence
  • A table could lose its closing rows
  • A link [text](url) could be cut in the middle
  • A multi-line blockquote could be cut mid-content

The markdown renderer will handle these gracefully enough (unclosed blocks just render as-is), so this isn't a blocker, but the claim in the description should be corrected. Truncating before rendering is still better than trying to truncate rendered HTML.

3. Naming convention: MarkdownToHTMLWithPreviewLimit vs MarkdownToHtml

The existing method uses MarkdownToHtml (lowercase tml) with a nolint:revive comment. The new method uses MarkdownToHTMLWithPreviewLimit (uppercase HTML). Consider being consistent — either follow the existing convention or take the opportunity to note why the new method uses the Go-standard casing.

4. Minor: "text grey" CSS classes

output = template.HTML(string(output) + `<span class="text grey">…</span>`)

Is text grey still the correct class combination? Some parts of the codebase seem to use tw- prefixed Tailwind classes. Worth double-checking that this renders as intended.

Overall the approach is sound and the tests are thorough. The main actionable item is #1 (missing truncation for dismissed reviews in the feed).

@silverwind
Copy link
Copy Markdown
Member

Ignore the suggestion regarding fenced code block truncation, that is WIP separately in #36604.

Extract renderCommentPreview helper to deduplicate feed truncation
logic. Apply truncation to ActionPullReviewDismissed. Migrate
truncation indicator from text grey to tw-text-text-light.
@alessandroferra
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! Addressed in ae885cc:

1. Missing truncation for ActionPullReviewDismissed

Fixed. Extracted a renderCommentPreview() helper in convert.go that handles truncation + markdown rendering + ellipsis. Both the comment actions and dismissed review case now go through the same path.

Worth noting this was already unbounded before this PR, the switch to live content just made it more visible. Before this PR, act.GetIssueInfos()[2] was used directly without truncation either. Now it's consistently capped, thank you for pointing it out.

2. Markdown truncation wording

Agreed, good catch, the wording overstated it. I tested all the cases you mentioned (unclosed fences, partial tables, broken links, broken inline code, and mermaid diagrams) against the live instance. The renderer auto-closes blocks and falls back to plain text for incomplete constructs, no broken HTML or visual artifacts. Only updated the summary to:

Truncates markdown source to 5 lines / 1000 runes before rendering to bound preview size. Incomplete markdown constructs are handled gracefully by the renderer.

So it's more correct.

3. Naming: MarkdownToHTMLWithPreviewLimit vs MarkdownToHtml

MarkdownToHtml is the only function in the repo using lowercase Html, everything else uses HTML (Go initialism convention). It was introduced in #24417 (Apr 2023) and suppressed with a nolint:revive comment. My method follows the correct convention. Happy to follow up with a rename if that's useful, but i don't want to mix unrelated churn into this one. Unless you want me to.

4. CSS class: text grey

Updated to tw-text-text-light. Both resolve to the same var(--color-text-light) CSS variable, but the Tailwind class aligns with the ongoing migration away from Fomantic UI.

@alessandroferra
Copy link
Copy Markdown
Contributor Author

About the open issue, i tried to search for it but i couldn't find it. This is the closest match but is closed: #26868

@silverwind
Copy link
Copy Markdown
Member

Yeah, there's no issue about. I was imagining things 😆.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants