From 8c34e6c80c52b221852a0dc392f64c3dbd37d1d9 Mon Sep 17 00:00:00 2001 From: Shivashis Padhi Date: Wed, 1 Mar 2023 17:08:23 +0530 Subject: [PATCH] MM-49547: Handle Top DM case for deleted second user (#22049) * Ignore archived DM channels with deleted second user * Add tests * Add store test * Add error check for post delete in test --------- Co-authored-by: Mattermost Build --- app/post_test.go | 15 +++++++++++++++ store/sqlstore/post_store.go | 8 ++++++-- store/storetest/post_store.go | 26 ++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/app/post_test.go b/app/post_test.go index 463b3c40dc..2fedbf8a75 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -3292,4 +3292,19 @@ func TestGetTopDMsForUserSince(t *testing.T) { require.Equal(t, topDMs.Items[0].SecondParticipant.Id, u3.Id) require.Equal(t, topDMs.Items[0].MessageCount, int64(4)) }) + + t.Run("topDMs will not consider deleted second user", func(t *testing.T) { + // u4 only takes part in one conversation + topDMs, err := th.App.GetTopDMsForUserSince(u4.Id, &model.InsightsOpts{StartUnixMilli: 100, Page: 0, PerPage: 100}) + require.Nil(t, err) + // len of topDMs.Items should be 1 + require.Len(t, topDMs.Items, 1) + // delete user3 + err = th.App.PermanentDeleteUser(th.Context, u3) + require.Nil(t, err) + topDMs, err = th.App.GetTopDMsForUserSince(u4.Id, &model.InsightsOpts{StartUnixMilli: 100, Page: 0, PerPage: 100}) + require.Nil(t, err) + // len of topDMs.Items should be 0 since u3 is deleted + require.Len(t, topDMs.Items, 0) + }) } diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 192b8d2834..7e447c7516 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -3170,10 +3170,14 @@ func (s *SqlPostStore) GetTopDMsForUserSince(userID string, since int64, offset }, }).GroupBy("vch.id") - topDMsBuilder = topDMsBuilder.OrderBy("MessageCount DESC").Limit(uint64(limit + 1)).Offset(uint64(offset)) + // following where clause filters out all archived DMs with "deleted" users, that has only 1 user-id in Participants column. + archivedDMsFilter := s.getQueryBuilder().Select("MessageCount", "Participants", "ChannelId").FromSelect(topDMsBuilder, "top_dms"). + Where(sq.Expr("POSITION(',' IN Participants) > 0")) + + archivedDMsFilter = archivedDMsFilter.OrderBy("MessageCount DESC").Limit(uint64(limit + 1)).Offset(uint64(offset)) topDMs := make([]*model.TopDM, 0) - sql, args, err := topDMsBuilder.ToSql() + sql, args, err := archivedDMsFilter.ToSql() if err != nil { return nil, errors.Wrap(err, "GetTopDMsForUserSince_ToSql") } diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 8486f89adf..473268342c 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -4901,7 +4901,7 @@ func testGetTopDMsForUserSince(t *testing.T, ss store.Store, s SqlStore) { require.NoError(t, err) } // for u4-u3: 4 posts - _, err = ss.Post().Save(&model.Post{ + u3u4Post1, err := ss.Post().Save(&model.Post{ ChannelId: chUser3User4.Id, UserId: u3.Id, }) @@ -4911,7 +4911,7 @@ func testGetTopDMsForUserSince(t *testing.T, ss store.Store, s SqlStore) { UserId: u4.Id, }) require.NoError(t, err) - _, err = ss.Post().Save(&model.Post{ + u3u4Post2, err := ss.Post().Save(&model.Post{ ChannelId: chUser3User4.Id, UserId: u3.Id, }) @@ -4965,6 +4965,28 @@ func testGetTopDMsForUserSince(t *testing.T, ss store.Store, s SqlStore) { // len of topDMs.Items should be 3 require.Len(t, topDMs.Items, 2) }) + t.Run("topDMs will not consider deleted second user", func(t *testing.T) { + // u4 only takes part in one conversation + topDMs, err := ss.Post().GetTopDMsForUserSince(u4.Id, 100, 0, 100) + require.NoError(t, err) + // len of topDMs.Items should be 1 + require.Len(t, topDMs.Items, 1) + // delete user3 + err = ss.User().PermanentDelete(u3.Id) + require.NoError(t, err) + // delete user3 posts + err = ss.Post().Delete(u3u4Post1.Id, 200, u3.Id) + require.NoError(t, err) + err = ss.Post().Delete(u3u4Post2.Id, 200, u3.Id) + require.NoError(t, err) + // delete channel memberships + err = ss.Channel().PermanentDeleteMembersByUser(u3.Id) + require.NoError(t, err) + topDMs, err = ss.Post().GetTopDMsForUserSince(u4.Id, 100, 0, 100) + require.NoError(t, err) + // len of topDMs.Items should be 0 since u3 is deleted + require.Len(t, topDMs.Items, 0) + }) } func testGetEditHistoryForPost(t *testing.T, ss store.Store) {