diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index c0ce94a936..84f054d888 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -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 diff --git a/server/channels/app/email/email_batching.go b/server/channels/app/email/email_batching.go index 3d9fac3cc7..8877fef72c 100644 --- a/server/channels/app/email/email_batching.go +++ b/server/channels/app/email/email_batching.go @@ -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, diff --git a/server/channels/app/email/mocks/ServiceInterface.go b/server/channels/app/email/mocks/ServiceInterface.go index deaca07976..ea78445a1a 100644 --- a/server/channels/app/email/mocks/ServiceInterface.go +++ b/server/channels/app/email/mocks/ServiceInterface.go @@ -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() diff --git a/server/channels/app/email/notification_email.go b/server/channels/app/email/notification_email.go index 9ff1f37995..22850af547 100644 --- a/server/channels/app/email/notification_email.go +++ b/server/channels/app/email/notification_email.go @@ -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("%s", channelURL, "~"+ch.Name) + postMessage = strings.Replace(postMessage, "~"+ch.Name, channelHyperLink, -1) + visited[ch.Id] = true + } + } + return postMessage, nil +} diff --git a/server/channels/app/email/service.go b/server/channels/app/email/service.go index adf21e531e..4df5e608b8 100644 --- a/server/channels/app/email/service.go +++ b/server/channels/app/email/service.go @@ -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 } diff --git a/server/channels/app/notification_email.go b/server/channels/app/notification_email.go index a7ad3e3cf7..755548196d 100644 --- a/server/channels/app/notification_email.go +++ b/server/channels/app/notification_email.go @@ -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("%s", 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) } diff --git a/server/channels/app/notification_email_test.go b/server/channels/app/notification_email_test.go index ea983d0c3d..967241811d 100644 --- a/server/channels/app/notification_email_test.go +++ b/server/channels/app/notification_email_test.go @@ -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+""+mention+"", 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) } diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index a6e07582de..c214c7d542 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -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 }