From 2980d341d482093aaf847ee5c24cfd95b8d3b3f4 Mon Sep 17 00:00:00 2001 From: Micah Thompson Date: Thu, 29 Aug 2019 06:09:09 -0400 Subject: [PATCH] MM-11102 Show channel links in notification emails (#11670) * Added code to hyperlink channels in HTML email Adjusted imports * Rending channel display name in hyperlink instead of channel URL handler Addressed go formatting warning * [MM-11102] Channel links should show as links in notification emails * Escaped post;ChannelMentions, GetChannelsByName functions; Changed tests * remove mlog.info * MultiMention and Escape tests; Change logging --- app/notification_email.go | 36 ++++++- app/notification_email_test.go | 176 +++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) diff --git a/app/notification_email.go b/app/notification_email.go index 07fba3449e..004ee085bc 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -6,6 +6,7 @@ package app import ( "fmt" "html" + "html/template" "net/url" "path/filepath" "strings" @@ -171,7 +172,10 @@ func (a *App) getNotificationEmailBody(recipient *model.User, post *model.Post, var bodyPage *utils.HTMLTemplate if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { bodyPage = a.NewEmailTemplate("post_body_full", recipient.Locale) - bodyPage.Props["PostMessage"] = a.GetMessageForNotification(post, translateFunc) + postMessage := a.GetMessageForNotification(post, translateFunc) + postMessage = html.EscapeString(postMessage) + normalizedPostMessage := a.generateHyperlinkForChannels(postMessage, teamName, teamURL) + bodyPage.Props["PostMessage"] = template.HTML(normalizedPostMessage) } else { bodyPage = a.NewEmailTemplate("post_body_generic", recipient.Locale) } @@ -283,6 +287,36 @@ func getFormattedPostTime(user *model.User, post *model.Post, useMilitaryTime bo } } +func (a *App) generateHyperlinkForChannels(postMessage, teamName, teamURL string) string { + team, err := a.GetTeamByName(teamName) + if err != nil { + mlog.Error("Encountered error while looking up team by name", mlog.String("Team Name", teamName), mlog.Err(err)) + return postMessage + } + + channelNames := model.ChannelMentions(postMessage) + if len(channelNames) == 0 { + return postMessage + } + + channels, err := a.GetChannelsByNames(channelNames, team.Id) + if err != nil { + mlog.Error("Encountered error while getting channels", mlog.Err(err)) + return postMessage + } + + visited := make(map[string]bool) + for _, ch := range channels { + if !visited[ch.Id] && ch.Type == model.CHANNEL_OPEN { + 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 +} + func (a *App) GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string { if len(strings.TrimSpace(post.Message)) != 0 || len(post.FileIds) == 0 { return post.Message diff --git a/app/notification_email_test.go b/app/notification_email_test.go index 1c920a9b0a..267694c2a1 100644 --- a/app/notification_email_test.go +++ b/app/notification_email_test.go @@ -10,6 +10,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/timezones" "github.com/mattermost/mattermost-server/utils" @@ -499,3 +501,177 @@ func TestGetNotificationEmailBodyGenericNotificationDirectChannel(t *testing.T) t.Fatal("Expected email text '" + teamURL + "'. Got " + body) } } + +func TestGetNotificationEmailEscapingChars(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + ch := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_OPEN, + } + channelName := "ChannelName" + recipient := &model.User{} + message := "Bold Test" + post := &model.Post{ + Message: message, + } + + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := th.App.getNotificationEmailBody(recipient, post, ch, + channelName, senderName, teamName, teamURL, + emailNotificationContentsType, true, translateFunc) + + fmt.Println(body) + assert.NotContains(t, body, message) +} + +func TestGetNotificationEmailBodyPublicChannelMention(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ch := th.BasicChannel + recipient := th.BasicUser2 + post := &model.Post{ + Message: "This is the message ~" + ch.Name, + } + + senderName := th.BasicUser.Username + teamName := th.BasicTeam.Name + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := th.App.getNotificationEmailBody(recipient, post, ch, + ch.Name, senderName, teamName, teamURL, + emailNotificationContentsType, true, translateFunc) + channelURL := teamURL + "/channels/" + ch.Name + mention := "~" + ch.Name + assert.Contains(t, body, ""+mention+"") +} + +func TestGetNotificationEmailBodyMultiPublicChannelMention(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ch := th.BasicChannel + mention := "~" + ch.Name + + ch2 := th.CreateChannel(th.BasicTeam) + mention2 := "~" + ch2.Name + + ch3 := th.CreateChannel(th.BasicTeam) + mention3 := "~" + ch3.Name + + message := fmt.Sprintf("This is the message Channel1: %s; Channel2: %s;"+ + " Channel3: %s", mention, mention2, mention3) + recipient := th.BasicUser2 + post := &model.Post{ + Message: message, + } + + senderName := th.BasicUser.Username + teamName := th.BasicTeam.Name + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := th.App.getNotificationEmailBody(recipient, post, ch, + ch.Name, senderName, teamName, teamURL, + emailNotificationContentsType, true, translateFunc) + channelURL := teamURL + "/channels/" + ch.Name + channelURL2 := teamURL + "/channels/" + ch2.Name + channelURL3 := teamURL + "/channels/" + ch3.Name + expMessage := fmt.Sprintf("This is the message Channel1: %s;"+ + " Channel2: %s; Channel3: %s", + channelURL, mention, channelURL2, mention2, channelURL3, mention3) + assert.Contains(t, body, expMessage) +} + +func TestGetNotificationEmailBodyPrivateChannelMention(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ch := th.CreatePrivateChannel(th.BasicTeam) + recipient := th.BasicUser2 + post := &model.Post{ + Message: "This is the message ~" + ch.Name, + } + + senderName := th.BasicUser.Username + teamName := ch.Name + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := th.App.getNotificationEmailBody(recipient, post, ch, + ch.Name, senderName, teamName, teamURL, + emailNotificationContentsType, true, translateFunc) + channelURL := teamURL + "/channels/" + ch.Name + mention := "~" + ch.Name + assert.NotContains(t, body, ""+mention+"") +} + +func TestGenerateHyperlinkForChannelsPublic(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ch := th.BasicChannel + message := "This is the message " + mention := "~" + ch.Name + + teamName := th.BasicTeam.Name + teamURL := "http://localhost:8065/" + teamName + + outMessage := th.App.generateHyperlinkForChannels(message+mention, teamName, teamURL) + channelURL := teamURL + "/channels/" + ch.Name + assert.Equal(t, message+""+mention+"", outMessage) +} + +func TestGenerateHyperlinkForChannelsMultiPublic(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ch := th.BasicChannel + mention := "~" + ch.Name + + ch2 := th.CreateChannel(th.BasicTeam) + mention2 := "~" + ch2.Name + + ch3 := th.CreateChannel(th.BasicTeam) + mention3 := "~" + ch3.Name + + message := fmt.Sprintf("This is the message Channel1: %s; Channel2: %s;"+ + " Channel3: %s", mention, mention2, mention3) + + teamName := th.BasicTeam.Name + teamURL := "http://localhost:8065/" + teamName + + outMessage := th.App.generateHyperlinkForChannels(message, teamName, teamURL) + channelURL := teamURL + "/channels/" + ch.Name + channelURL2 := teamURL + "/channels/" + ch2.Name + channelURL3 := teamURL + "/channels/" + ch3.Name + expMessage := fmt.Sprintf("This is the message Channel1: %s;"+ + " Channel2: %s; Channel3: %s", + channelURL, mention, channelURL2, mention2, channelURL3, mention3) + assert.Equal(t, expMessage, outMessage) +} + +func TestGenerateHyperlinkForChannelsPrivate(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + ch := th.CreatePrivateChannel(th.BasicTeam) + message := "This is the message ~" + ch.Name + + teamName := th.BasicTeam.Name + teamURL := "http://localhost:8065/" + teamName + + outMessage := th.App.generateHyperlinkForChannels(message, teamName, teamURL) + assert.Equal(t, message, outMessage) +}