diff --git a/app/email_batching.go b/app/email_batching.go index e698708148..a578daf041 100644 --- a/app/email_batching.go +++ b/app/email_batching.go @@ -177,9 +177,30 @@ func sendBatchedEmailNotification(userId string, notifications []*batchedNotific var contents string for _, notification := range notifications { - template := utils.NewHTMLTemplate("post_batched_post", user.Locale) + var sender *model.User + schan := Srv.Store.User().Get(notification.post.UserId) + if result := <-schan; result.Err != nil { + l4g.Warn(utils.T("api.email_batching.render_batched_post.sender.app_error")) + continue + } else { + sender = result.Data.(*model.User) + } - contents += renderBatchedPost(template, notification.post, notification.teamName, displayNameFormat, translateFunc) + var channel *model.Channel + cchan := Srv.Store.Channel().Get(notification.post.ChannelId, true) + if result := <-cchan; result.Err != nil { + l4g.Warn(utils.T("api.email_batching.render_batched_post.channel.app_error")) + continue + } else { + channel = result.Data.(*model.Channel) + } + + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + if utils.IsLicensed && *utils.License.Features.EmailNotificationContents { + emailNotificationContentsType = *utils.Cfg.EmailSettings.EmailNotificationContentsType + } + + contents += renderBatchedPost(notification, channel, sender, *utils.Cfg.ServiceSettings.SiteURL, displayNameFormat, translateFunc, user.Locale, emailNotificationContentsType) } tm := time.Unix(notifications[0].post.CreateAt/1000, 0) @@ -201,15 +222,21 @@ func sendBatchedEmailNotification(userId string, notifications []*batchedNotific } } -func renderBatchedPost(template *utils.HTMLTemplate, post *model.Post, teamName string, displayNameFormat string, translateFunc i18n.TranslateFunc) string { - schan := Srv.Store.User().Get(post.UserId) - cchan := Srv.Store.Channel().Get(post.ChannelId, true) +func renderBatchedPost(notification *batchedNotification, channel *model.Channel, sender *model.User, siteURL string, displayNameFormat string, translateFunc i18n.TranslateFunc, userLocale string, emailNotificationContentsType string) string { + // don't include message contents if email notification contents type is set to generic + var template *utils.HTMLTemplate + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + template = utils.NewHTMLTemplate("post_batched_post_full", userLocale) + } else { + template = utils.NewHTMLTemplate("post_batched_post_generic", userLocale) + } template.Props["Button"] = translateFunc("api.email_batching.render_batched_post.go_to_post") - template.Props["PostMessage"] = GetMessageForNotification(post, translateFunc) - template.Props["PostLink"] = *utils.Cfg.ServiceSettings.SiteURL + "/" + teamName + "/pl/" + post.Id + template.Props["PostMessage"] = GetMessageForNotification(notification.post, translateFunc) + template.Props["PostLink"] = siteURL + "/" + notification.teamName + "/pl/" + notification.post.Id + template.Props["SenderName"] = sender.GetDisplayName(displayNameFormat) - tm := time.Unix(post.CreateAt/1000, 0) + tm := time.Unix(notification.post.CreateAt/1000, 0) timezone, _ := tm.Zone() template.Props["Date"] = translateFunc("api.email_batching.render_batched_post.date", map[string]interface{}{ @@ -221,22 +248,17 @@ func renderBatchedPost(template *utils.HTMLTemplate, post *model.Post, teamName "Timezone": timezone, }) - if result := <-schan; result.Err != nil { - l4g.Warn(utils.T("api.email_batching.render_batched_post.sender.app_error")) - return "" - } else { - template.Props["SenderName"] = result.Data.(*model.User).GetDisplayName(displayNameFormat) - } - - if result := <-cchan; result.Err != nil { - l4g.Warn(utils.T("api.email_batching.render_batched_post.channel.app_error")) - return "" - } else if channel := result.Data.(*model.Channel); channel.Type == model.CHANNEL_DIRECT { + if channel.Type == model.CHANNEL_DIRECT { template.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.direct_message") } else if channel.Type == model.CHANNEL_GROUP { template.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.group_message") } else { - template.Props["ChannelName"] = channel.DisplayName + // don't include channel name if email notification contents type is set to generic + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + template.Props["ChannelName"] = channel.DisplayName + } else { + template.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.notification") + } } return template.Render() diff --git a/app/email_batching_test.go b/app/email_batching_test.go index b5c18d3788..24acc8a656 100644 --- a/app/email_batching_test.go +++ b/app/email_batching_test.go @@ -4,6 +4,7 @@ package app import ( + "strings" "testing" "time" @@ -271,3 +272,53 @@ func TestCheckPendingNotificationsCantParseInterval(t *testing.T) { t.Fatal("should have sent queued post") } } + +/* + * Ensures that post contents are not included in notification email when email notification content type is set to generic + */ +func TestRenderBatchedPostGeneric(t *testing.T) { + Setup() + var post = &model.Post{} + post.Message = "This is the message" + var notification = &batchedNotification{} + notification.post = post + var channel = &model.Channel{} + channel.DisplayName = "Some Test Channel" + var sender = &model.User{} + sender.Email = "sender@test.com" + + translateFunc := func(translationID string, args ...interface{}) string { + // mock translateFunc just returns the translation id - this is good enough for our purposes + return translationID + } + + var rendered = renderBatchedPost(notification, channel, sender, "http://localhost:8065", "", translateFunc, "en", model.EMAIL_NOTIFICATION_CONTENTS_GENERIC) + if strings.Contains(rendered, post.Message) { + t.Fatal("Rendered email should not contain post contents when email notification contents type is set to Generic.") + } +} + +/* + * Ensures that post contents included in notification email when email notification content type is set to full + */ +func TestRenderBatchedPostFull(t *testing.T) { + Setup() + var post = &model.Post{} + post.Message = "This is the message" + var notification = &batchedNotification{} + notification.post = post + var channel = &model.Channel{} + channel.DisplayName = "Some Test Channel" + var sender = &model.User{} + sender.Email = "sender@test.com" + + translateFunc := func(translationID string, args ...interface{}) string { + // mock translateFunc just returns the translation id - this is good enough for our purposes + return translationID + } + + var rendered = renderBatchedPost(notification, channel, sender, "http://localhost:8065", "", translateFunc, "en", model.EMAIL_NOTIFICATION_CONTENTS_FULL) + if !strings.Contains(rendered, post.Message) { + t.Fatal("Rendered email should contain post contents when email notification contents type is set to Full.") + } +} diff --git a/app/notification.go b/app/notification.go index d4af6463b2..b9153037e9 100644 --- a/app/notification.go +++ b/app/notification.go @@ -349,67 +349,25 @@ func sendNotificationEmail(post *model.Post, user *model.User, channel *model.Ch // fall back to sending a single email if we can't batch it for some reason } - var channelName string - var bodyText string + translateFunc := utils.GetUserTranslations(user.Locale) + var subjectText string - var mailTemplate string - var mailParameters map[string]interface{} + if channel.Type == model.CHANNEL_DIRECT { + subjectText = getDirectMessageNotificationEmailSubject(post, translateFunc, utils.Cfg.TeamSettings.SiteName, senderName) + } else { + subjectText = getNotificationEmailSubject(post, translateFunc, utils.Cfg.TeamSettings.SiteName, team.DisplayName) + } + + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + if utils.IsLicensed && *utils.License.Features.EmailNotificationContents { + emailNotificationContentsType = *utils.Cfg.EmailSettings.EmailNotificationContentsType + } teamURL := utils.GetSiteURL() + "/" + team.Name - tm := time.Unix(post.CreateAt/1000, 0) - - userLocale := utils.GetUserTranslations(user.Locale) - month := userLocale(tm.Month().String()) - day := fmt.Sprintf("%d", tm.Day()) - year := fmt.Sprintf("%d", tm.Year()) - zone, _ := tm.Zone() - - if channel.Type == model.CHANNEL_DIRECT { - bodyText = userLocale("api.post.send_notifications_and_forget.message_body") - subjectText = userLocale("api.post.send_notifications_and_forget.message_subject") - - senderDisplayName := senderName - - mailTemplate = "api.templates.post_subject_in_direct_message" - mailParameters = map[string]interface{}{"SubjectText": subjectText, - "SenderDisplayName": senderDisplayName, "Month": month, "Day": day, "Year": year} - } else if channel.Type == model.CHANNEL_GROUP { - bodyText = userLocale("api.post.send_notifications_and_forget.mention_body") - - senderDisplayName := senderName - - mailTemplate = "api.templates.post_subject_in_group_message" - mailParameters = map[string]interface{}{"SenderDisplayName": senderDisplayName, "Month": month, "Day": day, "Year": year} - channelName = userLocale("api.templates.channel_name.group") - } else { - bodyText = userLocale("api.post.send_notifications_and_forget.mention_body") - subjectText = userLocale("api.post.send_notifications_and_forget.mention_subject") - channelName = channel.DisplayName - mailTemplate = "api.templates.post_subject_in_channel" - mailParameters = map[string]interface{}{"SubjectText": subjectText, "TeamDisplayName": team.DisplayName, - "ChannelName": channelName, "Month": month, "Day": day, "Year": year} - } - - subject := fmt.Sprintf("[%v] %v", utils.Cfg.TeamSettings.SiteName, userLocale(mailTemplate, mailParameters)) - - bodyPage := utils.NewHTMLTemplate("post_body", user.Locale) - bodyPage.Props["SiteURL"] = utils.GetSiteURL() - bodyPage.Props["PostMessage"] = GetMessageForNotification(post, userLocale) - if team.Name != "select_team" { - bodyPage.Props["TeamLink"] = teamURL + "/pl/" + post.Id - } else { - bodyPage.Props["TeamLink"] = teamURL - } - - bodyPage.Props["BodyText"] = bodyText - bodyPage.Props["Button"] = userLocale("api.templates.post_body.button") - bodyPage.Html["Info"] = template.HTML(userLocale("api.templates.post_body.info", - map[string]interface{}{"ChannelName": channelName, "SenderName": senderName, - "Hour": fmt.Sprintf("%02d", tm.Hour()), "Minute": fmt.Sprintf("%02d", tm.Minute()), - "TimeZone": zone, "Month": month, "Day": day})) + var bodyText = getNotificationEmailBody(user, post, channel, senderName, team.Name, teamURL, emailNotificationContentsType, translateFunc) go func() { - if err := utils.SendMail(user.Email, html.UnescapeString(subject), bodyPage.Render()); err != nil { + if err := utils.SendMail(user.Email, html.UnescapeString(subjectText), bodyText); err != nil { l4g.Error(utils.T("api.post.send_notifications_and_forget.send.error"), user.Email, err) } }() @@ -421,6 +379,149 @@ func sendNotificationEmail(post *model.Post, user *model.User, channel *model.Ch return nil } +/** + * Computes the subject line for direct notification email messages + */ +func getDirectMessageNotificationEmailSubject(post *model.Post, translateFunc i18n.TranslateFunc, siteName string, senderName string) string { + t := getFormattedPostTime(post, translateFunc) + var subjectParameters = map[string]interface{}{ + "SiteName": siteName, + "SenderDisplayName": senderName, + "Month": t.Month, + "Day": t.Day, + "Year": t.Year, + } + return translateFunc("app.notification.subject.direct.full", subjectParameters) +} + +/** + * Computes the subject line for group, public, and private email messages + */ +func getNotificationEmailSubject(post *model.Post, translateFunc i18n.TranslateFunc, siteName string, teamName string) string { + t := getFormattedPostTime(post, translateFunc) + var subjectParameters = map[string]interface{}{ + "SiteName": siteName, + "TeamName": teamName, + "Month": t.Month, + "Day": t.Day, + "Year": t.Year, + } + return translateFunc("app.notification.subject.notification.full", subjectParameters) +} + +/** + * Computes the email body for notification messages + */ +func getNotificationEmailBody(recipient *model.User, post *model.Post, channel *model.Channel, senderName string, teamName string, teamURL string, emailNotificationContentsType string, translateFunc i18n.TranslateFunc) string { + // only include message contents in notification email if email notification contents type is set to full + var bodyPage *utils.HTMLTemplate + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + bodyPage = utils.NewHTMLTemplate("post_body_full", recipient.Locale) + bodyPage.Props["PostMessage"] = GetMessageForNotification(post, translateFunc) + } else { + bodyPage = utils.NewHTMLTemplate("post_body_generic", recipient.Locale) + } + + bodyPage.Props["SiteURL"] = utils.GetSiteURL() + if teamName != "select_team" { + bodyPage.Props["TeamLink"] = teamURL + "/pl/" + post.Id + } else { + bodyPage.Props["TeamLink"] = teamURL + } + + var channelName = channel.DisplayName + if channel.Type == model.CHANNEL_GROUP { + channelName = translateFunc("api.templates.channel_name.group") + } + t := getFormattedPostTime(post, translateFunc) + + var bodyText string + var info string + if channel.Type == model.CHANNEL_DIRECT { + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + bodyText = translateFunc("app.notification.body.intro.direct.full") + info = translateFunc("app.notification.body.text.direct.full", + map[string]interface{}{ + "SenderName": senderName, + "Hour": t.Hour, + "Minute": t.Minute, + "TimeZone": t.TimeZone, + "Month": t.Month, + "Day": t.Day, + }) + } else { + bodyText = translateFunc("app.notification.body.intro.direct.generic", map[string]interface{}{ + "SenderName": senderName, + }) + info = translateFunc("app.notification.body.text.direct.generic", + map[string]interface{}{ + "Hour": t.Hour, + "Minute": t.Minute, + "TimeZone": t.TimeZone, + "Month": t.Month, + "Day": t.Day, + }) + } + } else { + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + bodyText = translateFunc("app.notification.body.intro.notification.full") + info = translateFunc("app.notification.body.text.notification.full", + map[string]interface{}{ + "ChannelName": channelName, + "SenderName": senderName, + "Hour": t.Hour, + "Minute": t.Minute, + "TimeZone": t.TimeZone, + "Month": t.Month, + "Day": t.Day, + }) + } else { + bodyText = translateFunc("app.notification.body.intro.notification.generic", map[string]interface{}{ + "SenderName": senderName, + }) + info = translateFunc("app.notification.body.text.notification.generic", + map[string]interface{}{ + "Hour": t.Hour, + "Minute": t.Minute, + "TimeZone": t.TimeZone, + "Month": t.Month, + "Day": t.Day, + }) + } + } + + bodyPage.Props["BodyText"] = bodyText + bodyPage.Html["Info"] = template.HTML(info) + bodyPage.Props["Button"] = translateFunc("api.templates.post_body.button") + + return bodyPage.Render() +} + +type formattedPostTime struct { + Time time.Time + Year string + Month string + Day string + Hour string + Minute string + TimeZone string +} + +func getFormattedPostTime(post *model.Post, translateFunc i18n.TranslateFunc) formattedPostTime { + tm := time.Unix(post.CreateAt/1000, 0) + zone, _ := tm.Zone() + + return formattedPostTime{ + Time: tm, + Year: fmt.Sprintf("%d", tm.Year()), + Month: translateFunc(tm.Month().String()), + Day: fmt.Sprintf("%d", tm.Day()), + Hour: fmt.Sprintf("%02d", tm.Hour()), + Minute: fmt.Sprintf("%02d", tm.Minute()), + TimeZone: zone, + } +} + func 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_test.go b/app/notification_test.go index 022f671ae4..5f57290e73 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -4,9 +4,11 @@ package app import ( + "strings" "testing" "github.com/mattermost/platform/model" + "github.com/mattermost/platform/utils" ) func TestSendNotifications(t *testing.T) { @@ -817,3 +819,287 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { t.Fatal("Should have been false") } } + +func TestGetDirectMessageNotificationEmailSubject(t *testing.T) { + Setup() + expectedPrefix := "[http://localhost:8065] New Direct Message from sender on" + post := &model.Post{ + CreateAt: 1501804801000, + } + translateFunc := utils.GetUserTranslations("en") + subject := getDirectMessageNotificationEmailSubject(post, translateFunc, "http://localhost:8065", "sender") + if !strings.HasPrefix(subject, expectedPrefix) { + t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject) + } +} + +func TestGetNotificationEmailSubject(t *testing.T) { + Setup() + expectedPrefix := "[http://localhost:8065] Notification in team on" + post := &model.Post{ + CreateAt: 1501804801000, + } + translateFunc := utils.GetUserTranslations("en") + subject := getNotificationEmailSubject(post, translateFunc, "http://localhost:8065", "team") + if !strings.HasPrefix(subject, expectedPrefix) { + t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject) + } +} + +func TestGetNotificationEmailBodyFullNotificationPublicChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_OPEN, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new notification.") { + t.Fatal("Expected email text 'You have a new notification. Got " + body) + } + if !strings.Contains(body, "CHANNEL: "+channel.DisplayName) { + t.Fatal("Expected email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) + } + if !strings.Contains(body, senderName+" - ") { + t.Fatal("Expected email text '" + senderName + " - '. Got " + body) + } + if !strings.Contains(body, post.Message) { + t.Fatal("Expected email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +func TestGetNotificationEmailBodyFullNotificationGroupChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_GROUP, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new notification.") { + t.Fatal("Expected email text 'You have a new notification. Got " + body) + } + if !strings.Contains(body, "CHANNEL: Group Message") { + t.Fatal("Expected email text 'CHANNEL: Group Message'. Got " + body) + } + if !strings.Contains(body, senderName+" - ") { + t.Fatal("Expected email text '" + senderName + " - '. Got " + body) + } + if !strings.Contains(body, post.Message) { + t.Fatal("Expected email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +func TestGetNotificationEmailBodyFullNotificationPrivateChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_PRIVATE, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new notification.") { + t.Fatal("Expected email text 'You have a new notification. Got " + body) + } + if !strings.Contains(body, "CHANNEL: "+channel.DisplayName) { + t.Fatal("Expected email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) + } + if !strings.Contains(body, senderName+" - ") { + t.Fatal("Expected email text '" + senderName + " - '. Got " + body) + } + if !strings.Contains(body, post.Message) { + t.Fatal("Expected email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +func TestGetNotificationEmailBodyFullNotificationDirectChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_DIRECT, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new direct message.") { + t.Fatal("Expected email text 'You have a new direct message. Got " + body) + } + if !strings.Contains(body, senderName+" - ") { + t.Fatal("Expected email text '" + senderName + " - '. Got " + body) + } + if !strings.Contains(body, post.Message) { + t.Fatal("Expected email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +// from here +func TestGetNotificationEmailBodyGenericNotificationPublicChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_OPEN, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new notification from "+senderName) { + t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body) + } + if strings.Contains(body, "CHANNEL: "+channel.DisplayName) { + t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) + } + if strings.Contains(body, post.Message) { + t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +func TestGetNotificationEmailBodyGenericNotificationGroupChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_GROUP, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new notification from "+senderName) { + t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body) + } + if strings.Contains(body, "CHANNEL: "+channel.DisplayName) { + t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) + } + if strings.Contains(body, post.Message) { + t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +func TestGetNotificationEmailBodyGenericNotificationPrivateChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_PRIVATE, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new notification from "+senderName) { + t.Fatal("Expected email text 'You have a new notification from " + senderName + "'. Got " + body) + } + if strings.Contains(body, "CHANNEL: "+channel.DisplayName) { + t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) + } + if strings.Contains(body, post.Message) { + t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} + +func TestGetNotificationEmailBodyGenericNotificationDirectChannel(t *testing.T) { + Setup() + recipient := &model.User{} + post := &model.Post{ + Message: "This is the message", + } + channel := &model.Channel{ + DisplayName: "ChannelName", + Type: model.CHANNEL_DIRECT, + } + senderName := "sender" + teamName := "team" + teamURL := "http://localhost:8065/" + teamName + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC + translateFunc := utils.GetUserTranslations("en") + + body := getNotificationEmailBody(recipient, post, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new direct message from "+senderName) { + t.Fatal("Expected email text 'You have a new direct message from " + senderName + "'. Got " + body) + } + if strings.Contains(body, "CHANNEL: "+channel.DisplayName) { + t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) + } + if strings.Contains(body, post.Message) { + t.Fatal("Did not expect email text '" + post.Message + "'. Got " + body) + } + if !strings.Contains(body, teamURL) { + t.Fatal("Expected email text '" + teamURL + "'. Got " + body) + } +} diff --git a/config/config.json b/config/config.json index 310fe1ce79..23d1d2ec04 100644 --- a/config/config.json +++ b/config/config.json @@ -143,7 +143,8 @@ "EnableEmailBatching": false, "EmailBatchingBufferSize": 256, "EmailBatchingInterval": 30, - "SkipServerCertificateVerification": false + "SkipServerCertificateVerification": false, + "EmailNotificationContentsType": "full" }, "RateLimitSettings": { "Enable": false, diff --git a/i18n/en.json b/i18n/en.json index 4769d26146..730017e956 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1117,7 +1117,7 @@ }, { "id": "api.email_batching.render_batched_post.direct_message", - "translation": "Direct Message" + "translation": "Direct Message from " }, { "id": "api.email_batching.render_batched_post.go_to_post", @@ -1125,7 +1125,11 @@ }, { "id": "api.email_batching.render_batched_post.group_message", - "translation": "Group Message" + "translation": "Group Message from " + }, + { + "id": "api.email_batching.render_batched_post.notification", + "translation": "Notification from " }, { "id": "api.email_batching.render_batched_post.sender.app_error", @@ -1134,8 +1138,8 @@ { "id": "api.email_batching.send_batched_email_notification.body_text", "translation": { - "one": "You have a new message.", - "other": "You have {{.Count}} new messages." + "one": "You have a new notification.", + "other": "You have {{.Count}} new notifications." } }, { @@ -1803,22 +1807,10 @@ "id": "api.post.send_notifications_and_forget.get_teams.error", "translation": "Failed to get teams when sending cross-team DM user_id=%v, err=%v" }, - { - "id": "api.post.send_notifications_and_forget.mention_body", - "translation": "You have one new mention." - }, { "id": "api.post.send_notifications_and_forget.mention_subject", "translation": "New Mention" }, - { - "id": "api.post.send_notifications_and_forget.message_body", - "translation": "You have one new message." - }, - { - "id": "api.post.send_notifications_and_forget.message_subject", - "translation": "New Direct Message" - }, { "id": "api.post.send_notifications_and_forget.push_image_only", "translation": " Uploaded one or more files in " @@ -2459,22 +2451,6 @@ "id": "api.templates.post_body.button", "translation": "Go To Post" }, - { - "id": "api.templates.post_body.info", - "translation": "CHANNEL: {{.ChannelName}}
{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}" - }, - { - "id": "api.templates.post_subject_in_channel", - "translation": "{{.SubjectText}} in {{.TeamDisplayName}} ({{.ChannelName}}) on {{.Month}} {{.Day}}, {{.Year}}" - }, - { - "id": "api.templates.post_subject_in_direct_message", - "translation": "{{.SubjectText}} from {{.SenderDisplayName}} on {{.Month}} {{.Day}}, {{.Year}}" - }, - { - "id": "api.templates.post_subject_in_group_message", - "translation": "New Group Message from {{ .SenderDisplayName}} on {{.Month}} {{.Day}}, {{.Year}}" - }, { "id": "api.templates.reset_body.button", "translation": "Reset Password" @@ -3139,6 +3115,46 @@ "id": "api.websocket_handler.invalid_param.app_error", "translation": "Invalid {{.Name}} parameter" }, + { + "id": "app.notification.subject.direct.full", + "translation": "[{{.SiteName}}] New Direct Message from {{.SenderDisplayName}} on {{.Month}} {{.Day}}, {{.Year}}" + }, + { + "id": "app.notification.subject.notification.full", + "translation": "[{{ .SiteName }}] Notification in {{ .TeamName}} on {{.Month}} {{.Day}}, {{.Year}}" + }, + { + "id": "app.notification.body.intro.direct.full", + "translation": "You have a new direct message." + }, + { + "id": "app.notification.body.intro.direct.generic", + "translation": "You have a new direct message from {{.SenderName}}" + }, + { + "id": "app.notification.body.intro.notification.full", + "translation": "You have a new notification." + }, + { + "id": "app.notification.body.intro.notification.generic", + "translation": "You have a new notification from {{.SenderName}}" + }, + { + "id": "app.notification.body.text.direct.full", + "translation": "{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}" + }, + { + "id": "app.notification.body.text.direct.generic", + "translation": "{{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}" + }, + { + "id": "app.notification.body.text.notification.full", + "translation": "CHANNEL: {{.ChannelName}}
{{.SenderName}} - {{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}" + }, + { + "id": "app.notification.body.text.notification.generic", + "translation": "{{.Hour}}:{{.Minute}} {{.TimeZone}}, {{.Month}} {{.Day}}" + }, { "id": "app.user_access_token.disabled", "translation": "User access tokens are disabled on this server. Please contact your system administrator for details." @@ -4439,6 +4455,10 @@ "id": "model.config.is_valid.email_batching_interval.app_error", "translation": "Invalid email batching interval for email settings. Must be 30 seconds or more." }, + { + "id": "model.config.is_valid.email_notification_contents_type.app_error", + "translation": "Invalid email notification contents type for email settings. Must be one of either 'full' or 'generic'." + }, { "id": "model.config.is_valid.email_reset_salt.app_error", "translation": "Invalid password reset salt for email settings. Must be 32 chars or more." diff --git a/model/config.go b/model/config.go index 5b0916cd34..55fe8490be 100644 --- a/model/config.go +++ b/model/config.go @@ -66,6 +66,9 @@ const ( EMAIL_BATCHING_BUFFER_SIZE = 256 EMAIL_BATCHING_INTERVAL = 30 + EMAIL_NOTIFICATION_CONTENTS_FULL = "full" + EMAIL_NOTIFICATION_CONTENTS_GENERIC = "generic" + SITENAME_MAX_LENGTH = 30 SERVICE_SETTINGS_DEFAULT_SITE_URL = "" @@ -284,6 +287,7 @@ type EmailSettings struct { EmailBatchingBufferSize *int EmailBatchingInterval *int SkipServerCertificateVerification *bool + EmailNotificationContentsType *string } type RateLimitSettings struct { @@ -819,6 +823,11 @@ func (o *Config) SetDefaults() { *o.EmailSettings.SkipServerCertificateVerification = false } + if o.EmailSettings.EmailNotificationContentsType == nil { + o.EmailSettings.EmailNotificationContentsType = new(string) + *o.EmailSettings.EmailNotificationContentsType = EMAIL_NOTIFICATION_CONTENTS_FULL + } + if !IsSafeLink(o.SupportSettings.TermsOfServiceLink) { *o.SupportSettings.TermsOfServiceLink = SUPPORT_SETTINGS_DEFAULT_TERMS_OF_SERVICE_LINK } @@ -1550,6 +1559,10 @@ func (o *Config) IsValid() *AppError { return NewLocAppError("Config.IsValid", "model.config.is_valid.email_batching_interval.app_error", nil, "") } + if !(*o.EmailSettings.EmailNotificationContentsType == EMAIL_NOTIFICATION_CONTENTS_FULL || *o.EmailSettings.EmailNotificationContentsType == EMAIL_NOTIFICATION_CONTENTS_GENERIC) { + return NewLocAppError("Config.IsValid", "model.config.is_valid.email_notification_contents_type.app_error", nil, "") + } + if o.RateLimitSettings.MemoryStoreSize <= 0 { return NewLocAppError("Config.IsValid", "model.config.is_valid.rate_mem.app_error", nil, "") } diff --git a/model/config_test.go b/model/config_test.go index 62a77c1335..1a944710fe 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -15,3 +15,12 @@ func TestConfigDefaultFileSettingsDirectory(t *testing.T) { t.Fatal("FileSettings.Directory should default to './data/'") } } + +func TestConfigDefaultEmailNotificationContentsType(t *testing.T) { + c1 := Config{} + c1.SetDefaults() + + if *c1.EmailSettings.EmailNotificationContentsType != EMAIL_NOTIFICATION_CONTENTS_FULL { + t.Fatal("EmailSettings.EmailNotificationContentsType should default to 'full'") + } +} diff --git a/model/license.go b/model/license.go index 8d53bd4cdc..ea10897230 100644 --- a/model/license.go +++ b/model/license.go @@ -37,39 +37,42 @@ type Customer struct { } type Features struct { - Users *int `json:"users"` - LDAP *bool `json:"ldap"` - MFA *bool `json:"mfa"` - GoogleOAuth *bool `json:"google_oauth"` - Office365OAuth *bool `json:"office365_oauth"` - Compliance *bool `json:"compliance"` - Cluster *bool `json:"cluster"` - Metrics *bool `json:"metrics"` - CustomBrand *bool `json:"custom_brand"` - MHPNS *bool `json:"mhpns"` - SAML *bool `json:"saml"` - PasswordRequirements *bool `json:"password_requirements"` - Elasticsearch *bool `json:"elastic_search"` - Announcement *bool `json:"announcement"` + Users *int `json:"users"` + LDAP *bool `json:"ldap"` + MFA *bool `json:"mfa"` + GoogleOAuth *bool `json:"google_oauth"` + Office365OAuth *bool `json:"office365_oauth"` + Compliance *bool `json:"compliance"` + Cluster *bool `json:"cluster"` + Metrics *bool `json:"metrics"` + CustomBrand *bool `json:"custom_brand"` + MHPNS *bool `json:"mhpns"` + SAML *bool `json:"saml"` + PasswordRequirements *bool `json:"password_requirements"` + Elasticsearch *bool `json:"elastic_search"` + Announcement *bool `json:"announcement"` + EmailNotificationContents *bool `json:"email_notification_contents"` + // after we enabled more features for webrtc we'll need to control them with this FutureFeatures *bool `json:"future_features"` } func (f *Features) ToMap() map[string]interface{} { return map[string]interface{}{ - "ldap": *f.LDAP, - "mfa": *f.MFA, - "google": *f.GoogleOAuth, - "office365": *f.Office365OAuth, - "compliance": *f.Compliance, - "cluster": *f.Cluster, - "metrics": *f.Metrics, - "custom_brand": *f.CustomBrand, - "mhpns": *f.MHPNS, - "saml": *f.SAML, - "password": *f.PasswordRequirements, - "elastic_search": *f.Elasticsearch, - "future": *f.FutureFeatures, + "ldap": *f.LDAP, + "mfa": *f.MFA, + "google": *f.GoogleOAuth, + "office365": *f.Office365OAuth, + "compliance": *f.Compliance, + "cluster": *f.Cluster, + "metrics": *f.Metrics, + "custom_brand": *f.CustomBrand, + "mhpns": *f.MHPNS, + "saml": *f.SAML, + "password": *f.PasswordRequirements, + "elastic_search": *f.Elasticsearch, + "email_notification_contents": *f.EmailNotificationContents, + "future": *f.FutureFeatures, } } @@ -148,6 +151,11 @@ func (f *Features) SetDefaults() { f.Announcement = new(bool) *f.Announcement = true } + + if f.EmailNotificationContents == nil { + f.EmailNotificationContents = new(bool) + *f.EmailNotificationContents = *f.FutureFeatures + } } func (l *License) IsExpired() bool { diff --git a/model/license_test.go b/model/license_test.go index 952ab493e3..2338c9b93d 100644 --- a/model/license_test.go +++ b/model/license_test.go @@ -27,6 +27,7 @@ func TestLicenseFeaturesToMap(t *testing.T) { CheckTrue(t, m["password"].(bool)) CheckTrue(t, m["elastic_search"].(bool)) CheckTrue(t, m["future"].(bool)) + CheckTrue(t, m["email_notification_contents"].(bool)) } func TestLicenseFeaturesSetDefaults(t *testing.T) { @@ -46,6 +47,7 @@ func TestLicenseFeaturesSetDefaults(t *testing.T) { CheckTrue(t, *f.SAML) CheckTrue(t, *f.PasswordRequirements) CheckTrue(t, *f.Elasticsearch) + CheckTrue(t, *f.EmailNotificationContents) CheckTrue(t, *f.FutureFeatures) f = Features{} @@ -65,6 +67,7 @@ func TestLicenseFeaturesSetDefaults(t *testing.T) { *f.SAML = true *f.PasswordRequirements = true *f.Elasticsearch = true + *f.EmailNotificationContents = true f.SetDefaults() @@ -81,6 +84,7 @@ func TestLicenseFeaturesSetDefaults(t *testing.T) { CheckTrue(t, *f.SAML) CheckTrue(t, *f.PasswordRequirements) CheckTrue(t, *f.Elasticsearch) + CheckTrue(t, *f.EmailNotificationContents) CheckFalse(t, *f.FutureFeatures) } diff --git a/templates/post_batched_post.html b/templates/post_batched_post_full.html similarity index 97% rename from templates/post_batched_post.html rename to templates/post_batched_post_full.html index f3dbf9d6d4..7e12da46eb 100644 --- a/templates/post_batched_post.html +++ b/templates/post_batched_post_full.html @@ -1,4 +1,4 @@ -{{define "post_batched_post"}} +{{define "post_batched_post_full"}} + + + + + + + + +
+ + {{.Props.ChannelName}} + + + @{{.Props.SenderName}} + +
+
+ + {{.Props.Date}} + +
+
+ + {{.Props.Button}} + +
+ +{{end}} diff --git a/templates/post_body.html b/templates/post_body_full.html similarity index 99% rename from templates/post_body.html rename to templates/post_body_full.html index 54f34d1dd4..fa27aba550 100644 --- a/templates/post_body.html +++ b/templates/post_body_full.html @@ -1,4 +1,4 @@ -{{define "post_body"}} +{{define "post_body_full"}} diff --git a/templates/post_body_generic.html b/templates/post_body_generic.html new file mode 100644 index 0000000000..bdd358e993 --- /dev/null +++ b/templates/post_body_generic.html @@ -0,0 +1,45 @@ +{{define "post_body_generic"}} + +
+ + + +
+ + + + +
+ + + + + + + + + {{template "email_footer" . }} + +
+ +
+ + + + + + {{template "email_info" . }} + +
+

{{.Props.BodyText}}

+

{{.Html.Info}}

+

+ {{.Props.Button}} +

+
+
+
+
+ +{{end}} + diff --git a/utils/config.go b/utils/config.go index 6a973fe1c8..c0c7ecc209 100644 --- a/utils/config.go +++ b/utils/config.go @@ -442,6 +442,7 @@ func getClientConfig(c *model.Config) map[string]string { props["EnableSignInWithUsername"] = strconv.FormatBool(*c.EmailSettings.EnableSignInWithUsername) props["RequireEmailVerification"] = strconv.FormatBool(c.EmailSettings.RequireEmailVerification) props["EnableEmailBatching"] = strconv.FormatBool(*c.EmailSettings.EnableEmailBatching) + props["EmailNotificationContentsType"] = *c.EmailSettings.EmailNotificationContentsType props["EnableSignUpWithGitLab"] = strconv.FormatBool(c.GitLabSettings.Enable) diff --git a/utils/config_test.go b/utils/config_test.go index a6bfa4e82d..e49073b8e3 100644 --- a/utils/config_test.go +++ b/utils/config_test.go @@ -277,3 +277,13 @@ func TestValidateLocales(t *testing.T) { t.Fatal("Should have returned an error validating AvailableLocales") } } + +func TestGetClientConfig(t *testing.T) { + TranslationsPreInit() + LoadConfig("config.json") + + configMap := getClientConfig(Cfg) + if configMap["EmailNotificationContentsType"] != *Cfg.EmailSettings.EmailNotificationContentsType { + t.Fatal("EmailSettings.EmailNotificationContentsType not exposed to client config") + } +} diff --git a/utils/license.go b/utils/license.go index 3647b51ccd..e28a43e29e 100644 --- a/utils/license.go +++ b/utils/license.go @@ -186,6 +186,7 @@ func getClientLicense(l *model.License) map[string]string { props["Email"] = l.Customer.Email props["Company"] = l.Customer.Company props["PhoneNumber"] = l.Customer.PhoneNumber + props["EmailNotificationContents"] = strconv.FormatBool(*l.Features.EmailNotificationContents) } return props diff --git a/webapp/components/admin_console/email_settings.jsx b/webapp/components/admin_console/email_settings.jsx index e48bc46ab5..e630402bc5 100644 --- a/webapp/components/admin_console/email_settings.jsx +++ b/webapp/components/admin_console/email_settings.jsx @@ -11,11 +11,15 @@ import * as Utils from 'utils/utils.jsx'; import AdminSettings from './admin_settings.jsx'; import BooleanSetting from './boolean_setting.jsx'; import {ConnectionSecurityDropdownSettingEmail} from './connection_security_dropdown_setting.jsx'; +import DropdownSetting from './dropdown_setting.jsx'; import EmailConnectionTest from './email_connection_test.jsx'; import {FormattedHTMLMessage, FormattedMessage} from 'react-intl'; import SettingsGroup from './settings_group.jsx'; import TextSetting from './text_setting.jsx'; +const EMAIL_NOTIFICATION_CONTENTS_FULL = 'full'; +const EMAIL_NOTIFICATION_CONTENTS_GENERIC = 'generic'; + export default class EmailSettings extends AdminSettings { constructor(props) { super(props); @@ -39,6 +43,7 @@ export default class EmailSettings extends AdminSettings { config.EmailSettings.EnableEmailBatching = this.state.enableEmailBatching; config.ServiceSettings.EnableSecurityFixAlert = this.state.enableSecurityFixAlert; config.EmailSettings.SkipServerCertificateVerification = this.state.skipServerCertificateVerification; + config.EmailSettings.EmailNotificationContentsType = this.state.emailNotificationContentsType; return config; } @@ -63,7 +68,8 @@ export default class EmailSettings extends AdminSettings { connectionSecurity: config.EmailSettings.ConnectionSecurity, enableEmailBatching: config.EmailSettings.EnableEmailBatching, skipServerCertificateVerification: config.EmailSettings.SkipServerCertificateVerification, - enableSecurityFixAlert: config.ServiceSettings.EnableSecurityFixAlert + enableSecurityFixAlert: config.ServiceSettings.EnableSecurityFixAlert, + emailNotificationContentsType: config.EmailSettings.EmailNotificationContentsType }; } @@ -105,6 +111,48 @@ export default class EmailSettings extends AdminSettings { ); } + let emailNotificationContentsTypeDropdown = null; + let emailNotificationContentsHelpText = null; + if (window.mm_license.EmailNotificationContents === 'true') { + const emailNotificationContentsTypes = []; + emailNotificationContentsTypes.push({value: EMAIL_NOTIFICATION_CONTENTS_FULL, text: Utils.localizeMessage('admin.email.notification.contents.full', 'Send full message contents')}); + emailNotificationContentsTypes.push({value: EMAIL_NOTIFICATION_CONTENTS_GENERIC, text: Utils.localizeMessage('admin.email.notification.contents.generic', 'Send generic description with only sender name')}); + + if (this.state.emailNotificationContentsType === EMAIL_NOTIFICATION_CONTENTS_FULL) { + emailNotificationContentsHelpText = ( + + ); + } else if (this.state.emailNotificationContentsType === EMAIL_NOTIFICATION_CONTENTS_GENERIC) { + emailNotificationContentsHelpText = ( + + ); + } + + emailNotificationContentsTypeDropdown = ( + + } + value={this.state.emailNotificationContentsType} + onChange={this.handleChange} + helpText={emailNotificationContentsHelpText} + /> + ); + } + return ( + {emailNotificationContentsTypeDropdown} Typically used for compliance reasons if Mattermost contains confidential information and policy dictates it cannot be stored in email.", + "admin.email.notification.contents.generic.description": "Only the name of the person who sent the message, with no information about channel name or message contents are included in email notifications.
Typically used for compliance reasons if Mattermost contains confidential information and policy dictates it cannot be stored in email.", "admin.false": "false", "admin.file.enableFileAttachments": "Allow File Sharing:", "admin.file.enableFileAttachmentsDesc": "When false, disables file sharing on the server. All file and image uploads on messages are forbidden across clients and devices, including mobile.",