PLT-7404 Return viewed at times in view channel API response (#7428)

* Return viewed at times in view channel API response

* Updated transaction to read and write once

* Remove transaction and only update if new value greater than older

api/channel.go

@@ -779,7 +779,7 @@ func viewChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := c.App.ViewChannel(view, c.Session.UserId, !c.Session.IsMobileApp()); err != nil {
+	if _, err := c.App.ViewChannel(view, c.Session.UserId, !c.Session.IsMobileApp()); err != nil {
 		c.Err = err
 		return
 	}

api4/channel.go

@@ -709,12 +709,20 @@ func viewChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := c.App.ViewChannel(view, c.Params.UserId, !c.Session.IsMobileApp()); err != nil {
+	times, err := c.App.ViewChannel(view, c.Params.UserId, !c.Session.IsMobileApp())
+
+	if err != nil {
 		c.Err = err
 		return
 	}
 
-	ReturnStatusOK(w)
+	// Returning {"status": "OK", ...} for backwards compatability
+	resp := &model.ChannelViewResponse{
+		Status:            "OK",
+		LastViewedAtTimes: times,
+	}
+
+	w.Write([]byte(resp.ToJson()))
 }
 
 func updateChannelMemberRoles(c *Context, w http.ResponseWriter, r *http.Request) {

api4/channel_test.go

@@ -1375,13 +1375,19 @@ func TestViewChannel(t *testing.T) {
 		ChannelId: th.BasicChannel.Id,
 	}
 
-	pass, resp := Client.ViewChannel(th.BasicUser.Id, view)
+	viewResp, resp := Client.ViewChannel(th.BasicUser.Id, view)
 	CheckNoError(t, resp)
 
-	if !pass {
+	if viewResp.Status != "OK" {
 		t.Fatal("should have passed")
 	}
 
+	channel, _ := th.App.GetChannel(th.BasicChannel.Id)
+
+	if lastViewedAt := viewResp.LastViewedAtTimes[channel.Id]; lastViewedAt != channel.LastPostAt {
+		t.Fatal("LastPostAt does not match returned LastViewedAt time")
+	}
+
 	view.PrevChannelId = th.BasicChannel.Id
 	_, resp = Client.ViewChannel(th.BasicUser.Id, view)
 	CheckNoError(t, resp)
@@ -1396,7 +1402,7 @@ func TestViewChannel(t *testing.T) {
 
 	member, resp := Client.GetChannelMember(th.BasicChannel.Id, th.BasicUser.Id, "")
 	CheckNoError(t, resp)
-	channel, resp := Client.GetChannel(th.BasicChannel.Id, "")
+	channel, resp = Client.GetChannel(th.BasicChannel.Id, "")
 	CheckNoError(t, resp)
 
 	if member.MsgCount != channel.TotalMsgCount {

app/channel.go

@@ -1136,9 +1136,9 @@ func (a *App) SearchChannelsUserNotIn(teamId string, userId string, term string)
 	}
 }
 
-func (a *App) ViewChannel(view *model.ChannelView, userId string, clearPushNotifications bool) *model.AppError {
+func (a *App) ViewChannel(view *model.ChannelView, userId string, clearPushNotifications bool) (map[string]int64, *model.AppError) {
 	if err := a.SetActiveChannel(userId, view.ChannelId); err != nil {
-		return err
+		return nil, err
 	}
 
 	channelIds := []string{}
@@ -1157,14 +1157,14 @@ func (a *App) ViewChannel(view *model.ChannelView, userId string, clearPushNotif
 	}
 
 	if len(channelIds) == 0 {
-		return nil
+		return map[string]int64{}, nil
 	}
 
 	uchan := a.Srv.Store.Channel().UpdateLastViewedAt(channelIds, userId)
 
 	if pchan != nil {
 		if result := <-pchan; result.Err != nil {
-			return result.Err
+			return nil, result.Err
 		} else {
 			if result.Data.(int64) > 0 {
 				a.ClearPushNotification(userId, view.ChannelId)
@@ -1172,8 +1172,11 @@ func (a *App) ViewChannel(view *model.ChannelView, userId string, clearPushNotif
 		}
 	}
 
+	times := map[string]int64{}
 	if result := <-uchan; result.Err != nil {
-		return result.Err
+		return nil, result.Err
+	} else {
+		times = result.Data.(map[string]int64)
 	}
 
 	if *utils.Cfg.ServiceSettings.EnableChannelViewedMessages && model.IsValidId(view.ChannelId) {
@@ -1182,7 +1185,7 @@ func (a *App) ViewChannel(view *model.ChannelView, userId string, clearPushNotif
 		go a.Publish(message)
 	}
 
-	return nil
+	return times, nil
 }
 
 func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError {

model/channel_view.go

@@ -32,3 +32,28 @@ func ChannelViewFromJson(data io.Reader) *ChannelView {
 		return nil
 	}
 }
+
+type ChannelViewResponse struct {
+	Status            string           `json:"status"`
+	LastViewedAtTimes map[string]int64 `json:"last_viewed_at_times"`
+}
+
+func (o *ChannelViewResponse) ToJson() string {
+	b, err := json.Marshal(o)
+	if err != nil {
+		return ""
+	} else {
+		return string(b)
+	}
+}
+
+func ChannelViewResponseFromJson(data io.Reader) *ChannelViewResponse {
+	decoder := json.NewDecoder(data)
+	var o ChannelViewResponse
+	err := decoder.Decode(&o)
+	if err == nil {
+		return &o
+	} else {
+		return nil
+	}
+}

model/channel_view_test.go

@@ -0,0 +1,38 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package model
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestChannelViewJson(t *testing.T) {
+	o := ChannelView{ChannelId: NewId(), PrevChannelId: NewId()}
+	json := o.ToJson()
+	ro := ChannelViewFromJson(strings.NewReader(json))
+
+	if o.ChannelId != ro.ChannelId {
+		t.Fatal("ChannelIdIds do not match")
+	}
+
+	if o.PrevChannelId != ro.PrevChannelId {
+		t.Fatal("PrevChannelIds do not match")
+	}
+}
+
+func TestChannelViewResponseJson(t *testing.T) {
+	id := NewId()
+	o := ChannelViewResponse{Status: "OK", LastViewedAtTimes: map[string]int64{id: 12345}}
+	json := o.ToJson()
+	ro := ChannelViewResponseFromJson(strings.NewReader(json))
+
+	if o.Status != ro.Status {
+		t.Fatal("ChannelIdIds do not match")
+	}
+
+	if o.LastViewedAtTimes[id] != ro.LastViewedAtTimes[id] {
+		t.Fatal("LastViewedAtTimes do not match")
+	}
+}

model/client4.go

@@ -1570,13 +1570,13 @@ func (c *Client4) GetChannelMembersForUser(userId, teamId, etag string) (*Channe
 }
 
 // ViewChannel performs a view action for a user. Synonymous with switching channels or marking channels as read by a user.
-func (c *Client4) ViewChannel(userId string, view *ChannelView) (bool, *Response) {
+func (c *Client4) ViewChannel(userId string, view *ChannelView) (*ChannelViewResponse, *Response) {
 	url := fmt.Sprintf(c.GetChannelsRoute()+"/members/%v/view", userId)
 	if r, err := c.DoApiPost(url, view.ToJson()); err != nil {
-		return false, BuildErrorResponse(r, err)
+		return nil, BuildErrorResponse(r, err)
 	} else {
 		defer closeBody(r)
-		return CheckStatusOK(r), BuildResponse(r)
+		return ChannelViewResponseFromJson(r.Body), BuildResponse(r)
 	}
 }
 

store/sqlstore/channel_store.go

@@ -1284,52 +1284,82 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string)
 	go func() {
 		result := store.StoreResult{}
 
-		var query string
 		props := make(map[string]interface{})
 
-		idQuery := ""
+		updateIdQuery := ""
 		for index, channelId := range channelIds {
-			if len(idQuery) > 0 {
-				idQuery += " OR "
+			if len(updateIdQuery) > 0 {
+				updateIdQuery += " OR "
 			}
 
 			props["channelId"+strconv.Itoa(index)] = channelId
-			idQuery += "ChannelId = :channelId" + strconv.Itoa(index)
+			updateIdQuery += "ChannelId = :channelId" + strconv.Itoa(index)
 		}
 
+		selectIdQuery := strings.Replace(updateIdQuery, "ChannelId", "Id", -1)
+
+		var lastPostAtTimes []struct {
+			Id            string
+			LastPostAt    int64
+			TotalMsgCount int64
+		}
+
+		selectQuery := "SELECT Id, LastPostAt, TotalMsgCount FROM Channels WHERE (" + selectIdQuery + ")"
+
+		if _, err := s.GetMaster().Select(&lastPostAtTimes, selectQuery, props); err != nil {
+			result.Err = model.NewAppError("SqlChannelStore.UpdateLastViewedAt", "store.sql_channel.update_last_viewed_at.app_error", nil, "channel_ids="+strings.Join(channelIds, ",")+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
+			storeChannel <- result
+			close(storeChannel)
+			return
+		}
+
+		times := map[string]int64{}
+		msgCountQuery := ""
+		lastViewedQuery := ""
+		for index, t := range lastPostAtTimes {
+			times[t.Id] = t.LastPostAt
+
+			props["msgCount"+strconv.Itoa(index)] = t.TotalMsgCount
+			msgCountQuery += fmt.Sprintf("WHEN :channelId%d THEN GREATEST(MsgCount, :msgCount%d) ", index, index)
+
+			props["lastViewed"+strconv.Itoa(index)] = t.LastPostAt
+			lastViewedQuery += fmt.Sprintf("WHEN :channelId%d THEN GREATEST(LastViewedAt, :lastViewed%d) ", index, index)
+
+			props["channelId"+strconv.Itoa(index)] = t.Id
+		}
+
+		var updateQuery string
+
 		if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
-			query = `UPDATE
+			updateQuery = `UPDATE
 				ChannelMembers
 			SET
 			    MentionCount = 0,
-			    MsgCount = Channels.TotalMsgCount,
-			    LastViewedAt = Channels.LastPostAt,
-			    LastUpdateAt = Channels.LastPostAt
-			FROM
-				Channels
+			    MsgCount = CAST(CASE ChannelId ` + msgCountQuery + ` END AS BIGINT),
+			    LastViewedAt = CAST(CASE ChannelId ` + lastViewedQuery + ` END AS BIGINT),
+			    LastUpdateAt = CAST(CASE ChannelId ` + lastViewedQuery + ` END AS BIGINT)
 			WHERE
-			    Channels.Id = ChannelMembers.ChannelId
-			        AND UserId = :UserId
-			        AND (` + idQuery + `)`
+			        UserId = :UserId
+			        AND (` + updateIdQuery + `)`
 		} else if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
-			query = `UPDATE
-				ChannelMembers, Channels
+			updateQuery = `UPDATE
+				ChannelMembers
 			SET
-			    ChannelMembers.MentionCount = 0,
-			    ChannelMembers.MsgCount = Channels.TotalMsgCount,
-			    ChannelMembers.LastViewedAt = Channels.LastPostAt,
-			    ChannelMembers.LastUpdateAt = Channels.LastPostAt
+			    MentionCount = 0,
+			    MsgCount = CASE ChannelId ` + msgCountQuery + ` END,
+			    LastViewedAt = CASE ChannelId ` + lastViewedQuery + ` END,
+			    LastUpdateAt = CASE ChannelId ` + lastViewedQuery + ` END
 			WHERE
-			    Channels.Id = ChannelMembers.ChannelId
-			        AND UserId = :UserId
-			        AND (` + idQuery + `)`
+			        UserId = :UserId
+			        AND (` + updateIdQuery + `)`
 		}
 
 		props["UserId"] = userId
 
-		_, err := s.GetMaster().Exec(query, props)
-		if err != nil {
+		if _, err := s.GetMaster().Exec(updateQuery, props); err != nil {
 			result.Err = model.NewAppError("SqlChannelStore.UpdateLastViewedAt", "store.sql_channel.update_last_viewed_at.app_error", nil, "channel_ids="+strings.Join(channelIds, ",")+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
+		} else {
+			result.Data = times
 		}
 
 		storeChannel <- result

store/sqlstore/channel_store_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
 )
@@ -1249,6 +1251,7 @@ func TestChannelStoreUpdateLastViewedAt(t *testing.T) {
 	o1.Name = "zz" + model.NewId() + "b"
 	o1.Type = model.CHANNEL_OPEN
 	o1.TotalMsgCount = 25
+	o1.LastPostAt = 12345
 	store.Must(ss.Channel().Save(&o1))
 
 	m1 := model.ChannelMember{}
@@ -1257,13 +1260,44 @@ func TestChannelStoreUpdateLastViewedAt(t *testing.T) {
 	m1.NotifyProps = model.GetDefaultChannelNotifyProps()
 	store.Must(ss.Channel().SaveMember(&m1))
 
-	err := (<-ss.Channel().UpdateLastViewedAt([]string{m1.ChannelId}, m1.UserId)).Err
-	if err != nil {
-		t.Fatal("failed to update", err)
+	o2 := model.Channel{}
+	o2.TeamId = model.NewId()
+	o2.DisplayName = "Channel1"
+	o2.Name = "zz" + model.NewId() + "c"
+	o2.Type = model.CHANNEL_OPEN
+	o2.TotalMsgCount = 26
+	o2.LastPostAt = 123456
+	store.Must(ss.Channel().Save(&o2))
+
+	m2 := model.ChannelMember{}
+	m2.ChannelId = o2.Id
+	m2.UserId = m1.UserId
+	m2.NotifyProps = model.GetDefaultChannelNotifyProps()
+	store.Must(ss.Channel().SaveMember(&m2))
+
+	if result := <-ss.Channel().UpdateLastViewedAt([]string{m1.ChannelId}, m1.UserId); result.Err != nil {
+		t.Fatal("failed to update", result.Err)
+	} else if result.Data.(map[string]int64)[o1.Id] != o1.LastPostAt {
+		t.Fatal("last viewed at time incorrect")
 	}
 
-	err = (<-ss.Channel().UpdateLastViewedAt([]string{m1.ChannelId}, "missing id")).Err
-	if err != nil {
+	if result := <-ss.Channel().UpdateLastViewedAt([]string{m1.ChannelId, m2.ChannelId}, m1.UserId); result.Err != nil {
+		t.Fatal("failed to update", result.Err)
+	} else if result.Data.(map[string]int64)[o2.Id] != o2.LastPostAt {
+		t.Fatal("last viewed at time incorrect")
+	}
+
+	rm1 := store.Must(ss.Channel().GetMember(m1.ChannelId, m1.UserId)).(*model.ChannelMember)
+	assert.Equal(t, rm1.LastViewedAt, o1.LastPostAt)
+	assert.Equal(t, rm1.LastUpdateAt, o1.LastPostAt)
+	assert.Equal(t, rm1.MsgCount, o1.TotalMsgCount)
+
+	rm2 := store.Must(ss.Channel().GetMember(m2.ChannelId, m2.UserId)).(*model.ChannelMember)
+	assert.Equal(t, rm2.LastViewedAt, o2.LastPostAt)
+	assert.Equal(t, rm2.LastUpdateAt, o2.LastPostAt)
+	assert.Equal(t, rm2.MsgCount, o2.TotalMsgCount)
+
+	if result := <-ss.Channel().UpdateLastViewedAt([]string{m1.ChannelId}, "missing id"); result.Err != nil {
 		t.Fatal("failed to update")
 	}
 }