From d7699e51e7dc25fdfa484b3b0d5ac96deb2aa803 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Wed, 1 Mar 2017 19:26:21 -0300 Subject: [PATCH 1/4] Fix push notifications where channel is set to all activity --- app/notification.go | 11 ++++++----- app/status.go | 6 ++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/notification.go b/app/notification.go index ae365b4179a87..4fe75ec596afb 100644 --- a/app/notification.go +++ b/app/notification.go @@ -102,9 +102,10 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe // find which users in the channel are set up to always receive mobile notifications for _, profile := range profileMap { - if profile.NotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_ALL && + if (profile.NotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_ALL && (post.UserId != profile.Id || post.Props["from_webhook"] == "true") && - !post.IsSystemMessage() { + !post.IsSystemMessage()) || + channelMemberNotifyPropsMap[profile.Id][model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_ALL { allActivityPushUserIds = append(allActivityPushUserIds, profile.Id) } } @@ -257,7 +258,7 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe status = &model.Status{UserId: id, Status: model.STATUS_OFFLINE, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if DoesStatusAllowPushNotification(profileMap[id], status, post.ChannelId) { + if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], status, post.ChannelId) { sendPushNotification(post, profileMap[id], channel, senderName[id], channelMemberNotifyPropsMap[id], true) } } @@ -270,7 +271,7 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe status = &model.Status{UserId: id, Status: model.STATUS_OFFLINE, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if DoesStatusAllowPushNotification(profileMap[id], status, post.ChannelId) { + if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], status, post.ChannelId) { sendPushNotification(post, profileMap[id], channel, senderName[id], channelMemberNotifyPropsMap[id], false) } } @@ -466,7 +467,7 @@ func sendPushNotification(post *model.Post, user *model.User, channel *model.Cha if channelNotify, ok := channelNotifyProps[model.PUSH_NOTIFY_PROP]; ok { if channelNotify == model.USER_NOTIFY_NONE { return nil - } else if channelNotify == model.USER_NOTIFY_MENTION && !wasMentioned { + } else if channelNotify == model.CHANNEL_NOTIFY_MENTION && !wasMentioned { return nil } } diff --git a/app/status.go b/app/status.go index bef17165fc789..9bd89fe77f02d 100644 --- a/app/status.go +++ b/app/status.go @@ -236,10 +236,12 @@ func IsUserAway(lastActivityAt int64) bool { return model.GetMillis()-lastActivityAt >= *utils.Cfg.TeamSettings.UserStatusAwayTimeout*1000 } -func DoesStatusAllowPushNotification(user *model.User, status *model.Status, channelId string) bool { +func DoesStatusAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, status *model.Status, channelId string) bool { props := user.NotifyProps - if props["push"] == "none" { + if props[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE && + (channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE || + channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_DEFAULT) { return false } From 63ee19bec7eaf9c2ecfffe462040d16eef89a164 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Wed, 1 Mar 2017 19:42:41 -0300 Subject: [PATCH 2/4] feedback review --- app/notification.go | 6 +++--- app/status.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/notification.go b/app/notification.go index 4fe75ec596afb..7bf336c694850 100644 --- a/app/notification.go +++ b/app/notification.go @@ -102,10 +102,10 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe // find which users in the channel are set up to always receive mobile notifications for _, profile := range profileMap { - if (profile.NotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_ALL && + if (profile.NotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_ALL || + channelMemberNotifyPropsMap[profile.Id][model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_ALL) && (post.UserId != profile.Id || post.Props["from_webhook"] == "true") && - !post.IsSystemMessage()) || - channelMemberNotifyPropsMap[profile.Id][model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_ALL { + !post.IsSystemMessage() { allActivityPushUserIds = append(allActivityPushUserIds, profile.Id) } } diff --git a/app/status.go b/app/status.go index 9bd89fe77f02d..27010096c1715 100644 --- a/app/status.go +++ b/app/status.go @@ -241,7 +241,7 @@ func DoesStatusAllowPushNotification(user *model.User, channelNotifyProps model. if props[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE && (channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE || - channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_DEFAULT) { + channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_DEFAULT) { return false } From aa7b5f78056ea8b87953cffb62eafda52628410e Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Wed, 1 Mar 2017 20:01:16 -0300 Subject: [PATCH 3/4] moved push notification logic to DoesStatusAllowPushNotification --- app/notification.go | 18 +++++------------- app/status.go | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/app/notification.go b/app/notification.go index 7bf336c694850..56f620c8b8b8e 100644 --- a/app/notification.go +++ b/app/notification.go @@ -258,8 +258,8 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe status = &model.Status{UserId: id, Status: model.STATUS_OFFLINE, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], status, post.ChannelId) { - sendPushNotification(post, profileMap[id], channel, senderName[id], channelMemberNotifyPropsMap[id], true) + if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], true, status, post.ChannelId) { + sendPushNotification(post, profileMap[id], channel, senderName[id], true) } } @@ -271,8 +271,8 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe status = &model.Status{UserId: id, Status: model.STATUS_OFFLINE, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], status, post.ChannelId) { - sendPushNotification(post, profileMap[id], channel, senderName[id], channelMemberNotifyPropsMap[id], false) + if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], false, status, post.ChannelId) { + sendPushNotification(post, profileMap[id], channel, senderName[id], false) } } } @@ -456,7 +456,7 @@ func GetMessageForNotification(post *model.Post, translateFunc i18n.TranslateFun } } -func sendPushNotification(post *model.Post, user *model.User, channel *model.Channel, senderName string, channelNotifyProps model.StringMap, wasMentioned bool) *model.AppError { +func sendPushNotification(post *model.Post, user *model.User, channel *model.Channel, senderName string, wasMentioned bool) *model.AppError { sessions, err := getMobileAppSessions(user.Id) if err != nil { return err @@ -464,14 +464,6 @@ func sendPushNotification(post *model.Post, user *model.User, channel *model.Cha var channelName string - if channelNotify, ok := channelNotifyProps[model.PUSH_NOTIFY_PROP]; ok { - if channelNotify == model.USER_NOTIFY_NONE { - return nil - } else if channelNotify == model.CHANNEL_NOTIFY_MENTION && !wasMentioned { - return nil - } - } - if channel.Type == model.CHANNEL_DIRECT { channelName = senderName } else { diff --git a/app/status.go b/app/status.go index 27010096c1715..a6566891dbc29 100644 --- a/app/status.go +++ b/app/status.go @@ -236,12 +236,21 @@ func IsUserAway(lastActivityAt int64) bool { return model.GetMillis()-lastActivityAt >= *utils.Cfg.TeamSettings.UserStatusAwayTimeout*1000 } -func DoesStatusAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, status *model.Status, channelId string) bool { +func DoesStatusAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, wasMentioned bool, status *model.Status, channelId string) bool { props := user.NotifyProps + channelNotify := "" + ok := false + if channelNotify, ok = channelNotifyProps[model.PUSH_NOTIFY_PROP]; ok { + if channelNotify == model.USER_NOTIFY_NONE { + return false + } else if channelNotify == model.CHANNEL_NOTIFY_MENTION && !wasMentioned { + return false + } + } + if props[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE && - (channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE || - channelNotifyProps[model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_DEFAULT) { + (!ok || channelNotify == model.CHANNEL_NOTIFY_DEFAULT) { return false } From 9c80b8e0474b337fb1954926c082b413a9110451 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Wed, 1 Mar 2017 22:48:14 -0300 Subject: [PATCH 4/4] Have every option handled in ShouldSendPushNotification --- app/notification.go | 49 +++++++++++++++++++++++++++++++++++++++++++-- app/status.go | 29 --------------------------- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/app/notification.go b/app/notification.go index 56f620c8b8b8e..aba0e83b3e32d 100644 --- a/app/notification.go +++ b/app/notification.go @@ -258,7 +258,7 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe status = &model.Status{UserId: id, Status: model.STATUS_OFFLINE, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], true, status, post.ChannelId) { + if ShouldSendPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], true, status, post) { sendPushNotification(post, profileMap[id], channel, senderName[id], true) } } @@ -271,7 +271,7 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe status = &model.Status{UserId: id, Status: model.STATUS_OFFLINE, Manual: false, LastActivityAt: 0, ActiveChannel: ""} } - if DoesStatusAllowPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], false, status, post.ChannelId) { + if ShouldSendPushNotification(profileMap[id], channelMemberNotifyPropsMap[id], false, status, post) { sendPushNotification(post, profileMap[id], channel, senderName[id], false) } } @@ -735,3 +735,48 @@ func GetMentionKeywordsInChannel(profiles map[string]*model.User) map[string][]s return keywords } + +func ShouldSendPushNotification(user *model.User, channelNotifyProps model.StringMap, wasMentioned bool, status *model.Status, post *model.Post) bool { + return DoesNotifyPropsAllowPushNotification(user, channelNotifyProps, post, wasMentioned) && + DoesStatusAllowPushNotification(user.NotifyProps, status, post.ChannelId) +} + +func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, post *model.Post, wasMentioned bool) bool { + userNotifyProps := user.NotifyProps + userNotify := userNotifyProps[model.PUSH_NOTIFY_PROP] + channelNotify, ok := channelNotifyProps[model.PUSH_NOTIFY_PROP] + + if post.IsSystemMessage() { + return false + } + + if channelNotify == model.USER_NOTIFY_NONE { + return false + } else if (userNotify == model.USER_NOTIFY_MENTION || channelNotify == model.CHANNEL_NOTIFY_MENTION) && !wasMentioned { + return false + } + + if (userNotify == model.USER_NOTIFY_ALL || channelNotify == model.CHANNEL_NOTIFY_ALL) && + (post.UserId != user.Id || post.Props["from_webhook"] == "true") { + return true + } + + if userNotify == model.USER_NOTIFY_NONE && + (!ok || channelNotify == model.CHANNEL_NOTIFY_DEFAULT) { + return false + } + + return true +} + +func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *model.Status, channelId string) bool { + if pushStatus, ok := userNotifyProps["push_status"]; (pushStatus == model.STATUS_ONLINE || !ok) && (status.ActiveChannel != channelId || model.GetMillis()-status.LastActivityAt > model.STATUS_CHANNEL_TIMEOUT) { + return true + } else if pushStatus == model.STATUS_AWAY && (status.Status == model.STATUS_AWAY || status.Status == model.STATUS_OFFLINE) { + return true + } else if pushStatus == model.STATUS_OFFLINE && status.Status == model.STATUS_OFFLINE { + return true + } + + return false +} diff --git a/app/status.go b/app/status.go index a6566891dbc29..51e7d1ed8ed77 100644 --- a/app/status.go +++ b/app/status.go @@ -235,32 +235,3 @@ func GetStatus(userId string) (*model.Status, *model.AppError) { func IsUserAway(lastActivityAt int64) bool { return model.GetMillis()-lastActivityAt >= *utils.Cfg.TeamSettings.UserStatusAwayTimeout*1000 } - -func DoesStatusAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, wasMentioned bool, status *model.Status, channelId string) bool { - props := user.NotifyProps - - channelNotify := "" - ok := false - if channelNotify, ok = channelNotifyProps[model.PUSH_NOTIFY_PROP]; ok { - if channelNotify == model.USER_NOTIFY_NONE { - return false - } else if channelNotify == model.CHANNEL_NOTIFY_MENTION && !wasMentioned { - return false - } - } - - if props[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_NONE && - (!ok || channelNotify == model.CHANNEL_NOTIFY_DEFAULT) { - return false - } - - if pushStatus, ok := props["push_status"]; (pushStatus == model.STATUS_ONLINE || !ok) && (status.ActiveChannel != channelId || model.GetMillis()-status.LastActivityAt > model.STATUS_CHANNEL_TIMEOUT) { - return true - } else if pushStatus == model.STATUS_AWAY && (status.Status == model.STATUS_AWAY || status.Status == model.STATUS_OFFLINE) { - return true - } else if pushStatus == model.STATUS_OFFLINE && status.Status == model.STATUS_OFFLINE { - return true - } - - return false -}