diff --git a/app/channel.go b/app/channel.go index ed800bf039..73c801187a 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1787,13 +1787,21 @@ func (a *App) UpdateChannelLastViewedAt(channelIds []string, userId string) *mod // MarkChanelAsUnreadFromPost will take a post and set the channel as unread from that one. func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string) (*model.ChannelUnreadAt, *model.AppError) { - post, err := a.GetSinglePost(postID) if err != nil { return nil, err } - unreadMentions := 0 // TODO: calculate this value, setting it to zero for now. + user, err := a.GetUser(userID) + if err != nil { + return nil, err + } + + unreadMentions, err := a.countMentionsFromPost(user, post) + if err != nil { + return nil, err + } + return a.Srv.Store.Channel().UpdateLastViewedAtPost(post, userID, unreadMentions) } diff --git a/app/channel_test.go b/app/channel_test.go index 2458efada2..3bc394b1db 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -1128,7 +1128,6 @@ func TestMarkChannelAsUnreadFromPost(t *testing.T) { unread, err := th.App.GetChannelUnread(c1.Id, u2.Id) require.Nil(t, err) assert.Equal(t, int64(0), unread.MsgCount) - }) t.Run("Unread on a private channel", func(t *testing.T) { @@ -1150,6 +1149,48 @@ func TestMarkChannelAsUnreadFromPost(t *testing.T) { assert.Equal(t, pp2.CreateAt-1, response.LastViewedAt) }) + t.Run("Unread with mentions", func(t *testing.T) { + c2 := th.CreateChannel(th.BasicTeam) + _, err := th.App.AddUserToChannel(u2, c2) + require.Nil(t, err) + + p4, err := th.App.CreatePost(&model.Post{ + UserId: u2.Id, + ChannelId: c2.Id, + Message: "@" + u1.Username, + }, c2, false) + require.Nil(t, err) + th.CreatePost(c2) + + response, err := th.App.MarkChannelAsUnreadFromPost(p4.Id, u1.Id) + assert.Nil(t, err) + assert.Equal(t, int64(1), response.MsgCount) + assert.Equal(t, int64(1), response.MentionCount) + + unread, err := th.App.GetChannelUnread(c2.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(1), unread.MsgCount) + assert.Equal(t, int64(1), unread.MentionCount) + }) + + t.Run("Unread on a DM channel", func(t *testing.T) { + dc := th.CreateDmChannel(u2) + + dm1 := th.CreatePost(dc) + th.CreatePost(dc) + th.CreatePost(dc) + + response, err := th.App.MarkChannelAsUnreadFromPost(dm1.Id, u1.Id) + assert.Nil(t, err) + assert.Equal(t, int64(0), response.MsgCount) + assert.Equal(t, int64(3), response.MentionCount) + + unread, err := th.App.GetChannelUnread(dc.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(3), unread.MsgCount) + assert.Equal(t, int64(3), unread.MentionCount) + }) + t.Run("Can't unread an imaginary post", func(t *testing.T) { response, err := th.App.MarkChannelAsUnreadFromPost("invalid4ofngungryquinj976y", u1.Id) assert.NotNil(t, err) diff --git a/app/notification.go b/app/notification.go index 1dc1fdbd95..27e4d82b4f 100644 --- a/app/notification.go +++ b/app/notification.go @@ -16,11 +16,6 @@ import ( "github.com/mattermost/mattermost-server/utils/markdown" ) -const ( - THREAD_ANY = "any" - THREAD_ROOT = "root" -) - func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *model.Channel, sender *model.User, parentPostList *model.PostList) ([]string, error) { // Do not send notifications in archived channels if channel.DeleteAt > 0 { @@ -63,67 +58,57 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } channelMemberNotifyPropsMap := result.Data.(map[string]model.StringMap) - mentionedUserIds := make(map[string]bool) - threadMentionedUserIds := make(map[string]string) + mentions := &ExplicitMentions{} allActivityPushUserIds := []string{} - hereNotification := false - channelNotification := false - allNotification := false - updateMentionChans := []chan *model.AppError{} if channel.Type == model.CHANNEL_DIRECT { otherUserId := channel.GetOtherUserIdForDM(post.UserId) _, ok := profileMap[otherUserId] if ok { - mentionedUserIds[otherUserId] = true + mentions.addMention(otherUserId, DMMention) } if post.Props["from_webhook"] == "true" { - mentionedUserIds[post.UserId] = true + mentions.addMention(post.UserId, DMMention) } } else { - keywords := a.getMentionKeywordsInChannel(profileMap, post.Type != model.POST_HEADER_CHANGE && post.Type != model.POST_PURPOSE_CHANGE, channelMemberNotifyPropsMap) + allowChannelMentions := a.allowChannelMentions(post, len(profileMap)) + keywords := a.getMentionKeywordsInChannel(profileMap, allowChannelMentions, channelMemberNotifyPropsMap) - m := getExplicitMentions(post, keywords) + mentions = getExplicitMentions(post, keywords) // Add an implicit mention when a user is added to a channel // even if the user has set 'username mentions' to false in account settings. if post.Type == model.POST_ADD_TO_CHANNEL { - val := post.Props[model.POST_PROPS_ADDED_USER_ID] - if val != nil { - uid := val.(string) - m.MentionedUserIds[uid] = true + addedUserId, ok := post.Props[model.POST_PROPS_ADDED_USER_ID].(string) + if ok { + mentions.addMention(addedUserId, KeywordMention) } } - mentionedUserIds, hereNotification, channelNotification, allNotification = m.MentionedUserIds, m.HereMentioned, m.ChannelMentioned, m.AllMentioned - // get users that have comment thread mentions enabled if len(post.RootId) > 0 && parentPostList != nil { for _, threadPost := range parentPostList.Posts { profile := profileMap[threadPost.UserId] - if profile != nil && (profile.NotifyProps[model.COMMENTS_NOTIFY_PROP] == THREAD_ANY || (profile.NotifyProps[model.COMMENTS_NOTIFY_PROP] == THREAD_ROOT && threadPost.Id == parentPostList.Order[0])) { + if profile != nil && (profile.NotifyProps[model.COMMENTS_NOTIFY_PROP] == model.COMMENTS_NOTIFY_ANY || (profile.NotifyProps[model.COMMENTS_NOTIFY_PROP] == model.COMMENTS_NOTIFY_ROOT && threadPost.Id == parentPostList.Order[0])) { + mentionType := ThreadMention if threadPost.Id == parentPostList.Order[0] { - threadMentionedUserIds[threadPost.UserId] = THREAD_ROOT - } else { - threadMentionedUserIds[threadPost.UserId] = THREAD_ANY + mentionType = CommentMention } - if _, ok := mentionedUserIds[threadPost.UserId]; !ok { - mentionedUserIds[threadPost.UserId] = false - } + mentions.addMention(threadPost.UserId, mentionType) } } } // prevent the user from mentioning themselves if post.Props["from_webhook"] != "true" { - delete(mentionedUserIds, post.UserId) + mentions.removeMention(post.UserId) } go func() { - _, err := a.sendOutOfChannelMentions(sender, post, channel, m.OtherPotentialMentions) + _, err := a.sendOutOfChannelMentions(sender, post, channel, mentions.OtherPotentialMentions) if err != nil { mlog.Error("Failed to send warning for out of channel mentions", mlog.String("user_id", sender.Id), mlog.String("post_id", post.Id), mlog.Err(err)) } @@ -140,9 +125,12 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } } - mentionedUsersList := make([]string, 0, len(mentionedUserIds)) - for id := range mentionedUserIds { + mentionedUsersList := make([]string, 0, len(mentions.Mentions)) + updateMentionChans := []chan *model.AppError{} + + for id := range mentions.Mentions { mentionedUsersList = append(mentionedUsersList, id) + umc := make(chan *model.AppError, 1) go func(userId string) { umc <- a.Srv.Store.Channel().IncrementMentionCount(post.ChannelId, userId) @@ -205,43 +193,42 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } } - T := utils.GetUserTranslations(sender.Locale) + // Check for channel-wide mentions in channels that have too many members for those to work + if int64(len(profileMap)) > *a.Config().TeamSettings.MaxNotificationsPerChannel { + T := utils.GetUserTranslations(sender.Locale) - // If the channel has more than 1K users then @here is disabled - if hereNotification && int64(len(profileMap)) > *a.Config().TeamSettings.MaxNotificationsPerChannel { - hereNotification = false - a.SendEphemeralPost( - post.UserId, - &model.Post{ - ChannelId: post.ChannelId, - Message: T("api.post.disabled_here", map[string]interface{}{"Users": *a.Config().TeamSettings.MaxNotificationsPerChannel}), - CreateAt: post.CreateAt + 1, - }, - ) - } + if mentions.HereMentioned { + a.SendEphemeralPost( + post.UserId, + &model.Post{ + ChannelId: post.ChannelId, + Message: T("api.post.disabled_here", map[string]interface{}{"Users": *a.Config().TeamSettings.MaxNotificationsPerChannel}), + CreateAt: post.CreateAt + 1, + }, + ) + } - // If the channel has more than 1K users then @channel is disabled - if channelNotification && int64(len(profileMap)) > *a.Config().TeamSettings.MaxNotificationsPerChannel { - a.SendEphemeralPost( - post.UserId, - &model.Post{ - ChannelId: post.ChannelId, - Message: T("api.post.disabled_channel", map[string]interface{}{"Users": *a.Config().TeamSettings.MaxNotificationsPerChannel}), - CreateAt: post.CreateAt + 1, - }, - ) - } + if mentions.ChannelMentioned { + a.SendEphemeralPost( + post.UserId, + &model.Post{ + ChannelId: post.ChannelId, + Message: T("api.post.disabled_channel", map[string]interface{}{"Users": *a.Config().TeamSettings.MaxNotificationsPerChannel}), + CreateAt: post.CreateAt + 1, + }, + ) + } - // If the channel has more than 1K users then @all is disabled - if allNotification && int64(len(profileMap)) > *a.Config().TeamSettings.MaxNotificationsPerChannel { - a.SendEphemeralPost( - post.UserId, - &model.Post{ - ChannelId: post.ChannelId, - Message: T("api.post.disabled_all", map[string]interface{}{"Users": *a.Config().TeamSettings.MaxNotificationsPerChannel}), - CreateAt: post.CreateAt + 1, - }, - ) + if mentions.AllMentioned { + a.SendEphemeralPost( + post.UserId, + &model.Post{ + ChannelId: post.ChannelId, + Message: T("api.post.disabled_all", map[string]interface{}{"Users": *a.Config().TeamSettings.MaxNotificationsPerChannel}), + CreateAt: post.CreateAt + 1, + }, + ) + } } // Make sure all mention updates are complete to prevent race @@ -282,16 +269,20 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } if ShouldSendPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], true, status, post) { + mentionType := mentions.Mentions[id] + replyToThreadType := "" - if value, ok := threadMentionedUserIds[id]; ok { - replyToThreadType = value + if mentionType == ThreadMention { + replyToThreadType = model.COMMENTS_NOTIFY_ANY + } else if mentionType == CommentMention { + replyToThreadType = model.COMMENTS_NOTIFY_ROOT } a.sendPushNotification( notification, profileMap[id], - mentionedUserIds[id], - (channelNotification || hereNotification || allNotification), + mentionType == KeywordMention || mentionType == ChannelMention || mentionType == DMMention, + mentionType == ChannelMention, replyToThreadType, ) } else { @@ -311,7 +302,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod continue } - if _, ok := mentionedUserIds[id]; !ok { + if _, ok := mentions.Mentions[id]; !ok { var status *model.Status var err *model.AppError if status, err = a.GetStatus(id); err != nil { @@ -521,8 +512,8 @@ func splitAtFinal(items []string) (preliminary []string, final string) { } type ExplicitMentions struct { - // MentionedUserIds contains a key for each user mentioned by keyword. - MentionedUserIds map[string]bool + // Mentions contains the ID of each user that was mentioned and how they were mentioned. + Mentions map[string]MentionType // OtherPotentialMentions contains a list of strings that looked like mentions, but didn't have // a corresponding keyword. @@ -538,12 +529,56 @@ type ExplicitMentions struct { ChannelMentioned bool } +type MentionType int + +const ( + // Different types of mentions ordered by their priority from lowest to highest + + // A placeholder that should never be used in practice + NoMention MentionType = iota + + // The post is in a thread that the user has commented on + ThreadMention + + // The post is a comment on a thread started by the user + CommentMention + + // The post contains an at-channel, at-all, or at-here + ChannelMention + + // The post is a DM + DMMention + + // The post contains an at-mention for the user + KeywordMention +) + +func (m *ExplicitMentions) addMention(userId string, mentionType MentionType) { + if m.Mentions == nil { + m.Mentions = make(map[string]MentionType) + } + + if currentType, ok := m.Mentions[userId]; ok && currentType >= mentionType { + return + } + + m.Mentions[userId] = mentionType +} + +func (m *ExplicitMentions) addMentions(userIds []string, mentionType MentionType) { + for _, userId := range userIds { + m.addMention(userId, mentionType) + } +} + +func (m *ExplicitMentions) removeMention(userId string) { + delete(m.Mentions, userId) +} + // Given a message and a map mapping mention keywords to the users who use them, returns a map of mentioned // users and a slice of potential mention users not in the channel and whether or not @here was mentioned. func getExplicitMentions(post *model.Post, keywords map[string][]string) *ExplicitMentions { - ret := &ExplicitMentions{ - MentionedUserIds: make(map[string]bool), - } + ret := &ExplicitMentions{} buf := "" mentionsEnabledFields := getMentionsEnabledFields(post) @@ -582,49 +617,67 @@ func getMentionsEnabledFields(post *model.Post) model.StringArray { return ret } +// allowChannelMentions returns whether or not the channel mentions are allowed for the given post. +func (a *App) allowChannelMentions(post *model.Post, numProfiles int) bool { + if post.Type == model.POST_HEADER_CHANGE || post.Type == model.POST_PURPOSE_CHANGE { + return false + } + + if int64(numProfiles) >= *a.Config().TeamSettings.MaxNotificationsPerChannel { + return false + } + + return true +} + // Given a map of user IDs to profiles, returns a list of mention // keywords for all users in the channel. -func (a *App) getMentionKeywordsInChannel(profiles map[string]*model.User, lookForSpecialMentions bool, channelMemberNotifyPropsMap map[string]model.StringMap) map[string][]string { +func (a *App) getMentionKeywordsInChannel(profiles map[string]*model.User, allowChannelMentions bool, channelMemberNotifyPropsMap map[string]model.StringMap) map[string][]string { keywords := make(map[string][]string) - for id, profile := range profiles { - userMention := "@" + strings.ToLower(profile.Username) - keywords[userMention] = append(keywords[userMention], id) + for _, profile := range profiles { + addMentionKeywordsForUser( + keywords, + profile, + channelMemberNotifyPropsMap[profile.Id], + GetStatusFromCache(profile.Id), + allowChannelMentions, + ) + } - if len(profile.NotifyProps[model.MENTION_KEYS_NOTIFY_PROP]) > 0 { - // Add all the user's mention keys - splitKeys := strings.Split(profile.NotifyProps[model.MENTION_KEYS_NOTIFY_PROP], ",") - for _, k := range splitKeys { - // note that these are made lower case so that we can do a case insensitive check for them - key := strings.ToLower(k) - if key != "" { - keywords[key] = append(keywords[key], id) - } - } + return keywords +} + +// addMentionKeywordsForUser adds the mention keywords for a given user to the given keyword map. Returns the provided keyword map. +func addMentionKeywordsForUser(keywords map[string][]string, profile *model.User, channelNotifyProps map[string]string, status *model.Status, allowChannelMentions bool) map[string][]string { + userMention := "@" + strings.ToLower(profile.Username) + keywords[userMention] = append(keywords[userMention], profile.Id) + + // Add all the user's mention keys + for _, k := range profile.GetMentionKeys() { + // note that these are made lower case so that we can do a case insensitive check for them + key := strings.ToLower(k) + + if key != "" { + keywords[key] = append(keywords[key], profile.Id) } + } - // If turned on, add the user's case sensitive first name - if profile.NotifyProps[model.FIRST_NAME_NOTIFY_PROP] == "true" { - keywords[profile.FirstName] = append(keywords[profile.FirstName], profile.Id) - } + // If turned on, add the user's case sensitive first name + if profile.NotifyProps[model.FIRST_NAME_NOTIFY_PROP] == "true" { + keywords[profile.FirstName] = append(keywords[profile.FirstName], profile.Id) + } - ignoreChannelMentions := false - if ignoreChannelMentionsNotifyProp, ok := channelMemberNotifyPropsMap[profile.Id][model.IGNORE_CHANNEL_MENTIONS_NOTIFY_PROP]; ok { - if ignoreChannelMentionsNotifyProp == model.IGNORE_CHANNEL_MENTIONS_ON { - ignoreChannelMentions = true - } - } + // Add @channel and @all to keywords if user has them turned on and the server allows them + if allowChannelMentions { + ignoreChannelMentions := channelNotifyProps[model.IGNORE_CHANNEL_MENTIONS_NOTIFY_PROP] == model.IGNORE_CHANNEL_MENTIONS_ON - // Add @channel and @all to keywords if user has them turned on - if lookForSpecialMentions { - if int64(len(profiles)) <= *a.Config().TeamSettings.MaxNotificationsPerChannel && profile.NotifyProps[model.CHANNEL_MENTIONS_NOTIFY_PROP] == "true" && !ignoreChannelMentions { - keywords["@channel"] = append(keywords["@channel"], profile.Id) - keywords["@all"] = append(keywords["@all"], profile.Id) + if profile.NotifyProps[model.CHANNEL_MENTIONS_NOTIFY_PROP] == "true" && !ignoreChannelMentions { + keywords["@channel"] = append(keywords["@channel"], profile.Id) + keywords["@all"] = append(keywords["@all"], profile.Id) - status := GetStatusFromCache(profile.Id) - if status != nil && status.Status == model.STATUS_ONLINE { - keywords["@here"] = append(keywords["@here"], profile.Id) - } + if status != nil && status.Status == model.STATUS_ONLINE { + keywords["@here"] = append(keywords["@here"], profile.Id) } } } @@ -679,38 +732,36 @@ func (n *postNotification) GetSenderName(userNameFormat string, overridesAllowed return n.sender.GetDisplayNameWithPrefix(userNameFormat, "@") } -// addMentionedUsers will add the mentioned user id in the struct's list for mentioned users -func (e *ExplicitMentions) addMentionedUsers(ids []string) { - for _, id := range ids { - e.MentionedUserIds[id] = true - } -} - // checkForMention checks if there is a mention to a specific user or to the keywords here / channel / all func (e *ExplicitMentions) checkForMention(word string, keywords map[string][]string) bool { - isMention := false + var mentionType MentionType switch strings.ToLower(word) { case "@here": e.HereMentioned = true + mentionType = ChannelMention case "@channel": e.ChannelMentioned = true + mentionType = ChannelMention case "@all": e.AllMentioned = true + mentionType = ChannelMention + default: + mentionType = KeywordMention } if ids, match := keywords[strings.ToLower(word)]; match { - e.addMentionedUsers(ids) - isMention = true + e.addMentions(ids, mentionType) + return true } // Case-sensitive check for first name if ids, match := keywords[word]; match { - e.addMentionedUsers(ids) - isMention = true + e.addMentions(ids, mentionType) + return true } - return isMention + return false } // isKeywordMultibyte checks if a word containing a multibyte character contains a multibyte keyword @@ -785,8 +836,9 @@ func (e *ExplicitMentions) processText(text string, keywords map[string][]string } } } + if ids, match := isKeywordMultibyte(keywords, word); match { - e.addMentionedUsers(ids) + e.addMentions(ids, KeywordMention) } } } diff --git a/app/notification_push.go b/app/notification_push.go index 2b4e963186..2fc9aa748a 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -160,11 +160,11 @@ func (a *App) getPushNotificationMessage(postMessage string, explicitMention, ch return senderName + userLocale("api.post.send_notifications_and_forget.push_explicit_mention") } - if replyToThreadType == THREAD_ROOT { + if replyToThreadType == model.COMMENTS_NOTIFY_ROOT { return senderName + userLocale("api.post.send_notification_and_forget.push_comment_on_post") } - if replyToThreadType == THREAD_ANY { + if replyToThreadType == model.COMMENTS_NOTIFY_ANY { return senderName + userLocale("api.post.send_notification_and_forget.push_comment_on_thread") } diff --git a/app/notification_push_test.go b/app/notification_push_test.go index 5efd7228a1..a7f952df33 100644 --- a/app/notification_push_test.go +++ b/app/notification_push_test.go @@ -572,13 +572,13 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "full message, public channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, ChannelType: model.CHANNEL_OPEN, ExpectedMessage: "user: this is a message", }, "full message, public channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, ChannelType: model.CHANNEL_OPEN, ExpectedMessage: "user: this is a message", }, @@ -595,13 +595,13 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "full message, private channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, ChannelType: model.CHANNEL_PRIVATE, ExpectedMessage: "user: this is a message", }, "full message, private channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, ChannelType: model.CHANNEL_PRIVATE, ExpectedMessage: "user: this is a message", }, @@ -618,13 +618,13 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "full message, group message channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, ChannelType: model.CHANNEL_GROUP, ExpectedMessage: "user: this is a message", }, "full message, group message channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, ChannelType: model.CHANNEL_GROUP, ExpectedMessage: "user: this is a message", }, @@ -641,13 +641,13 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "full message, direct message channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, ChannelType: model.CHANNEL_DIRECT, ExpectedMessage: "this is a message", }, "full message, direct message channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, ChannelType: model.CHANNEL_DIRECT, ExpectedMessage: "this is a message", }, @@ -673,14 +673,14 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "generic message, public channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_OPEN, ExpectedMessage: "user commented on your post.", }, "generic message, public channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_OPEN, ExpectedMessage: "user commented on a thread you participated in.", @@ -707,14 +707,14 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "generic message, public private, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_PRIVATE, ExpectedMessage: "user commented on your post.", }, "generic message, public private, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_PRIVATE, ExpectedMessage: "user commented on a thread you participated in.", @@ -741,14 +741,14 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "generic message, group message channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_GROUP, ExpectedMessage: "user commented on your post.", }, "generic message, group message channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_GROUP, ExpectedMessage: "user commented on a thread you participated in.", @@ -775,14 +775,14 @@ func TestGetPushNotificationMessage(t *testing.T) { }, "generic message, direct message channel, commented on post": { Message: "this is a message", - replyToThreadType: THREAD_ROOT, + replyToThreadType: model.COMMENTS_NOTIFY_ROOT, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_DIRECT, ExpectedMessage: "sent you a message.", }, "generic message, direct message channel, commented on thread": { Message: "this is a message", - replyToThreadType: THREAD_ANY, + replyToThreadType: model.COMMENTS_NOTIFY_ANY, PushNotificationContents: model.GENERIC_NOTIFICATION, ChannelType: model.CHANNEL_DIRECT, ExpectedMessage: "sent you a message.", diff --git a/app/notification_test.go b/app/notification_test.go index 13ee5ca588..16b441b0aa 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -321,8 +321,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -330,8 +330,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user.name.", Keywords: map[string][]string{"@user.name.": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -339,8 +339,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user.name.", Keywords: map[string][]string{"@user.name.": {id1}, "@user.name": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -348,8 +348,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user.", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -357,8 +357,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user", Keywords: map[string][]string{"this": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, OtherPotentialMentions: []string{"user"}, }, @@ -367,8 +367,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user.", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -376,8 +376,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for .@user", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -385,8 +385,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user:", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -394,8 +394,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for :@user", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -403,8 +403,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user.", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -412,8 +412,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for -@user", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -421,9 +421,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user", Keywords: map[string][]string{"@user": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, + id2: KeywordMention, }, }, }, @@ -431,8 +431,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user", Keywords: map[string][]string{"@user": {id1}, "@mention": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -440,9 +440,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an @mention for @user", Keywords: map[string][]string{"@user": {id1}, "@mention": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, + id2: KeywordMention, }, }, }, @@ -450,9 +450,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @channel", Keywords: map[string][]string{"@channel": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: ChannelMention, + id2: ChannelMention, }, ChannelMentioned: true, }, @@ -462,9 +462,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @channel:", Keywords: map[string][]string{"@channel": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: ChannelMention, + id2: ChannelMention, }, ChannelMentioned: true, }, @@ -473,9 +473,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @cHaNNeL", Keywords: map[string][]string{"@channel": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: ChannelMention, + id2: ChannelMention, }, ChannelMentioned: true, }, @@ -484,9 +484,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @all", Keywords: map[string][]string{"@all": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: ChannelMention, + id2: ChannelMention, }, AllMentioned: true, }, @@ -495,9 +495,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @all:", Keywords: map[string][]string{"@all": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: ChannelMention, + id2: ChannelMention, }, AllMentioned: true, }, @@ -506,9 +506,9 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @ALL", Keywords: map[string][]string{"@all": {id1, id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: ChannelMention, + id2: ChannelMention, }, AllMentioned: true, }, @@ -517,8 +517,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "user.period doesn't complicate things at all by including periods in their username", Keywords: map[string][]string{"user.period": {id1}, "user": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -526,8 +526,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user:", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -535,8 +535,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for @user.period.", Keywords: map[string][]string{"@user.period": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -544,8 +544,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for user.period.", Keywords: map[string][]string{"user.period": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -553,8 +553,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is a message for user:", Keywords: map[string][]string{"user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -562,8 +562,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @potential and @user", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, OtherPotentialMentions: []string{"potential"}, }, @@ -588,10 +588,10 @@ func TestGetExplicitMentions(t *testing.T) { Message: "*@aaa @bbb @ccc*", Keywords: map[string][]string{"@aaa": {id1}, "@bbb": {id2}, "@ccc": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, - id3: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, + id2: KeywordMention, + id3: KeywordMention, }, }, }, @@ -599,10 +599,10 @@ func TestGetExplicitMentions(t *testing.T) { Message: "**@aaa @bbb @ccc**", Keywords: map[string][]string{"@aaa": {id1}, "@bbb": {id2}, "@ccc": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, - id3: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, + id2: KeywordMention, + id3: KeywordMention, }, }, }, @@ -610,10 +610,10 @@ func TestGetExplicitMentions(t *testing.T) { Message: "~~@aaa @bbb @ccc~~", Keywords: map[string][]string{"@aaa": {id1}, "@bbb": {id2}, "@ccc": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, - id3: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, + id2: KeywordMention, + id3: KeywordMention, }, }, }, @@ -621,8 +621,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "### @aaa", Keywords: map[string][]string{"@aaa": {id1}, "@bbb": {id2}, "@ccc": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -630,8 +630,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "> @aaa", Keywords: map[string][]string{"@aaa": {id1}, "@bbb": {id2}, "@ccc": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -644,8 +644,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "smile", Keywords: map[string][]string{"smile": {id1}, "smiley": {id2}, "smiley_cat": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -653,8 +653,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: ":smile", Keywords: map[string][]string{"smile": {id1}, "smiley": {id2}, "smiley_cat": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -662,8 +662,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "smile:", Keywords: map[string][]string{"smile": {id1}, "smiley": {id2}, "smiley_cat": {id3}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -688,8 +688,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "My name is 萌", Keywords: map[string][]string{"萌": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -697,8 +697,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "이메일을 보내다.", Keywords: map[string][]string{"이메일": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -706,8 +706,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "我爱吃番茄炒饭", Keywords: map[string][]string{"番茄": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -715,8 +715,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "こんにちは、世界", Keywords: map[string][]string{"世界": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -724,8 +724,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "石橋さんが石橋を渡る", Keywords: map[string][]string{"石橋": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -736,8 +736,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @user.name", Keywords: map[string][]string{"@user.name": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -745,8 +745,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @user.name.", Keywords: map[string][]string{"@user.name": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -754,8 +754,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @user.name...", Keywords: map[string][]string{"@user.name": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -763,8 +763,8 @@ func TestGetExplicitMentions(t *testing.T) { Message: "this is an message for @user...name...", Keywords: map[string][]string{"@user...name": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -778,9 +778,9 @@ func TestGetExplicitMentions(t *testing.T) { }, Keywords: map[string][]string{"@user1": {id1}, "@user2": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, + id2: KeywordMention, }, HereMentioned: true, }, @@ -801,16 +801,15 @@ func TestGetExplicitMentions(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - - post := &model.Post{Message: tc.Message, Props: model.StringInterface{ - "attachments": tc.Attachments, - }, + post := &model.Post{ + Message: tc.Message, + Props: model.StringInterface{ + "attachments": tc.Attachments, + }, } m := getExplicitMentions(post, tc.Keywords) - if tc.Expected.MentionedUserIds == nil { - tc.Expected.MentionedUserIds = make(map[string]bool) - } + assert.EqualValues(t, tc.Expected, m) }) } @@ -873,13 +872,59 @@ func TestGetExplicitMentionsAtHere(t *testing.T) { id := model.NewId() if m := getExplicitMentions(&model.Post{Message: "@here @user @potential"}, map[string][]string{"@user": {id}}); !m.HereMentioned { t.Fatal("should've mentioned @here with \"@here @user\"") - } else if len(m.MentionedUserIds) != 1 || !m.MentionedUserIds[id] { + } else if len(m.Mentions) != 1 || m.Mentions[id] != KeywordMention { t.Fatal("should've mentioned @user with \"@here @user\"") } else if len(m.OtherPotentialMentions) > 1 { t.Fatal("should've potential mentions for @potential") } } +func TestAllowChannelMentions(t *testing.T) { + t.Run("should return true for a regular post with few channel members", func(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + post := &model.Post{} + + allowChannelMentions := th.App.allowChannelMentions(post, 5) + + assert.True(t, allowChannelMentions) + }) + + t.Run("should return false for a channel header post", func(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + post := &model.Post{Type: model.POST_HEADER_CHANGE} + + allowChannelMentions := th.App.allowChannelMentions(post, 5) + + assert.False(t, allowChannelMentions) + }) + + t.Run("should return false for a channel purpose post", func(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + post := &model.Post{Type: model.POST_PURPOSE_CHANGE} + + allowChannelMentions := th.App.allowChannelMentions(post, 5) + + assert.False(t, allowChannelMentions) + }) + + t.Run("should return false for a regular post with many channel members", func(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + post := &model.Post{} + + allowChannelMentions := th.App.allowChannelMentions(post, int(*th.App.Config().TeamSettings.MaxNotificationsPerChannel)+1) + + assert.False(t, allowChannelMentions) + }) +} + func TestGetMentionKeywords(t *testing.T) { th := Setup(t) defer th.TearDown() @@ -1116,10 +1161,9 @@ func TestGetMentionKeywords(t *testing.T) { } // multiple users and more than MaxNotificationsPerChannel - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.MaxNotificationsPerChannel = 3 }) - mentions = th.App.getMentionKeywordsInChannel(profiles, true, channelMemberNotifyPropsMap4Off) + mentions = th.App.getMentionKeywordsInChannel(profiles, false, channelMemberNotifyPropsMap4Off) if len(mentions) != 4 { - t.Fatal("should've returned four mention keywords") + t.Fatal("should've returned four mention keywords", mentions) } else if _, ok := mentions["@channel"]; ok { t.Fatal("should not have mentioned any user with @channel") } else if _, ok := mentions["@all"]; ok { @@ -1175,6 +1219,228 @@ func TestGetMentionKeywords(t *testing.T) { assert.Equal(t, userNoMentionKeys.Id, ids[0], "should've returned mention key of @user") } +func TestAddMentionKeywordsForUser(t *testing.T) { + t.Run("should add @user", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + } + channelNotifyProps := map[string]string{} + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, nil, false) + + assert.Contains(t, keywords["@user"], user.Id) + }) + + t.Run("should add custom mention keywords", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.MENTION_KEYS_NOTIFY_PROP: "apple,BANANA,OrAnGe", + }, + } + channelNotifyProps := map[string]string{} + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, nil, false) + + assert.Contains(t, keywords["apple"], user.Id) + assert.Contains(t, keywords["banana"], user.Id) + assert.Contains(t, keywords["orange"], user.Id) + }) + + t.Run("should not add empty custom keywords", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.MENTION_KEYS_NOTIFY_PROP: ",,", + }, + } + channelNotifyProps := map[string]string{} + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, nil, false) + + assert.Nil(t, keywords[""]) + }) + + t.Run("should add case sensitive first name if enabled", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + FirstName: "William", + LastName: "Robert", + NotifyProps: map[string]string{ + model.FIRST_NAME_NOTIFY_PROP: "true", + }, + } + channelNotifyProps := map[string]string{} + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, nil, false) + + assert.Contains(t, keywords["William"], user.Id) + assert.NotContains(t, keywords["william"], user.Id) + assert.NotContains(t, keywords["Robert"], user.Id) + }) + + t.Run("should not add case sensitive first name if disabled", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + FirstName: "William", + LastName: "Robert", + NotifyProps: map[string]string{ + model.FIRST_NAME_NOTIFY_PROP: "false", + }, + } + channelNotifyProps := map[string]string{} + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, nil, false) + + assert.NotContains(t, keywords["William"], user.Id) + assert.NotContains(t, keywords["william"], user.Id) + assert.NotContains(t, keywords["Robert"], user.Id) + }) + + t.Run("should add @channel/@all/@here when allowed", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "true", + }, + } + channelNotifyProps := map[string]string{} + status := &model.Status{ + Status: model.STATUS_ONLINE, + } + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, status, true) + + assert.Contains(t, keywords["@channel"], user.Id) + assert.Contains(t, keywords["@all"], user.Id) + assert.Contains(t, keywords["@here"], user.Id) + }) + + t.Run("should not add @channel/@all/@here when not allowed", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "true", + }, + } + channelNotifyProps := map[string]string{} + status := &model.Status{ + Status: model.STATUS_ONLINE, + } + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, status, false) + + assert.NotContains(t, keywords["@channel"], user.Id) + assert.NotContains(t, keywords["@all"], user.Id) + assert.NotContains(t, keywords["@here"], user.Id) + }) + + t.Run("should not add @channel/@all/@here when disabled for user", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "false", + }, + } + channelNotifyProps := map[string]string{} + status := &model.Status{ + Status: model.STATUS_ONLINE, + } + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, status, true) + + assert.NotContains(t, keywords["@channel"], user.Id) + assert.NotContains(t, keywords["@all"], user.Id) + assert.NotContains(t, keywords["@here"], user.Id) + }) + + t.Run("should not add @channel/@all/@here when disabled for channel", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "true", + }, + } + channelNotifyProps := map[string]string{ + model.IGNORE_CHANNEL_MENTIONS_NOTIFY_PROP: model.IGNORE_CHANNEL_MENTIONS_ON, + } + status := &model.Status{ + Status: model.STATUS_ONLINE, + } + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, status, true) + + assert.NotContains(t, keywords["@channel"], user.Id) + assert.NotContains(t, keywords["@all"], user.Id) + assert.NotContains(t, keywords["@here"], user.Id) + }) + + t.Run("should not add @here when when user is not online", func(t *testing.T) { + user := &model.User{ + Id: model.NewId(), + Username: "user", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "true", + }, + } + channelNotifyProps := map[string]string{} + status := &model.Status{ + Status: model.STATUS_AWAY, + } + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user, channelNotifyProps, status, true) + + assert.Contains(t, keywords["@channel"], user.Id) + assert.Contains(t, keywords["@all"], user.Id) + assert.NotContains(t, keywords["@here"], user.Id) + }) + + t.Run("should add for multiple users", func(t *testing.T) { + user1 := &model.User{ + Id: model.NewId(), + Username: "user1", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "true", + }, + } + user2 := &model.User{ + Id: model.NewId(), + Username: "user2", + NotifyProps: map[string]string{ + model.CHANNEL_MENTIONS_NOTIFY_PROP: "true", + }, + } + + keywords := map[string][]string{} + addMentionKeywordsForUser(keywords, user1, map[string]string{}, nil, true) + addMentionKeywordsForUser(keywords, user2, map[string]string{}, nil, true) + + assert.Contains(t, keywords["@user1"], user1.Id) + assert.Contains(t, keywords["@user2"], user2.Id) + assert.Contains(t, keywords["@all"], user1.Id) + assert.Contains(t, keywords["@all"], user2.Id) + }) +} + func TestGetMentionsEnabledFields(t *testing.T) { attachmentWithTextAndPreText := model.SlackAttachment{ @@ -1383,8 +1649,8 @@ func TestIsKeywordMultibyte(t *testing.T) { Message: "My name is 萌", Keywords: map[string][]string{"萌": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1392,15 +1658,15 @@ func TestIsKeywordMultibyte(t *testing.T) { Message: "My name is 萌", Keywords: map[string][]string{"萌": {}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, + Mentions: nil, }, }, "MultibyteCharacterAtBeginningOfSentence": { Message: "이메일을 보내다.", Keywords: map[string][]string{"이메일": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1408,15 +1674,15 @@ func TestIsKeywordMultibyte(t *testing.T) { Message: "이메일을 보내다.", Keywords: map[string][]string{"이메일": {}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, + Mentions: nil, }, }, "MultibyteCharacterInPartOfSentence": { Message: "我爱吃番茄炒饭", Keywords: map[string][]string{"番茄": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1424,15 +1690,15 @@ func TestIsKeywordMultibyte(t *testing.T) { Message: "我爱吃番茄炒饭", Keywords: map[string][]string{"番茄": {}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, + Mentions: nil, }, }, "MultibyteCharacterAtEndOfSentence": { Message: "こんにちは、世界", Keywords: map[string][]string{"世界": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1440,15 +1706,15 @@ func TestIsKeywordMultibyte(t *testing.T) { Message: "こんにちは、世界", Keywords: map[string][]string{"世界": {}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, + Mentions: nil, }, }, "MultibyteCharacterTwiceInSentence": { Message: "石橋さんが石橋を渡る", Keywords: map[string][]string{"石橋": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1456,117 +1722,78 @@ func TestIsKeywordMultibyte(t *testing.T) { Message: "石橋さんが石橋を渡る", Keywords: map[string][]string{"石橋": {}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, + Mentions: nil, }, }, } { t.Run(name, func(t *testing.T) { - - post := &model.Post{Message: tc.Message, Props: model.StringInterface{ - "attachments": tc.Attachments, - }, + post := &model.Post{ + Message: tc.Message, + Props: model.StringInterface{ + "attachments": tc.Attachments, + }, } m := getExplicitMentions(post, tc.Keywords) - if tc.Expected.MentionedUserIds == nil { - tc.Expected.MentionedUserIds = make(map[string]bool) - } + // if tc.Expected.MentionedUserIds == nil { + // tc.Expected.MentionedUserIds = make(map[string]bool) + // } assert.EqualValues(t, tc.Expected, m) }) } } -func TestAddMentionedUsers(t *testing.T) { - id1 := model.NewId() - id2 := model.NewId() - id3 := model.NewId() - id4 := model.NewId() - id5 := model.NewId() - id6 := model.NewId() - id7 := model.NewId() - id8 := model.NewId() - id9 := model.NewId() +func TestAddMention(t *testing.T) { + t.Run("should initialize Mentions and store new mentions", func(t *testing.T) { + m := &ExplicitMentions{} - for name, tc := range map[string]struct { - Mentions []string - ExplicitMentions *ExplicitMentions - Expected *ExplicitMentions - }{ - "test": { - Mentions: []string{id1}, - ExplicitMentions: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, - }, - Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - }, - }, - }, - "two users": { - Mentions: []string{id1, id2}, - ExplicitMentions: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, - }, - Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, - }, - }, - }, - "no users": { - Mentions: []string{}, - ExplicitMentions: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, - }, - Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, - }, - }, - "five users": { - Mentions: []string{id1, id5, id4, id8, id9}, - ExplicitMentions: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, - }, - Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id4: true, - id5: true, - id8: true, - id9: true, - }, - }, - }, - "nine users": { - Mentions: []string{id1, id2, id3, id4, id5, id6, id7, id8, id9}, - ExplicitMentions: &ExplicitMentions{ - MentionedUserIds: map[string]bool{}, - }, - Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, - id2: true, - id3: true, - id4: true, - id5: true, - id6: true, - id7: true, - id8: true, - id9: true, - }, - }, - }, - } { - t.Run(name, func(t *testing.T) { - tc.ExplicitMentions.addMentionedUsers(tc.Mentions) - if tc.ExplicitMentions.MentionedUserIds == nil { - tc.ExplicitMentions.MentionedUserIds = make(map[string]bool) - } - assert.EqualValues(t, tc.Expected.MentionedUserIds, tc.ExplicitMentions.MentionedUserIds) - }) - } + userId1 := model.NewId() + userId2 := model.NewId() + + m.addMention(userId1, KeywordMention) + m.addMention(userId2, CommentMention) + + assert.Equal(t, map[string]MentionType{ + userId1: KeywordMention, + userId2: CommentMention, + }, m.Mentions) + }) + + t.Run("should replace existing mentions with higher priority ones", func(t *testing.T) { + m := &ExplicitMentions{} + + userId1 := model.NewId() + userId2 := model.NewId() + + m.addMention(userId1, ThreadMention) + m.addMention(userId2, DMMention) + + m.addMention(userId1, ChannelMention) + m.addMention(userId2, KeywordMention) + + assert.Equal(t, map[string]MentionType{ + userId1: ChannelMention, + userId2: KeywordMention, + }, m.Mentions) + }) + + t.Run("should not replace high priority mentions with low priority ones", func(t *testing.T) { + m := &ExplicitMentions{} + + userId1 := model.NewId() + userId2 := model.NewId() + + m.addMention(userId1, KeywordMention) + m.addMention(userId2, CommentMention) + + m.addMention(userId1, DMMention) + m.addMention(userId2, ThreadMention) + + assert.Equal(t, map[string]MentionType{ + userId1: KeywordMention, + userId2: CommentMention, + }, m.Mentions) + }) } func TestCheckForMentionUsers(t *testing.T) { @@ -1588,8 +1815,8 @@ func TestCheckForMentionUsers(t *testing.T) { Word: "@User", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1597,8 +1824,8 @@ func TestCheckForMentionUsers(t *testing.T) { Word: "@user", Keywords: map[string][]string{"@user": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1606,8 +1833,8 @@ func TestCheckForMentionUsers(t *testing.T) { Word: "@user2", Keywords: map[string][]string{"@user2": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id2: true, + Mentions: map[string]MentionType{ + id2: KeywordMention, }, }, }, @@ -1615,8 +1842,8 @@ func TestCheckForMentionUsers(t *testing.T) { Word: "@UsEr2", Keywords: map[string][]string{"@user2": {id2}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id2: true, + Mentions: map[string]MentionType{ + id2: KeywordMention, }, }, }, @@ -1659,13 +1886,9 @@ func TestCheckForMentionUsers(t *testing.T) { } { t.Run(name, func(t *testing.T) { - e := &ExplicitMentions{ - MentionedUserIds: make(map[string]bool), - } + e := &ExplicitMentions{} e.checkForMention(tc.Word, tc.Keywords) - if tc.Expected.MentionedUserIds == nil { - tc.Expected.MentionedUserIds = make(map[string]bool) - } + assert.EqualValues(t, tc.Expected, e) }) } @@ -1682,8 +1905,8 @@ func TestProcessText(t *testing.T) { Text: "hello user @user1", Keywords: map[string][]string{"@user1": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1691,8 +1914,8 @@ func TestProcessText(t *testing.T) { Text: "hello user.@user1", Keywords: map[string][]string{"@user1": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1700,8 +1923,8 @@ func TestProcessText(t *testing.T) { Text: "hello user-@user1", Keywords: map[string][]string{"@user1": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1709,8 +1932,8 @@ func TestProcessText(t *testing.T) { Text: "hello user:@user1", Keywords: map[string][]string{"@user1": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, }, }, @@ -1742,26 +1965,21 @@ func TestProcessText(t *testing.T) { OtherPotentialMentions: []string{"potentialuser", "otherpotentialuser"}, }, }, - "Mention a user and another pontential users or system calls": { + "Mention a real user and another potential user": { Text: "@user1, you can use @systembot to get help", Keywords: map[string][]string{"@user1": {id1}}, Expected: &ExplicitMentions{ - MentionedUserIds: map[string]bool{ - id1: true, + Mentions: map[string]MentionType{ + id1: KeywordMention, }, OtherPotentialMentions: []string{"systembot"}, }, }, } { t.Run(name, func(t *testing.T) { - - e := &ExplicitMentions{ - MentionedUserIds: make(map[string]bool), - } - if tc.Expected.MentionedUserIds == nil { - tc.Expected.MentionedUserIds = make(map[string]bool) - } + e := &ExplicitMentions{} e.processText(tc.Text, tc.Keywords) + assert.EqualValues(t, tc.Expected, e) }) } diff --git a/app/post.go b/app/post.go index d6f5e1d6a8..aa463e4b35 100644 --- a/app/post.go +++ b/app/post.go @@ -1178,3 +1178,137 @@ func (a *App) MaxPostSize() int { return maxPostSize } + +// countMentionsFromPost returns the number of posts in the post's channel that mention the user after and including the +// given post. Returns the number of mentions or store.MentionAllPosts if the post is in a direct message channel. +func (a *App) countMentionsFromPost(user *model.User, post *model.Post) (int, *model.AppError) { + channel, err := a.GetChannel(post.ChannelId) + if err != nil { + return 0, err + } + + if channel.Type == model.CHANNEL_DIRECT { + return store.MentionAllPosts, nil + } + + channelMember, err := a.GetChannelMember(channel.Id, user.Id) + if err != nil { + return 0, err + } + + keywords := addMentionKeywordsForUser( + map[string][]string{}, + user, + channelMember.NotifyProps, + &model.Status{Status: model.STATUS_ONLINE}, // Assume the user is online since they would've triggered this + true, // Assume channel mentions are always allowed for simplicity + ) + commentMentions := user.NotifyProps[model.COMMENTS_NOTIFY_PROP] + checkForCommentMentions := commentMentions == model.COMMENTS_NOTIFY_ROOT || commentMentions == model.COMMENTS_NOTIFY_ANY + + // A mapping of thread root IDs to whether or not a post in that thread mentions the user + mentionedByThread := make(map[string]bool) + + thread, err := a.GetPostThread(post.Id) + if err != nil { + return 0, err + } + + count := 0 + + if isPostMention(user, post, keywords, thread.Posts, mentionedByThread, checkForCommentMentions) { + count += 1 + } + + page := 0 + perPage := 200 + for { + postList, err := a.GetPostsAfterPost(model.GetPostsOptions{ + ChannelId: post.ChannelId, + PostId: post.Id, + Page: page, + PerPage: perPage, + }) + if err != nil { + return 0, err + } + + for _, postId := range postList.Order { + if isPostMention(user, postList.Posts[postId], keywords, postList.Posts, mentionedByThread, checkForCommentMentions) { + count += 1 + } + } + + if len(postList.Order) < perPage { + break + } + + page += 1 + } + + return count, nil +} + +func isCommentMention(user *model.User, post *model.Post, otherPosts map[string]*model.Post, mentionedByThread map[string]bool) bool { + if post.RootId == "" { + // Not a comment + return false + } + + if mentioned, ok := mentionedByThread[post.RootId]; ok { + // We've already figured out if the user was mentioned by this thread + return mentioned + } + + // Whether or not the user was mentioned because they started the thread + mentioned := otherPosts[post.RootId].UserId == user.Id + + // Or because they commented on it before this post + if !mentioned && user.NotifyProps[model.COMMENTS_NOTIFY_PROP] == model.COMMENTS_NOTIFY_ANY { + for _, otherPost := range otherPosts { + if otherPost.Id == post.Id { + continue + } + + if otherPost.RootId != post.RootId { + continue + } + + if otherPost.UserId == user.Id && otherPost.CreateAt < post.CreateAt { + // Found a comment made by the user from before this post + mentioned = true + break + } + } + } + + mentionedByThread[post.RootId] = mentioned + return mentioned +} + +func isPostMention(user *model.User, post *model.Post, keywords map[string][]string, otherPosts map[string]*model.Post, mentionedByThread map[string]bool, checkForCommentMentions bool) bool { + // Prevent the user from mentioning themselves + if post.UserId == user.Id && post.Props["from_webhook"] != "true" { + return false + } + + // Check for keyword mentions + mentions := getExplicitMentions(post, keywords) + if _, ok := mentions.Mentions[user.Id]; ok { + return true + } + + // Check for mentions caused by being added to the channel + if post.Type == model.POST_ADD_TO_CHANNEL { + if addedUserId, ok := post.Props[model.POST_PROPS_ADDED_USER_ID].(string); ok && addedUserId == user.Id { + return true + } + } + + // Check for comment mentions + if checkForCommentMentions && isCommentMention(user, post, otherPosts, mentionedByThread) { + return true + } + + return false +} diff --git a/app/post_test.go b/app/post_test.go index f07a541312..5092aa9cd7 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -16,6 +16,7 @@ import ( "github.com/mattermost/mattermost-server/einterfaces/mocks" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/store/storetest" ) @@ -930,3 +931,567 @@ func TestSearchPostsInTeamForUser(t *testing.T) { es.AssertExpectations(t) }) } + +func TestCountMentionsFromPost(t *testing.T) { + t.Run("should not count posts without mentions", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test3", + }, channel, false) + require.Nil(t, err) + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 0, count) + }) + + t.Run("should count keyword mentions", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.MENTION_KEYS_NOTIFY_PROP] = "apple" + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "apple", + }, channel, false) + require.Nil(t, err) + + // post1 and post3 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 2, count) + }) + + t.Run("should count channel-wide mentions when enabled", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.CHANNEL_MENTIONS_NOTIFY_PROP] = "true" + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "@channel", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "@all", + }, channel, false) + require.Nil(t, err) + + // post2 and post3 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 2, count) + }) + + t.Run("should not count channel-wide mentions when disabled for user", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.CHANNEL_MENTIONS_NOTIFY_PROP] = "false" + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "@channel", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "@all", + }, channel, false) + require.Nil(t, err) + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 0, count) + }) + + t.Run("should not count channel-wide mentions when disabled for channel", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.CHANNEL_MENTIONS_NOTIFY_PROP] = "true" + + _, err := th.App.UpdateChannelMemberNotifyProps(map[string]string{ + model.IGNORE_CHANNEL_MENTIONS_NOTIFY_PROP: model.IGNORE_CHANNEL_MENTIONS_ON, + }, channel.Id, user2.Id) + require.Nil(t, err) + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "@channel", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "@all", + }, channel, false) + require.Nil(t, err) + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 0, count) + }) + + t.Run("should count comment mentions when using COMMENTS_NOTIFY_ROOT", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.COMMENTS_NOTIFY_PROP] = model.COMMENTS_NOTIFY_ROOT + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + RootId: post1.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + post3, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test3", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + RootId: post3.Id, + Message: "test4", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + RootId: post3.Id, + Message: "test5", + }, channel, false) + require.Nil(t, err) + + // post2 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 1, count) + }) + + t.Run("should count comment mentions when using COMMENTS_NOTIFY_ANY", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.COMMENTS_NOTIFY_PROP] = model.COMMENTS_NOTIFY_ANY + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + RootId: post1.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + post3, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test3", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + RootId: post3.Id, + Message: "test4", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + RootId: post3.Id, + Message: "test5", + }, channel, false) + require.Nil(t, err) + + // post2 and post5 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 2, count) + }) + + t.Run("should count mentions caused by being added to the channel", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test", + Type: model.POST_ADD_TO_CHANNEL, + Props: map[string]interface{}{ + model.POST_PROPS_ADDED_USER_ID: model.NewId(), + }, + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test2", + Type: model.POST_ADD_TO_CHANNEL, + Props: map[string]interface{}{ + model.POST_PROPS_ADDED_USER_ID: user2.Id, + }, + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test3", + Type: model.POST_ADD_TO_CHANNEL, + Props: map[string]interface{}{ + model.POST_PROPS_ADDED_USER_ID: user2.Id, + }, + }, channel, false) + require.Nil(t, err) + + // should be mentioned by post2 and post3 + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 2, count) + }) + + t.Run("should return store.MentionAllPosts for a direct channel", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel, err := th.App.createDirectChannel(user1.Id, user2.Id) + require.Nil(t, err) + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test", + }, channel, false) + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, store.MentionAllPosts, count) + }) + + t.Run("should not count mentions from the before the given post", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + _, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + post2, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + + // post1 and post3 should mention the user, but we only count post3 + + count, err := th.App.countMentionsFromPost(user2, post2) + + assert.Nil(t, err) + assert.Equal(t, 1, count) + }) + + t.Run("should not count mentions from the user's own posts", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + + // post2 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 1, count) + }) + + t.Run("should include comments made before the given post when counting comment mentions", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + user2.NotifyProps[model.COMMENTS_NOTIFY_PROP] = model.COMMENTS_NOTIFY_ANY + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test1", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + RootId: post1.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + post3, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test3", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + RootId: post1.Id, + Message: "test4", + }, channel, false) + require.Nil(t, err) + + // post4 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post3) + + assert.Nil(t, err) + assert.Equal(t, 1, count) + }) + + t.Run("should count mentions from the user's webhook posts", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test1", + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user2.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + Props: map[string]interface{}{ + "from_webhook": "true", + }, + }, channel, false) + require.Nil(t, err) + + // post3 should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, 1, count) + }) + + t.Run("should count multiple pages of mentions", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user1 := th.BasicUser + user2 := th.BasicUser2 + + channel := th.CreateChannel(th.BasicTeam) + th.AddUserToChannel(user2, channel) + + numPosts := 215 + + post1, err := th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + + for i := 0; i < numPosts-1; i++ { + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: fmt.Sprintf("@%s", user2.Username), + }, channel, false) + require.Nil(t, err) + } + + // Every post should mention the user + + count, err := th.App.countMentionsFromPost(user2, post1) + + assert.Nil(t, err) + assert.Equal(t, numPosts, count) + }) +} diff --git a/model/user.go b/model/user.go index ee8c8e7b6b..30baef167f 100644 --- a/model/user.go +++ b/model/user.go @@ -397,8 +397,7 @@ func (u *User) SetDefaultNotifications() { func (user *User) UpdateMentionKeysFromUsername(oldUsername string) { nonUsernameKeys := []string{} - splitKeys := strings.Split(user.NotifyProps[MENTION_KEYS_NOTIFY_PROP], ",") - for _, key := range splitKeys { + for _, key := range user.GetMentionKeys() { if key != oldUsername && key != "@"+oldUsername { nonUsernameKeys = append(nonUsernameKeys, key) } @@ -410,6 +409,22 @@ func (user *User) UpdateMentionKeysFromUsername(oldUsername string) { } } +func (user *User) GetMentionKeys() []string { + var keys []string + + for _, key := range strings.Split(user.NotifyProps[MENTION_KEYS_NOTIFY_PROP], ",") { + trimmedKey := strings.TrimSpace(key) + + if trimmedKey == "" { + continue + } + + keys = append(keys, trimmedKey) + } + + return keys +} + func (u *User) Patch(patch *UserPatch) { if patch.Username != nil { u.Username = *patch.Username diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index fdb9bc37ef..376ac1eb34 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1678,7 +1678,7 @@ func (s SqlChannelStore) GetPinnedPostCount(channelId string, allowFromCache boo FROM Posts WHERE IsPinned = true - AND ChannelId = :ChannelId + AND ChannelId = :ChannelId AND DeleteAt = 0`, map[string]interface{}{"ChannelId": channelId}) if err != nil { @@ -1880,8 +1880,8 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string) return times, nil } -// CountPostsSince gives the number of posts in a channel created since a given date. -func (s SqlChannelStore) CountPostsSince(channelID string, since int64) (int64, *model.AppError) { +// countPostsAfter returns the number of posts in the given channel created after but not including the given timestamp. +func (s SqlChannelStore) countPostsAfter(channelID string, since int64) (int64, *model.AppError) { countUnreadQuery := ` SELECT count(*) FROM Posts @@ -1898,22 +1898,27 @@ func (s SqlChannelStore) CountPostsSince(channelID string, since int64) (int64, unread, err := s.GetReplica().SelectInt(countUnreadQuery, countParams) if err != nil { - return 0, model.NewAppError("SqlChannelStore.CountPostsSince", "store.sql_channel.count_posts_since.app_error", countParams, fmt.Sprintf("channel_id=%s, since=%d, err=%s", channelID, since, err), http.StatusInternalServerError) + return 0, model.NewAppError("SqlChannelStore.countPostsAfter", "store.sql_channel.count_posts_since.app_error", countParams, fmt.Sprintf("channel_id=%s, since=%d, err=%s", channelID, since, err), http.StatusInternalServerError) } return unread, nil } -// UpdateLastViewedAtPost sets a channel as unread for a user at the time of the post selected and update the MentionCount -// it returns a channelunread so redux can update the apps easily. +// UpdateLastViewedAtPost updates a ChannelMember as if the user last read the channel at the time of the given post. +// If the provided mentionCount is -1, the given post and all posts after it are considered to be mentions. Returns +// an updated model.ChannelUnreadAt that can be returned to the client. func (s SqlChannelStore) UpdateLastViewedAtPost(unreadPost *model.Post, userID string, mentionCount int) (*model.ChannelUnreadAt, *model.AppError) { - unreadDate := unreadPost.CreateAt - 1 - unread, appErr := s.CountPostsSince(unreadPost.ChannelId, unreadDate) + unread, appErr := s.countPostsAfter(unreadPost.ChannelId, unreadDate) if appErr != nil { return nil, appErr } + if mentionCount == store.MentionAllPosts { + // Treat every unread post as a mention (like in a DM channel) + mentionCount = int(unread) + } + params := map[string]interface{}{ "mentions": mentionCount, "unreadCount": unread, @@ -1943,7 +1948,7 @@ func (s SqlChannelStore) UpdateLastViewedAtPost(unreadPost *model.Post, userID s } chanUnreadQuery := ` - SELECT + SELECT c.TeamId TeamId, cm.UserId UserId, cm.ChannelId ChannelId, @@ -1951,11 +1956,11 @@ func (s SqlChannelStore) UpdateLastViewedAtPost(unreadPost *model.Post, userID s cm.MentionCount MentionCount, cm.LastViewedAt LastViewedAt, cm.NotifyProps NotifyProps - FROM + FROM ChannelMembers cm LEFT JOIN Channels c ON c.Id=cm.ChannelId WHERE - cm.UserId = :userId + cm.UserId = :userId AND cm.channelId = :channelId AND c.DeleteAt = 0 ` diff --git a/store/store.go b/store/store.go index 090459a8f2..0e76a10e21 100644 --- a/store/store.go +++ b/store/store.go @@ -9,6 +9,10 @@ import ( "github.com/mattermost/mattermost-server/model" ) +const ( + MentionAllPosts = -1 +) + type StoreResult struct { Data interface{} Err *model.AppError