MM-18623 - invalid reply-count displayed (#12364)

* fixed thread creator, fixed default behaviour of fetching posts
* handle reply-count in getPostThread code path

api4/post.go

@@ -384,8 +384,11 @@ func getPostThread(c *Context, w http.ResponseWriter, r *http.Request) {
 	if c.Err != nil {
 		return
 	}
-
+	skipFetchThreads := false
-	list, err := c.App.GetPostThread(c.Params.PostId)
+	if r.URL.Query().Get("fetchThreads") == "false" {
+		skipFetchThreads = true
+	}
+	list, err := c.App.GetPostThread(c.Params.PostId, skipFetchThreads)
 	if err != nil {
 		c.Err = err
 		return

app/auto_posts.go

@@ -92,8 +92,8 @@ func (cfg *AutoPostCreator) CreateRandomPostNested(parentId, rootId string) (*mo
 		RootId:    rootId,
 		Message:   postText,
 		FileIds:   fileIds}
-	rpost, err2 := cfg.client.CreatePost(post)
+	rpost, resp := cfg.client.CreatePost(post)
-	if err2 != nil {
+	if resp != nil && resp.Error != nil {
 		return nil, false
 	}
 	return rpost, true

app/plugin_api.go

@@ -466,7 +466,7 @@ func (api *PluginAPI) DeletePost(postId string) *model.AppError {
 }
 
 func (api *PluginAPI) GetPostThread(postId string) (*model.PostList, *model.AppError) {
-	return api.app.GetPostThread(postId)
+	return api.app.GetPostThread(postId, false)
 }
 
 func (api *PluginAPI) GetPost(postId string) (*model.Post, *model.AppError) {

app/post.go

@@ -167,7 +167,7 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo
 	if len(post.RootId) > 0 {
 		pchan = make(chan store.StoreResult, 1)
 		go func() {
-			r, pErr := a.Srv.Store.Post().Get(post.RootId, true)
+			r, pErr := a.Srv.Store.Post().Get(post.RootId, false)
 			pchan <- store.StoreResult{Data: r, Err: pErr}
 			close(pchan)
 		}()
@@ -475,7 +475,7 @@ func (a *App) DeleteEphemeralPost(userId, postId string) {
 func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) {
 	post.SanitizeProps()
 
-	postLists, err := a.Srv.Store.Post().Get(post.Id, true)
+	postLists, err := a.Srv.Store.Post().Get(post.Id, false)
 	if err != nil {
 		return nil, err
 	}
@@ -634,8 +634,8 @@ func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) {
 	return a.Srv.Store.Post().GetSingle(postId)
 }
 
-func (a *App) GetPostThread(postId string) (*model.PostList, *model.AppError) {
+func (a *App) GetPostThread(postId string, skipFetchThreads bool) (*model.PostList, *model.AppError) {
-	return a.Srv.Store.Post().Get(postId, false)
+	return a.Srv.Store.Post().Get(postId, skipFetchThreads)
 }
 
 func (a *App) GetFlaggedPosts(userId string, offset int, limit int) (*model.PostList, *model.AppError) {
@@ -789,7 +789,7 @@ func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limit
 		return model.NewPostList(), nil
 	}
 
-	postList, err := a.GetPostThread(lastUnreadPostId)
+	postList, err := a.GetPostThread(lastUnreadPostId, false)
 	if err != nil {
 		return nil, err
 	}

store/sqlstore/post_store.go

@@ -282,7 +282,7 @@ func (s *SqlPostStore) Get(id string, skipFetchThreads bool) (*model.PostList, *
 	var post model.Post
 	var postFetchQuery string
 	if skipFetchThreads {
-		postFetchQuery = "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.RootId)  FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0"
+		postFetchQuery = "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id) as ReplyCount  FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0"
 	} else {
 		postFetchQuery = "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0"
 	}
@@ -541,20 +541,28 @@ func (s *SqlPostStore) GetPostsSince(options model.GetPostsSinceOptions, allowFr
 	}
 
 	var posts []*model.Post
+
+	replyCountQuery1 := ""
+	replyCountQuery2 := ""
+	if options.SkipFetchThreads {
+		replyCountQuery1 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p1.RootId = '' AND Posts.RootId = p1.Id) as ReplyCount`
+		replyCountQuery2 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p2.RootId = '' AND Posts.RootId = p2.Id) as ReplyCount`
+	}
+
 	_, err := s.GetReplica().Select(&posts,
 		`(SELECT
-			*
+			*`+replyCountQuery1+`
 		FROM
-			Posts
+			Posts p1
 		WHERE
 			(UpdateAt > :Time
 				AND ChannelId = :ChannelId)
 			LIMIT 1000)
 		UNION
 			(SELECT
-			    *
+			    *`+replyCountQuery2+`
 			FROM
-			    Posts
+			    Posts p2
 			WHERE
 			    Id
 			IN
@@ -647,15 +655,18 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions
 			}
 		}
 		rootQuery := s.getQueryBuilder().Select("p.*")
+		idQuery := sq.Or{
+			sq.Eq{"Id": rootIds},
+		}
 		if options.SkipFetchThreads {
 			rootQuery = rootQuery.Column(sq.Alias(replyCountSubQuery, "ReplyCount"))
+		} else {
+			idQuery = append(idQuery, sq.Eq{"RootId": rootIds}) // preserve original behaviour
 		}
+
 		rootQuery = rootQuery.From("Posts p").
 			Where(sq.And{
-				sq.Or{
+				idQuery,
-					sq.Eq{"RootId": rootIds},
-					sq.Eq{"Id": rootIds},
-				},
 				sq.Eq{"ChannelId": options.ChannelId},
 				sq.Eq{"DeleteAt": 0},
 			}).
@@ -785,8 +796,11 @@ func (s *SqlPostStore) getRootPosts(channelId string, offset int, limit int, ski
 func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int, skipFetchThreads bool) ([]*model.Post, *model.AppError) {
 	var posts []*model.Post
 	replyCountQuery := ""
+	onStatement := "q1.RootId = q2.Id"
 	if skipFetchThreads {
 		replyCountQuery = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id) as ReplyCount`
+	} else {
+		onStatement += " OR q1.RootId = q2.RootId"
 	}
 	_, err := s.GetReplica().Select(&posts,
 		`SELECT q2.*`+replyCountQuery+`
@@ -806,7 +820,7 @@ func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int,
 				ORDER BY CreateAt DESC
 				LIMIT :Limit OFFSET :Offset) q3
 			WHERE q3.RootId != '') q1
-			ON q1.RootId = q2.Id OR q1.RootId = q2.RootId
+			ON `+onStatement+`
 		WHERE
 			ChannelId = :ChannelId2
 				AND DeleteAt = 0

store/storetest/post_store.go

@@ -1008,8 +1008,6 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 				post1.Id: post1,
 				post2.Id: post2,
 				post3.Id: post3,
-				post4.Id: post4,
-				post6.Id: post6,
 			}, postList.Posts)
 		})
 
@@ -1031,7 +1029,6 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) {
 			assert.Equal(t, []string{post6.Id, post5.Id}, postList.Order)
 			assert.Equal(t, map[string]*model.Post{
 				post2.Id: post2,
-				post4.Id: post4,
 				post5.Id: post5,
 				post6.Id: post6,
 			}, postList.Posts)