Commit 6c2a5555 authored by Harrison Healey's avatar Harrison Healey Committed by Sudheer

MM-11700 Clean up handling of user display names for notifications (#9343)

* MM-11700 Clean up handling of user display names for notifications
parent ab99f065
......@@ -514,7 +514,7 @@ func (a *App) ImportUser(data *UserImportData, dryRun bool) *model.AppError {
preferences = append(preferences, model.Preference{
UserId: savedUser.Id,
Category: model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS,
Name: "use_military_time",
Name: model.PREFERENCE_NAME_USE_MILITARY_TIME,
Value: *data.UseMilitaryTime,
})
}
......@@ -523,7 +523,7 @@ func (a *App) ImportUser(data *UserImportData, dryRun bool) *model.AppError {
preferences = append(preferences, model.Preference{
UserId: savedUser.Id,
Category: model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS,
Name: "collapse_previews",
Name: model.PREFERENCE_NAME_COLLAPSE_SETTING,
Value: *data.CollapsePreviews,
})
}
......@@ -532,7 +532,7 @@ func (a *App) ImportUser(data *UserImportData, dryRun bool) *model.AppError {
preferences = append(preferences, model.Preference{
UserId: savedUser.Id,
Category: model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS,
Name: "message_display",
Name: model.PREFERENCE_NAME_MESSAGE_DISPLAY,
Value: *data.MessageDisplay,
})
}
......
......@@ -1091,10 +1091,10 @@ func TestImportImportUser(t *testing.T) {
}
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_THEME, "", *data.Theme)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "use_military_time", *data.UseMilitaryTime)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "collapse_previews", *data.CollapsePreviews)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "message_display", *data.MessageDisplay)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "channel_display_mode", *data.ChannelDisplayMode)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_USE_MILITARY_TIME, *data.UseMilitaryTime)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_COLLAPSE_SETTING, *data.CollapsePreviews)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_MESSAGE_DISPLAY, *data.MessageDisplay)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_CHANNEL_DISPLAY_MODE, *data.ChannelDisplayMode)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_TUTORIAL_STEPS, user.Id, *data.TutorialStep)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_ADVANCED_SETTINGS, "feature_enabled_markdown_preview", *data.UseMarkdownPreview)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_ADVANCED_SETTINGS, "formatting", *data.UseFormatting)
......@@ -1117,10 +1117,10 @@ func TestImportImportUser(t *testing.T) {
// Check their values again.
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_THEME, "", *data.Theme)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "use_military_time", *data.UseMilitaryTime)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "collapse_previews", *data.CollapsePreviews)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "message_display", *data.MessageDisplay)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "channel_display_mode", *data.ChannelDisplayMode)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_USE_MILITARY_TIME, *data.UseMilitaryTime)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_COLLAPSE_SETTING, *data.CollapsePreviews)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_MESSAGE_DISPLAY, *data.MessageDisplay)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_CHANNEL_DISPLAY_MODE, *data.ChannelDisplayMode)
checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_TUTORIAL_STEPS, user.Id, *data.TutorialStep)
// Set Notify Props
......
......@@ -157,32 +157,11 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
updateMentionChans = append(updateMentionChans, a.Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id))
}
var senderUsername string
senderName := ""
channelName := ""
if post.IsSystemMessage() {
senderName = utils.T("system.message.name")
} else {
if value, ok := post.Props["override_username"]; ok && post.Props["from_webhook"] == "true" && channel.Type != model.CHANNEL_DIRECT {
senderName = value.(string)
senderUsername = value.(string)
} else {
senderName = sender.Username
senderUsername = sender.Username
}
}
if channel.Type == model.CHANNEL_GROUP {
userList := []*model.User{}
for _, u := range profileMap {
if u.Id != sender.Id {
userList = append(userList, u)
}
}
userList = append(userList, sender)
channelName = model.GetGroupDisplayNameFromUsers(userList, false)
} else {
channelName = channel.DisplayName
notification := &postNotification{
post: post,
channel: channel,
profileMap: profileMap,
sender: sender,
}
if a.Config().EmailSettings.SendEmailNotifications {
......@@ -227,7 +206,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
autoResponderRelated := status.Status == model.STATUS_OUT_OF_OFFICE || post.Type == model.POST_AUTO_RESPONDER
if userAllowsEmails && status.Status != model.STATUS_ONLINE && profileMap[id].DeleteAt == 0 && !autoResponderRelated {
a.sendNotificationEmail(post, profileMap[id], channel, team, channelName, senderName, sender)
a.sendNotificationEmail(notification, profileMap[id], team)
}
}
}
......@@ -310,12 +289,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
}
a.sendPushNotification(
post,
notification,
profileMap[id],
channel,
channelName,
sender,
senderName,
mentionedUserIds[id],
(channelNotification || hereNotification || allNotification),
replyToThreadType,
......@@ -337,12 +312,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
if ShouldSendPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], false, status, post) {
a.sendPushNotification(
post,
notification,
profileMap[id],
channel,
channelName,
sender,
senderName,
false,
false,
"",
......@@ -355,9 +326,9 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_POSTED, "", post.ChannelId, "", nil)
message.Add("post", a.PostWithProxyAddedToImageURLs(post).ToJson())
message.Add("channel_type", channel.Type)
message.Add("channel_display_name", channelName)
message.Add("channel_display_name", notification.GetChannelName(model.SHOW_USERNAME, ""))
message.Add("channel_name", channel.Name)
message.Add("sender_name", senderUsername)
message.Add("sender_name", notification.GetSenderName(model.SHOW_USERNAME, a.Config().ServiceSettings.EnablePostUsernameOverride))
message.Add("team_id", team.Id)
if len(post.FileIds) != 0 && fchan != nil {
......@@ -628,3 +599,50 @@ func (a *App) GetMentionKeywordsInChannel(profiles map[string]*model.User, lookF
return keywords
}
// Represents either an email or push notification and contains the fields required to send it to any user.
type postNotification struct {
channel *model.Channel
post *model.Post
profileMap map[string]*model.User
sender *model.User
}
// Returns the name of the channel for this notification. For direct messages, this is the sender's name
// preceeded by an at sign. For group messages, this is a comma-separated list of the members of the
// channel, with an option to exclude the recipient of the message from that list.
func (n *postNotification) GetChannelName(userNameFormat string, excludeId string) string {
switch n.channel.Type {
case model.CHANNEL_DIRECT:
return fmt.Sprintf("@%s", n.sender.GetDisplayName(userNameFormat))
case model.CHANNEL_GROUP:
names := []string{}
for _, user := range n.profileMap {
if user.Id != excludeId {
names = append(names, user.GetDisplayName(userNameFormat))
}
}
sort.Strings(names)
return strings.Join(names, ", ")
default:
return n.channel.DisplayName
}
}
// Returns the name of the sender of this notification, accounting for things like system messages
// and whether or not the username has been overridden by an integration.
func (n *postNotification) GetSenderName(userNameFormat string, overridesAllowed bool) string {
if n.post.IsSystemMessage() {
return utils.T("system.message.name")
}
if overridesAllowed && n.channel.Type != model.CHANNEL_DIRECT {
if value, ok := n.post.Props["override_username"]; ok && n.post.Props["from_webhook"] == "true" {
return value.(string)
}
}
return n.sender.GetDisplayName(userNameFormat)
}
......@@ -17,7 +17,10 @@ import (
"github.com/nicksnyder/go-i18n/i18n"
)
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 {
func (a *App) sendNotificationEmail(notification *postNotification, user *model.User, team *model.Team) *model.AppError {
channel := notification.channel
post := notification.post
if channel.IsGroupOrDirect() {
if result := <-a.Srv.Store.Team().GetTeamsByUserId(user.Id); result.Err != nil {
return result.Err
......@@ -41,6 +44,7 @@ func (a *App) sendNotificationEmail(post *model.Post, user *model.User, channel
}
}
}
if *a.Config().EmailSettings.EnableEmailBatching {
var sendBatched bool
if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL); result.Err != nil {
......@@ -60,14 +64,25 @@ func (a *App) sendNotificationEmail(post *model.Post, user *model.User, channel
// fall back to sending a single email if we can't batch it for some reason
}
var useMilitaryTime bool
translateFunc := utils.GetUserTranslations(user.Locale)
if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "use_military_time"); result.Err != nil {
var useMilitaryTime bool
if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_USE_MILITARY_TIME); result.Err != nil {
useMilitaryTime = true
} else {
useMilitaryTime = result.Data.(model.Preference).Value == "true"
}
var nameFormat string
if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_NAME_FORMAT); result.Err != nil {
nameFormat = *a.Config().TeamSettings.TeammateNameDisplay
} else {
nameFormat = result.Data.(model.Preference).Value
}
channelName := notification.GetChannelName(nameFormat, "")
senderName := notification.GetSenderName(nameFormat, a.Config().ServiceSettings.EnablePostUsernameOverride)
emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL
if license := a.License(); license != nil && *license.Features.EmailNotificationContents {
emailNotificationContentsType = *a.Config().EmailSettings.EmailNotificationContentsType
......@@ -79,7 +94,7 @@ func (a *App) sendNotificationEmail(post *model.Post, user *model.User, channel
} else if channel.Type == model.CHANNEL_GROUP {
subjectText = getGroupMessageNotificationEmailSubject(user, post, translateFunc, a.Config().TeamSettings.SiteName, channelName, emailNotificationContentsType, useMilitaryTime)
} else if *a.Config().EmailSettings.UseChannelInEmailNotifications {
subjectText = getNotificationEmailSubject(user, post, translateFunc, a.Config().TeamSettings.SiteName, team.DisplayName+" ("+channel.DisplayName+")", useMilitaryTime)
subjectText = getNotificationEmailSubject(user, post, translateFunc, a.Config().TeamSettings.SiteName, team.DisplayName+" ("+channelName+")", useMilitaryTime)
} else {
subjectText = getNotificationEmailSubject(user, post, translateFunc, a.Config().TeamSettings.SiteName, team.DisplayName, useMilitaryTime)
}
......
......@@ -15,13 +15,23 @@ import (
"github.com/nicksnyder/go-i18n/i18n"
)
func (a *App) sendPushNotification(post *model.Post, user *model.User, channel *model.Channel, channelName string, sender *model.User, senderName string,
explicitMention, channelWideMention bool, replyToThreadType string) *model.AppError {
func (a *App) sendPushNotification(notification *postNotification, user *model.User, explicitMention, channelWideMention bool, replyToThreadType string) *model.AppError {
channel := notification.channel
post := notification.post
cfg := a.Config()
contentsConfig := *cfg.EmailSettings.PushNotificationContents
teammateNameConfig := *cfg.TeamSettings.TeammateNameDisplay
var nameFormat string
if result := <-a.Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_NAME_FORMAT); result.Err != nil {
nameFormat = *a.Config().TeamSettings.TeammateNameDisplay
} else {
nameFormat = result.Data.(model.Preference).Value
}
channelName := notification.GetChannelName(nameFormat, user.Id)
senderName := notification.GetSenderName(nameFormat, cfg.ServiceSettings.EnablePostUsernameOverride)
sessions, err := a.getMobileAppSessions(user.Id)
sentBySystem := senderName == utils.T("system.message.name")
if err != nil {
return err
}
......@@ -43,25 +53,13 @@ func (a *App) sendPushNotification(post *model.Post, user *model.User, channel *
msg.RootId = post.RootId
msg.SenderId = post.UserId
if !sentBySystem {
senderName = sender.GetDisplayName(teammateNameConfig)
preference, prefError := a.GetPreferenceByCategoryAndNameForUser(user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, "name_format")
if prefError == nil && preference.Value != teammateNameConfig {
senderName = sender.GetDisplayName(preference.Value)
}
}
if channel.Type == model.CHANNEL_DIRECT {
channelName = fmt.Sprintf("@%v", senderName)
}
contentsConfig := *cfg.EmailSettings.PushNotificationContents
if contentsConfig != model.GENERIC_NO_CHANNEL_NOTIFICATION || channel.Type == model.CHANNEL_DIRECT {
msg.ChannelName = channelName
}
if ou, ok := post.Props["override_username"].(string); ok && cfg.ServiceSettings.EnablePostUsernameOverride {
msg.OverrideUsername = ou
senderName = ou
}
if oi, ok := post.Props["override_icon_url"].(string); ok && cfg.ServiceSettings.EnablePostIconOverride {
......
......@@ -817,3 +817,165 @@ func TestGetMentionsEnabledFields(t *testing.T) {
assert.EqualValues(t, 4, len(mentionEnabledFields))
assert.EqualValues(t, expectedFields, mentionEnabledFields)
}
func TestPostNotificationGetChannelName(t *testing.T) {
sender := &model.User{Id: model.NewId(), Username: "sender", FirstName: "Sender", LastName: "Sender", Nickname: "Sender"}
recipient := &model.User{Id: model.NewId(), Username: "recipient", FirstName: "Recipient", LastName: "Recipient", Nickname: "Recipient"}
otherUser := &model.User{Id: model.NewId(), Username: "other", FirstName: "Other", LastName: "Other", Nickname: "Other"}
profileMap := map[string]*model.User{
sender.Id: sender,
recipient.Id: recipient,
otherUser.Id: otherUser,
}
for name, testCase := range map[string]struct {
channel *model.Channel
nameFormat string
recipientId string
expected string
}{
"regular channel": {
channel: &model.Channel{Type: model.CHANNEL_OPEN, Name: "channel", DisplayName: "My Channel"},
expected: "My Channel",
},
"direct channel, unspecified": {
channel: &model.Channel{Type: model.CHANNEL_DIRECT},
expected: "@sender",
},
"direct channel, username": {
channel: &model.Channel{Type: model.CHANNEL_DIRECT},
nameFormat: model.SHOW_USERNAME,
expected: "@sender",
},
"direct channel, full name": {
channel: &model.Channel{Type: model.CHANNEL_DIRECT},
nameFormat: model.SHOW_FULLNAME,
expected: "@Sender Sender",
},
"direct channel, nickname": {
channel: &model.Channel{Type: model.CHANNEL_DIRECT},
nameFormat: model.SHOW_NICKNAME_FULLNAME,
expected: "@Sender",
},
"group channel, unspecified": {
channel: &model.Channel{Type: model.CHANNEL_GROUP},
expected: "other, sender",
},
"group channel, username": {
channel: &model.Channel{Type: model.CHANNEL_GROUP},
nameFormat: model.SHOW_USERNAME,
expected: "other, sender",
},
"group channel, full name": {
channel: &model.Channel{Type: model.CHANNEL_GROUP},
nameFormat: model.SHOW_FULLNAME,
expected: "Other Other, Sender Sender",
},
"group channel, nickname": {
channel: &model.Channel{Type: model.CHANNEL_GROUP},
nameFormat: model.SHOW_NICKNAME_FULLNAME,
expected: "Other, Sender",
},
"group channel, not excluding current user": {
channel: &model.Channel{Type: model.CHANNEL_GROUP},
nameFormat: model.SHOW_NICKNAME_FULLNAME,
expected: "Other, Sender",
recipientId: "",
},
} {
t.Run(name, func(t *testing.T) {
notification := &postNotification{
channel: testCase.channel,
sender: sender,
profileMap: profileMap,
}
recipientId := recipient.Id
if testCase.recipientId != "" {
recipientId = testCase.recipientId
}
assert.Equal(t, testCase.expected, notification.GetChannelName(testCase.nameFormat, recipientId))
})
}
}
func TestPostNotificationGetSenderName(t *testing.T) {
th := Setup()
defer th.TearDown()
defaultChannel := &model.Channel{Type: model.CHANNEL_OPEN}
defaultPost := &model.Post{Props: model.StringInterface{}}
sender := &model.User{Id: model.NewId(), Username: "sender", FirstName: "Sender", LastName: "Sender", Nickname: "Sender"}
overriddenPost := &model.Post{
Props: model.StringInterface{
"override_username": "Overridden",
"from_webhook": "true",
},
}
for name, testCase := range map[string]struct {
channel *model.Channel
post *model.Post
nameFormat string
allowOverrides bool
expected string
}{
"name format unspecified": {
expected: sender.Username,
},
"name format username": {
nameFormat: model.SHOW_USERNAME,
expected: sender.Username,
},
"name format full name": {
nameFormat: model.SHOW_FULLNAME,
expected: sender.FirstName + " " + sender.LastName,
},
"name format nickname": {
nameFormat: model.SHOW_NICKNAME_FULLNAME,
expected: sender.Nickname,
},
"system message": {
post: &model.Post{Type: model.POST_SYSTEM_MESSAGE_PREFIX + "custom"},
expected: utils.T("system.message.name"),
},
"overridden username": {
post: overriddenPost,
allowOverrides: true,
expected: overriddenPost.Props["override_username"].(string),
},
"overridden username, direct channel": {
channel: &model.Channel{Type: model.CHANNEL_DIRECT},
post: overriddenPost,
allowOverrides: true,
expected: sender.Username,
},
"overridden username, overrides disabled": {
post: overriddenPost,
allowOverrides: false,
expected: sender.Username,
},
} {
t.Run(name, func(t *testing.T) {
channel := defaultChannel
if testCase.channel != nil {
channel = testCase.channel
}
post := defaultPost
if testCase.post != nil {
post = testCase.post
}
notification := &postNotification{
channel: channel,
post: post,
sender: sender,
}
assert.Equal(t, testCase.expected, notification.GetSenderName(testCase.nameFormat, testCase.allowOverrides))
})
}
}
......@@ -21,7 +21,11 @@ const (
PREFERENCE_CATEGORY_SIDEBAR_SETTINGS = "sidebar_settings"
PREFERENCE_CATEGORY_DISPLAY_SETTINGS = "display_settings"
PREFERENCE_NAME_CHANNEL_DISPLAY_MODE = "channel_display_mode"
PREFERENCE_NAME_COLLAPSE_SETTING = "collapse_previews"
PREFERENCE_NAME_MESSAGE_DISPLAY = "message_display"
PREFERENCE_NAME_NAME_FORMAT = "name_format"
PREFERENCE_NAME_USE_MILITARY_TIME = "use_military_time"
PREFERENCE_CATEGORY_THEME = "theme"
// the name for theme props is the team id
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment