diff --git a/api4/user_test.go b/api4/user_test.go index 1eabb4f700..fec22f447c 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -5549,6 +5549,36 @@ func TestFollowThreads(t *testing.T) { }) } +func checkThreadListReplies(t *testing.T, th *TestHelper, client *model.Client4, userId string, expectedReplies, expectedThreads int, options *model.GetUserThreadsOpts) (*model.Threads, *model.Response) { + opts := model.GetUserThreadsOpts{} + if options != nil { + opts = *options + } + u, r := client.GetUserThreads(userId, th.BasicTeam.Id, opts) + CheckNoError(t, r) + require.Len(t, u.Threads, expectedThreads) + + count := int64(0) + sum := int64(0) + for _, thr := range u.Threads { + if thr.UnreadReplies > 0 { + count += 1 + } + sum += thr.UnreadReplies + } + require.EqualValues(t, expectedReplies, sum) + require.Equal(t, count, u.TotalUnreadThreads) + + return u, r +} + +func postAndCheck(t *testing.T, client *model.Client4, post *model.Post) (*model.Post, *model.Response) { + p, resp := client.CreatePost(post) + CheckNoError(t, resp) + CheckCreatedStatus(t, resp) + return p, resp +} + func TestMaintainUnreadRepliesInThread(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -5564,63 +5594,80 @@ func TestMaintainUnreadRepliesInThread(t *testing.T) { defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.SystemAdminUser.Id) // create a post by regular user - rpost, resp := Client.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"}) - CheckNoError(t, resp) - CheckCreatedStatus(t, resp) + rpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"}) // reply with another - _, resp2 := th.SystemAdminClient.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id}) - CheckNoError(t, resp2) - CheckCreatedStatus(t, resp2) + postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id}) - checkThreadList := func(client *model.Client4, userId string, expectedReplies, expectedThreads int) (*model.Threads, *model.Response) { - u, r := client.GetUserThreads(userId, th.BasicTeam.Id, model.GetUserThreadsOpts{ - Page: 0, - PageSize: 30, - Deleted: false, - }) - CheckNoError(t, r) - require.Len(t, u.Threads, expectedThreads) - require.EqualValues(t, expectedReplies, u.Threads[0].UnreadReplies) - - sum := int64(0) - for _, thr := range u.Threads { - sum += thr.UnreadReplies - } - require.Equal(t, sum, u.TotalUnreadReplies) - - return u, r - } // regular user should have one thread with one reply - checkThreadList(th.Client, th.BasicUser.Id, 1, 1) + checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 1, 1, nil) // add another reply by regular user - _, resp3 := Client.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply2", RootId: rpost.Id}) - CheckNoError(t, resp3) - CheckCreatedStatus(t, resp3) + postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply2", RootId: rpost.Id}) // replying to the thread clears reply count, so it should be 0 - checkThreadList(th.Client, th.BasicUser.Id, 0, 1) + checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 0, 1, nil) // the other user should have 2 replies - checkThreadList(th.SystemAdminClient, th.SystemAdminUser.Id, 2, 1) + checkThreadListReplies(t, th, th.SystemAdminClient, th.SystemAdminUser.Id, 2, 1, nil) // mark all as read for user - resp = th.Client.UpdateThreadsReadForUser(th.BasicUser.Id, th.BasicTeam.Id) + resp := th.Client.UpdateThreadsReadForUser(th.BasicUser.Id, th.BasicTeam.Id) CheckNoError(t, resp) CheckOKStatus(t, resp) // reply count should be 0 - checkThreadList(th.Client, th.BasicUser.Id, 0, 1) + checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 0, 1, nil) // the other user should also have 2 - checkThreadList(th.SystemAdminClient, th.SystemAdminUser.Id, 2, 1) + checkThreadListReplies(t, th, th.SystemAdminClient, th.SystemAdminUser.Id, 2, 1, nil) } -func postAndCheck(t *testing.T, client *model.Client4, post *model.Post) (*model.Post, *model.Response) { - p, resp := client.CreatePost(post) - CheckNoError(t, resp) - CheckCreatedStatus(t, resp) - return p, resp + +func TestThreadCounts(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") + defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.COLLAPSED_THREADS_DEFAULT_ON + }) + + Client := th.Client + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id) + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.SystemAdminUser.Id) + + // create a post by regular user + rpost, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"}) + // reply with another + time.Sleep(1) + postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id}) + + // create another post by regular user + time.Sleep(1) + rpost2, _ := postAndCheck(t, Client, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testMsg1"}) + // reply with another 2 times + time.Sleep(1) + postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testReply2", RootId: rpost2.Id}) + postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testReply22", RootId: rpost2.Id}) + + // regular user should have two threads with 3 replies total + checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 3, 2, &model.GetUserThreadsOpts{ + Deleted: false, + }) + + // delete first thread + th.App.Srv().Store.Post().Delete(rpost.Id, model.GetMillis(), th.BasicUser.Id) + + // we should now have 1 thread with 2 replies + checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 2, 1, &model.GetUserThreadsOpts{ + Deleted: false, + }) + // with Deleted we should get the same as before deleting, minus the reply in the deleted thread + checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 3, 2, &model.GetUserThreadsOpts{ + Deleted: true, + }) } + func TestMaintainUnreadMentionsInThread(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/model/thread.go b/model/thread.go index 794a86b7ef..02c50b9a13 100644 --- a/model/thread.go +++ b/model/thread.go @@ -28,7 +28,7 @@ type ThreadResponse struct { type Threads struct { Total int64 `json:"total"` - TotalUnreadReplies int64 `json:"total_unread_replies"` + TotalUnreadThreads int64 `json:"total_unread_threads"` TotalUnreadMentions int64 `json:"total_unread_mentions"` Threads []*ThreadResponse `json:"threads"` } diff --git a/store/sqlstore/thread_store.go b/store/sqlstore/thread_store.go index e43a3bf8bb..a689331dac 100644 --- a/store/sqlstore/thread_store.go +++ b/store/sqlstore/thread_store.go @@ -120,19 +120,22 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get model.Post } - unreadRepliesQuery := "SELECT COUNT(Posts.Id) From Posts Where Posts.RootId=ThreadMemberships.PostId AND Posts.UpdateAt >= ThreadMemberships.LastViewed AND Posts.DeleteAt=0" + unreadRepliesQuery := "SELECT COUNT(Posts.Id) From Posts Where Posts.RootId=ThreadMemberships.PostId AND Posts.UpdateAt >= ThreadMemberships.LastViewed" fetchConditions := sq.And{ sq.Or{sq.Eq{"Channels.TeamId": teamId}, sq.Eq{"Channels.TeamId": ""}}, sq.Eq{"ThreadMemberships.UserId": userId}, sq.Eq{"ThreadMemberships.Following": true}, } + if !opts.Deleted { + fetchConditions = sq.And{fetchConditions, sq.Eq{"Posts.DeleteAt": 0}} + } pageSize := uint64(30) - if opts.PageSize == 0 { + if opts.PageSize != 0 { pageSize = opts.PageSize } - totalUnreadRepliesChan := make(chan store.StoreResult, 1) + totalUnreadThreadsChan := make(chan store.StoreResult, 1) totalCountChan := make(chan store.StoreResult, 1) totalUnreadMentionsChan := make(chan store.StoreResult, 1) threadsChan := make(chan store.StoreResult, 1) @@ -140,20 +143,21 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get repliesQuery, repliesQueryArgs, _ := s.getQueryBuilder(). Select("COUNT(Posts.Id)"). From("Posts"). - LeftJoin("ThreadMemberships ON Posts.RootId = ThreadMemberships.PostId"). + LeftJoin("ThreadMemberships ON Posts.Id = ThreadMemberships.PostId"). LeftJoin("Channels ON Posts.ChannelId = Channels.Id"). Where(fetchConditions). Where("Posts.UpdateAt >= ThreadMemberships.LastViewed").ToSql() - totalUnreadReplies, err := s.GetMaster().SelectInt(repliesQuery, repliesQueryArgs...) - totalUnreadRepliesChan <- store.StoreResult{Data: totalUnreadReplies, NErr: errors.Wrapf(err, "failed to get count replies on threads for user id=%s", userId)} - close(totalUnreadRepliesChan) + totalUnreadThreads, err := s.GetMaster().SelectInt(repliesQuery, repliesQueryArgs...) + totalUnreadThreadsChan <- store.StoreResult{Data: totalUnreadThreads, NErr: errors.Wrapf(err, "failed to get count unread on threads for user id=%s", userId)} + close(totalUnreadThreadsChan) }() go func() { threadsQuery, threadsQueryArgs, _ := s.getQueryBuilder(). Select("COUNT(ThreadMemberships.PostId)"). LeftJoin("Threads ON Threads.PostId = ThreadMemberships.PostId"). LeftJoin("Channels ON Threads.ChannelId = Channels.Id"). + LeftJoin("Posts ON Posts.Id = ThreadMemberships.PostId"). From("ThreadMemberships"). Where(fetchConditions).ToSql() @@ -166,6 +170,7 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get Select("COALESCE(SUM(ThreadMemberships.UnreadMentions),0)"). From("ThreadMemberships"). LeftJoin("Threads ON Threads.PostId = ThreadMemberships.PostId"). + LeftJoin("Posts ON Posts.Id = ThreadMemberships.PostId"). LeftJoin("Channels ON Threads.ChannelId = Channels.Id"). Where(fetchConditions).ToSql() totalUnreadMentions, err := s.GetMaster().SelectInt(mentionsQuery, mentionsQueryArgs...) @@ -174,9 +179,6 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get }() go func() { newFetchConditions := fetchConditions - if !opts.Deleted { - newFetchConditions = sq.And{fetchConditions, sq.Eq{"Posts.DeleteAt": 0}} - } if opts.Since > 0 { newFetchConditions = sq.And{newFetchConditions, sq.GtOrEq{"Threads.LastReplyAt": opts.Since}} } @@ -216,11 +218,11 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get } totalCount := totalCountResult.Data.(int64) - totalUnreadRepliesResult := <-totalUnreadRepliesChan - if totalUnreadRepliesResult.NErr != nil { - return nil, totalUnreadRepliesResult.NErr + totalUnreadThreadsResult := <-totalUnreadThreadsChan + if totalUnreadThreadsResult.NErr != nil { + return nil, totalUnreadThreadsResult.NErr } - totalUnreadReplies := totalUnreadRepliesResult.Data.(int64) + totalUnreadThreads := totalUnreadThreadsResult.Data.(int64) var userIds []string userIdMap := map[string]bool{} @@ -249,7 +251,7 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get Total: totalCount, Threads: nil, TotalUnreadMentions: totalUnreadMentions, - TotalUnreadReplies: totalUnreadReplies, + TotalUnreadThreads: totalUnreadThreads, } for _, thread := range threads {