diff --git a/api4/post_test.go b/api4/post_test.go index f0366dd70f..2cc2c83e6a 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "net/url" "reflect" + "sort" "strconv" "strings" "testing" @@ -1716,7 +1717,7 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { post2 := th.CreatePost() post3 := th.CreatePost() post4 := th.CreatePost() - th.CreatePost() // post5 + post5 := th.CreatePost() replyPost := &model.Post{ChannelId: channelId, Message: model.NewId(), RootId: post4.Id, ParentId: post4.Id} post6, resp := Client.CreatePost(replyPost) CheckNoError(t, resp) @@ -1729,13 +1730,60 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { post10, resp := Client.CreatePost(replyPost) CheckNoError(t, resp) + postIdNames := map[string]string{ + post1.Id: "post1", + post2.Id: "post2", + post3.Id: "post3", + post4.Id: "post4", + post5.Id: "post5", + post6.Id: "post6 (reply to post4)", + post7.Id: "post7 (reply to post4)", + post8.Id: "post8 (reply to post4)", + post9.Id: "post9 (reply to post4)", + post10.Id: "post10 (reply to post4)", + } + + namePost := func(postId string) string { + name, ok := postIdNames[postId] + if ok { + return name + } + + return fmt.Sprintf("unknown (%s)", postId) + } + + namePosts := func(postIds []string) []string { + namedPostIds := make([]string, 0, len(postIds)) + for _, postId := range postIds { + namedPostIds = append(namedPostIds, namePost(postId)) + } + + return namedPostIds + } + + namePostsMap := func(posts map[string]*model.Post) []string { + namedPostIds := make([]string, 0, len(posts)) + for postId := range posts { + namedPostIds = append(namedPostIds, namePost(postId)) + } + sort.Strings(namedPostIds) + + return namedPostIds + } + + assertPostList := func(t *testing.T, expected, actual *model.PostList) { + t.Helper() + + require.Equal(t, namePosts(expected.Order), namePosts(actual.Order), "unexpected post order") + require.Equal(t, namePostsMap(expected.Posts), namePostsMap(actual.Posts), "unexpected posts") + require.Equal(t, namePost(expected.NextPostId), namePost(actual.NextPostId), "unexpected next post id") + require.Equal(t, namePost(expected.PrevPostId), namePost(actual.PrevPostId), "unexpected prev post id") + } + // All returned posts are all read by the user, since it's created by the user itself. posts, resp := Client.GetPostsAroundLastUnread(userId, channelId, 20, 20) CheckNoError(t, resp) - - if len(posts.Order) != 12 { - t.Fatal("Should return 12 posts only since there's no unread post") - } + require.Len(t, posts.Order, 12, "Should return 12 posts only since there's no unread post") // Set channel member's last viewed to 0. // All returned posts are latest posts as if all previous posts were already read by the user. @@ -1749,14 +1797,15 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 20, 20) CheckNoError(t, resp) - if len(posts.Order) != 12 { - t.Fatal("Should return 12 posts only since there's no unread post") - } + require.Len(t, posts.Order, 12, "Should return 12 posts only since there's no unread post") // get the first system post generated before the created posts above posts, resp = Client.GetPostsBefore(th.BasicChannel.Id, post1.Id, 0, 2, "") CheckNoError(t, resp) - systemPostId1 := posts.Order[1] + systemPost0 := posts.Posts[posts.Order[0]] + postIdNames[systemPost0.Id] = "system post 0" + systemPost1 := posts.Posts[posts.Order[1]] + postIdNames[systemPost1.Id] = "system post 1" // Set channel member's last viewed before post1. channelMember, err = th.App.Srv.Store.Channel().GetMember(channelId, userId) @@ -1769,15 +1818,18 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 3, 3) CheckNoError(t, resp) - if len(posts.Order) != 5 || posts.Order[0] != post3.Id || posts.Order[4] != systemPostId1 { - t.Fatal("Should return 5 posts and match order") - } - if posts.NextPostId != post4.Id { - t.Fatal("should return post4.Id as NextPostId") - } - if posts.PrevPostId != "" { - t.Fatal("should return an empty PrevPostId") - } + assertPostList(t, &model.PostList{ + Order: []string{post3.Id, post2.Id, post1.Id, systemPost0.Id, systemPost1.Id}, + Posts: map[string]*model.Post{ + systemPost0.Id: systemPost0, + systemPost1.Id: systemPost1, + post1.Id: post1, + post2.Id: post2, + post3.Id: post3, + }, + NextPostId: post4.Id, + PrevPostId: "", + }, posts) // Set channel member's last viewed before post6. channelMember, err = th.App.Srv.Store.Channel().GetMember(channelId, userId) @@ -1790,15 +1842,21 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 3, 3) CheckNoError(t, resp) - if len(posts.Order) != 6 || posts.Order[0] != post8.Id || posts.Order[5] != post3.Id { - t.Fatal("Should return 6 posts and match order") - } - if posts.NextPostId != post9.Id { - t.Fatal("should return post8.Id as NextPostId") - } - if posts.PrevPostId != post2.Id { - t.Fatal("should return post2.Id as PrevPostId") - } + assertPostList(t, &model.PostList{ + Order: []string{post8.Id, post7.Id, post6.Id, post5.Id, post4.Id, post3.Id}, + Posts: map[string]*model.Post{ + post3.Id: post3, + post4.Id: post4, + post5.Id: post5, + post6.Id: post6, + post7.Id: post7, + post8.Id: post8, + post9.Id: post9, + post10.Id: post10, + }, + NextPostId: post9.Id, + PrevPostId: post2.Id, + }, posts) // Set channel member's last viewed before post10. channelMember, err = th.App.Srv.Store.Channel().GetMember(channelId, userId) @@ -1811,15 +1869,19 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 3, 3) CheckNoError(t, resp) - if len(posts.Order) != 4 || posts.Order[0] != post10.Id || posts.Order[3] != post7.Id { - t.Fatal("Should return 4 posts and match order") - } - if posts.NextPostId != "" { - t.Fatal("should return an empty NextPostId") - } - if posts.PrevPostId != post6.Id { - t.Fatal("should return post6.Id as PrevPostId") - } + assertPostList(t, &model.PostList{ + Order: []string{post10.Id, post9.Id, post8.Id, post7.Id}, + Posts: map[string]*model.Post{ + post4.Id: post4, + post6.Id: post6, + post7.Id: post7, + post8.Id: post8, + post9.Id: post9, + post10.Id: post10, + }, + NextPostId: "", + PrevPostId: post6.Id, + }, posts) // Set channel member's last viewed equal to post10. channelMember, err = th.App.Srv.Store.Channel().GetMember(channelId, userId) @@ -1832,15 +1894,62 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) { posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 3, 3) CheckNoError(t, resp) - if len(posts.Order) != 3 || posts.Order[0] != post10.Id || posts.Order[2] != post8.Id { - t.Fatal("Should return 3 posts and match order") - } - if posts.NextPostId != "" { - t.Fatal("should return an empty NextPostId") - } - if posts.PrevPostId != post7.Id { - t.Fatal("should return post7.Id as PrevPostId") - } + assertPostList(t, &model.PostList{ + Order: []string{post10.Id, post9.Id, post8.Id}, + Posts: map[string]*model.Post{ + post4.Id: post4, + post6.Id: post6, + post7.Id: post7, + post8.Id: post8, + post9.Id: post9, + post10.Id: post10, + }, + NextPostId: "", + PrevPostId: post7.Id, + }, posts) + + // Set channel member's last viewed to just before a new reply to a previous thread, not + // otherwise in the requested window. + post11 := th.CreatePost() + post12, resp := Client.CreatePost(&model.Post{ + ChannelId: channelId, + Message: model.NewId(), + RootId: post4.Id, + ParentId: post4.Id, + }) + CheckNoError(t, resp) + post13 := th.CreatePost() + + postIdNames[post11.Id] = "post11" + postIdNames[post12.Id] = "post12 (reply to post4)" + postIdNames[post13.Id] = "post13" + + channelMember, err = th.App.Srv.Store.Channel().GetMember(channelId, userId) + require.Nil(t, err) + channelMember.LastViewedAt = post12.CreateAt - 1 + _, err = th.App.Srv.Store.Channel().UpdateMember(channelMember) + require.Nil(t, err) + th.App.Srv.Store.Post().InvalidateLastPostTimeCache(channelId) + + posts, resp = Client.GetPostsAroundLastUnread(userId, channelId, 1, 2) + CheckNoError(t, resp) + + assertPostList(t, &model.PostList{ + Order: []string{post13.Id, post12.Id, post11.Id}, + Posts: map[string]*model.Post{ + post4.Id: post4, + post6.Id: post6, + post7.Id: post7, + post8.Id: post8, + post9.Id: post9, + post10.Id: post10, + post11.Id: post11, + post12.Id: post12, + post13.Id: post13, + }, + NextPostId: "", + PrevPostId: post10.Id, + }, posts) } func TestGetPost(t *testing.T) { diff --git a/app/post.go b/app/post.go index d83f4e1670..8d5f20ab89 100644 --- a/app/post.go +++ b/app/post.go @@ -771,27 +771,33 @@ func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limit return model.NewPostList(), nil } - lastUnreadPost, err := a.GetPostAfterTime(channelId, member.LastViewedAt) + lastUnreadPostId, err := a.GetPostIdAfterTime(channelId, member.LastViewedAt) if err != nil { return nil, err - } else if lastUnreadPost == nil { + } else if lastUnreadPostId == "" { return model.NewPostList(), nil } - var postList *model.PostList - if postList, err = a.GetPostsBeforePost(channelId, lastUnreadPost.Id, PAGE_DEFAULT, limitBefore); err != nil { + postList, err := a.GetPostThread(lastUnreadPostId) + if err != nil { return nil, err } + // Reset order to only include the last unread post: if the thread appears in the centre + // channel organically, those replies will be added below. + postList.Order = []string{lastUnreadPostId} - if postListAfter, err := a.GetPostsAfterPost(channelId, lastUnreadPost.Id, PAGE_DEFAULT, limitAfter-1); err != nil { + if postListBefore, err := a.GetPostsBeforePost(channelId, lastUnreadPostId, PAGE_DEFAULT, limitBefore); err != nil { + return nil, err + } else if postListBefore != nil { + postList.Extend(postListBefore) + } + + if postListAfter, err := a.GetPostsAfterPost(channelId, lastUnreadPostId, PAGE_DEFAULT, limitAfter-1); err != nil { return nil, err } else if postListAfter != nil { postList.Extend(postListAfter) } - postList.AddPost(lastUnreadPost) - postList.AddOrder(lastUnreadPost.Id) - postList.SortByCreateAt() return postList, nil }