Do not send push notifications for channels being actively viewed (#3931)

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

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)
 	}
 

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
 	}

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
+}

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")
+	}
+}

model/client.go

@@ -1140,8 +1140,13 @@ func (c *Client) RemoveChannelMember(id, user_id string) (*Result, *AppError) {
 	}
 }
 
-func (c *Client) UpdateLastViewedAt(channelId string) (*Result, *AppError) {
+// UpdateLastViewedAt will mark a channel as read.
-	if r, err := c.DoApiPost(c.GetChannelRoute(channelId)+"/update_last_viewed_at", ""); err != nil {
+// The channelId indicates the channel to mark as read. If active is true, push notifications
+// will be cleared if there are unread messages. The default for active is true.
+func (c *Client) UpdateLastViewedAt(channelId string, active bool) (*Result, *AppError) {
+	data := make(map[string]interface{})
+	data["active"] = active
+	if r, err := c.DoApiPost(c.GetChannelRoute(channelId)+"/update_last_viewed_at", StringInterfaceToJson(data)); err != nil {
 		return nil, err
 	} else {
 		defer closeBody(r)
@@ -1463,6 +1468,21 @@ func (c *Client) GetStatuses() (*Result, *AppError) {
 	}
 }
 
+// SetActiveChannel sets the the channel id the user is currently viewing.
+// The channelId key is required but the value can be blank. Returns standard
+// response.
+func (c *Client) SetActiveChannel(channelId string) (*Result, *AppError) {
+	data := map[string]string{}
+	data["channel_id"] = channelId
+	if r, err := c.DoApiPost("/users/status/set_active_channel", MapToJson(data)); err != nil {
+		return nil, err
+	} else {
+		defer closeBody(r)
+		return &Result{r.Header.Get(HEADER_REQUEST_ID),
+			r.Header.Get(HEADER_ETAG_SERVER), MapFromJson(r.Body)}, nil
+	}
+}
+
 func (c *Client) GetMyTeam(etag string) (*Result, *AppError) {
 	if r, err := c.DoApiGet(c.GetTeamRoute()+"/me", "", etag); err != nil {
 		return nil, err

model/status.go

@@ -9,10 +9,11 @@ import (
 )
 
 const (
-	STATUS_OFFLINE    = "offline"
+	STATUS_OFFLINE         = "offline"
-	STATUS_AWAY       = "away"
+	STATUS_AWAY            = "away"
-	STATUS_ONLINE     = "online"
+	STATUS_ONLINE          = "online"
-	STATUS_CACHE_SIZE = 10000
+	STATUS_CACHE_SIZE      = 10000
+	STATUS_CHANNEL_TIMEOUT = 20000 // 20 seconds
 )
 
 type Status struct {
@@ -20,6 +21,7 @@ type Status struct {
 	Status         string `json:"status"`
 	Manual         bool   `json:"manual"`
 	LastActivityAt int64  `json:"last_activity_at"`
+	ActiveChannel  string `json:"active_channel"`
 }
 
 func (o *Status) ToJson() string {

model/status_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func TestStatus(t *testing.T) {
-	status := Status{NewId(), STATUS_ONLINE, true, 0}
+	status := Status{NewId(), STATUS_ONLINE, true, 0, ""}
 	json := status.ToJson()
 	status2 := StatusFromJson(strings.NewReader(json))
 

model/user.go

@@ -378,24 +378,6 @@ func (u *User) IsLDAPUser() bool {
 	return false
 }
 
-func (u *User) StatusAllowsPushNotification(status *Status) bool {
-	props := u.NotifyProps
-
-	if props["push"] == "none" {
-		return false
-	}
-
-	if pushStatus, ok := props["push_status"]; pushStatus == STATUS_ONLINE || !ok {
-		return true
-	} else if pushStatus == STATUS_AWAY && (status.Status == STATUS_AWAY || status.Status == STATUS_OFFLINE) {
-		return true
-	} else if pushStatus == STATUS_OFFLINE && status.Status == STATUS_OFFLINE {
-		return true
-	}
-
-	return false
-}
-
 // UserFromJson will decode the input and return a User
 func UserFromJson(data io.Reader) *User {
 	decoder := json.NewDecoder(data)

store/sql_status_store.go

@@ -24,6 +24,7 @@ func NewSqlStatusStore(sqlStore *SqlStore) StatusStore {
 		table := db.AddTableWithName(model.Status{}, "Status").SetKeys(false, "UserId")
 		table.ColMap("UserId").SetMaxSize(26)
 		table.ColMap("Status").SetMaxSize(32)
+		table.ColMap("ActiveChannel").SetMaxSize(26)
 	}
 
 	return s

store/sql_status_store_test.go

@@ -12,7 +12,7 @@ import (
 func TestSqlStatusStore(t *testing.T) {
 	Setup()
 
-	status := &model.Status{model.NewId(), model.STATUS_ONLINE, false, 0}
+	status := &model.Status{model.NewId(), model.STATUS_ONLINE, false, 0, ""}
 
 	if err := (<-store.Status().SaveOrUpdate(status)).Err; err != nil {
 		t.Fatal(err)
@@ -28,12 +28,12 @@ func TestSqlStatusStore(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	status2 := &model.Status{model.NewId(), model.STATUS_AWAY, false, 0}
+	status2 := &model.Status{model.NewId(), model.STATUS_AWAY, false, 0, ""}
 	if err := (<-store.Status().SaveOrUpdate(status2)).Err; err != nil {
 		t.Fatal(err)
 	}
 
-	status3 := &model.Status{model.NewId(), model.STATUS_OFFLINE, false, 0}
+	status3 := &model.Status{model.NewId(), model.STATUS_OFFLINE, false, 0, ""}
 	if err := (<-store.Status().SaveOrUpdate(status3)).Err; err != nil {
 		t.Fatal(err)
 	}
@@ -81,7 +81,7 @@ func TestSqlStatusStore(t *testing.T) {
 func TestActiveUserCount(t *testing.T) {
 	Setup()
 
-	status := &model.Status{model.NewId(), model.STATUS_ONLINE, false, model.GetMillis()}
+	status := &model.Status{model.NewId(), model.STATUS_ONLINE, false, model.GetMillis(), ""}
 	Must(store.Status().SaveOrUpdate(status))
 
 	if result := <-store.Status().GetTotalActiveUsersCount(); result.Err != nil {

store/sql_upgrade.go

@@ -180,7 +180,9 @@ func UpgradeDatabaseToVersion33(sqlStore *SqlStore) {
 
 func UpgradeDatabaseToVersion34(sqlStore *SqlStore) {
 	if shouldPerformUpgrade(sqlStore, VERSION_3_3_0, VERSION_3_4_0) {
+
 		sqlStore.CreateColumnIfNotExists("Status", "Manual", "BOOLEAN", "BOOLEAN", "0")
+		sqlStore.CreateColumnIfNotExists("Status", "ActiveChannel", "varchar(26)", "varchar(26)", "")
 
 		// TODO XXX FIXME should be removed before release
 		//saveSchemaVersion(sqlStore, VERSION_3_4_0)

webapp/actions/post_actions.jsx

@@ -19,7 +19,7 @@ const Preferences = Constants.Preferences;
 export function handleNewPost(post, msg) {
     if (ChannelStore.getCurrentId() === post.channel_id) {
         if (window.isActive) {
-            AsyncClient.updateLastViewedAt();
+            AsyncClient.updateLastViewedAt(null, false);
         } else {
             AsyncClient.getChannel(post.channel_id);
         }

webapp/actions/websocket_actions.jsx

@@ -174,7 +174,7 @@ function handlePostEditEvent(msg) {
     // Update channel state
     if (ChannelStore.getCurrentId() === msg.channel_id) {
         if (window.isActive) {
-            AsyncClient.updateLastViewedAt();
+            AsyncClient.updateLastViewedAt(null, false);
         }
     }
 }

webapp/client/client.jsx

@@ -1003,6 +1003,16 @@ export default class Client {
             end(this.handleResponse.bind(this, 'getStatuses', success, error));
     }
 
+    setActiveChannel(id, success, error) {
+        request.
+            post(`${this.getUsersRoute()}/status/set_active_channel`).
+            set(this.defaultHeaders).
+            type('application/json').
+            accept('application/json').
+            send({channel_id: id}).
+            end(this.handleResponse.bind(this, 'setActiveChannel', success, error));
+    }
+
     verifyEmail(uid, hid, success, error) {
         request.
             post(`${this.getUsersRoute()}/verify_email`).
@@ -1172,12 +1182,13 @@ export default class Client {
         this.track('api', 'api_channels_delete');
     }
 
-    updateLastViewedAt(channelId, success, error) {
+    updateLastViewedAt(channelId, active, success, error) {
         request.
             post(`${this.getChannelNeededRoute(channelId)}/update_last_viewed_at`).
             set(this.defaultHeaders).
             type('application/json').
             accept('application/json').
+            send({active}).
             end(this.handleResponse.bind(this, 'updateLastViewedAt', success, error));
     }
 

webapp/components/needs_team.jsx

@@ -94,6 +94,7 @@ export default class NeedsTeam extends React.Component {
 
         $(window).on('blur', () => {
             window.isActive = false;
+            AsyncClient.setActiveChannel('');
         });
 
         Utils.applyTheme(this.state.theme);

webapp/components/post_view/post_view_cache.jsx

@@ -4,6 +4,7 @@
 import PostViewController from './post_view_controller.jsx';
 
 import ChannelStore from 'stores/channel_store.jsx';
+import * as AsyncClient from 'utils/async_client.jsx';
 
 import React from 'react';
 
@@ -28,6 +29,7 @@ export default class PostViewCache extends React.Component {
     }
 
     componentWillUnmount() {
+        AsyncClient.setActiveChannel('');
         ChannelStore.removeChangeListener(this.onChannelChange);
     }
 

webapp/tests/client_channel.test.jsx

@@ -216,6 +216,7 @@ describe('Client.Channels', function() {
             var channel = TestHelper.basicChannel();
             TestHelper.basicClient().updateLastViewedAt(
                 channel.id,
+                true,
                 function(data) {
                     assert.equal(data.id, channel.id);
                     done();

webapp/tests/client_user.test.jsx

@@ -509,6 +509,23 @@ describe('Client.User', function() {
     });
     */
 
+    it('setActiveChannel', function(done) {
+        TestHelper.initBasic(() => {
+            var ids = [];
+            ids.push(TestHelper.basicUser().id);
+
+            TestHelper.basicClient().setActiveChannel(
+                TestHelper.basicChannel().id,
+                function() {
+                    done();
+                },
+                function(err) {
+                    done(new Error(err.message));
+                }
+            );
+        });
+    });
+
     it('verifyEmail', function(done) {
         TestHelper.initBasic(() => {
             TestHelper.basicClient().enableLogErrorsToConsole(false); // Disabling since this unit test causes an error

webapp/utils/async_client.jsx

@@ -113,7 +113,7 @@ export function getChannel(id) {
     );
 }
 
-export function updateLastViewedAt(id) {
+export function updateLastViewedAt(id, active) {
     let channelId;
     if (id) {
         channelId = id;
@@ -129,9 +129,17 @@ export function updateLastViewedAt(id) {
         return;
     }
 
+    let isActive;
+    if (active == null) {
+        isActive = true;
+    } else {
+        isActive = active;
+    }
+
     callTracker[`updateLastViewed${channelId}`] = utils.getTimestamp();
     Client.updateLastViewedAt(
         channelId,
+        isActive,
         () => {
             callTracker[`updateLastViewed${channelId}`] = 0;
             ErrorStore.clearLastError();
@@ -753,6 +761,24 @@ export function getStatuses() {
     );
 }
 
+export function setActiveChannel(channelId) {
+    if (isCallInProgress(`setActiveChannel${channelId}`)) {
+        return;
+    }
+
+    callTracker[`setActiveChannel${channelId}`] = utils.getTimestamp();
+    Client.setActiveChannel(
+        channelId,
+        () => {
+            callTracker[`setActiveChannel${channelId}`] = 0;
+        },
+        (err) => {
+            callTracker[`setActiveChannel${channelId}`] = 0;
+            dispatchError(err, 'setActiveChannel');
+        }
+    );
+}
+
 export function getMyTeam() {
     if (isCallInProgress('getMyTeam')) {
         return null;