MM-56629 Render Markdown in batched email notifications (#26217)

* [MM-56629] rendered markdown in batched email (#26116)

* [MM-56629] added unit test for GenerateHyperlinkForChannels (#26116)

* [MM-56629] refactored code to remove duplicates, updated test (#26116)

* [MM-56629] updated landing url and cleared check-style (#26116)

* [MM-56629] added app-layesrs and mocks (#26116)

* [MM-56629] updated unit tests (#26116)

* [MM-56629] reverted changes of removing unused context parameter (#26116)

* [MM-56629] removed unused method generateHyperlinkForChannels (#26116)

* [MM-56629] fixed check-style error (#26116)

* [MM-56629] refactored (#26116)

---------

Co-authored-by: Sazzad Hossain <sazzad.hossain@marginedge.com>
Co-authored-by: Harrison Healey <harrisonmhealey@gmail.com>
Этот коммит содержится в:
sazzad hossain
2024-03-07 22:27:08 +06:00
коммит произвёл GitHub
родитель ab029105fd
Коммит bc887441d0
8 изменённых файлов: 122 добавлений и 65 удалений

Просмотреть файл

@@ -715,7 +715,7 @@ type AppIface interface {
GetLogs(rctx request.CTX, page, perPage int) ([]string, *model.AppError)
GetLogsSkipSend(rctx request.CTX, page, perPage int, logFilter *model.LogFilter) ([]string, *model.AppError)
GetMemberCountsByGroup(rctx request.CTX, channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError)
GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string
GetMessageForNotification(post *model.Post, teamName, siteUrl string, translateFunc i18n.TranslateFunc) string
GetMultipleEmojiByName(c request.CTX, names []string) ([]*model.Emoji, *model.AppError)
GetNewUsersForTeamPage(teamID string, page, perPage int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError)
GetNextPostIdFromPostList(postList *model.PostList, collapsedThreads bool) string

Просмотреть файл

@@ -332,12 +332,13 @@ func (es *Service) sendBatchedEmailNotification(userID string, notifications []*
channelDisplayName = truncateUserNames(channel.DisplayName, 11)
}
postMessage := es.GetMessageForNotification(notification.post, notification.teamName, siteURL, translateFunc)
postsData = append(postsData, &postData{
SenderPhoto: senderPhoto,
SenderName: truncateUserNames(sender.GetDisplayName(displayNameFormat), 22),
Time: t,
ChannelName: channelDisplayName,
Message: template.HTML(es.GetMessageForNotification(notification.post, translateFunc)),
Message: template.HTML(postMessage),
MessageURL: MessageURL,
ShowChannelIcon: showChannelIcon,
OtherChannelMembersCount: otherChannelMembersCount,

Просмотреть файл

@@ -13,6 +13,8 @@ import (
model "github.com/mattermost/mattermost/server/public/model"
store "github.com/mattermost/mattermost/server/v8/channels/store"
templates "github.com/mattermost/mattermost/server/v8/platform/shared/templates"
throttled "github.com/throttled/throttled"
@@ -65,13 +67,37 @@ func (_m *ServiceInterface) CreateVerifyEmailToken(userID string, newEmail strin
return r0, r1
}
// GetMessageForNotification provides a mock function with given fields: post, translateFunc
func (_m *ServiceInterface) GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string {
ret := _m.Called(post, translateFunc)
// GenerateHyperlinkForChannels provides a mock function with given fields: postMessage, teamName, teamURL
func (_m *ServiceInterface) GenerateHyperlinkForChannels(postMessage string, teamName string, teamURL string) (string, error) {
ret := _m.Called(postMessage, teamName, teamURL)
var r0 string
if rf, ok := ret.Get(0).(func(*model.Post, i18n.TranslateFunc) string); ok {
r0 = rf(post, translateFunc)
var r1 error
if rf, ok := ret.Get(0).(func(string, string, string) (string, error)); ok {
return rf(postMessage, teamName, teamURL)
}
if rf, ok := ret.Get(0).(func(string, string, string) string); ok {
r0 = rf(postMessage, teamName, teamURL)
} else {
r0 = ret.Get(0).(string)
}
if rf, ok := ret.Get(1).(func(string, string, string) error); ok {
r1 = rf(postMessage, teamName, teamURL)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// GetMessageForNotification provides a mock function with given fields: post, teamName, siteUrl, translateFunc
func (_m *ServiceInterface) GetMessageForNotification(post *model.Post, teamName string, siteUrl string, translateFunc i18n.TranslateFunc) string {
ret := _m.Called(post, teamName, siteUrl, translateFunc)
var r0 string
if rf, ok := ret.Get(0).(func(*model.Post, string, string, i18n.TranslateFunc) string); ok {
r0 = rf(post, teamName, siteUrl, translateFunc)
} else {
r0 = ret.Get(0).(string)
}
@@ -608,6 +634,11 @@ func (_m *ServiceInterface) SendWelcomeEmail(userID string, _a1 string, verified
return r0
}
// SetStore provides a mock function with given fields: st
func (_m *ServiceInterface) SetStore(st store.Store) {
_m.Called(st)
}
// Stop provides a mock function with given fields:
func (_m *ServiceInterface) Stop() {
_m.Called()

Просмотреть файл

@@ -4,6 +4,7 @@
package email
import (
"fmt"
"html"
"html/template"
"net/url"
@@ -28,9 +29,9 @@ type EmailMessageAttachment struct {
FieldRows []FieldRow
}
func (es *Service) GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string {
func (es *Service) GetMessageForNotification(post *model.Post, teamName, siteUrl string, translateFunc i18n.TranslateFunc) string {
if strings.TrimSpace(post.Message) != "" || len(post.FileIds) == 0 {
return post.Message
return es.prepareNotificationMessageForEmail(post.Message, teamName, siteUrl)
}
// extract the filenames from their paths and determine what type of files are attached
@@ -134,3 +135,49 @@ func prepareTextForEmail(text, siteURL string) template.HTML {
return template.HTML(markdownText)
}
func (es *Service) prepareNotificationMessageForEmail(postMessage, teamName, siteURL string) string {
postMessage = html.EscapeString(postMessage)
mdPostMessage, mdErr := utils.MarkdownToHTML(postMessage, siteURL)
if mdErr != nil {
mlog.Warn("Encountered error while converting markdown to HTML", mlog.Err(mdErr))
mdPostMessage = postMessage
}
landingURL := siteURL + "/landing#/" + teamName
normalizedPostMessage, err := es.GenerateHyperlinkForChannels(mdPostMessage, teamName, landingURL)
if err != nil {
mlog.Warn("Encountered error while generating hyperlink for channels", mlog.String("team_name", teamName), mlog.Err(err))
normalizedPostMessage = mdPostMessage
}
return normalizedPostMessage
}
func (es *Service) GenerateHyperlinkForChannels(postMessage, teamName, landingURL string) (string, error) {
channelNames := model.ChannelMentions(postMessage)
if len(channelNames) == 0 {
return postMessage, nil
}
team, err := es.Store().Team().GetByName(teamName)
if err != nil {
mlog.Error("Team not found with the name", mlog.String("team_name", teamName), mlog.Err(err))
return postMessage, nil
}
channels, err := es.store.Channel().GetByNames(team.Id, channelNames, true)
if err != nil {
return "", err
}
visited := make(map[string]bool)
for _, ch := range channels {
if !visited[ch.Id] && ch.Type == model.ChannelTypeOpen {
channelURL := landingURL + "/channels/" + ch.Name
channelHyperLink := fmt.Sprintf("<a href='%s'>%s</a>", channelURL, "~"+ch.Name)
postMessage = strings.Replace(postMessage, "~"+ch.Name, channelHyperLink, -1)
visited[ch.Id] = true
}
}
return postMessage, nil
}

Просмотреть файл

@@ -162,14 +162,24 @@ type ServiceInterface interface {
SendNoCardPaymentFailedEmail(email string, locale string, siteURL string) error
SendRemoveExpiredLicenseEmail(ctaText, ctaLink, email, locale, siteURL string) error
AddNotificationEmailToBatch(user *model.User, post *model.Post, team *model.Team) *model.AppError
GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string
GetMessageForNotification(post *model.Post, teamName, siteUrl string, translateFunc i18n.TranslateFunc) string
GenerateHyperlinkForChannels(postMessage, teamName, teamURL string) (string, error)
InitEmailBatching()
SendChangeUsernameEmail(newUsername, email, locale, siteURL string) error
CreateVerifyEmailToken(userID string, newEmail string) (*model.Token, error)
SendIPFiltersChangedEmail(email string, userWhoChangedFilter *model.User, siteURL, portalURL, locale string, isWorkspaceOwner bool) error
SetStore(st store.Store)
Stop()
}
func (es *Service) Store() store.Store {
return es.store
}
func (es *Service) SetStore(st store.Store) {
es.store = st
}
func (es *Service) GetPerDayEmailRateLimiter() *throttled.GCRARateLimiter {
return es.perDayEmailRateLimiter
}

Просмотреть файл

@@ -9,7 +9,6 @@ import (
"html"
"html/template"
"io"
"strings"
"github.com/pkg/errors"
@@ -232,20 +231,8 @@ func (a *App) getNotificationEmailBody(c request.CTX, recipient *model.User, pos
}
if emailNotificationContentsType == model.EmailNotificationContentsFull {
postMessage := a.GetMessageForNotification(post, translateFunc)
postMessage = html.EscapeString(postMessage)
mdPostMessage, mdErr := utils.MarkdownToHTML(postMessage, a.GetSiteURL())
if mdErr != nil {
c.Logger().Warn("Encountered error while converting markdown to HTML", mlog.Err(mdErr))
mdPostMessage = postMessage
}
normalizedPostMessage, err := a.generateHyperlinkForChannels(c, mdPostMessage, teamName, landingURL)
if err != nil {
c.Logger().Warn("Encountered error while generating hyperlink for channels", mlog.String("team_name", teamName), mlog.Err(err))
normalizedPostMessage = mdPostMessage
}
pData.Message = template.HTML(normalizedPostMessage)
postMessage := a.GetMessageForNotification(post, teamName, a.GetSiteURL(), translateFunc)
pData.Message = template.HTML(postMessage)
pData.Time = translateFunc("app.notification.body.dm.time", messageTime)
pData.MessageAttachments = email.ProcessMessageAttachments(post, a.GetSiteURL())
}
@@ -309,34 +296,6 @@ func (a *App) getNotificationEmailBody(c request.CTX, recipient *model.User, pos
return a.Srv().TemplatesContainer().RenderToString("messages_notification", data)
}
func (a *App) generateHyperlinkForChannels(c request.CTX, postMessage, teamName, teamURL string) (string, *model.AppError) {
team, err := a.GetTeamByName(teamName)
if err != nil {
return "", err
}
channelNames := model.ChannelMentions(postMessage)
if len(channelNames) == 0 {
return postMessage, nil
}
channels, err := a.GetChannelsByNames(c, channelNames, team.Id)
if err != nil {
return "", err
}
visited := make(map[string]bool)
for _, ch := range channels {
if !visited[ch.Id] && ch.Type == model.ChannelTypeOpen {
channelURL := teamURL + "/channels/" + ch.Name
channelHyperLink := fmt.Sprintf("<a href='%s'>%s</a>", channelURL, "~"+ch.Name)
postMessage = strings.Replace(postMessage, "~"+ch.Name, channelHyperLink, -1)
visited[ch.Id] = true
}
}
return postMessage, nil
}
func (a *App) GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string {
return a.Srv().EmailService.GetMessageForNotification(post, translateFunc)
func (a *App) GetMessageForNotification(post *model.Post, teamName, siteUrl string, translateFunc i18n.TranslateFunc) string {
return a.Srv().EmailService.GetMessageForNotification(post, teamName, siteUrl, translateFunc)
}

Просмотреть файл

@@ -615,7 +615,7 @@ func TestGetNotificationEmailBodyPublicChannelMention(t *testing.T) {
senderName := "user1"
teamName := "testteam"
teamURL := "http://localhost:8065/testteam"
teamURL := th.App.GetSiteURL() + "/landing#" + "/testteam"
emailNotificationContentsType := model.EmailNotificationContentsFull
translateFunc := i18n.GetUserTranslations("en")
@@ -628,6 +628,8 @@ func TestGetNotificationEmailBodyPublicChannelMention(t *testing.T) {
channelStoreMock.On("GetByNames", "test", []string{ch.Name}, true).Return([]*model.Channel{ch}, nil)
storeMock.On("Channel").Return(&channelStoreMock)
th.App.Srv().EmailService.SetStore(storeMock)
body, err := th.App.getNotificationEmailBody(th.Context, recipient, post, ch,
ch.Name, senderName, teamName, teamURL,
emailNotificationContentsType, true, translateFunc, "user-avatar.png")
@@ -681,7 +683,7 @@ func TestGetNotificationEmailBodyMultiPublicChannelMention(t *testing.T) {
senderName := "user1"
teamName := "testteam"
teamURL := "http://localhost:8065/testteam"
teamURL := th.App.GetSiteURL() + "/landing#" + "/testteam"
emailNotificationContentsType := model.EmailNotificationContentsFull
translateFunc := i18n.GetUserTranslations("en")
@@ -694,6 +696,8 @@ func TestGetNotificationEmailBodyMultiPublicChannelMention(t *testing.T) {
channelStoreMock.On("GetByNames", "test", []string{ch.Name, ch2.Name, ch3.Name}, true).Return([]*model.Channel{ch, ch2, ch3}, nil)
storeMock.On("Channel").Return(&channelStoreMock)
th.App.Srv().EmailService.SetStore(storeMock)
body, err := th.App.getNotificationEmailBody(th.Context, recipient, post, ch,
ch.Name, senderName, teamName, teamURL,
emailNotificationContentsType, true, translateFunc, "user-avatar.png")
@@ -743,6 +747,8 @@ func TestGetNotificationEmailBodyPrivateChannelMention(t *testing.T) {
channelStoreMock.On("GetByNames", "test", []string{ch.Name}, true).Return([]*model.Channel{ch}, nil)
storeMock.On("Channel").Return(&channelStoreMock)
th.App.Srv().EmailService.SetStore(storeMock)
body, err := th.App.getNotificationEmailBody(th.Context, recipient, post, ch,
ch.Name, senderName, teamName, teamURL,
emailNotificationContentsType, true, translateFunc, "user-avatar.png")
@@ -776,8 +782,9 @@ func TestGenerateHyperlinkForChannelsPublic(t *testing.T) {
channelStoreMock.On("GetByNames", "test", []string{ch.Name}, true).Return([]*model.Channel{ch}, nil)
storeMock.On("Channel").Return(&channelStoreMock)
outMessage, err := th.App.generateHyperlinkForChannels(th.Context, message+mention, teamName, teamURL)
require.Nil(t, err)
th.App.Srv().EmailService.SetStore(storeMock)
outMessage, err := th.App.Srv().EmailService.GenerateHyperlinkForChannels(message+mention, teamName, teamURL)
require.NoError(t, err)
channelURL := teamURL + "/channels/" + ch.Name
assert.Equal(t, message+"<a href='"+channelURL+"'>"+mention+"</a>", outMessage)
}
@@ -826,8 +833,9 @@ func TestGenerateHyperlinkForChannelsMultiPublic(t *testing.T) {
channelStoreMock.On("GetByNames", "test", []string{ch.Name, ch2.Name, ch3.Name}, true).Return([]*model.Channel{ch, ch2, ch3}, nil)
storeMock.On("Channel").Return(&channelStoreMock)
outMessage, err := th.App.generateHyperlinkForChannels(th.Context, message, teamName, teamURL)
require.Nil(t, err)
th.App.Srv().EmailService.SetStore(storeMock)
outMessage, err := th.App.Srv().EmailService.GenerateHyperlinkForChannels(message, teamName, teamURL)
require.NoError(t, err)
channelURL := teamURL + "/channels/" + ch.Name
channelURL2 := teamURL + "/channels/" + ch2.Name
channelURL3 := teamURL + "/channels/" + ch3.Name
@@ -860,8 +868,9 @@ func TestGenerateHyperlinkForChannelsPrivate(t *testing.T) {
channelStoreMock.On("GetByNames", "test", []string{ch.Name}, true).Return([]*model.Channel{ch}, nil)
storeMock.On("Channel").Return(&channelStoreMock)
outMessage, err := th.App.generateHyperlinkForChannels(th.Context, message, teamName, teamURL)
require.Nil(t, err)
th.App.Srv().EmailService.SetStore(storeMock)
outMessage, err := th.App.Srv().EmailService.GenerateHyperlinkForChannels(message, teamName, teamURL)
require.NoError(t, err)
assert.Equal(t, message, outMessage)
}

Просмотреть файл

@@ -7401,7 +7401,7 @@ func (a *OpenTracingAppLayer) GetMemberCountsByGroup(rctx request.CTX, channelID
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string {
func (a *OpenTracingAppLayer) GetMessageForNotification(post *model.Post, teamName string, siteUrl string, translateFunc i18n.TranslateFunc) string {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetMessageForNotification")
@@ -7413,7 +7413,7 @@ func (a *OpenTracingAppLayer) GetMessageForNotification(post *model.Post, transl
}()
defer span.Finish()
resultVar0 := a.app.GetMessageForNotification(post, translateFunc)
resultVar0 := a.app.GetMessageForNotification(post, teamName, siteUrl, translateFunc)
return resultVar0
}