[MM-48521] Fix for batch notification email not rendering properly (#21736)

* Add MessageAttachment field to postdata while rendering batched email notifications

* Add test for SlackAttachments body generator

* Add context to changes

* Add license text to new test file

* Use app/email/notification_email.go as single source for attachment generation

app/email/email_batching.go

@@ -33,6 +33,7 @@ type postData struct {
 	Time                     string
 	ShowChannelIcon          bool
 	OtherChannelMembersCount int
+	MessageAttachments       []*EmailMessageAttachment
 }
 
 func (es *Service) InitEmailBatching() {
@@ -314,6 +315,7 @@ func (es *Service) sendBatchedEmailNotification(userID string, notifications []*
 				MessageURL:               MessageURL,
 				ShowChannelIcon:          showChannelIcon,
 				OtherChannelMembersCount: otherChannelMembersCount,
+				MessageAttachments:       ProcessMessageAttachments(notification.post),
 			})
 		}
 	}

app/email/notification_email.go

@@ -4,6 +4,8 @@
 package email
 
 import (
+	"html"
+	"html/template"
 	"net/url"
 	"path/filepath"
 	"strings"
@@ -11,8 +13,21 @@ import (
 	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/mattermost/mattermost-server/v6/shared/i18n"
 	"github.com/mattermost/mattermost-server/v6/shared/mlog"
+	"github.com/mattermost/mattermost-server/v6/utils"
 )
 
+type FieldRow struct {
+	Cells []*model.SlackAttachmentField
+}
+
+type EmailMessageAttachment struct {
+	model.SlackAttachment
+
+	Pretext   template.HTML
+	Text      template.HTML
+	FieldRows []FieldRow
+}
+
 func (es *Service) GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string {
 	if strings.TrimSpace(post.Message) != "" || len(post.FileIds) == 0 {
 		return post.Message
@@ -44,3 +59,78 @@ func (es *Service) GetMessageForNotification(post *model.Post, translateFunc i18
 	}
 	return translateFunc("api.post.get_message_for_notification.files_sent", len(filenames), props)
 }
+
+func ProcessMessageAttachments(post *model.Post) []*EmailMessageAttachment {
+	emailMessageAttachments := []*EmailMessageAttachment{}
+
+	for _, messageAttachment := range post.Attachments() {
+		emailMessageAttachment := &EmailMessageAttachment{
+			SlackAttachment: *messageAttachment,
+			Pretext:         prepareTextForEmail(messageAttachment.Pretext),
+			Text:            prepareTextForEmail(messageAttachment.Text),
+		}
+
+		stripedTitle, err := utils.StripMarkdown(emailMessageAttachment.Title)
+		if err != nil {
+			mlog.Warn("Failed parse to markdown from messageatatchment title", mlog.String("post_id", post.Id), mlog.Err(err))
+			stripedTitle = ""
+		}
+
+		emailMessageAttachment.Title = stripedTitle
+
+		shortFieldRow := FieldRow{}
+
+		for i := range messageAttachment.Fields {
+			// Create a new instance to avoid altering the original pointer reference
+			// We update field value to parse markdown.
+			// If we do that on the original pointer, the rendered text in mattermost
+			// becomes invalid as its no longer a markdown string, but rather an HTML string.
+			field := &model.SlackAttachmentField{
+				Title: messageAttachment.Fields[i].Title,
+				Value: messageAttachment.Fields[i].Value,
+				Short: messageAttachment.Fields[i].Short,
+			}
+
+			if stringValue, ok := field.Value.(string); ok {
+				field.Value = prepareTextForEmail(stringValue)
+			}
+
+			if !field.Short {
+				if len(shortFieldRow.Cells) > 0 {
+					emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, shortFieldRow)
+					shortFieldRow = FieldRow{}
+				}
+
+				emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, FieldRow{[]*model.SlackAttachmentField{field}})
+			} else {
+				shortFieldRow.Cells = append(shortFieldRow.Cells, field)
+
+				if len(shortFieldRow.Cells) == 2 {
+					emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, shortFieldRow)
+					shortFieldRow = FieldRow{}
+				}
+			}
+		}
+
+		// collect any leftover short fields
+		if len(shortFieldRow.Cells) > 0 {
+			emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, shortFieldRow)
+			shortFieldRow = FieldRow{}
+		}
+
+		emailMessageAttachments = append(emailMessageAttachments, emailMessageAttachment)
+	}
+
+	return emailMessageAttachments
+}
+
+func prepareTextForEmail(text string) template.HTML {
+	escapedText := html.EscapeString(text)
+	markdownText, err := utils.MarkdownToHTML(escapedText)
+	if err != nil {
+		mlog.Warn("Encountered error while converting markdown to HTML", mlog.Err(err))
+		return template.HTML(text)
+	}
+
+	return template.HTML(markdownText)
+}

app/email/notification_email_test.go

@@ -0,0 +1,72 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package email
+
+import (
+	"testing"
+
+	"github.com/mattermost/mattermost-server/v6/model"
+	"github.com/stretchr/testify/require"
+)
+
+func TestProcessMessageAttachments(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	post := &model.Post{
+		Message: "This is the message",
+	}
+
+	messageAttachments := []*model.SlackAttachment{
+		{
+			Color:      "#FF0000",
+			Pretext:    "message attachment 1 pretext",
+			AuthorName: "author name",
+			AuthorLink: "https://example.com/slack_attachment_1/author_link",
+			AuthorIcon: "https://example.com/slack_attachment_1/author_icon",
+			Title:      "message attachment 1 title",
+			TitleLink:  "https://example.com/slack_attachment_1/title_link",
+			Text:       "message attachment 1 text",
+			ImageURL:   "https://example.com/slack_attachment_1/image",
+			ThumbURL:   "https://example.com/slack_attachment_1/thumb",
+			Fields: []*model.SlackAttachmentField{
+				{
+					Short: true,
+					Title: "message attachment 1 field 1 title",
+					Value: "message attachment 1 field 1 value",
+				},
+				{
+					Short: false,
+					Title: "message attachment 1 field 2 title",
+					Value: "message attachment 1 field 2 value",
+				},
+				{
+					Short: true,
+					Title: "message attachment 1 field 3 title",
+					Value: "message attachment 1 field 3 value",
+				},
+				{
+					Short: true,
+					Title: "message attachment 1 field 4 title",
+					Value: "message attachment 1 field 4 value",
+				},
+			},
+		},
+		{
+			Color:      "#FF0000",
+			Pretext:    "message attachment 2 pretext",
+			AuthorName: "author name 2",
+			Text:       "message attachment 2 text",
+		},
+	}
+
+	model.ParseSlackAttachment(post, messageAttachments)
+
+	processedAttachcmentsPost := ProcessMessageAttachments(post)
+	require.NotNil(t, processedAttachcmentsPost)
+	require.Len(t, processedAttachcmentsPost, 2)
+	require.Equal(t, processedAttachcmentsPost[0].Color, "#FF0000")
+	require.Equal(t, processedAttachcmentsPost[0].FieldRows[0].Cells[0].Title, "message attachment 1 field 1 title")
+	require.Equal(t, processedAttachcmentsPost[1].Color, "#FF0000")
+}

app/notification_email.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	email "github.com/mattermost/mattermost-server/v6/app/email"
 	"github.com/mattermost/mattermost-server/v6/app/request"
 	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/mattermost/mattermost-server/v6/shared/i18n"
@@ -201,18 +202,6 @@ func truncateUserNames(name string, i int) string {
 	return name
 }
 
-type FieldRow struct {
-	Cells []*model.SlackAttachmentField
-}
-
-type EmailMessageAttachment struct {
-	model.SlackAttachment
-
-	Pretext   template.HTML
-	Text      template.HTML
-	FieldRows []FieldRow
-}
-
 type postData struct {
 	SenderName               string
 	ChannelName              string
@@ -223,7 +212,7 @@ type postData struct {
 	Time                     string
 	ShowChannelIcon          bool
 	OtherChannelMembersCount int
-	MessageAttachments       []*EmailMessageAttachment
+	MessageAttachments       []*email.EmailMessageAttachment
 }
 
 /**
@@ -258,7 +247,7 @@ func (a *App) getNotificationEmailBody(c request.CTX, recipient *model.User, pos
 		}
 		pData.Message = template.HTML(normalizedPostMessage)
 		pData.Time = translateFunc("app.notification.body.dm.time", messageTime)
-		pData.MessageAttachments = a.processMessageAttachments(post)
+		pData.MessageAttachments = email.ProcessMessageAttachments(post)
 	}
 
 	data := a.Srv().EmailService.NewEmailTemplateData(recipient.Locale)
@@ -320,81 +309,6 @@ func (a *App) getNotificationEmailBody(c request.CTX, recipient *model.User, pos
 	return a.Srv().TemplatesContainer().RenderToString("messages_notification", data)
 }
 
-func (a *App) processMessageAttachments(post *model.Post) []*EmailMessageAttachment {
-	emailMessageAttachments := []*EmailMessageAttachment{}
-
-	for _, messageAttachment := range post.Attachments() {
-		emailMessageAttachment := &EmailMessageAttachment{
-			SlackAttachment: *messageAttachment,
-			Pretext:         a.prepareTextForEmail(messageAttachment.Pretext),
-			Text:            a.prepareTextForEmail(messageAttachment.Text),
-		}
-
-		stripedTitle, err := utils.StripMarkdown(emailMessageAttachment.Title)
-		if err != nil {
-			mlog.Warn("Failed parse to markdown from messageatatchment title", mlog.String("post_id", post.Id), mlog.Err(err))
-			stripedTitle = ""
-		}
-
-		emailMessageAttachment.Title = stripedTitle
-
-		shortFieldRow := FieldRow{}
-
-		for i := range messageAttachment.Fields {
-			// Create a new instance to avoid altering the original pointer reference
-			// We update field value to parse markdown.
-			// If we do that on the original pointer, the rendered text in mattermost
-			// becomes invalid as its no longer a markdown string, but rather an HTML string.
-			field := &model.SlackAttachmentField{
-				Title: messageAttachment.Fields[i].Title,
-				Value: messageAttachment.Fields[i].Value,
-				Short: messageAttachment.Fields[i].Short,
-			}
-
-			if stringValue, ok := field.Value.(string); ok {
-				field.Value = a.prepareTextForEmail(stringValue)
-			}
-
-			if !field.Short {
-				if len(shortFieldRow.Cells) > 0 {
-					emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, shortFieldRow)
-					shortFieldRow = FieldRow{}
-				}
-
-				emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, FieldRow{[]*model.SlackAttachmentField{field}})
-			} else {
-				shortFieldRow.Cells = append(shortFieldRow.Cells, field)
-
-				if len(shortFieldRow.Cells) == 2 {
-					emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, shortFieldRow)
-					shortFieldRow = FieldRow{}
-				}
-			}
-		}
-
-		// collect any leftover short fields
-		if len(shortFieldRow.Cells) > 0 {
-			emailMessageAttachment.FieldRows = append(emailMessageAttachment.FieldRows, shortFieldRow)
-			shortFieldRow = FieldRow{}
-		}
-
-		emailMessageAttachments = append(emailMessageAttachments, emailMessageAttachment)
-	}
-
-	return emailMessageAttachments
-}
-
-func (a *App) prepareTextForEmail(text string) template.HTML {
-	escapedText := html.EscapeString(text)
-	markdownText, err := utils.MarkdownToHTML(escapedText)
-	if err != nil {
-		mlog.Warn("Encountered error while converting markdown to HTML", mlog.Err(err))
-		return template.HTML(text)
-	}
-
-	return template.HTML(markdownText)
-}
-
 type formattedPostTime struct {
 	Time     time.Time
 	Year     string