From 6b0f4f1aeeda6f21713611ffaac01e77bf333bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Wed, 21 Aug 2019 14:48:25 +0200 Subject: [PATCH] [MM-17069] Api endpoint to unread a channel (#11794) * [MM-17069] endpoint to unread a channel from post * [MM-17069] update mock * [MM-17069] first passing test * [MM-17069] fix SQL typo * [MM-17069] fix msgCount * add tests * [MM-17069] Fix tests * [MM-16069] Remove trash, add comments * [MM-16069] Add message to errors * [MM-17069] fix go fmt * [MM-17069] return an UnreadChannel response * [MM-17069] added unauthorized test * [MM-17069] fix operator * [MM-17069] refactor tests * [MM-16069] back to green tests * [MM-17069] change url to include user * [MM-17069] Fixing code review comments * [MM-17069] One shouldn't fix manually what a machine can fix better * [MM-17069] change response type, update tests * [MM-17069] fix permission error * [MM-17069] Add tests for edit_other_users permission * [MM-17069] no magic numbers --- api4/post.go | 23 ++++++ api4/post_test.go | 81 +++++++++++++++++++++ app/channel.go | 12 ++++ app/channel_test.go | 100 ++++++++++++++++++++++++++ i18n/en.json | 8 +++ model/channel_member.go | 21 ++++++ model/client4.go | 10 +++ store/sqlstore/channel_store.go | 84 ++++++++++++++++++++++ store/store.go | 1 + store/storetest/mocks/ChannelStore.go | 25 +++++++ 10 files changed, 365 insertions(+) diff --git a/api4/post.go b/api4/post.go index 6d314350e9..5d6d4a1449 100644 --- a/api4/post.go +++ b/api4/post.go @@ -28,6 +28,7 @@ func (api *API) InitPost() { api.BaseRoutes.Team.Handle("/posts/search", api.ApiSessionRequired(searchPosts)).Methods("POST") api.BaseRoutes.Post.Handle("", api.ApiSessionRequired(updatePost)).Methods("PUT") api.BaseRoutes.Post.Handle("/patch", api.ApiSessionRequired(patchPost)).Methods("PUT") + api.BaseRoutes.PostForUser.Handle("/set_unread", api.ApiSessionRequired(setPostUnread)).Methods("POST") api.BaseRoutes.Post.Handle("/pin", api.ApiSessionRequired(pinPost)).Methods("POST") api.BaseRoutes.Post.Handle("/unpin", api.ApiSessionRequired(unpinPost)).Methods("POST") } @@ -576,6 +577,28 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(patchedPost.ToJson())) } +func setPostUnread(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequirePostId().RequireUserId() + if c.Err != nil { + return + } + if c.App.Session.UserId != c.Params.UserId && !c.App.SessionHasPermissionToUser(c.App.Session, c.Params.UserId) { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + return + } + if !c.App.SessionHasPermissionToChannelByPost(c.App.Session, c.Params.PostId, model.PERMISSION_READ_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + return + } + + state, err := c.App.MarkChannelAsUnreadFromPost(c.Params.PostId, c.Params.UserId) + if err != nil { + c.Err = err + return + } + w.Write([]byte(state.ToJson())) +} + func saveIsPinnedPost(c *Context, w http.ResponseWriter, r *http.Request, isPinned bool) { c.RequirePostId() if c.Err != nil { diff --git a/api4/post_test.go b/api4/post_test.go index b4a9176d7a..22fd2c997f 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -2647,3 +2647,84 @@ func TestGetFileInfosForPost(t *testing.T) { _, resp = th.SystemAdminClient.GetFileInfosForPost(th.BasicPost.Id, "") CheckNoError(t, resp) } + +func TestSetChannelUnread(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + u1 := th.BasicUser + u2 := th.BasicUser2 + s2, _ := th.App.GetSession(th.Client.AuthToken) + th.Client.Login(u1.Email, u1.Password) + c1 := th.BasicChannel + c1toc2 := &model.ChannelView{ChannelId: th.BasicChannel2.Id, PrevChannelId: c1.Id} + now := utils.MillisFromTime(time.Now()) + th.CreateMessagePostNoClient(c1, "AAA", now) + p2 := th.CreateMessagePostNoClient(c1, "BBB", now+10) + th.CreateMessagePostNoClient(c1, "CCC", now+20) + + pp1 := th.CreateMessagePostNoClient(th.BasicPrivateChannel, "Sssh!", now) + pp2 := th.CreateMessagePostNoClient(th.BasicPrivateChannel, "You Sssh!", now+10) + require.NotNil(t, pp1) + require.NotNil(t, pp2) + + // Ensure that post have been read + unread, err := th.App.GetChannelUnread(c1.Id, u1.Id) + require.Nil(t, err) + require.Equal(t, int64(4), unread.MsgCount) + unread, err = th.App.GetChannelUnread(c1.Id, u2.Id) + require.Nil(t, err) + require.Equal(t, int64(4), unread.MsgCount) + _, err = th.App.ViewChannel(c1toc2, u2.Id, s2.Id) + require.Nil(t, err) + unread, err = th.App.GetChannelUnread(c1.Id, u2.Id) + require.Nil(t, err) + require.Equal(t, int64(0), unread.MsgCount) + + t.Run("Unread last one", func(t *testing.T) { + r := th.Client.SetPostUnread(u1.Id, p2.Id) + checkHTTPStatus(t, r, 200, false) + unread, err := th.App.GetChannelUnread(c1.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(1), unread.MsgCount) + }) + + t.Run("Unread on a private channel", func(t *testing.T) { + r := th.Client.SetPostUnread(u1.Id, pp2.Id) + assert.Equal(t, 200, r.StatusCode) + unread, err := th.App.GetChannelUnread(th.BasicPrivateChannel.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(0), unread.MsgCount) + r = th.Client.SetPostUnread(u1.Id, pp1.Id) + assert.Equal(t, 200, r.StatusCode) + unread, err = th.App.GetChannelUnread(th.BasicPrivateChannel.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(1), unread.MsgCount) + }) + + t.Run("Can't unread an imaginary post", func(t *testing.T) { + r := th.Client.SetPostUnread(u1.Id, "invalid4ofngungryquinj976y") + assert.Equal(t, http.StatusForbidden, r.StatusCode) + }) + + // let's create another user to test permissions + u3 := th.CreateUser() + c3 := th.CreateClient() + c3.Login(u3.Email, u3.Password) + + t.Run("Can't unread channels you don't belong to", func(t *testing.T) { + r := c3.SetPostUnread(u3.Id, pp1.Id) + assert.Equal(t, http.StatusForbidden, r.StatusCode) + }) + + t.Run("Can't unread users you don't have permission to edit", func(t *testing.T) { + r := c3.SetPostUnread(u1.Id, pp1.Id) + assert.Equal(t, http.StatusForbidden, r.StatusCode) + }) + + t.Run("Can't unread if user is not logged in", func(t *testing.T) { + th.Client.Logout() + response := th.Client.SetPostUnread(u1.Id, p2.Id) + checkHTTPStatus(t, response, http.StatusUnauthorized, true) + }) +} diff --git a/app/channel.go b/app/channel.go index 54e0f63984..5b46d59c37 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1754,6 +1754,18 @@ func (a *App) UpdateChannelLastViewedAt(channelIds []string, userId string) *mod return nil } +// MarkChanelAsUnreadFromPost will take a post and set the channel as unread from that one. +func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string) (*model.ChannelUnreadAt, *model.AppError) { + + post, err := a.GetSinglePost(postID) + if err != nil { + return nil, err + } + + unreadMentions := 0 // TODO: calculate this value, setting it to zero for now. + return a.Srv.Store.Channel().UpdateLastViewedAtPost(post, userID, unreadMentions) +} + func (a *App) esAutocompleteChannels(teamId, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) { channelIds, err := a.Elasticsearch.SearchChannels(teamId, term) if err != nil { diff --git a/app/channel_test.go b/app/channel_test.go index 99eed39727..9f1f27600a 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -1041,3 +1041,103 @@ func TestSearchChannelsForUser(t *testing.T) { searchAndCheck(t, "dev", []string{"test-dev-1", "test-dev-2", "dev-3"}) }) } + +func TestMarkChannelAsUnreadFromPost(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + u1 := th.BasicUser + u2 := th.BasicUser2 + c1 := th.BasicChannel + pc1 := th.CreatePrivateChannel(th.BasicTeam) + th.AddUserToChannel(u2, c1) + th.AddUserToChannel(u1, pc1) + th.AddUserToChannel(u2, pc1) + + p1 := th.CreatePost(c1) + p2 := th.CreatePost(c1) + p3 := th.CreatePost(c1) + + pp1 := th.CreatePost(pc1) + require.NotNil(t, pp1) + pp2 := th.CreatePost(pc1) + + unread, err := th.App.GetChannelUnread(c1.Id, u1.Id) + require.Nil(t, err) + require.Equal(t, int64(4), unread.MsgCount) + unread, err = th.App.GetChannelUnread(c1.Id, u2.Id) + require.Nil(t, err) + require.Equal(t, int64(4), unread.MsgCount) + err = th.App.UpdateChannelLastViewedAt([]string{c1.Id, pc1.Id}, u1.Id) + require.Nil(t, err) + err = th.App.UpdateChannelLastViewedAt([]string{c1.Id, pc1.Id}, u2.Id) + require.Nil(t, err) + unread, err = th.App.GetChannelUnread(c1.Id, u2.Id) + require.Nil(t, err) + require.Equal(t, int64(0), unread.MsgCount) + + t.Run("Unread but last one", func(t *testing.T) { + response, err := th.App.MarkChannelAsUnreadFromPost(p2.Id, u1.Id) + require.Nil(t, err) + require.NotNil(t, response) + assert.Equal(t, int64(3), response.MsgCount) + unread, err := th.App.GetChannelUnread(c1.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(1), unread.MsgCount) + assert.Equal(t, p2.CreateAt, response.LastViewedAt) + }) + + t.Run("Unread last one", func(t *testing.T) { + response, err := th.App.MarkChannelAsUnreadFromPost(p3.Id, u1.Id) + require.Nil(t, err) + require.NotNil(t, response) + assert.Equal(t, int64(4), response.MsgCount) + unread, err := th.App.GetChannelUnread(c1.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(0), unread.MsgCount) + assert.Equal(t, p3.CreateAt, response.LastViewedAt) + }) + + t.Run("Unread first one", func(t *testing.T) { + response, err := th.App.MarkChannelAsUnreadFromPost(p1.Id, u1.Id) + require.Nil(t, err) + require.NotNil(t, response) + assert.Equal(t, int64(2), response.MsgCount) + unread, err := th.App.GetChannelUnread(c1.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(2), unread.MsgCount) + assert.Equal(t, p1.CreateAt, response.LastViewedAt) + }) + + t.Run("Other users are unaffected", func(t *testing.T) { + unread, err := th.App.GetChannelUnread(c1.Id, u2.Id) + require.Nil(t, err) + assert.Equal(t, int64(0), unread.MsgCount) + + }) + + t.Run("Unread on a private channel", func(t *testing.T) { + response, err := th.App.MarkChannelAsUnreadFromPost(pp1.Id, u1.Id) + require.Nil(t, err) + require.NotNil(t, response) + assert.Equal(t, int64(1), response.MsgCount) + unread, err := th.App.GetChannelUnread(pc1.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(1), unread.MsgCount) + assert.Equal(t, pp1.CreateAt, response.LastViewedAt) + + response, err = th.App.MarkChannelAsUnreadFromPost(pp2.Id, u1.Id) + assert.Nil(t, err) + assert.Equal(t, int64(2), response.MsgCount) + unread, err = th.App.GetChannelUnread(pc1.Id, u1.Id) + require.Nil(t, err) + assert.Equal(t, int64(0), unread.MsgCount) + assert.Equal(t, pp2.CreateAt, response.LastViewedAt) + }) + + t.Run("Can't unread an imaginary post", func(t *testing.T) { + response, err := th.App.MarkChannelAsUnreadFromPost("invalid4ofngungryquinj976y", u1.Id) + assert.NotNil(t, err) + assert.Nil(t, response) + }) +} diff --git a/i18n/en.json b/i18n/en.json index b855420e6e..e695d7bd07 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5530,6 +5530,10 @@ "id": "store.sql_channel.clear_all_custom_role_assignments.update.app_error", "translation": "Failed to update the channel member" }, + { + "id": "store.sql_channel.count_posts_since.app_error", + "translation": "Unable to count messages since given date" + }, { "id": "store.sql_channel.delete.channel.app_error", "translation": "Unable to delete the channel" @@ -5862,6 +5866,10 @@ "id": "store.sql_channel.update_last_viewed_at.app_error", "translation": "Unable to update the last viewed at time" }, + { + "id": "store.sql_channel.update_last_viewed_at_post.app_error", + "translation": "Unable to mark channel as unread" + }, { "id": "store.sql_channel.update_member.app_error", "translation": "We encountered an error updating the channel member" diff --git a/model/channel_member.go b/model/channel_member.go index 6bfa9bf656..9a3a3f2e9c 100644 --- a/model/channel_member.go +++ b/model/channel_member.go @@ -31,6 +31,16 @@ type ChannelUnread struct { NotifyProps StringMap `json:"-"` } +type ChannelUnreadAt struct { + TeamId string `json:"team_id"` + UserId string `json:"user_id"` + ChannelId string `json:"channel_id"` + MsgCount int64 `json:"msg_count"` + MentionCount int64 `json:"mention_count"` + LastViewedAt int64 `json:"last_viewed_at"` + NotifyProps StringMap `json:"-"` +} + type ChannelMember struct { ChannelId string `json:"channel_id"` UserId string `json:"user_id"` @@ -67,6 +77,11 @@ func (o *ChannelUnread) ToJson() string { return string(b) } +func (o *ChannelUnreadAt) ToJson() string { + b, _ := json.Marshal(o) + return string(b) +} + func ChannelMembersFromJson(data io.Reader) *ChannelMembers { var o *ChannelMembers json.NewDecoder(data).Decode(&o) @@ -79,6 +94,12 @@ func ChannelUnreadFromJson(data io.Reader) *ChannelUnread { return o } +func ChannelUnreadAtFromJson(data io.Reader) *ChannelUnreadAt { + var o *ChannelUnreadAt + json.NewDecoder(data).Decode(&o) + return o +} + func (o *ChannelMember) ToJson() string { b, _ := json.Marshal(o) return string(b) diff --git a/model/client4.go b/model/client4.go index 60f15383cf..cdc70f26b8 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2480,6 +2480,16 @@ func (c *Client4) PatchPost(postId string, patch *PostPatch) (*Post, *Response) return PostFromJson(r.Body), BuildResponse(r) } +// SetPostUnread marks channel where post belongs as unread on the time of the provided post. +func (c *Client4) SetPostUnread(userId string, postId string) *Response { + r, err := c.DoApiPost(c.GetUserRoute(userId)+c.GetPostRoute(postId)+"/set_unread", "") + if err != nil { + return BuildErrorResponse(r, err) + } + defer closeBody(r) + return BuildResponse(r) +} + // PinPost pin a post based on provided post id string. func (c *Client4) PinPost(postId string) (bool, *Response) { r, err := c.DoApiPost(c.GetPostRoute(postId)+"/pin", "") diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 5ff8af8513..749891d081 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1814,6 +1814,90 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string) return times, nil } +// CountPostsSince gives the number of posts in a channel created since a given date. +func (s SqlChannelStore) CountPostsSince(channelID string, since int64) (int64, *model.AppError) { + countUnreadQuery := ` + SELECT count(*) + FROM Posts + WHERE + ChannelId = :channelId + AND CreateAt > :createAt + AND Type = '' + AND DeleteAt = 0 + ` + countParams := map[string]interface{}{ + "channelId": channelID, + "createAt": since, + } + + unread, err := s.GetReplica().SelectInt(countUnreadQuery, countParams) + if err != nil { + return 0, model.NewAppError("SqlChannelStore.CountPostsSince", "store.sql_channel.count_posts_since.app_error", countParams, fmt.Sprintf("channel_id=%s, since=%d, err=%s", channelID, since, err), http.StatusInternalServerError) + } + return unread, nil +} + +// UpdateLastViewedAtPost sets a channel as unread for a user at the time of the post selected and update the MentionCount +// it returns a channelunread so redux can update the apps easily. +func (s SqlChannelStore) UpdateLastViewedAtPost(unreadPost *model.Post, userID string, mentionCount int) (*model.ChannelUnreadAt, *model.AppError) { + + unread, appErr := s.CountPostsSince(unreadPost.ChannelId, unreadPost.CreateAt) + if appErr != nil { + return nil, appErr + } + + params := map[string]interface{}{ + "mentions": mentionCount, + "unreadCount": unread, + "lastViewedAt": unreadPost.CreateAt, + "userId": userID, + "channelId": unreadPost.ChannelId, + "updatedAt": model.GetMillis(), + } + + // msg count uses the value from channels to prevent counting on older channels where no. of messages can be high. + // we only count the unread which will be a lot less in 99% cases + setUnreadQuery := ` + UPDATE + ChannelMembers + SET + MentionCount = :mentions, + MsgCount = (SELECT TotalMsgCount FROM Channels WHERE ID = :channelId) - :unreadCount, + LastViewedAt = :lastViewedAt, + LastUpdateAt = :updatedAt + WHERE + UserId = :userId + AND ChannelId = :channelId + ` + _, err := s.GetMaster().Exec(setUnreadQuery, params) + if err != nil { + return nil, model.NewAppError("SqlChannelStore.UpdateLastViewedAtPost", "store.sql_channel.update_last_viewed_at_post.app_error", params, "Error setting channel "+unreadPost.ChannelId+" as unread: "+err.Error(), http.StatusInternalServerError) + } + + chanUnreadQuery := ` + SELECT + c.TeamId TeamId, + cm.UserId UserId, + cm.ChannelId ChannelId, + cm.MsgCount MsgCount, + cm.MentionCount MentionCount, + cm.LastViewedAt LastViewedAt, + cm.NotifyProps NotifyProps + FROM + ChannelMembers cm + LEFT JOIN Channels c ON c.Id=cm.ChannelId + WHERE + cm.UserId = :userId + AND cm.channelId = :channelId + AND c.DeleteAt = 0 + ` + result := &model.ChannelUnreadAt{} + if err = s.GetMaster().SelectOne(result, chanUnreadQuery, params); err != nil { + return nil, model.NewAppError("SqlChannelStore.UpdateLastViewedAtPost", "store.sql_channel.update_last_viewed_at_post.app_error", params, "Error retrieving unread status from channel "+unreadPost.ChannelId+", error was: "+err.Error(), http.StatusInternalServerError) + } + return result, nil +} + func (s SqlChannelStore) IncrementMentionCount(channelId string, userId string) *model.AppError { _, err := s.GetMaster().Exec( `UPDATE diff --git a/store/store.go b/store/store.go index c0d1cabdf5..02d38e0f16 100644 --- a/store/store.go +++ b/store/store.go @@ -155,6 +155,7 @@ type ChannelStore interface { PermanentDeleteMembersByUser(userId string) *model.AppError PermanentDeleteMembersByChannel(channelId string) *model.AppError UpdateLastViewedAt(channelIds []string, userId string) (map[string]int64, *model.AppError) + UpdateLastViewedAtPost(unreadPost *model.Post, userID string, mentionCount int) (*model.ChannelUnreadAt, *model.AppError) IncrementMentionCount(channelId string, userId string) *model.AppError AnalyticsTypeCount(teamId string, channelType string) (int64, *model.AppError) GetMembersForUser(teamId string, userId string) (*model.ChannelMembers, *model.AppError) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 9c78f72efd..54c870ac02 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -1586,6 +1586,31 @@ func (_m *ChannelStore) UpdateLastViewedAt(channelIds []string, userId string) ( return r0, r1 } +// UpdateLastViewedAtPost provides a mock function with given fields: unreadPost, userID, mentionCount +func (_m *ChannelStore) UpdateLastViewedAtPost(unreadPost *model.Post, userID string, mentionCount int) (*model.ChannelUnreadAt, *model.AppError) { + ret := _m.Called(unreadPost, userID, mentionCount) + + var r0 *model.ChannelUnreadAt + if rf, ok := ret.Get(0).(func(*model.Post, string, int) *model.ChannelUnreadAt); ok { + r0 = rf(unreadPost, userID, mentionCount) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.ChannelUnreadAt) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.Post, string, int) *model.AppError); ok { + r1 = rf(unreadPost, userID, mentionCount) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // UpdateMember provides a mock function with given fields: member func (_m *ChannelStore) UpdateMember(member *model.ChannelMember) (*model.ChannelMember, *model.AppError) { ret := _m.Called(member)