From 38855327502420f06d1235d9ac34a66d0bcbca34 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Fri, 21 Oct 2016 16:23:59 -0400 Subject: Fix notifications for public/private channels and add basic unit test (#4295) --- api/post.go | 28 +++++++++++++--------------- api/post_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 15 deletions(-) (limited to 'api') diff --git a/api/post.go b/api/post.go index 5ae5f60db..293ee0af0 100644 --- a/api/post.go +++ b/api/post.go @@ -539,14 +539,14 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri return mentioned, potentialOthersMentioned, hereMentioned } -func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *model.Channel) { +func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *model.Channel) []string { pchan := Srv.Store.User().GetProfilesInChannel(channel.Id, -1, -1, true) fchan := Srv.Store.FileInfo().GetForPost(post.Id) var profileMap map[string]*model.User if result := <-pchan; result.Err != nil { l4g.Error(utils.T("api.post.handle_post_events_and_forget.profiles.error"), c.TeamId, result.Err) - return + return nil } else { profileMap = result.Data.(map[string]*model.User) } @@ -554,7 +554,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * // If the user who made the post is mention don't send a notification if _, ok := profileMap[post.UserId]; !ok { l4g.Error(utils.T("api.post.send_notifications_and_forget.user_id.error"), post.UserId) - return + return nil } mentionedUserIds := make(map[string]bool) @@ -574,22 +574,21 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * } else { keywords := getMentionKeywordsInChannel(profileMap) - var mentioned map[string]bool var potentialOtherMentions []string - mentioned, potentialOtherMentions, hereNotification = getExplicitMentions(post.Message, keywords) + mentionedUserIds, potentialOtherMentions, hereNotification = getExplicitMentions(post.Message, keywords) // get users that have comment thread mentions enabled if len(post.RootId) > 0 { if result := <-Srv.Store.Post().Get(post.RootId); result.Err != nil { l4g.Error(utils.T("api.post.send_notifications_and_forget.comment_thread.error"), post.RootId, result.Err) - return + return nil } else { list := result.Data.(*model.PostList) for _, threadPost := range list.Posts { profile := profileMap[threadPost.UserId] if profile.NotifyProps["comments"] == "any" || (profile.NotifyProps["comments"] == "root" && threadPost.Id == list.Order[0]) { - mentioned[threadPost.UserId] = true + mentionedUserIds[threadPost.UserId] = true } } } @@ -597,7 +596,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * // prevent the user from mentioning themselves if post.Props["from_webhook"] != "true" { - delete(mentioned, post.UserId) + delete(mentionedUserIds, post.UserId) } if len(potentialOtherMentions) > 0 { @@ -618,6 +617,10 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * } mentionedUsersList := make([]string, 0, len(mentionedUserIds)) + for id := range mentionedUserIds { + mentionedUsersList = append(mentionedUsersList, id) + updateMentionChans = append(updateMentionChans, Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id)) + } senderName := "" @@ -633,11 +636,6 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * sender = profile } - for id := range mentionedUserIds { - mentionedUsersList = append(mentionedUsersList, id) - updateMentionChans = append(updateMentionChans, Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id)) - } - if utils.Cfg.EmailSettings.SendEmailNotifications { for _, id := range mentionedUsersList { userAllowsEmails := profileMap[id].NotifyProps["email"] != "false" @@ -657,7 +655,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * if hereNotification { if result := <-Srv.Store.Status().GetOnline(); result.Err != nil { l4g.Warn(utils.T("api.post.notification.here.warn"), result.Err) - return + return nil } else { statuses := result.Data.([]*model.Status) for _, status := range statuses { @@ -754,7 +752,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * } Publish(message) - return + return mentionedUsersList } func sendNotificationEmail(c *Context, post *model.Post, user *model.User, channel *model.Channel, team *model.Team, senderName string, sender *model.User) { diff --git a/api/post_test.go b/api/post_test.go index 3c917aec3..0bafb5d20 100644 --- a/api/post_test.go +++ b/api/post_test.go @@ -1273,3 +1273,41 @@ func TestGetFileInfosForPost(t *testing.T) { t.Fatal("should've returned nothing because of etag") } } + +// TODO: Needs to be vastly fleshed out +func TestSendNotifications(t *testing.T) { + th := Setup().InitBasic() + Client := th.BasicClient + + AddUserToChannel(th.BasicUser2, th.BasicChannel) + + mockSession := model.Session{ + UserId: th.BasicUser.Id, + TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, UserId: th.BasicUser.Id}}, + IsOAuth: false, + } + + newContext := &Context{ + Session: mockSession, + RequestId: model.NewId(), + IpAddress: "", + Path: "fake", + Err: nil, + siteURL: *utils.Cfg.ServiceSettings.SiteURL, + TeamId: th.BasicTeam.Id, + } + + post1 := Client.Must(Client.CreatePost(&model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "@" + th.BasicUser2.Username, + })).Data.(*model.Post) + + mentions := sendNotifications(newContext, post1, th.BasicTeam, th.BasicChannel) + if mentions == nil { + t.Log(mentions) + t.Fatal("user should have been mentioned") + } else if mentions[0] != th.BasicUser2.Id { + t.Log(mentions) + t.Fatal("user should have been mentioned") + } +} -- cgit v1.2.3-1-g7c22