From 562388501e48b0048fbbb1fb755e448d6b62e376 Mon Sep 17 00:00:00 2001 From: Kyriakos Z <3829551+koox00@users.noreply.github.com> Date: Fri, 29 Jul 2022 17:03:52 +0300 Subject: [PATCH] MM-43939: fixes LastReplyAt when deleting the last reply (#20615) * MM-43939: fixes lastreplyat when deleting the last reply Currently we are not updating the Threads.LastReplyAt when the last reply gets deleted. This can lead to threads appearing unread when actually there is no unread thread. This commit updates the value of Threads.LastReplyAt when a reply gets deleted, to the most recent post's timestamp in the thread. * Updates ReplyCount to current value on post delete * Addresses review comments Co-authored-by: Mattermod --- store/sqlstore/post_store.go | 17 ++++++++- store/storetest/post_store.go | 68 +++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 03ee58888e..044ee6ca27 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -2840,8 +2840,23 @@ func (s *SqlPostStore) updateThreadAfterReplyDeletion(transaction *sqlxTxWrapper } } + lastReplyAtSubquery := sq.Select("COALESCE(MAX(CreateAt), 0)"). + From("Posts"). + Where(sq.Eq{ + "RootId": rootId, + "DeleteAt": 0, + }) + + lastReplyCountSubquery := sq.Select("Count(*)"). + From("Posts"). + Where(sq.Eq{ + "RootId": rootId, + "DeleteAt": 0, + }) + updateQueryString, updateArgs, err := updateQuery. - Set("ReplyCount", sq.Expr("ReplyCount - 1")). + Set("LastReplyAt", lastReplyAtSubquery). + Set("ReplyCount", lastReplyCountSubquery). Where(sq.And{ sq.Eq{"PostId": rootId}, sq.Gt{"ReplyCount": 0}, diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 98efff5321..2489847905 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -979,6 +979,74 @@ func testPostStoreDelete(t *testing.T, ss store.Store) { _, err = ss.Post().Get(context.Background(), rootPost2.Id, model.GetPostsOptions{}, "", map[string]bool{}) require.NoError(t, err) }) + + t.Run("thread with multiple replies, update thread last reply at", func(t *testing.T) { + // Create a root post + rootPost1, err := ss.Post().Save(&model.Post{ + ChannelId: model.NewId(), + UserId: model.NewId(), + Message: NewTestId(), + }) + require.NoError(t, err) + + // Reply to that root post + replyPost1, err := ss.Post().Save(&model.Post{ + ChannelId: rootPost1.ChannelId, + UserId: model.NewId(), + Message: NewTestId(), + RootId: rootPost1.Id, + }) + require.NoError(t, err) + + // Reply to that root post a second time + replyPost2, err := ss.Post().Save(&model.Post{ + ChannelId: rootPost1.ChannelId, + UserId: model.NewId(), + Message: NewTestId(), + RootId: rootPost1.Id, + }) + require.NoError(t, err) + + // Reply to that root post a third time + replyPost3, err := ss.Post().Save(&model.Post{ + ChannelId: rootPost1.ChannelId, + UserId: model.NewId(), + Message: NewTestId(), + RootId: rootPost1.Id, + }) + require.NoError(t, err) + + thread, err := ss.Thread().Get(rootPost1.Id) + require.NoError(t, err) + require.Equal(t, replyPost3.CreateAt, thread.LastReplyAt) + + // Delete the reply previous to last + err = ss.Post().Delete(replyPost2.Id, model.GetMillis(), "") + require.NoError(t, err) + + thread, err = ss.Thread().Get(rootPost1.Id) + require.NoError(t, err) + // last reply at should be unchanged + require.Equal(t, replyPost3.CreateAt, thread.LastReplyAt) + + // Delete the last reply + err = ss.Post().Delete(replyPost3.Id, model.GetMillis(), "") + require.NoError(t, err) + + thread, err = ss.Thread().Get(rootPost1.Id) + require.NoError(t, err) + // last reply at should have changed + require.Equal(t, replyPost1.CreateAt, thread.LastReplyAt) + + // Delete the last reply + err = ss.Post().Delete(replyPost1.Id, model.GetMillis(), "") + require.NoError(t, err) + + thread, err = ss.Thread().Get(rootPost1.Id) + require.NoError(t, err) + // last reply at should be 0 + require.Equal(t, int64(0), thread.LastReplyAt) + }) } func testPostStorePermDelete1Level(t *testing.T, ss store.Store) {