feat: format user mentions with display names in email notifications (#1930)

Email notifications now display user mentions with inline avatar images for improved visual recognition and easier identification. Mentions gracefully fall back to display names if avatars are unavailable.
2026-04-29 11:00:07 -05:00 · 2025-12-10 12:39:05 +01:00
parent d4eccccbfe
commit fb7764d9f1
14 changed files with 610 additions and 25 deletions
--- a/pkg/models/mentions.go
+++ b/pkg/models/mentions.go
@@ -17,11 +17,15 @@
 package models

 import (
+	"bytes"
 	"strings"

+	"code.vikunja.io/api/pkg/log"
+	"code.vikunja.io/api/pkg/modules/avatar"
 	"code.vikunja.io/api/pkg/user"

 	"golang.org/x/net/html"
+	"golang.org/x/net/html/atom"
 	"xorm.io/xorm"
 )

@@ -74,3 +78,188 @@ func extractMentionedUsernames(htmlText string) []string {
 	traverse(doc)
 	return usernames
 }
+
+// formatMentionsForEmail replaces mention-user tags with user avatars and names for email display.
+// It converts <mention-user data-id="username" data-label="Display Name"> tags to
+// <strong><img src="data:..."/> Display Name</strong> with a 20x20 avatar image.
+// If data-label is missing, it falls back to data-id. Returns the original HTML unchanged on any error.
+func formatMentionsForEmail(s *xorm.Session, htmlText string) string {
+	if htmlText == "" {
+		return htmlText
+	}
+
+	// Create a synthetic body node for fragment parsing
+	bodyNode := &html.Node{
+		Type:     html.ElementNode,
+		Data:     "body",
+		DataAtom: atom.Body,
+	}
+
+	fragments, err := html.ParseFragment(strings.NewReader(htmlText), bodyNode)
+	if err != nil {
+		log.Debugf("Failed to parse HTML fragment for mention formatting: %v", err)
+		return htmlText
+	}
+
+	// If no fragments, return original
+	if len(fragments) == 0 {
+		return htmlText
+	}
+
+	// Extract all usernames first to batch fetch users
+	usernames := extractMentionedUsernames(htmlText)
+	var usersMap map[int64]*user.User
+	var usernameToUser map[string]*user.User
+
+	if len(usernames) == 0 {
+		return htmlText
+	}
+
+	// Create maps for user data and avatar data URIs
+	usernameToAvatarURI := make(map[string]string)
+
+	// Only fetch users if we have a valid session
+	usersMap, err = user.GetUsersByUsername(s, usernames, true)
+	if err != nil {
+		log.Debugf("Failed to fetch users for mention formatting: %v", err)
+		// Continue without user data - we'll fall back to display names from attributes
+	} else {
+		// Create username -> user map for easy lookup and fetch avatar data URIs
+		usernameToUser = make(map[string]*user.User)
+		for _, u := range usersMap {
+			usernameToUser[u.Username] = u
+
+			// Fetch avatar data URI for this user
+			provider := avatar.GetProvider(u)
+			avatarDataURI, err := provider.AsDataURI(u, 20)
+			if err == nil && avatarDataURI != "" {
+				usernameToAvatarURI[u.Username] = avatarDataURI
+			}
+		}
+	}
+
+	// Track nodes to replace (can't modify while traversing)
+	type replacement struct {
+		oldNode *html.Node
+		newNode *html.Node
+	}
+	replacements := []replacement{}
+
+	var traverse func(*html.Node)
+	traverse = func(n *html.Node) {
+		if n.Type == html.ElementNode && n.Data == "mention-user" {
+			var dataLabel, dataID string
+
+			// Extract data-label and data-id attributes
+			for _, attr := range n.Attr {
+				switch attr.Key {
+				case "data-label":
+					dataLabel = attr.Val
+				case "data-id":
+					dataID = attr.Val
+				}
+			}
+
+			// Determine what to display
+			displayName := dataLabel
+			if displayName == "" {
+				displayName = dataID
+			}
+
+			// If still empty and has text content (old format), use that
+			if displayName == "" && n.FirstChild != nil && n.FirstChild.Type == html.TextNode {
+				displayName = strings.TrimPrefix(n.FirstChild.Data, "@")
+			}
+
+			if displayName == "" {
+				log.Debugf("Mention node has no data-label, data-id, or text content, skipping")
+				// Continue traversing children in case there are nested elements
+				for child := n.FirstChild; child != nil; child = child.NextSibling {
+					traverse(child)
+				}
+				return
+			}
+
+			// Create <strong> wrapper
+			strongNode := &html.Node{
+				Type: html.ElementNode,
+				Data: "strong",
+			}
+
+			// Get pre-fetched avatar data URI for the user
+			var avatarDataURI string
+			if dataID != "" {
+				avatarDataURI = usernameToAvatarURI[dataID]
+			}
+
+			// If we have an avatar, add the img element
+			if avatarDataURI != "" {
+				imgNode := &html.Node{
+					Type: html.ElementNode,
+					Data: "img",
+					Attr: []html.Attribute{
+						{Key: "src", Val: avatarDataURI},
+						{Key: "width", Val: "20"},
+						{Key: "height", Val: "20"},
+						{Key: "style", Val: "border-radius: 50%; vertical-align: middle; margin-right: 4px;"},
+						{Key: "alt", Val: displayName},
+					},
+				}
+				strongNode.AppendChild(imgNode)
+
+				// Add display name without @ since we have the avatar
+				textNode := &html.Node{
+					Type: html.TextNode,
+					Data: displayName,
+				}
+				strongNode.AppendChild(textNode)
+			} else {
+				// Fall back to @DisplayName without avatar
+				textNode := &html.Node{
+					Type: html.TextNode,
+					Data: "@" + displayName,
+				}
+				strongNode.AppendChild(textNode)
+			}
+
+			// Schedule replacement
+			replacements = append(replacements, replacement{
+				oldNode: n,
+				newNode: strongNode,
+			})
+
+			// Don't traverse children of mention-user since we're replacing it
+			return
+		}
+
+		// Traverse child nodes
+		for child := n.FirstChild; child != nil; child = child.NextSibling {
+			traverse(child)
+		}
+	}
+
+	// Traverse all fragment nodes
+	for _, fragment := range fragments {
+		traverse(fragment)
+	}
+
+	// Apply replacements
+	for _, r := range replacements {
+		if r.oldNode.Parent != nil {
+			r.oldNode.Parent.InsertBefore(r.newNode, r.oldNode)
+			r.oldNode.Parent.RemoveChild(r.oldNode)
+		}
+	}
+
+	// Render each fragment node back to HTML
+	var buf bytes.Buffer
+	for _, fragment := range fragments {
+		err = html.Render(&buf, fragment)
+		if err != nil {
+			log.Debugf("Failed to render HTML fragment after mention formatting: %v", err)
+			return htmlText
+		}
+	}
+
+	return buf.String()
+}
--- a/pkg/models/mentions_test.go
+++ b/pkg/models/mentions_test.go
@@ -19,6 +19,8 @@ package models
 import (
 	"testing"

+	"regexp"
+
 	"code.vikunja.io/api/pkg/db"
 	"code.vikunja.io/api/pkg/notifications"
 	"code.vikunja.io/api/pkg/user"
@@ -179,3 +181,214 @@ func TestSendingMentionNotification(t *testing.T) {
 		assert.Len(t, dbNotifications, 1)
 	})
 }
+
+func TestFormatMentionsForEmail(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+		useRegex bool // If true, expected is treated as a regex pattern
+	}{
+		{
+			name:     "empty string",
+			input:    "",
+			expected: "",
+		},
+		{
+			name:     "no mentions",
+			input:    "<p>Lorem Ipsum dolor sit amet</p>",
+			expected: "<p>Lorem Ipsum dolor sit amet</p>",
+		},
+		{
+			name:     "single mention with data-label (new format)",
+			input:    `<p><mention-user data-id="frederick" data-label="Frederick" data-mention-suggestion-char="@"></mention-user> hello</p>`,
+			expected: `<p><strong>@Frederick</strong> hello</p>`,
+		},
+		{
+			name:     "single mention with full name in data-label",
+			input:    `<p><mention-user data-id="johndoe" data-label="John Doe" data-mention-suggestion-char="@"></mention-user> please help</p>`,
+			expected: `<p><strong>@John Doe</strong> please help</p>`,
+		},
+		{
+			name:     "mention without data-label (fallback to data-id)",
+			input:    `<p><mention-user data-id="johndoe"></mention-user> test</p>`,
+			expected: `<p><strong>@johndoe</strong> test</p>`,
+		},
+		{
+			name:     "old format with text node inside",
+			input:    `<p><mention-user data-id="user1">@user1</mention-user> Lorem Ipsum</p>`,
+			expected: `<p><strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="user1"/>user1</strong> Lorem Ipsum</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "old format with text node (data-id takes precedence over text)",
+			input:    `<p><mention-user data-id="actualuser">@differentuser</mention-user> text</p>`,
+			expected: `<p><strong>@actualuser</strong> text</p>`,
+		},
+		{
+			name:     "multiple mentions in one paragraph",
+			input:    `<p>Hey <mention-user data-id="john" data-label="John"></mention-user> and <mention-user data-id="jane" data-label="Jane Doe"></mention-user>, please review</p>`,
+			expected: `<p>Hey <strong>@John</strong> and <strong>@Jane Doe</strong>, please review</p>`,
+		},
+		{
+			name:     "mention at beginning",
+			input:    `<p><mention-user data-id="user1" data-label="User One"></mention-user> Lorem Ipsum</p>`,
+			expected: `<p><strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User One"/>User One</strong> Lorem Ipsum</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "mention at end",
+			input:    `<p>Lorem Ipsum <mention-user data-id="user1" data-label="User One"></mention-user></p>`,
+			expected: `<p>Lorem Ipsum <strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User One"/>User One</strong></p>`,
+			useRegex: true,
+		},
+		{
+			name:     "mention in middle",
+			input:    `<p>Lorem <mention-user data-id="user1" data-label="User One"></mention-user> Ipsum</p>`,
+			expected: `<p>Lorem <strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User One"/>User One</strong> Ipsum</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "same user mentioned multiple times",
+			input:    `<p><mention-user data-id="user1" data-label="User"></mention-user> and <mention-user data-id="user1" data-label="User"></mention-user> again</p>`,
+			expected: `<p><strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User"/>User</strong> and <strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User"/>User</strong> again</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "HTML preservation with links",
+			input:    `<p>Check <a href="http://example.com">this link</a> and ask <mention-user data-id="expert" data-label="Expert"></mention-user></p>`,
+			expected: `<p>Check <a href="http://example.com">this link</a> and ask <strong>@Expert</strong></p>`,
+		},
+		{
+			name:     "HTML preservation with multiple paragraphs",
+			input:    `<p>First paragraph with <mention-user data-id="user1" data-label="User"></mention-user></p><p>Second paragraph</p>`,
+			expected: `<p>First paragraph with <strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User"/>User</strong></p><p>Second paragraph</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "HTML preservation with bold and italic",
+			input:    `<p><strong>Bold text</strong> and <em>italic</em> with <mention-user data-id="user1" data-label="User"></mention-user></p>`,
+			expected: `<p><strong>Bold text</strong> and <em>italic</em> with <strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="User"/>User</strong></p>`,
+			useRegex: true,
+		},
+		{
+			name:     "special characters in data-label",
+			input:    `<p><mention-user data-id="user1" data-label="O'Brien"></mention-user> test</p>`,
+			expected: `<p><strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="O&#39;Brien"/>O&#39;Brien</strong> test</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "special characters - ampersand in data-label",
+			input:    `<p><mention-user data-id="user1" data-label="Tom &amp; Jerry"></mention-user> test</p>`,
+			expected: `<p><strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="Tom &amp; Jerry"/>Tom &amp; Jerry</strong> test</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "special characters - quotes in data-label",
+			input:    `<p><mention-user data-id="user1" data-label="&quot;Nickname&quot;"></mention-user> test</p>`,
+			expected: `<p><strong><img src="data:image/svg\+xml;base64,[A-Za-z0-9+/=]+" width="20" height="20" style="border-radius: 50%; vertical-align: middle; margin-right: 4px;" alt="&#34;Nickname&#34;"/>&#34;Nickname&#34;</strong> test</p>`,
+			useRegex: true,
+		},
+		{
+			name:     "mixed old and new format",
+			input:    `<p><mention-user data-id="new" data-label="New User"></mention-user> and <mention-user data-id="old">@old</mention-user></p>`,
+			expected: `<p><strong>@New User</strong> and <strong>@old</strong></p>`,
+		},
+		{
+			name:     "self-closing tag format (XML-style)",
+			input:    `<p><mention-user data-id="user" data-label="User"/> hello</p>`,
+			expected: `<p><strong>@User</strong></p>`,
+		},
+		{
+			name:     "mention with only text content (no attributes) - old format edge case",
+			input:    `<p><mention-user>@someuser</mention-user> test</p>`,
+			expected: `<p><mention-user>@someuser</mention-user> test</p>`,
+		},
+		{
+			name:     "data-label takes precedence over data-id",
+			input:    `<p><mention-user data-id="username123" data-label="John Smith"></mention-user> test</p>`,
+			expected: `<p><strong>@John Smith</strong> test</p>`,
+		},
+		{
+			name:     "unicode characters in data-label",
+			input:    `<p><mention-user data-id="user" data-label="Müller François"></mention-user> test</p>`,
+			expected: `<p><strong>@Müller François</strong> test</p>`,
+		},
+		{
+			name:     "emoji in data-label",
+			input:    `<p><mention-user data-id="user" data-label="Cool User 😎"></mention-user> test</p>`,
+			expected: `<p><strong>@Cool User 😎</strong> test</p>`,
+		},
+		{
+			name:     "nested HTML structure",
+			input:    `<div><p>Text with <mention-user data-id="user" data-label="User"></mention-user> in div</p></div>`,
+			expected: `<div><p>Text with <strong>@User</strong> in div</p></div>`,
+		},
+		{
+			name:     "mention in list",
+			input:    `<ul><li>Item with <mention-user data-id="user" data-label="User"></mention-user></li></ul>`,
+			expected: `<ul><li>Item with <strong>@User</strong></li></ul>`,
+		},
+		{
+			name:     "very long name",
+			input:    `<p><mention-user data-id="user" data-label="Christopher Montgomery Bartholomew Johnson-Smith III"></mention-user> test</p>`,
+			expected: `<p><strong>@Christopher Montgomery Bartholomew Johnson-Smith III</strong> test</p>`,
+		},
+		{
+			name:     "empty data-label and data-id with text content",
+			input:    `<p><mention-user>@fallback</mention-user> test</p>`,
+			expected: `<p><mention-user>@fallback</mention-user> test</p>`,
+		},
+		{
+			name:     "whitespace in data-label",
+			input:    `<p><mention-user data-id="user" data-label="  Spaces  "></mention-user> test</p>`,
+			expected: `<p><strong>@  Spaces  </strong> test</p>`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			db.LoadAndAssertFixtures(t)
+			s := db.NewSession()
+			defer s.Close()
+
+			result := formatMentionsForEmail(s, tt.input)
+			if tt.useRegex {
+				matched, err := regexp.MatchString(tt.expected, result)
+				require.NoError(t, err, "Invalid regex pattern: %s", tt.expected)
+				assert.True(t, matched, "Result does not match regex pattern.\nExpected pattern: %s\nActual result: %s", tt.expected, result)
+			} else {
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestFormatMentionsForEmail_MalformedHTML(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+	}{
+		{
+			name:  "unclosed tag - returns original",
+			input: `<p>Test <mention-user data-id="user" data-label="User">`,
+		},
+		{
+			name:  "invalid HTML entities",
+			input: `<p>Test &invalid; entity</p>`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			db.LoadAndAssertFixtures(t)
+			s := db.NewSession()
+			defer s.Close()
+
+			result := formatMentionsForEmail(s, tt.input)
+			// For malformed HTML, we expect it to either be fixed by the parser or returned as-is
+			// The key is that it shouldn't panic or error
+			assert.NotEmpty(t, result)
+		})
+	}
+}
--- a/pkg/models/notifications.go
+++ b/pkg/models/notifications.go
@@ -24,6 +24,7 @@ import (
 	"time"

 	"code.vikunja.io/api/pkg/config"
+	"code.vikunja.io/api/pkg/db"
 	"code.vikunja.io/api/pkg/i18n"
 	"code.vikunja.io/api/pkg/notifications"
 	"code.vikunja.io/api/pkg/user"
@@ -93,6 +94,9 @@ func (n *TaskCommentNotification) SubjectID() int64 {

 // ToMail returns the mail notification for TaskCommentNotification
 func (n *TaskCommentNotification) ToMail(lang string) *notifications.Mail {
+	s := db.NewSession()
+	defer s.Close()
+	formattedComment := formatMentionsForEmail(s, n.Comment.Comment)

 	mail := notifications.NewMail().
 		From(n.Doer.GetNameAndFromEmail()).
@@ -104,7 +108,7 @@ func (n *TaskCommentNotification) ToMail(lang string) *notifications.Mail {
 			Subject(i18n.T(lang, "notifications.task.comment.mentioned_subject", n.Doer.GetName(), n.Task.Title))
 	}

-	mail.HTML(n.Comment.Comment)
+	mail.HTML(formattedComment)

 	return mail.
 		Action(i18n.T(lang, "notifications.common.actions.open_task"), n.Task.GetFrontendURL())
@@ -350,6 +354,10 @@ func (n *UserMentionedInTaskNotification) SubjectID() int64 {

 // ToMail returns the mail notification for UserMentionedInTaskNotification
 func (n *UserMentionedInTaskNotification) ToMail(lang string) *notifications.Mail {
+	s := db.NewSession()
+	defer s.Close()
+	formattedDescription := formatMentionsForEmail(s, n.Task.Description)
+
 	var subject string
 	if n.IsNew {
 		subject = i18n.T(lang, "notifications.task.mentioned.subject_new", n.Doer.GetName(), n.Task.Title)
@@ -361,7 +369,7 @@ func (n *UserMentionedInTaskNotification) ToMail(lang string) *notifications.Mai
 		From(n.Doer.GetNameAndFromEmail()).
 		Subject(subject).
 		Line(i18n.T(lang, "notifications.task.mentioned.message", n.Doer.GetName())).
-		HTML(n.Task.Description)
+		HTML(formattedDescription)

 	return mail.
 		Action(i18n.T(lang, "notifications.common.actions.open_task"), n.Task.GetFrontendURL())