PLT-5012 Combine updateLastViewedAt, setLastViewedAt and setActiveChannel into a single API (#4840)

* Combine updateLastViewedAt, setLastViewedAt and setActiveChannel into a single API

* Remove preference DB writes

api/channel.go

@@ -25,6 +25,7 @@ func InitChannel() {
 	BaseRoutes.Channels.Handle("/counts", ApiUserRequired(getChannelCounts)).Methods("GET")
 	BaseRoutes.Channels.Handle("/members", ApiUserRequired(getMyChannelMembers)).Methods("GET")
 	BaseRoutes.Channels.Handle("/create", ApiUserRequired(createChannel)).Methods("POST")
+	BaseRoutes.Channels.Handle("/view", ApiUserRequired(viewChannel)).Methods("POST")
 	BaseRoutes.Channels.Handle("/create_direct", ApiUserRequired(createDirectChannel)).Methods("POST")
 	BaseRoutes.Channels.Handle("/update", ApiUserRequired(updateChannel)).Methods("POST")
 	BaseRoutes.Channels.Handle("/update_header", ApiUserRequired(updateChannelHeader)).Methods("POST")
@@ -43,9 +44,6 @@ func InitChannel() {
 	BaseRoutes.NeedChannel.Handle("/delete", ApiUserRequired(deleteChannel)).Methods("POST")
 	BaseRoutes.NeedChannel.Handle("/add", ApiUserRequired(addMember)).Methods("POST")
 	BaseRoutes.NeedChannel.Handle("/remove", ApiUserRequired(removeMember)).Methods("POST")
-	BaseRoutes.NeedChannel.Handle("/update_last_viewed_at", ApiUserRequired(updateLastViewedAt)).Methods("POST")
-	BaseRoutes.NeedChannel.Handle("/set_last_viewed_at", ApiUserRequired(setLastViewedAt)).Methods("POST")
-
 }
 
 func createChannel(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -873,103 +871,6 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func setLastViewedAt(c *Context, w http.ResponseWriter, r *http.Request) {
-	params := mux.Vars(r)
-	id := params["channel_id"]
-
-	data := model.StringInterfaceFromJson(r.Body)
-	newLastViewedAt := int64(data["last_viewed_at"].(float64))
-
-	Srv.Store.Channel().SetLastViewedAt(id, c.Session.UserId, newLastViewedAt)
-
-	chanPref := model.Preference{
-		UserId:   c.Session.UserId,
-		Category: c.TeamId,
-		Name:     model.PREFERENCE_NAME_LAST_CHANNEL,
-		Value:    id,
-	}
-
-	teamPref := model.Preference{
-		UserId:   c.Session.UserId,
-		Category: model.PREFERENCE_CATEGORY_LAST,
-		Name:     model.PREFERENCE_NAME_LAST_TEAM,
-		Value:    c.TeamId,
-	}
-
-	Srv.Store.Preference().Save(&model.Preferences{teamPref, chanPref})
-
-	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
-	message.Add("channel_id", id)
-
-	go Publish(message)
-
-	result := make(map[string]string)
-	result["id"] = id
-	w.Write([]byte(model.MapToJson(result)))
-}
-
-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() && 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 {
-			if result.Data.(int64) > 0 {
-				doClearPush = true
-			}
-		}
-	}
-
-	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
-	if doClearPush {
-		go clearPushNotification(c.Session.UserId, id)
-	}
-
-	chanPref := model.Preference{
-		UserId:   c.Session.UserId,
-		Category: c.TeamId,
-		Name:     model.PREFERENCE_NAME_LAST_CHANNEL,
-		Value:    id,
-	}
-
-	teamPref := model.Preference{
-		UserId:   c.Session.UserId,
-		Category: model.PREFERENCE_CATEGORY_LAST,
-		Name:     model.PREFERENCE_NAME_LAST_TEAM,
-		Value:    c.TeamId,
-	}
-
-	Srv.Store.Preference().Save(&model.Preferences{teamPref, chanPref})
-
-	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
-	message.Add("channel_id", id)
-
-	go Publish(message)
-
-	result := make(map[string]string)
-	result["id"] = id
-	w.Write([]byte(model.MapToJson(result)))
-}
-
 func getChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	params := mux.Vars(r)
 	id := params["channel_id"]
@@ -1001,7 +902,27 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 			w.Write([]byte(data.ToJson()))
 		}
 	}
+}
 
+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
+		if !status.Manual {
+			status.Status = model.STATUS_ONLINE
+		}
+		status.LastActivityAt = model.GetMillis()
+	}
+
+	AddStatusCache(status)
+
+	if result := <-Srv.Store.Status().SaveOrUpdate(status); result.Err != nil {
+		return result.Err
+	}
+
+	return nil
 }
 
 func getChannelByName(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -1332,3 +1253,50 @@ func autocompleteChannels(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	w.Write([]byte(channels.ToJson()))
 }
+
+func viewChannel(c *Context, w http.ResponseWriter, r *http.Request) {
+	view := model.ChannelViewFromJson(r.Body)
+
+	go func() {
+		if err := SetActiveChannel(c.Session.UserId, view.ChannelId); err != nil {
+			l4g.Error(err.Error())
+		}
+	}()
+
+	if len(view.ChannelId) > 0 {
+
+		if view.Time == 0 {
+			if result := <-Srv.Store.Channel().UpdateLastViewedAt(view.ChannelId, c.Session.UserId); result.Err != nil {
+				c.Err = result.Err
+				return
+			}
+		} else {
+			if result := <-Srv.Store.Channel().SetLastViewedAt(view.ChannelId, c.Session.UserId, view.Time); result.Err != nil {
+				c.Err = result.Err
+				return
+			}
+		}
+
+		if len(view.PrevChannelId) > 0 {
+			Srv.Store.Channel().UpdateLastViewedAt(view.PrevChannelId, c.Session.UserId)
+
+			// Only clear push notifications if a channel switch occured
+			if *utils.Cfg.EmailSettings.SendPushNotifications && !c.Session.IsMobileApp() {
+				go func() {
+					if result := <-Srv.Store.User().GetUnreadCountForChannel(c.Session.UserId, view.ChannelId); result.Err != nil {
+						l4g.Error(utils.T("api.channel.update_last_viewed_at.get_unread_count_for_channel.error"), c.Session.UserId, view.ChannelId, result.Err.Error())
+					} else {
+						if result.Data.(int64) > 0 {
+							clearPushNotification(c.Session.UserId, view.ChannelId)
+						}
+					}
+				}()
+			}
+		}
+
+		message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
+		message.Add("channel_id", view.ChannelId)
+	}
+
+	ReturnStatusOK(w)
+}

api/channel_test.go

@@ -724,8 +724,9 @@ func TestGetChannel(t *testing.T) {
 		t.Fatal("cache should be empty")
 	}
 
-	if _, err := Client.UpdateLastViewedAt(channel2.Id, true); err != nil {
-		t.Fatal(err)
+	view := model.ChannelView{ChannelId: channel2.Id, PrevChannelId: channel1.Id}
+	if _, resp := Client.ViewChannel(view); resp.Error != nil {
+		t.Fatal(resp.Error)
 	}
 
 	if resp, err := Client.GetChannel(channel1.Id, ""); err != nil {
@@ -1735,3 +1736,44 @@ func TestGetChannelByName(t *testing.T) {
 		t.Fatal("Should fail due to not enough permissions")
 	}
 }
+
+func TestViewChannel(t *testing.T) {
+	th := Setup().InitBasic()
+	Client := th.BasicClient
+
+	view := model.ChannelView{
+		ChannelId: th.BasicChannel.Id,
+	}
+
+	if _, resp := Client.ViewChannel(view); resp.Error != nil {
+		t.Fatal(resp.Error)
+	}
+
+	view.PrevChannelId = th.BasicChannel.Id
+
+	if _, resp := Client.ViewChannel(view); resp.Error != nil {
+		t.Fatal(resp.Error)
+	}
+
+	view.PrevChannelId = ""
+	view.Time = 1234567890
+
+	if _, resp := Client.ViewChannel(view); resp.Error != nil {
+		t.Fatal(resp.Error)
+	}
+
+	view.PrevChannelId = "junk"
+	view.Time = 0
+
+	if _, resp := Client.ViewChannel(view); resp.Error != nil {
+		t.Fatal(resp.Error)
+	}
+
+	rdata := Client.Must(Client.GetChannel(th.BasicChannel.Id, "")).Data.(*model.ChannelData)
+
+	if rdata.Channel.TotalMsgCount != rdata.Member.MsgCount {
+		t.Log(rdata.Channel.TotalMsgCount)
+		t.Log(rdata.Member.MsgCount)
+		t.Fatal("message counts don't match")
+	}
+}

api/deprecated.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	l4g "github.com/alecthomas/log4go"
+	"github.com/gorilla/mux"
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/utils"
 )
@@ -16,7 +17,15 @@ import (
 func InitDeprecated() {
 	l4g.Debug(utils.T("api.channel.init.debug"))
 
-	BaseRoutes.Channels.Handle("/more", ApiUserRequired(getMoreChannels)).Methods("GET") // SCHEDULED FOR DEPRECATION IN 3.7
+	/* start - SCHEDULED FOR DEPRECATION IN 3.7 */
+	BaseRoutes.Channels.Handle("/more", ApiUserRequired(getMoreChannels)).Methods("GET")
+	/* end - SCHEDULED FOR DEPRECATION IN 3.7 */
+
+	/* start - SCHEDULED FOR DEPRECATION IN 3.8 */
+	BaseRoutes.NeedChannel.Handle("/update_last_viewed_at", ApiUserRequired(updateLastViewedAt)).Methods("POST")
+	BaseRoutes.NeedChannel.Handle("/set_last_viewed_at", ApiUserRequired(setLastViewedAt)).Methods("POST")
+	BaseRoutes.Users.Handle("/status/set_active_channel", ApiUserRequired(setActiveChannel)).Methods("POST")
+	/* end - SCHEDULED FOR DEPRECATION IN 3.8 */
 }
 
 func getMoreChannels(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -37,3 +46,118 @@ func getMoreChannels(c *Context, w http.ResponseWriter, r *http.Request) {
 		w.Write([]byte(data.ToJson()))
 	}
 }
+
+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() && 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 {
+			if result.Data.(int64) > 0 {
+				doClearPush = true
+			}
+		}
+	}
+
+	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
+	if doClearPush {
+		go clearPushNotification(c.Session.UserId, id)
+	}
+
+	chanPref := model.Preference{
+		UserId:   c.Session.UserId,
+		Category: c.TeamId,
+		Name:     model.PREFERENCE_NAME_LAST_CHANNEL,
+		Value:    id,
+	}
+
+	teamPref := model.Preference{
+		UserId:   c.Session.UserId,
+		Category: model.PREFERENCE_CATEGORY_LAST,
+		Name:     model.PREFERENCE_NAME_LAST_TEAM,
+		Value:    c.TeamId,
+	}
+
+	Srv.Store.Preference().Save(&model.Preferences{teamPref, chanPref})
+
+	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
+	message.Add("channel_id", id)
+
+	go Publish(message)
+
+	result := make(map[string]string)
+	result["id"] = id
+	w.Write([]byte(model.MapToJson(result)))
+}
+
+func setLastViewedAt(c *Context, w http.ResponseWriter, r *http.Request) {
+	params := mux.Vars(r)
+	id := params["channel_id"]
+
+	data := model.StringInterfaceFromJson(r.Body)
+	newLastViewedAt := int64(data["last_viewed_at"].(float64))
+
+	Srv.Store.Channel().SetLastViewedAt(id, c.Session.UserId, newLastViewedAt)
+
+	chanPref := model.Preference{
+		UserId:   c.Session.UserId,
+		Category: c.TeamId,
+		Name:     model.PREFERENCE_NAME_LAST_CHANNEL,
+		Value:    id,
+	}
+
+	teamPref := model.Preference{
+		UserId:   c.Session.UserId,
+		Category: model.PREFERENCE_CATEGORY_LAST,
+		Name:     model.PREFERENCE_NAME_LAST_TEAM,
+		Value:    c.TeamId,
+	}
+
+	Srv.Store.Preference().Save(&model.Preferences{teamPref, chanPref})
+
+	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_VIEWED, c.TeamId, "", c.Session.UserId, nil)
+	message.Add("channel_id", id)
+
+	go Publish(message)
+
+	result := make(map[string]string)
+	result["id"] = id
+	w.Write([]byte(model.MapToJson(result)))
+}
+
+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)
+}

api/deprecated_test.go

@@ -41,3 +41,44 @@ func TestGetMoreChannel(t *testing.T) {
 		t.Fatal("cache should be empty")
 	}
 }
+
+/*
+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)
+	}
+
+	time.Sleep(500 * time.Millisecond)
+
+	status, _ = GetStatus(th.BasicUser.Id)
+	 need to check if offline to catch race
+	if status.Status != model.STATUS_OFFLINE && status.ActiveChannel != th.BasicChannel.Id {
+		t.Fatal("active channel should be set")
+	}
+}
+*/

api/status.go

@@ -37,7 +37,6 @@ func InitStatus() {
 
 	BaseRoutes.Users.Handle("/status", ApiUserRequired(getStatusesHttp)).Methods("GET")
 	BaseRoutes.Users.Handle("/status/ids", ApiUserRequired(getStatusesByIdsHttp)).Methods("POST")
-	BaseRoutes.Users.Handle("/status/set_active_channel", ApiUserRequired(setActiveChannel)).Methods("POST")
 	BaseRoutes.WebSocket.Handle("get_statuses", ApiWebSocketHandler(getStatusesWebSocket))
 	BaseRoutes.WebSocket.Handle("get_statuses_by_ids", ApiWebSocketHandler(getStatusesByIdsWebSocket))
 }
@@ -305,42 +304,3 @@ func DoesStatusAllowPushNotification(user *model.User, status *model.Status, cha
 
 	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
-		if !status.Manual {
-			status.Status = model.STATUS_ONLINE
-		}
-		status.LastActivityAt = model.GetMillis()
-	}
-
-	AddStatusCache(status)
-
-	if result := <-Srv.Store.Status().SaveOrUpdate(status); result.Err != nil {
-		return result.Err
-	}
-
-	return nil
-}

api/status_test.go

@@ -229,44 +229,3 @@ func TestGetStatusesByIds(t *testing.T) {
 		t.Fatal("should have errored")
 	}
 }
-
-/*
-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)
-	}
-
-	time.Sleep(500 * time.Millisecond)
-
-	status, _ = GetStatus(th.BasicUser.Id)
-	 need to check if offline to catch race
-	if status.Status != model.STATUS_OFFLINE && status.ActiveChannel != th.BasicChannel.Id {
-		t.Fatal("active channel should be set")
-	}
-}
-*/