From 26f96b240ddc8cf8c56decea72102b10238e0a43 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Wed, 31 Aug 2016 12:52:14 -0400 Subject: PLT-3462 Add the ability to clear push notifications after channel is viewed (#3834) * Add the ability to clear push notifications after channel is viewed * Fix race condition between updating the mention count and reading it when sending push notifications --- api/channel.go | 16 +++++ api/post.go | 162 ++++++++++++++++++++++++++----------------- i18n/en.json | 16 ++++- model/push_notification.go | 15 ++++ model/session.go | 6 ++ store/sql_user_store.go | 19 +++++ store/sql_user_store_test.go | 10 +++ store/store.go | 1 + 8 files changed, 178 insertions(+), 67 deletions(-) diff --git a/api/channel.go b/api/channel.go index faa39e13c..e2c67f18b 100644 --- a/api/channel.go +++ b/api/channel.go @@ -846,8 +846,24 @@ func updateLastViewedAt(c *Context, w http.ResponseWriter, r *http.Request) { params := mux.Vars(r) id := params["channel_id"] + doClearPush := false + if *utils.Cfg.EmailSettings.SendPushNotifications && !c.Session.IsMobileApp() { + if result := <-Srv.Store.User().GetUnreadCountForChannel(c.Session.UserId, id); result.Err != nil { + l4g.Error(utils.T("api.channel.update_last_viewed_at.get_unread_count_for_channel.error"), c.Session.UserId, id, result.Err.Error()) + } else { + if result.Data.(int64) > 0 { + doClearPush = true + } + } + } + Srv.Store.Channel().UpdateLastViewedAt(id, c.Session.UserId) + // Must be after update so that unread count is correct + if doClearPush { + go clearPushNotification(c.Session.UserId, id) + } + preference := model.Preference{ UserId: c.Session.UserId, Category: model.PREFERENCE_CATEGORY_LAST, diff --git a/api/post.go b/api/post.go index 8639938e2..bfd22084b 100644 --- a/api/post.go +++ b/api/post.go @@ -711,6 +711,15 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * } } + // Make sure all mention updates are complete to prevent race + // Probably better to batch these DB updates in the future + // MUST be completed before push notifications send + for _, uchan := range updateMentionChans { + if result := <-uchan; result.Err != nil { + l4g.Warn(utils.T("api.post.update_mention_count_and_forget.update_error"), post.Id, post.ChannelId, result.Err) + } + } + sendPushNotifications := false if *utils.Cfg.EmailSettings.SendPushNotifications { pushServer := *utils.Cfg.EmailSettings.PushNotificationServer @@ -773,14 +782,6 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * message.Add("mentions", model.ArrayToJson(mentionedUsersList)) } - // Make sure all mention updates are complete to prevent race - // Probably better to batch these DB updates in the future - for _, uchan := range updateMentionChans { - if result := <-uchan; result.Err != nil { - l4g.Warn(utils.T("api.post.update_mention_count_and_forget.update_error"), post.Id, post.ChannelId, result.Err) - } - } - go Publish(message) } @@ -888,12 +889,10 @@ func getMessageForNotification(post *model.Post, translateFunc i18n.TranslateFun } func sendPushNotification(post *model.Post, user *model.User, channel *model.Channel, senderName string, wasMentioned bool) { - var sessions []*model.Session - if result := <-Srv.Store.Session().GetSessions(user.Id); result.Err != nil { - l4g.Error(utils.T("api.post.send_notifications_and_forget.sessions.error"), user.Id, result.Err) + session := getMobileAppSession(user.Id) + + if session == nil { return - } else { - sessions = result.Data.([]*model.Session) } var channelName string @@ -906,65 +905,98 @@ func sendPushNotification(post *model.Post, user *model.User, channel *model.Cha userLocale := utils.GetUserTranslations(user.Locale) - for _, session := range sessions { - if len(session.DeviceId) > 0 && - (strings.HasPrefix(session.DeviceId, model.PUSH_NOTIFY_APPLE+":") || strings.HasPrefix(session.DeviceId, model.PUSH_NOTIFY_ANDROID+":")) { + msg := model.PushNotification{} + if badge := <-Srv.Store.User().GetUnreadCount(user.Id); badge.Err != nil { + msg.Badge = 1 + l4g.Error(utils.T("store.sql_user.get_unread_count.app_error"), user.Id, badge.Err) + } else { + msg.Badge = int(badge.Data.(int64)) + } + msg.Type = model.PUSH_TYPE_MESSAGE + msg.ChannelId = channel.Id + msg.ChannelName = channel.Name - msg := model.PushNotification{} - if badge := <-Srv.Store.User().GetUnreadCount(user.Id); badge.Err != nil { - msg.Badge = 1 - l4g.Error(utils.T("store.sql_user.get_unread_count.app_error"), user.Id, badge.Err) - } else { - msg.Badge = int(badge.Data.(int64)) - } - msg.ServerId = utils.CfgDiagnosticId - msg.ChannelId = channel.Id - msg.ChannelName = channel.Name - - if strings.HasPrefix(session.DeviceId, model.PUSH_NOTIFY_APPLE+":") { - msg.Platform = model.PUSH_NOTIFY_APPLE - msg.DeviceId = strings.TrimPrefix(session.DeviceId, model.PUSH_NOTIFY_APPLE+":") - } else if strings.HasPrefix(session.DeviceId, model.PUSH_NOTIFY_ANDROID+":") { - msg.Platform = model.PUSH_NOTIFY_ANDROID - msg.DeviceId = strings.TrimPrefix(session.DeviceId, model.PUSH_NOTIFY_ANDROID+":") - } + msg.SetDeviceIdAndPlatform(session.DeviceId) - if *utils.Cfg.EmailSettings.PushNotificationContents == model.FULL_NOTIFICATION { - if channel.Type == model.CHANNEL_DIRECT { - msg.Category = model.CATEGORY_DM - msg.Message = "@" + senderName + ": " + model.ClearMentionTags(post.Message) - } else { - msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_in") + channelName + ": " + model.ClearMentionTags(post.Message) - } - } else { - if channel.Type == model.CHANNEL_DIRECT { - msg.Category = model.CATEGORY_DM - msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_message") - } else if wasMentioned { - msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_mention") + channelName - } else { - msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_non_mention") + channelName - } - } + if *utils.Cfg.EmailSettings.PushNotificationContents == model.FULL_NOTIFICATION { + if channel.Type == model.CHANNEL_DIRECT { + msg.Category = model.CATEGORY_DM + msg.Message = "@" + senderName + ": " + model.ClearMentionTags(post.Message) + } else { + msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_in") + channelName + ": " + model.ClearMentionTags(post.Message) + } + } else { + if channel.Type == model.CHANNEL_DIRECT { + msg.Category = model.CATEGORY_DM + msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_message") + } else if wasMentioned { + msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_mention") + channelName + } else { + msg.Message = senderName + userLocale("api.post.send_notifications_and_forget.push_non_mention") + channelName + } + } - tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: *utils.Cfg.ServiceSettings.EnableInsecureOutgoingConnections}, - } - httpClient := &http.Client{Transport: tr} - request, _ := http.NewRequest("POST", *utils.Cfg.EmailSettings.PushNotificationServer+model.API_URL_SUFFIX_V1+"/send_push", strings.NewReader(msg.ToJson())) + l4g.Debug(utils.T("api.post.send_notifications_and_forget.push_notification.debug"), msg.DeviceId, msg.Message) + sendToPushProxy(msg) +} - l4g.Debug(utils.T("api.post.send_notifications_and_forget.push_notification.debug"), msg.DeviceId, msg.Message) - if resp, err := httpClient.Do(request); err != nil { - l4g.Error(utils.T("api.post.send_notifications_and_forget.push_notification.error"), user.Id, err) - } else { - ioutil.ReadAll(resp.Body) - resp.Body.Close() - } +func clearPushNotification(userId string, channelId string) { + session := getMobileAppSession(userId) + + if session == nil { + return + } + + msg := model.PushNotification{} + msg.Type = model.PUSH_TYPE_CLEAR + msg.ChannelId = channelId + msg.ContentAvailable = 0 + if badge := <-Srv.Store.User().GetUnreadCount(userId); badge.Err != nil { + msg.Badge = 0 + l4g.Error(utils.T("store.sql_user.get_unread_count.app_error"), userId, badge.Err) + } else { + msg.Badge = int(badge.Data.(int64)) + } + + msg.SetDeviceIdAndPlatform(session.DeviceId) - // notification sent, don't need to check other sessions - break + l4g.Debug(utils.T("api.post.send_notifications_and_forget.clear_push_notification.debug"), msg.DeviceId, msg.ChannelId) + sendToPushProxy(msg) +} + +func sendToPushProxy(msg model.PushNotification) { + msg.ServerId = utils.CfgDiagnosticId + + tr := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: *utils.Cfg.ServiceSettings.EnableInsecureOutgoingConnections}, + } + httpClient := &http.Client{Transport: tr} + request, _ := http.NewRequest("POST", *utils.Cfg.EmailSettings.PushNotificationServer+model.API_URL_SUFFIX_V1+"/send_push", strings.NewReader(msg.ToJson())) + + if resp, err := httpClient.Do(request); err != nil { + l4g.Error(utils.T("api.post.send_notifications_and_forget.push_notification.error"), msg.DeviceId, err) + } else { + ioutil.ReadAll(resp.Body) + resp.Body.Close() + } +} + +func getMobileAppSession(userId string) *model.Session { + var sessions []*model.Session + if result := <-Srv.Store.Session().GetSessions(userId); result.Err != nil { + l4g.Error(utils.T("api.post.send_notifications_and_forget.sessions.error"), userId, result.Err) + return nil + } else { + sessions = result.Data.([]*model.Session) + } + + for _, session := range sessions { + if session.IsMobileApp() { + return session } } + + return nil } func sendOutOfChannelMentions(c *Context, post *model.Post, profiles map[string]*model.User, outOfChannelMentions map[string]bool) { diff --git a/i18n/en.json b/i18n/en.json index f2a2d2cd0..cdb05f14b 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -151,6 +151,10 @@ "id": "api.channel.add_member.added", "translation": "%v added to the channel by %v" }, + { + "id": "api.channel.update_last_viewed_at.get_unread_count_for_channel.errord", + "translation": "Unable to get the unread count for user_id=%v and channel_id=%v, err=%v" + }, { "id": "api.channel.add_member.find_channel.app_error", "translation": "Failed to find channel" @@ -1297,11 +1301,15 @@ }, { "id": "api.post.send_notifications_and_forget.push_notification.debug", - "translation": "Sending push notification to %v wi msg of '%v'" + "translation": "Sending push notification to %v with msg of '%v'" + }, + { + "id": "api.post.send_notifications_and_forget.clear_push_notification.debug", + "translation": "Clearing push notification to %v with channel_id %v" }, { "id": "api.post.send_notifications_and_forget.push_notification.error", - "translation": "Failed to send push notificationid=%v, err=%v" + "translation": "Failed to send push device_id=%v, err=%v" }, { "id": "api.post.send_notifications_and_forget.send.error", @@ -4455,6 +4463,10 @@ "id": "store.sql_user.get_unread_count.app_error", "translation": "We could not get the unread message count for the user" }, + { + "id": "store.sql_user.get_unread_count_for_channel.app_error", + "translation": "We could not get the unread message count for the user and channel" + }, { "id": "store.sql_user.migrate_theme.critical", "translation": "Failed to migrate User.ThemeProps to Preferences table %v" diff --git a/model/push_notification.go b/model/push_notification.go index 666dd8f7d..d4c380291 100644 --- a/model/push_notification.go +++ b/model/push_notification.go @@ -6,12 +6,16 @@ package model import ( "encoding/json" "io" + "strings" ) const ( PUSH_NOTIFY_APPLE = "apple" PUSH_NOTIFY_ANDROID = "android" + PUSH_TYPE_MESSAGE = "message" + PUSH_TYPE_CLEAR = "clear" + CATEGORY_DM = "DIRECT_MESSAGE" MHPNS = "https://push.mattermost.com" @@ -28,6 +32,7 @@ type PushNotification struct { ContentAvailable int `json:"cont_ava"` ChannelId string `json:"channel_id"` ChannelName string `json:"channel_name"` + Type string `json:"type"` } func (me *PushNotification) ToJson() string { @@ -39,6 +44,16 @@ func (me *PushNotification) ToJson() string { } } +func (me *PushNotification) SetDeviceIdAndPlatform(deviceId string) { + if strings.HasPrefix(deviceId, PUSH_NOTIFY_APPLE+":") { + me.Platform = PUSH_NOTIFY_APPLE + me.DeviceId = strings.TrimPrefix(deviceId, PUSH_NOTIFY_APPLE+":") + } else if strings.HasPrefix(deviceId, PUSH_NOTIFY_ANDROID+":") { + me.Platform = PUSH_NOTIFY_ANDROID + me.DeviceId = strings.TrimPrefix(deviceId, PUSH_NOTIFY_ANDROID+":") + } +} + func PushNotificationFromJson(data io.Reader) *PushNotification { decoder := json.NewDecoder(data) var me PushNotification diff --git a/model/session.go b/model/session.go index ef51374db..e8b04fbe2 100644 --- a/model/session.go +++ b/model/session.go @@ -6,6 +6,7 @@ package model import ( "encoding/json" "io" + "strings" ) const ( @@ -109,6 +110,11 @@ func (me *Session) GetTeamByTeamId(teamId string) *TeamMember { return nil } +func (me *Session) IsMobileApp() bool { + return len(me.DeviceId) > 0 && + (strings.HasPrefix(me.DeviceId, PUSH_NOTIFY_APPLE+":") || strings.HasPrefix(me.DeviceId, PUSH_NOTIFY_ANDROID+":")) +} + func SessionsToJson(o []*Session) string { if b, err := json.Marshal(o); err != nil { return "[]" diff --git a/store/sql_user_store.go b/store/sql_user_store.go index 027dcbd75..61bfa35b8 100644 --- a/store/sql_user_store.go +++ b/store/sql_user_store.go @@ -921,3 +921,22 @@ func (us SqlUserStore) GetUnreadCount(userId string) StoreChannel { return storeChannel } + +func (us SqlUserStore) GetUnreadCountForChannel(userId string, channelId string) StoreChannel { + storeChannel := make(StoreChannel) + + go func() { + result := StoreResult{} + + if count, err := us.GetReplica().SelectInt("SELECT SUM(CASE WHEN c.Type = 'D' THEN (c.TotalMsgCount - cm.MsgCount) ELSE cm.MentionCount END) FROM Channels c INNER JOIN ChannelMembers cm ON c.Id = :ChannelId AND cm.ChannelId = :ChannelId AND cm.UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}); err != nil { + result.Err = model.NewLocAppError("SqlUserStore.GetMentionCountForChannel", "store.sql_user.get_unread_count_for_channel.app_error", nil, err.Error()) + } else { + result.Data = count + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} diff --git a/store/sql_user_store_test.go b/store/sql_user_store_test.go index e44cee99d..74c27dbea 100644 --- a/store/sql_user_store_test.go +++ b/store/sql_user_store_test.go @@ -660,6 +660,16 @@ func TestUserUnreadCount(t *testing.T) { if badge != 3 { t.Fatal("should have 3 unread messages") } + + badge = (<-store.User().GetUnreadCountForChannel(u2.Id, c1.Id)).Data.(int64) + if badge != 1 { + t.Fatal("should have 1 unread messages for that channel") + } + + badge = (<-store.User().GetUnreadCountForChannel(u2.Id, c2.Id)).Data.(int64) + if badge != 2 { + t.Fatal("should have 2 unread messages for that channel") + } } func TestUserStoreUpdateMfaSecret(t *testing.T) { diff --git a/store/store.go b/store/store.go index 78db41e77..d3b908106 100644 --- a/store/store.go +++ b/store/store.go @@ -153,6 +153,7 @@ type UserStore interface { PermanentDelete(userId string) StoreChannel AnalyticsUniqueUserCount(teamId string) StoreChannel GetUnreadCount(userId string) StoreChannel + GetUnreadCountForChannel(userId string, channelId string) StoreChannel } type SessionStore interface { -- cgit v1.2.3-1-g7c22