From bb05e7c412df9832e2f7028b35cf1065a4d0aa05 Mon Sep 17 00:00:00 2001 From: Kyriakos Z <3829551+koox00@users.noreply.github.com> Date: Tue, 31 Aug 2021 00:13:55 +0300 Subject: [PATCH] MM-35017: order and inclusion of participants (#18235) * MM-35017: order and inclusion of participants From now on root poster is not considered a thread participant, only repliers are participants. Ordering: A second reply to the thread from the same user should place the user last at the participants list. * Fixes shadow * Fixes tests * Order By CreateAt for participants of old threads * Removes unnecessary check on root id * Removes GetParticipantProfilesByIds it's not needed At first GetParticipantProfilesByIds was created because GetProfileByIds gets users by order of username. This was not needed, we can order by user ids in Go. This commit removes GetParticipantProfilesByIds and replaces it by GetProfileByIds as it was done before. * Fixes participants order for pre CRT threads Older threads (before CRT) are being converted to CRT threads upon reply. This commit considers the order of participants to be inserted to the new thread. * Adds tests for thread participants Co-authored-by: Mattermod --- store/sqlstore/post_store.go | 19 +++++++-- store/sqlstore/thread_store.go | 14 ++++++- store/storetest/post_store.go | 70 +++++++++++++++++++++++++++++++++ store/storetest/thread_store.go | 2 +- 4 files changed, 99 insertions(+), 6 deletions(-) diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index b500a8c6ff..7762f7d4fa 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -2402,11 +2402,21 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *gorp.Transaction, pos for rootId, posts := range postsByRoot { if thread, found := threadByRoot[rootId]; !found { + var data []struct { + UserId string `db:"userid"` + RepliedAt int64 `db:"mc"` + } + // calculate participants - var participants model.StringArray - if _, err := transaction.Select(&participants, "SELECT DISTINCT UserId FROM Posts WHERE RootId=:RootId OR Id=:RootId", map[string]interface{}{"RootId": rootId}); err != nil { + if _, err := transaction.Select(&data, "SELECT UserId, MAX(CreateAt) as mc FROM Posts WHERE RootId=:RootId AND DeleteAt=0 GROUP BY UserId ORDER BY mc ASC", map[string]interface{}{"RootId": rootId}); err != nil { return err } + + var participants model.StringArray + for _, item := range data { + participants = append(participants, item.UserId) + } + // calculate reply count count, err := transaction.SelectInt("SELECT COUNT(Id) FROM Posts WHERE RootId=:RootId And DeleteAt=0", map[string]interface{}{"RootId": rootId}) if err != nil { @@ -2431,9 +2441,10 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *gorp.Transaction, pos // metadata exists, update it for _, post := range posts { thread.ReplyCount += 1 - if !thread.Participants.Contains(post.UserId) { - thread.Participants = append(thread.Participants, post.UserId) + if thread.Participants.Contains(post.UserId) { + thread.Participants = thread.Participants.Remove(post.UserId) } + thread.Participants = append(thread.Participants, post.UserId) if post.CreateAt > thread.LastReplyAt { thread.LastReplyAt = post.CreateAt } diff --git a/store/sqlstore/thread_store.go b/store/sqlstore/thread_store.go index 8d7d8108e7..5585addddd 100644 --- a/store/sqlstore/thread_store.go +++ b/store/sqlstore/thread_store.go @@ -448,6 +448,18 @@ func (s *SqlThreadStore) GetThreadForUser(teamId string, threadMembership *model } } + var participants []*model.User + for _, participantId := range thread.Participants { + var participant *model.User + for _, u := range users { + if u.Id == participantId { + participant = u + break + } + } + participants = append(participants, participant) + } + result := &model.ThreadResponse{ PostId: thread.PostId, ReplyCount: thread.ReplyCount, @@ -455,7 +467,7 @@ func (s *SqlThreadStore) GetThreadForUser(teamId string, threadMembership *model LastViewedAt: thread.LastViewedAt, UnreadReplies: thread.UnreadReplies, UnreadMentions: thread.UnreadMentions, - Participants: users, + Participants: participants, Post: thread.Post.ToNilIfInvalid(), } diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 27dfe5dc5b..1619bf65bc 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -357,6 +357,76 @@ func testPostStoreSaveMultiple(t *testing.T, ss store.Store) { assert.Greater(t, rchannel.LastPostAt, channel.LastPostAt) assert.Equal(t, int64(3), rchannel.TotalMsgCount) }) + + t.Run("Thread participants", func(t *testing.T) { + o1 := model.Post{} + o1.ChannelId = model.NewId() + o1.UserId = model.NewId() + o1.Message = "jessica hyde" + model.NewId() + "b" + + root, err := ss.Post().Save(&o1) + require.NoError(t, err) + + o2 := model.Post{} + o2.ChannelId = model.NewId() + o2.UserId = model.NewId() + o2.RootId = root.Id + o2.Message = "zz" + model.NewId() + "b" + + o3 := model.Post{} + o3.ChannelId = model.NewId() + o3.UserId = model.NewId() + o3.RootId = root.Id + o3.Message = "zz" + model.NewId() + "b" + + o4 := model.Post{} + o4.ChannelId = model.NewId() + o4.UserId = o2.UserId + o4.RootId = root.Id + o4.Message = "zz" + model.NewId() + "b" + + o5 := model.Post{} + o5.ChannelId = model.NewId() + o5.UserId = o1.UserId + o5.RootId = root.Id + o5.Message = "zz" + model.NewId() + "b" + + _, err = ss.Post().Save(&o2) + require.NoError(t, err) + thread, errT := ss.Thread().Get(root.Id) + require.NoError(t, errT) + + assert.Equal(t, int64(1), thread.ReplyCount) + assert.Equal(t, int(1), len(thread.Participants)) + assert.Equal(t, model.StringArray{o2.UserId}, thread.Participants) + + _, err = ss.Post().Save(&o3) + require.NoError(t, err) + thread, errT = ss.Thread().Get(root.Id) + require.NoError(t, errT) + + assert.Equal(t, int64(2), thread.ReplyCount) + assert.Equal(t, int(2), len(thread.Participants)) + assert.Equal(t, model.StringArray{o2.UserId, o3.UserId}, thread.Participants) + + _, err = ss.Post().Save(&o4) + require.NoError(t, err) + thread, errT = ss.Thread().Get(root.Id) + require.NoError(t, errT) + + assert.Equal(t, int64(3), thread.ReplyCount) + assert.Equal(t, int(2), len(thread.Participants)) + assert.Equal(t, model.StringArray{o3.UserId, o2.UserId}, thread.Participants) + + _, err = ss.Post().Save(&o5) + require.NoError(t, err) + thread, errT = ss.Thread().Get(root.Id) + require.NoError(t, errT) + + assert.Equal(t, int64(4), thread.ReplyCount) + assert.Equal(t, int(3), len(thread.Participants)) + assert.Equal(t, model.StringArray{o3.UserId, o2.UserId, o1.UserId}, thread.Participants) + }) } func testPostStoreSaveChannelMsgCounts(t *testing.T, ss store.Store) { diff --git a/store/storetest/thread_store.go b/store/storetest/thread_store.go index dc701349ac..cf02dad3c1 100644 --- a/store/storetest/thread_store.go +++ b/store/storetest/thread_store.go @@ -231,7 +231,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) { thread1, err := ss.Thread().Get(newPosts1[0].Id) require.NoError(t, err) require.EqualValues(t, thread1.ReplyCount, 1) - require.Len(t, thread1.Participants, 2) + require.Len(t, thread1.Participants, 1) err = ss.Post().PermanentDeleteByUser(rootPost.UserId) require.NoError(t, err)