From 02f8c18f40cd0e973e4c75b751e8fcbbbd019728 Mon Sep 17 00:00:00 2001 From: Ugurcan Turkdogan Date: Tue, 15 May 2018 13:43:59 -0700 Subject: Update email notification subject line and contents for Group Messages (#8689) Reordered notification strings MM-10335 Changed uppercase CHANNEL to Lowercase Channel, added @ sign before username on notifications Added @ sign in front of username in all email notifications. Capitalized Direct Message and Group Message in email notifications. Fixed the issue with long group message names. Removed executable bit --- app/notification.go | 77 ++++++++++++++++++++++++++------ app/notification_test.go | 114 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 141 insertions(+), 50 deletions(-) (limited to 'app') diff --git a/app/notification.go b/app/notification.go index 4bdc6c94f..0fbc33060 100644 --- a/app/notification.go +++ b/app/notification.go @@ -217,7 +217,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } if userAllowsEmails && status.Status != model.STATUS_ONLINE && profileMap[id].DeleteAt == 0 { - a.sendNotificationEmail(post, profileMap[id], channel, team, senderName, sender) + a.sendNotificationEmail(post, profileMap[id], channel, team, channelName, senderName, sender) } } } @@ -351,7 +351,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod return mentionedUsersList, nil } -func (a *App) sendNotificationEmail(post *model.Post, user *model.User, channel *model.Channel, team *model.Team, senderName string, sender *model.User) *model.AppError { +func (a *App) sendNotificationEmail(post *model.Post, user *model.User, channel *model.Channel, team *model.Team, channelName string, senderName string, sender *model.User) *model.AppError { if channel.IsGroupOrDirect() { if result := <-a.Srv.Store.Team().GetTeamsByUserId(user.Id); result.Err != nil { return result.Err @@ -396,22 +396,24 @@ func (a *App) sendNotificationEmail(post *model.Post, user *model.User, channel translateFunc := utils.GetUserTranslations(user.Locale) + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + if license := a.License(); license != nil && *license.Features.EmailNotificationContents { + emailNotificationContentsType = *a.Config().EmailSettings.EmailNotificationContentsType + } + var subjectText string if channel.Type == model.CHANNEL_DIRECT { subjectText = getDirectMessageNotificationEmailSubject(post, translateFunc, a.Config().TeamSettings.SiteName, senderName) + } else if channel.Type == model.CHANNEL_GROUP { + subjectText = getGroupMessageNotificationEmailSubject(post, translateFunc, a.Config().TeamSettings.SiteName, channelName, emailNotificationContentsType) } else if *a.Config().EmailSettings.UseChannelInEmailNotifications { subjectText = getNotificationEmailSubject(post, translateFunc, a.Config().TeamSettings.SiteName, team.DisplayName+" ("+channel.DisplayName+")") } else { subjectText = getNotificationEmailSubject(post, translateFunc, a.Config().TeamSettings.SiteName, team.DisplayName) } - emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL - if license := a.License(); license != nil && *license.Features.EmailNotificationContents { - emailNotificationContentsType = *a.Config().EmailSettings.EmailNotificationContentsType - } - teamURL := a.GetSiteURL() + "/" + team.Name - var bodyText = a.getNotificationEmailBody(user, post, channel, senderName, team.Name, teamURL, emailNotificationContentsType, translateFunc) + var bodyText = a.getNotificationEmailBody(user, post, channel, channelName, senderName, team.Name, teamURL, emailNotificationContentsType, translateFunc) a.Go(func() { if err := a.SendMail(user.Email, html.UnescapeString(subjectText), bodyText); err != nil { @@ -456,10 +458,37 @@ func getNotificationEmailSubject(post *model.Post, translateFunc i18n.TranslateF return translateFunc("app.notification.subject.notification.full", subjectParameters) } +/** + * Computes the subject line for group email messages + */ +func getGroupMessageNotificationEmailSubject(post *model.Post, translateFunc i18n.TranslateFunc, siteName string, channelName string, emailNotificationContentsType string) string { + t := getFormattedPostTime(post, translateFunc) + var subjectText string + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + var subjectParameters = map[string]interface{}{ + "SiteName": siteName, + "ChannelName": channelName, + "Month": t.Month, + "Day": t.Day, + "Year": t.Year, + } + subjectText = translateFunc("app.notification.subject.group_message.full", subjectParameters) + } else { + var subjectParameters = map[string]interface{}{ + "SiteName": siteName, + "Month": t.Month, + "Day": t.Day, + "Year": t.Year, + } + subjectText = translateFunc("app.notification.subject.group_message.generic", subjectParameters) + } + return subjectText +} + /** * Computes the email body for notification messages */ -func (a *App) getNotificationEmailBody(recipient *model.User, post *model.Post, channel *model.Channel, senderName string, teamName string, teamURL string, emailNotificationContentsType string, translateFunc i18n.TranslateFunc) string { +func (a *App) getNotificationEmailBody(recipient *model.User, post *model.Post, channel *model.Channel, channelName string, 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 { @@ -476,10 +505,6 @@ func (a *App) getNotificationEmailBody(recipient *model.User, post *model.Post, 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 @@ -509,6 +534,32 @@ func (a *App) getNotificationEmailBody(recipient *model.User, post *model.Post, "Day": t.Day, }) } + } else if channel.Type == model.CHANNEL_GROUP { + if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL { + bodyText = translateFunc("app.notification.body.intro.group_message.full") + info = utils.TranslateAsHtml(translateFunc, "app.notification.body.text.group_message.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.group_message.generic", map[string]interface{}{ + "SenderName": senderName, + }) + info = utils.TranslateAsHtml(translateFunc, "app.notification.body.text.group_message.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") diff --git a/app/notification_test.go b/app/notification_test.go index 818ad1d9d..8fbcf3a78 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -1043,7 +1043,7 @@ func TestGetDirectMessageNotificationEmailSubject(t *testing.T) { th := Setup() defer th.TearDown() - expectedPrefix := "[http://localhost:8065] New Direct Message from sender on" + expectedPrefix := "[http://localhost:8065] New Direct Message from @sender on" post := &model.Post{ CreateAt: 1501804801000, } @@ -1054,6 +1054,38 @@ func TestGetDirectMessageNotificationEmailSubject(t *testing.T) { } } +func TestGetGroupMessageNotificationEmailSubjectFull(t *testing.T) { + th := Setup() + defer th.TearDown() + + expectedPrefix := "[http://localhost:8065] New Group Message in sender on" + post := &model.Post{ + CreateAt: 1501804801000, + } + translateFunc := utils.GetUserTranslations("en") + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL + subject := getGroupMessageNotificationEmailSubject(post, translateFunc, "http://localhost:8065", "sender", emailNotificationContentsType) + if !strings.HasPrefix(subject, expectedPrefix) { + t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject) + } +} + +func TestGetGroupMessageNotificationEmailSubjectGeneric(t *testing.T) { + th := Setup() + defer th.TearDown() + + expectedPrefix := "[http://localhost:8065] New Group Message on" + post := &model.Post{ + CreateAt: 1501804801000, + } + translateFunc := utils.GetUserTranslations("en") + emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC + subject := getGroupMessageNotificationEmailSubject(post, translateFunc, "http://localhost:8065", "sender", emailNotificationContentsType) + if !strings.HasPrefix(subject, expectedPrefix) { + t.Fatal("Expected subject line prefix '" + expectedPrefix + "', got " + subject) + } +} + func TestGetNotificationEmailSubject(t *testing.T) { th := Setup() defer th.TearDown() @@ -1081,21 +1113,22 @@ func TestGetNotificationEmailBodyFullNotificationPublicChannel(t *testing.T) { DisplayName: "ChannelName", Type: model.CHANNEL_OPEN, } + channelName := "ChannelName" 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, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, 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, "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, "@"+senderName+" - ") { + t.Fatal("Expected email text '@" + senderName + " - '. Got " + body) } if !strings.Contains(body, post.Message) { t.Fatal("Expected email text '" + post.Message + "'. Got " + body) @@ -1117,21 +1150,22 @@ func TestGetNotificationEmailBodyFullNotificationGroupChannel(t *testing.T) { DisplayName: "ChannelName", Type: model.CHANNEL_GROUP, } + channelName := "ChannelName" 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, 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) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new Group Message.") { + t.Fatal("Expected email text 'You have a new Group Message. Got " + body) } - if !strings.Contains(body, "CHANNEL: Group Message") { - t.Fatal("Expected email text 'CHANNEL: Group Message'. Got " + body) + if !strings.Contains(body, "Channel: ChannelName") { + t.Fatal("Expected email text 'Channel: ChannelName'. Got " + body) } - if !strings.Contains(body, senderName+" - ") { - t.Fatal("Expected email text '" + senderName + " - '. 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) @@ -1153,21 +1187,22 @@ func TestGetNotificationEmailBodyFullNotificationPrivateChannel(t *testing.T) { DisplayName: "ChannelName", Type: model.CHANNEL_PRIVATE, } + channelName := "ChannelName" 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, channel, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, 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, "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, "@"+senderName+" - ") { + t.Fatal("Expected email text '@" + senderName + " - '. Got " + body) } if !strings.Contains(body, post.Message) { t.Fatal("Expected email text '" + post.Message + "'. Got " + body) @@ -1189,18 +1224,19 @@ func TestGetNotificationEmailBodyFullNotificationDirectChannel(t *testing.T) { DisplayName: "ChannelName", Type: model.CHANNEL_DIRECT, } + channelName := "ChannelName" 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, 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) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, 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, "@"+senderName+" - ") { + t.Fatal("Expected email text '@" + senderName + " - '. Got " + body) } if !strings.Contains(body, post.Message) { t.Fatal("Expected email text '" + post.Message + "'. Got " + body) @@ -1223,18 +1259,19 @@ func TestGetNotificationEmailBodyGenericNotificationPublicChannel(t *testing.T) DisplayName: "ChannelName", Type: model.CHANNEL_OPEN, } + channelName := "ChannelName" senderName := "sender" teamName := "team" teamURL := "http://localhost:8065/" + teamName emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC translateFunc := utils.GetUserTranslations("en") - body := th.App.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) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, 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, "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) @@ -1256,15 +1293,16 @@ func TestGetNotificationEmailBodyGenericNotificationGroupChannel(t *testing.T) { DisplayName: "ChannelName", Type: model.CHANNEL_GROUP, } + channelName := "ChannelName" senderName := "sender" teamName := "team" teamURL := "http://localhost:8065/" + teamName emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC translateFunc := utils.GetUserTranslations("en") - body := th.App.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) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, senderName, teamName, teamURL, emailNotificationContentsType, translateFunc) + if !strings.Contains(body, "You have a new Group Message from @"+senderName) { + t.Fatal("Expected email text 'You have a new Group Message from @" + senderName + "'. Got " + body) } if strings.Contains(body, "CHANNEL: "+channel.DisplayName) { t.Fatal("Did not expect email text 'CHANNEL: " + channel.DisplayName + "'. Got " + body) @@ -1289,15 +1327,16 @@ func TestGetNotificationEmailBodyGenericNotificationPrivateChannel(t *testing.T) DisplayName: "ChannelName", Type: model.CHANNEL_PRIVATE, } + channelName := "ChannelName" senderName := "sender" teamName := "team" teamURL := "http://localhost:8065/" + teamName emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC translateFunc := utils.GetUserTranslations("en") - body := th.App.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) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, 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) @@ -1322,15 +1361,16 @@ func TestGetNotificationEmailBodyGenericNotificationDirectChannel(t *testing.T) DisplayName: "ChannelName", Type: model.CHANNEL_DIRECT, } + channelName := "ChannelName" senderName := "sender" teamName := "team" teamURL := "http://localhost:8065/" + teamName emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_GENERIC translateFunc := utils.GetUserTranslations("en") - body := th.App.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) + body := th.App.getNotificationEmailBody(recipient, post, channel, channelName, 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) -- cgit v1.2.3-1-g7c22