MM-16949 Fix posts returned from unread API when the two different list contains parent post and comments (#11617)

* fix posts returned from unread API when the list contains parent post/s

* add ExtendAll to PostList and update test per feedback

* revert unintentional change to the other test and fix comment

* update the existing postlist.Extend, filter unique values and update unit tests

api4/post_test.go

@@ -1717,11 +1717,17 @@ func TestGetPostsForChannelAroundLastUnread(t *testing.T) {
 	post3 := th.CreatePost()
 	post4 := th.CreatePost()
 	th.CreatePost() // post5
-	post6 := th.CreatePost()
+	replyPost := &model.Post{ChannelId: channelId, Message: model.NewId(), RootId: post4.Id, ParentId: post4.Id}
-	post7 := th.CreatePost()
+	post6, resp := Client.CreatePost(replyPost)
-	post8 := th.CreatePost()
+	CheckNoError(t, resp)
-	post9 := th.CreatePost()
+	post7, resp := Client.CreatePost(replyPost)
-	post10 := th.CreatePost()
+	CheckNoError(t, resp)
+	post8, resp := Client.CreatePost(replyPost)
+	CheckNoError(t, resp)
+	post9, resp := Client.CreatePost(replyPost)
+	CheckNoError(t, resp)
+	post10, resp := Client.CreatePost(replyPost)
+	CheckNoError(t, resp)
 
 	// 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)

model/post_list.go

@@ -95,13 +95,29 @@ func (o *PostList) AddPost(post *Post) {
 	o.Posts[post.Id] = post
 }
 
-func (o *PostList) Extend(other *PostList) {
+func (o *PostList) UniqueOrder() {
-	for _, postId := range other.Order {
+	keys := make(map[string]bool)
-		if _, ok := o.Posts[postId]; !ok {
+	order := []string{}
-			o.AddPost(other.Posts[postId])
+	for _, postId := range o.Order {
-			o.AddOrder(postId)
+		if _, value := keys[postId]; !value {
+			keys[postId] = true
+			order = append(order, postId)
 		}
 	}
+
+	o.Order = order
+}
+
+func (o *PostList) Extend(other *PostList) {
+	for postId := range other.Posts {
+		o.AddPost(other.Posts[postId])
+	}
+
+	for _, postId := range other.Order {
+		o.AddOrder(postId)
+	}
+
+	o.UniqueOrder()
 }
 
 func (o *PostList) SortByCreateAt() {

model/post_list_test.go

@@ -38,37 +38,80 @@ func TestPostListJson(t *testing.T) {
 }
 
 func TestPostListExtend(t *testing.T) {
-	l1 := PostList{}
-
 	p1 := &Post{Id: NewId(), Message: NewId()}
+	p2 := &Post{Id: NewId(), Message: NewId()}
+	p3 := &Post{Id: NewId(), Message: NewId()}
+
+	l1 := PostList{}
 	l1.AddPost(p1)
 	l1.AddOrder(p1.Id)
-
-	p2 := &Post{Id: NewId(), Message: NewId()}
 	l1.AddPost(p2)
 	l1.AddOrder(p2.Id)
 
 	l2 := PostList{}
-
-	p3 := &Post{Id: NewId(), Message: NewId()}
 	l2.AddPost(p3)
 	l2.AddOrder(p3.Id)
 
 	l2.Extend(&l1)
 
-	if len(l1.Posts) != 2 || len(l1.Order) != 2 {
+	// should not changed l1
-		t.Fatal("extending l2 changed l1")
+	assert.Len(t, l1.Posts, 2)
-	} else if len(l2.Posts) != 3 {
+	assert.Len(t, l1.Order, 2)
-		t.Fatal("failed to extend posts l2")
+
-	} else if l2.Order[0] != p3.Id || l2.Order[1] != p1.Id || l2.Order[2] != p2.Id {
+	// should extend l2
-		t.Fatal("failed to extend order of l2")
+	assert.Len(t, l2.Posts, 3)
+	assert.Len(t, l2.Order, 3)
+
+	// should extend the Order of l2 correctly
+	assert.Equal(t, l2.Order[0], p3.Id)
+	assert.Equal(t, l2.Order[1], p1.Id)
+	assert.Equal(t, l2.Order[2], p2.Id)
+
+	// extend l2 again
+	l2.Extend(&l1)
+	// extending l2 again should not changed l1
+	assert.Len(t, l1.Posts, 2)
+	assert.Len(t, l1.Order, 2)
+
+	// extending l2 again should extend l2
+	assert.Len(t, l2.Posts, 3)
+	assert.Len(t, l2.Order, 3)
+
+	// p3 could be last unread
+	p4 := &Post{Id: NewId(), Message: NewId()}
+	p5 := &Post{Id: NewId(), RootId: p1.Id, Message: NewId()}
+	p6 := &Post{Id: NewId(), RootId: p1.Id, Message: NewId()}
+
+	// Create before and after post list where p3 could be last unread
+
+	// Order has 2 but Posts are 4 which includes additional 2 comments (p5 & p6) to parent post (p1)
+	beforePostList := PostList{
+		Order: []string{p1.Id, p2.Id},
+		Posts: map[string]*Post{p1.Id: p1, p2.Id: p2, p5.Id: p5, p6.Id: p6},
 	}
 
-	if len(l1.Posts) != 2 || len(l1.Order) != 2 {
+	// Order has 3 but Posts are 4 which includes 1 parent post (p1) of comments (p5 & p6)
-		t.Fatal("extending l2 again changed l1")
+	afterPostList := PostList{
-	} else if len(l2.Posts) != 3 || len(l2.Order) != 3 {
+		Order: []string{p4.Id, p5.Id, p6.Id},
-		t.Fatal("extending l2 again changed l2")
+		Posts: map[string]*Post{p1.Id: p1, p4.Id: p4, p5.Id: p5, p6.Id: p6},
 	}
+
+	beforePostList.Extend(&afterPostList)
+
+	// should not changed afterPostList
+	assert.Len(t, afterPostList.Posts, 4)
+	assert.Len(t, afterPostList.Order, 3)
+
+	// should extend beforePostList
+	assert.Len(t, beforePostList.Posts, 5)
+	assert.Len(t, beforePostList.Order, 5)
+
+	// should extend the Order of beforePostList correctly
+	assert.Equal(t, beforePostList.Order[0], p1.Id)
+	assert.Equal(t, beforePostList.Order[1], p2.Id)
+	assert.Equal(t, beforePostList.Order[2], p4.Id)
+	assert.Equal(t, beforePostList.Order[3], p5.Id)
+	assert.Equal(t, beforePostList.Order[4], p6.Id)
 }
 
 func TestPostListSortByCreateAt(t *testing.T) {