From f32eb525f3fa0828a23f589d765c267e3b2aea86 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Fri, 2 Sep 2016 12:50:15 -0400 Subject: Do not send push notifications for channels being actively viewed (#3931) --- api/channel.go | 16 +++++++++++++- api/channel_test.go | 2 +- api/post.go | 11 +++++---- api/status.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++++----- api/status_test.go | 36 ++++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 14 deletions(-) (limited to 'api') diff --git a/api/channel.go b/api/channel.go index e2c67f18b..c477a5ee4 100644 --- a/api/channel.go +++ b/api/channel.go @@ -846,8 +846,16 @@ func updateLastViewedAt(c *Context, w http.ResponseWriter, r *http.Request) { params := mux.Vars(r) id := params["channel_id"] + data := model.StringInterfaceFromJson(r.Body) + + var active bool + var ok bool + if active, ok = data["active"].(bool); !ok { + active = true + } + doClearPush := false - if *utils.Cfg.EmailSettings.SendPushNotifications && !c.Session.IsMobileApp() { + if *utils.Cfg.EmailSettings.SendPushNotifications && !c.Session.IsMobileApp() && active { 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 { @@ -857,6 +865,12 @@ func updateLastViewedAt(c *Context, w http.ResponseWriter, r *http.Request) { } } + go func() { + if err := SetActiveChannel(c.Session.UserId, id); err != nil { + l4g.Error(err.Error()) + } + }() + Srv.Store.Channel().UpdateLastViewedAt(id, c.Session.UserId) // Must be after update so that unread count is correct diff --git a/api/channel_test.go b/api/channel_test.go index 450aac877..7046a9868 100644 --- a/api/channel_test.go +++ b/api/channel_test.go @@ -647,7 +647,7 @@ func TestGetChannel(t *testing.T) { t.Fatal("cache should be empty") } - if _, err := Client.UpdateLastViewedAt(channel2.Id); err != nil { + if _, err := Client.UpdateLastViewedAt(channel2.Id, true); err != nil { t.Fatal(err) } diff --git a/api/post.go b/api/post.go index 55e63cd23..d62b85059 100644 --- a/api/post.go +++ b/api/post.go @@ -682,7 +682,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * var status *model.Status var err *model.AppError if status, err = GetStatus(id); err != nil { - status = &model.Status{id, model.STATUS_OFFLINE, false, 0} + status = &model.Status{id, model.STATUS_OFFLINE, false, 0, ""} } if userAllowsEmails && status.Status != model.STATUS_ONLINE { @@ -739,10 +739,10 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * var status *model.Status var err *model.AppError if status, err = GetStatus(id); err != nil { - status = &model.Status{id, model.STATUS_OFFLINE, false, 0} + status = &model.Status{id, model.STATUS_OFFLINE, false, 0, ""} } - if profileMap[id].StatusAllowsPushNotification(status) { + if DoesStatusAllowPushNotification(profileMap[id], status, post.ChannelId) { sendPushNotification(post, profileMap[id], channel, senderName, true) } } @@ -752,10 +752,10 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel * var status *model.Status var err *model.AppError if status, err = GetStatus(id); err != nil { - status = &model.Status{id, model.STATUS_OFFLINE, false, 0} + status = &model.Status{id, model.STATUS_OFFLINE, false, 0, ""} } - if profileMap[id].StatusAllowsPushNotification(status) { + if DoesStatusAllowPushNotification(profileMap[id], status, post.ChannelId) { sendPushNotification(post, profileMap[id], channel, senderName, false) } } @@ -945,7 +945,6 @@ func sendPushNotification(post *model.Post, user *model.User, channel *model.Cha func clearPushNotification(userId string, channelId string) { session := getMobileAppSession(userId) - if session == nil { return } diff --git a/api/status.go b/api/status.go index d83eac033..e8e324778 100644 --- a/api/status.go +++ b/api/status.go @@ -28,6 +28,7 @@ func InitStatus() { l4g.Debug(utils.T("api.status.init.debug")) BaseRoutes.Users.Handle("/status", ApiUserRequiredActivity(getStatusesHttp, false)).Methods("GET") + BaseRoutes.Users.Handle("/status/set_active_channel", ApiUserRequiredActivity(setActiveChannel, false)).Methods("POST") BaseRoutes.WebSocket.Handle("get_statuses", ApiWebSocketHandler(getStatusesWebSocket)) } @@ -66,13 +67,12 @@ func GetAllStatuses() (map[string]interface{}, *model.AppError) { } func SetStatusOnline(userId string, sessionId string, manual bool) { - l4g.Debug(userId, "online") broadcast := false var status *model.Status var err *model.AppError if status, err = GetStatus(userId); err != nil { - status = &model.Status{userId, model.STATUS_ONLINE, false, model.GetMillis()} + status = &model.Status{userId, model.STATUS_ONLINE, false, model.GetMillis(), ""} broadcast = true } else { if status.Manual && !manual { @@ -113,13 +113,12 @@ func SetStatusOnline(userId string, sessionId string, manual bool) { } func SetStatusOffline(userId string, manual bool) { - l4g.Debug(userId, "offline") status, err := GetStatus(userId) if err == nil && status.Manual && !manual { return // manually set status always overrides non-manual one } - status = &model.Status{userId, model.STATUS_OFFLINE, manual, model.GetMillis()} + status = &model.Status{userId, model.STATUS_OFFLINE, manual, model.GetMillis(), ""} AddStatusCache(status) @@ -133,11 +132,10 @@ func SetStatusOffline(userId string, manual bool) { } func SetStatusAwayIfNeeded(userId string, manual bool) { - l4g.Debug(userId, "away") status, err := GetStatus(userId) if err != nil { - status = &model.Status{userId, model.STATUS_OFFLINE, manual, 0} + status = &model.Status{userId, model.STATUS_OFFLINE, manual, 0, ""} } if !manual && status.Manual { @@ -156,6 +154,7 @@ func SetStatusAwayIfNeeded(userId string, manual bool) { status.Status = model.STATUS_AWAY status.Manual = manual + status.ActiveChannel = "" AddStatusCache(status) @@ -183,3 +182,56 @@ 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, status *model.Status, channelId string) bool { + props := user.NotifyProps + + if props["push"] == "none" { + 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 +} + +func setActiveChannel(c *Context, w http.ResponseWriter, r *http.Request) { + data := model.MapFromJson(r.Body) + + var channelId string + var ok bool + if channelId, ok = data["channel_id"]; !ok || len(channelId) > 26 { + c.SetInvalidParam("setActiveChannel", "channel_id") + return + } + + if err := SetActiveChannel(c.Session.UserId, channelId); err != nil { + c.Err = err + return + } + + ReturnStatusOK(w) +} + +func SetActiveChannel(userId string, channelId string) *model.AppError { + status, err := GetStatus(userId) + if err != nil { + status = &model.Status{userId, model.STATUS_ONLINE, false, model.GetMillis(), channelId} + } else { + status.ActiveChannel = channelId + } + + AddStatusCache(status) + + if result := <-Srv.Store.Status().SaveOrUpdate(status); result.Err != nil { + return result.Err + } + + return nil +} diff --git a/api/status_test.go b/api/status_test.go index 451f39e14..a8292d323 100644 --- a/api/status_test.go +++ b/api/status_test.go @@ -151,3 +151,39 @@ func TestStatuses(t *testing.T) { t.Fatal("didn't get offline event") } } + +func TestSetActiveChannel(t *testing.T) { + th := Setup().InitBasic() + Client := th.BasicClient + + if _, err := Client.SetActiveChannel(th.BasicChannel.Id); err != nil { + t.Fatal(err) + } + + status, _ := GetStatus(th.BasicUser.Id) + if status.ActiveChannel != th.BasicChannel.Id { + t.Fatal("active channel should be set") + } + + if _, err := Client.SetActiveChannel(""); err != nil { + t.Fatal(err) + } + + status, _ = GetStatus(th.BasicUser.Id) + if status.ActiveChannel != "" { + t.Fatal("active channel should be blank") + } + + if _, err := Client.SetActiveChannel("123456789012345678901234567890"); err == nil { + t.Fatal("should have failed, id too long") + } + + if _, err := Client.UpdateLastViewedAt(th.BasicChannel.Id, true); err != nil { + t.Fatal(err) + } + + status, _ = GetStatus(th.BasicUser.Id) + if status.ActiveChannel != th.BasicChannel.Id { + t.Fatal("active channel should be set") + } +} -- cgit v1.2.3-1-g7c22