From 34b4bbcb4645e97e0d8b27104b9d444f3c41599d Mon Sep 17 00:00:00 2001 From: Eli Yukelzon Date: Thu, 10 Oct 2019 17:14:44 +0300 Subject: [PATCH] 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 | 7 +++++-- app/auto_posts.go | 4 ++-- app/plugin_api.go | 2 +- app/post.go | 10 +++++----- store/sqlstore/post_store.go | 34 ++++++++++++++++++++++++---------- store/storetest/post_store.go | 3 --- 6 files changed, 37 insertions(+), 23 deletions(-) diff --git a/api4/post.go b/api4/post.go index 2cb20ec040..1441a1dd9f 100644 --- a/api4/post.go +++ b/api4/post.go @@ -384,8 +384,11 @@ func getPostThread(c *Context, w http.ResponseWriter, r *http.Request) { if c.Err != nil { return } - - list, err := c.App.GetPostThread(c.Params.PostId) + skipFetchThreads := false + 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 diff --git a/app/auto_posts.go b/app/auto_posts.go index b8837b4c9f..bded3f202e 100644 --- a/app/auto_posts.go +++ b/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) - if err2 != nil { + rpost, resp := cfg.client.CreatePost(post) + if resp != nil && resp.Error != nil { return nil, false } return rpost, true diff --git a/app/plugin_api.go b/app/plugin_api.go index 867e2f587d..04da1e7ce1 100644 --- a/app/plugin_api.go +++ b/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) { diff --git a/app/post.go b/app/post.go index d6f5e1d6a8..f05ffce078 100644 --- a/app/post.go +++ b/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) { - return a.Srv.Store.Post().Get(postId, false) +func (a *App) GetPostThread(postId string, skipFetchThreads bool) (*model.PostList, *model.AppError) { + 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 } diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 4c8698cdbd..1f77730da4 100644 --- a/store/sqlstore/post_store.go +++ b/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{ - sq.Eq{"RootId": rootIds}, - sq.Eq{"Id": rootIds}, - }, + idQuery, 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 diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index ad18de2f53..21162536e6 100644 --- a/store/storetest/post_store.go +++ b/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)