Fixing reply count on new posts (#14312)

* Fixing reply count on new posts

* Fixing tests

* Fixing post reply count on getPostsAround

Co-authored-by: mattermod <mattermod@users.noreply.github.com>

store/sqlstore/post_store.go

@@ -204,13 +204,13 @@ func (s *SqlPostStore) Save(post *model.Post) (*model.Post, *model.AppError) {
 func (s *SqlPostStore) populateReplyCount(posts []*model.Post) *model.AppError {
 	rootIds := []string{}
 	for _, post := range posts {
-		rootIds = append(rootIds, post.Id)
+		rootIds = append(rootIds, post.RootId)
 	}
 	countList := []struct {
 		RootId string
 		Count  int64
 	}{}
-	query := s.getQueryBuilder().Select("RootId, COUNT(Id)").From("Posts").Where(sq.Eq{"RootId": rootIds}).Where(sq.Eq{"DeleteAt": 0}).GroupBy("RootId")
+	query := s.getQueryBuilder().Select("RootId, COUNT(Id) AS Count").From("Posts").Where(sq.Eq{"RootId": rootIds}).Where(sq.Eq{"DeleteAt": 0}).GroupBy("RootId")
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
@@ -703,11 +703,9 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 		direction = ">"
 		sort = "ASC"
 	}
-	replyCountSubQuery := s.getQueryBuilder().Select("COUNT(Posts.Id)").From("Posts").Where(sq.Expr("p.RootId = '' AND RootId = p.Id AND DeleteAt = 0"))
+	replyCountSubQuery := s.getQueryBuilder().Select("COUNT(Posts.Id)").From("Posts").Where(sq.Expr("Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0"))
 	query := s.getQueryBuilder().Select("p.*")
-	if options.SkipFetchThreads {
 	query = query.Column(sq.Alias(replyCountSubQuery, "ReplyCount"))
-	}
 	query = query.From("Posts p").
 		Where(sq.And{
 			sq.Expr(`CreateAt `+direction+` (SELECT CreateAt FROM Posts WHERE Id = ?)`, options.PostId),
@@ -742,9 +740,8 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 		idQuery := sq.Or{
 			sq.Eq{"Id": rootIds},
 		}
-		if options.SkipFetchThreads {
 		rootQuery = rootQuery.Column(sq.Alias(replyCountSubQuery, "ReplyCount"))
-		} else {
+		if !options.SkipFetchThreads {
 			idQuery = append(idQuery, sq.Eq{"RootId": rootIds}) // preserve original behaviour
 		}
 

store/storetest/post_store.go

@@ -62,8 +62,41 @@ func testPostStoreSave(t *testing.T, ss store.Store) {
 		o1.UserId = model.NewId()
 		o1.Message = "zz" + model.NewId() + "b"
 
-		_, err := ss.Post().Save(&o1)
+		p, err := ss.Post().Save(&o1)
 		require.Nil(t, err, "couldn't save item")
+		assert.Equal(t, int64(0), p.ReplyCount)
+	})
+
+	t.Run("Save replies", func(t *testing.T) {
+		o1 := model.Post{}
+		o1.ChannelId = model.NewId()
+		o1.UserId = model.NewId()
+		o1.RootId = model.NewId()
+		o1.Message = "zz" + model.NewId() + "b"
+
+		o2 := model.Post{}
+		o2.ChannelId = model.NewId()
+		o2.UserId = model.NewId()
+		o2.RootId = o1.RootId
+		o2.Message = "zz" + model.NewId() + "b"
+
+		o3 := model.Post{}
+		o3.ChannelId = model.NewId()
+		o3.UserId = model.NewId()
+		o3.RootId = model.NewId()
+		o3.Message = "zz" + model.NewId() + "b"
+
+		p1, err := ss.Post().Save(&o1)
+		require.Nil(t, err, "couldn't save item")
+		assert.Equal(t, int64(1), p1.ReplyCount)
+
+		p2, err := ss.Post().Save(&o2)
+		require.Nil(t, err, "couldn't save item")
+		assert.Equal(t, int64(2), p2.ReplyCount)
+
+		p3, err := ss.Post().Save(&o3)
+		require.Nil(t, err, "couldn't save item")
+		assert.Equal(t, int64(1), p3.ReplyCount)
 	})
 
 	t.Run("Try to save existing post", func(t *testing.T) {
@@ -196,6 +229,39 @@ func testPostStoreSaveMultiple(t *testing.T, ss store.Store) {
 		}
 	})
 
+	t.Run("Save replies", func(t *testing.T) {
+		o1 := model.Post{}
+		o1.ChannelId = model.NewId()
+		o1.UserId = model.NewId()
+		o1.RootId = model.NewId()
+		o1.Message = "zz" + model.NewId() + "b"
+
+		o2 := model.Post{}
+		o2.ChannelId = model.NewId()
+		o2.UserId = model.NewId()
+		o2.RootId = o1.RootId
+		o2.Message = "zz" + model.NewId() + "b"
+
+		o3 := model.Post{}
+		o3.ChannelId = model.NewId()
+		o3.UserId = model.NewId()
+		o3.RootId = model.NewId()
+		o3.Message = "zz" + model.NewId() + "b"
+
+		o4 := model.Post{}
+		o4.ChannelId = model.NewId()
+		o4.UserId = model.NewId()
+		o4.Message = "zz" + model.NewId() + "b"
+
+		newPosts, err := ss.Post().SaveMultiple([]*model.Post{&o1, &o2, &o3, &o4})
+		require.Nil(t, err, "couldn't save item")
+		assert.Len(t, newPosts, 4)
+		assert.Equal(t, int64(2), newPosts[0].ReplyCount)
+		assert.Equal(t, int64(2), newPosts[1].ReplyCount)
+		assert.Equal(t, int64(1), newPosts[2].ReplyCount)
+		assert.Equal(t, int64(0), newPosts[3].ReplyCount)
+	})
+
 	t.Run("Try to save mixed, already saved and not saved posts", func(t *testing.T) {
 		newPosts, err := ss.Post().SaveMultiple([]*model.Post{&p4, &p3})
 		require.NotNil(t, err)
@@ -906,6 +972,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			UserId:    userId,
 			Message:   "message",
 		})
+		post1.ReplyCount = 1
 		require.Nil(t, err)
 		time.Sleep(time.Millisecond)
 
@@ -915,6 +982,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			Message:   "message",
 		})
 		require.Nil(t, err)
+		post2.ReplyCount = 2
 		time.Sleep(time.Millisecond)
 
 		post3, err := ss.Post().Save(&model.Post{
@@ -925,6 +993,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			Message:   "message",
 		})
 		require.Nil(t, err)
+		post3.ReplyCount = 1
 		time.Sleep(time.Millisecond)
 
 		post4, err := ss.Post().Save(&model.Post{
@@ -935,6 +1004,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			Message:   "message",
 		})
 		require.Nil(t, err)
+		post4.ReplyCount = 2
 		time.Sleep(time.Millisecond)
 
 		post5, err := ss.Post().Save(&model.Post{
@@ -952,6 +1022,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			RootId:    post2.Id,
 			Message:   "message",
 		})
+		post6.ReplyCount = 2
 		require.Nil(t, err)
 
 		// Adding a post to a thread changes the UpdateAt timestamp of the parent post
@@ -1023,6 +1094,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			Message:   "post3",
 		})
 		require.Nil(t, err)
+		post3.ReplyCount = 1
 		time.Sleep(time.Millisecond)
 
 		post4, err := ss.Post().Save(&model.Post{
@@ -1033,6 +1105,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			Message:   "post4",
 		})
 		require.Nil(t, err)
+		post4.ReplyCount = 2
 		time.Sleep(time.Millisecond)
 
 		post5, err := ss.Post().Save(&model.Post{
@@ -1050,6 +1123,7 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			RootId:    post2.Id,
 			Message:   "post6",
 		})
+		post6.ReplyCount = 2
 		require.Nil(t, err)
 
 		// Adding a post to a thread changes the UpdateAt timestamp of the parent post