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

2 files changed, 141 insertions, 50 deletions

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 {
@@ -457,9 +459,36 @@ func getNotificationEmailSubject(post *model.Post, translateFunc i18n.TranslateF
 }
 
 /**
+ * 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)