MM-36862: should remove user from participants (#18451)

* MM-36862: should remove user from participants When deleting a reply in a thread we should also delete the participant from the participants array. This should happen if they have no other replies in that thread. This commit fixes that. * Adds warning logs * Delete Post: fetches participants only when needed * Minor refactor * Remove participant should check for error in count * Fixes error when binary_parameters=true * Fixes: removes ArrayToJSON so we can handle the error * Satisfies the linter * Better error handling * Satisfy govet Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
2021-10-15 19:40:09 +03:00
--- a/store/sqlstore/post_store.go
+++ b/store/sqlstore/post_store.go
@@ -6,6 +6,7 @@ package sqlstore
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"fmt"
 	"reflect"
 	"regexp"
@@ -695,8 +696,9 @@ func (s *SqlPostStore) Delete(postID string, time int64, deleteByID string) erro
 		return errors.Wrap(err, "failed to update Posts")
 	}

+	ids := postIds{}
 	// TODO: change this to later delete thread directly from postID
-	rootID, err := s.GetReplica().SelectStr("SELECT RootId FROM Posts WHERE Id = :Id", map[string]interface{}{"Id": postID})
+	err = s.GetReplica().SelectOne(&ids, "SELECT RootId, UserId FROM Posts WHERE Id = :Id", map[string]interface{}{"Id": postID})
 	if err != nil {
 		if err == sql.ErrNoRows {
 			return store.NewErrNotFound("Post", postID)
@@ -705,7 +707,7 @@ func (s *SqlPostStore) Delete(postID string, time int64, deleteByID string) erro
 		return errors.Wrapf(err, "failed to delete Post with id=%s", postID)
 	}

-	return s.cleanupThreads(postID, rootID, false)
+	return s.cleanupThreads(postID, ids.RootId, false, ids.UserId)
 }

 func (s *SqlPostStore) permanentDelete(postId string) error {
@@ -714,7 +716,7 @@ func (s *SqlPostStore) permanentDelete(postId string) error {
 	if err != nil && err != sql.ErrNoRows {
 		return errors.Wrapf(err, "failed to get Post with id=%s", postId)
 	}
-	if err = s.cleanupThreads(post.Id, post.RootId, true); err != nil {
+	if err = s.cleanupThreads(post.Id, post.RootId, true, post.UserId); err != nil {
 		return errors.Wrapf(err, "failed to cleanup threads for Post with id=%s", postId)
 	}

@@ -739,7 +741,7 @@ func (s *SqlPostStore) permanentDeleteAllCommentByUser(userId string) error {
 	}

 	for _, ids := range results {
-		if err = s.cleanupThreads(ids.Id, ids.RootId, true); err != nil {
+		if err = s.cleanupThreads(ids.Id, ids.RootId, true, userId); err != nil {
 			return err
 		}
 	}
@@ -795,7 +797,7 @@ func (s *SqlPostStore) PermanentDeleteByChannel(channelId string) error {
 	}

 	for _, ids := range results {
-		if err = s.cleanupThreads(ids.Id, ids.RootId, true); err != nil {
+		if err = s.cleanupThreads(ids.Id, ids.RootId, true, ids.UserId); err != nil {
 			return err
 		}
 	}
@@ -2375,7 +2377,7 @@ func (s *SqlPostStore) GetOldestEntityCreationTime() (int64, error) {
 	return oldest, nil
 }

-func (s *SqlPostStore) cleanupThreads(postId, rootId string, permanent bool) error {
+func (s *SqlPostStore) cleanupThreads(postId, rootId string, permanent bool, userId string) error {
 	if permanent {
 		if _, err := s.GetMaster().Exec("DELETE FROM Threads WHERE PostId = :Id", map[string]interface{}{"Id": postId}); err != nil {
 			return errors.Wrap(err, "failed to delete Threads")
@@ -2386,7 +2388,67 @@ func (s *SqlPostStore) cleanupThreads(postId, rootId string, permanent bool) err
 		return nil
 	}
 	if rootId != "" {
-		_, err := s.GetMaster().Exec(`UPDATE Threads SET ReplyCount = ReplyCount - 1 WHERE PostId = :Id AND ReplyCount > 0`, map[string]interface{}{"Id": rootId})
+		queryString, args, err := s.getQueryBuilder().
+			Select("COUNT(Id)").
+			From("Posts").
+			Where(sq.And{
+				sq.Eq{"RootId": rootId},
+				sq.Eq{"UserId": userId},
+				sq.Eq{"DeleteAt": 0},
+			}).
+			ToSql()
+
+		if err != nil {
+			return errors.Wrap(err, "failed to create SQL query to count user's posts")
+		}
+
+		count, err := s.GetReplica().SelectInt(queryString, args...)
+
+		if err != nil {
+			return errors.Wrap(err, "failed to count user's posts in thread")
+		}
+
+		updateQuery := s.getQueryBuilder().Update("Threads")
+
+		if count == 0 {
+			var participants model.StringArray
+			err = s.getQueryBuilder().
+				Select("Participants").
+				From("Threads").
+				Where(sq.Eq{"PostId": rootId}).
+				RunWith(s.GetReplica()).
+				QueryRow().
+				Scan(&participants)
+
+			if err != nil {
+				return errors.Wrap(err, "failed getting thread participants")
+			}
+
+			if participants.Contains(userId) {
+				participants = participants.Remove(userId)
+				var participantsJSON []byte
+				participantsJSON, err = json.Marshal(participants)
+				if err != nil {
+					return errors.Wrap(err, "failed marshalling thread participants")
+				}
+				updateQuery = updateQuery.Set("Participants", string(participantsJSON))
+			}
+		}
+
+		updateQueryString, updateArgs, err := updateQuery.
+			Set("ReplyCount", sq.Expr("ReplyCount - 1")).
+			Where(sq.And{
+				sq.Eq{"PostId": rootId},
+				sq.Gt{"ReplyCount": 0},
+			}).
+			ToSql()
+
+		if err != nil {
+			return errors.Wrap(err, "failed to create SQL query to update thread")
+		}
+
+		_, err = s.GetMaster().Exec(updateQueryString, updateArgs...)
+
 		if err != nil {
 			return errors.Wrap(err, "failed to update Threads")
 		}
--- a/store/storetest/thread_store.go
+++ b/store/storetest/thread_store.go
@@ -130,7 +130,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
 		require.NoError(t, err, "couldn't get thread")
 		require.NotNil(t, thread)
 		require.Equal(t, int64(1), thread.ReplyCount)
-		require.ElementsMatch(t, model.StringArray{newPosts[0].UserId, newPosts[1].UserId}, thread.Participants)
+		require.ElementsMatch(t, model.StringArray{newPosts[0].UserId}, thread.Participants)
 	})

 	t.Run("Update reply should update the UpdateAt of the thread", func(t *testing.T) {
@@ -181,33 +181,55 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
 	})

 	t.Run("Deleting reply should update the thread", func(t *testing.T) {
-		rootPost := model.Post{}
-		rootPost.RootId = model.NewId()
-		rootPost.ChannelId = model.NewId()
-		rootPost.UserId = model.NewId()
-		rootPost.Message = NewTestId()
-
-		replyPost := model.Post{}
-		replyPost.ChannelId = rootPost.ChannelId
-		replyPost.UserId = model.NewId()
-		replyPost.Message = NewTestId()
-		replyPost.RootId = rootPost.RootId
-
-		newPosts, _, err := ss.Post().SaveMultiple([]*model.Post{&rootPost, &replyPost})
+		o1 := model.Post{}
+		o1.ChannelId = model.NewId()
+		o1.UserId = model.NewId()
+		o1.Message = NewTestId()
+		rootPost, err := ss.Post().Save(&o1)
 		require.NoError(t, err)

-		thread1, err := ss.Thread().Get(newPosts[0].RootId)
+		o2 := model.Post{}
+		o2.RootId = rootPost.Id
+		o2.ChannelId = rootPost.ChannelId
+		o2.UserId = model.NewId()
+		o2.Message = NewTestId()
+		replyPost, err := ss.Post().Save(&o2)
 		require.NoError(t, err)
-		require.EqualValues(t, thread1.ReplyCount, 2)
-		require.Len(t, thread1.Participants, 2)
+
+		o3 := model.Post{}
+		o3.RootId = rootPost.Id
+		o3.ChannelId = rootPost.ChannelId
+		o3.UserId = o2.UserId
+		o3.Message = NewTestId()
+		replyPost2, err := ss.Post().Save(&o3)
+		require.NoError(t, err)
+
+		o4 := model.Post{}
+		o4.RootId = rootPost.Id
+		o4.ChannelId = rootPost.ChannelId
+		o4.UserId = model.NewId()
+		o4.Message = NewTestId()
+		replyPost3, err := ss.Post().Save(&o4)
+		require.NoError(t, err)
+
+		thread, err := ss.Thread().Get(rootPost.Id)
+		require.NoError(t, err)
+		require.EqualValues(t, thread.ReplyCount, 3)
+		require.EqualValues(t, thread.Participants, model.StringArray{replyPost.UserId, replyPost3.UserId})
+
+		err = ss.Post().Delete(replyPost2.Id, 123, model.NewId())
+		require.NoError(t, err)
+		thread, err = ss.Thread().Get(rootPost.Id)
+		require.NoError(t, err)
+		require.EqualValues(t, thread.ReplyCount, 2)
+		require.EqualValues(t, thread.Participants, model.StringArray{replyPost.UserId, replyPost3.UserId})

 		err = ss.Post().Delete(replyPost.Id, 123, model.NewId())
 		require.NoError(t, err)
-
-		thread2, err := ss.Thread().Get(rootPost.RootId)
+		thread, err = ss.Thread().Get(rootPost.Id)
 		require.NoError(t, err)
-		require.EqualValues(t, thread2.ReplyCount, 1)
-		require.Len(t, thread2.Participants, 2)
+		require.EqualValues(t, thread.ReplyCount, 1)
+		require.EqualValues(t, thread.Participants, model.StringArray{replyPost3.UserId})
 	})

 	t.Run("Deleting root post should delete the thread", func(t *testing.T) {