From e6ed3436fb5469f92cfc695f39746c14fef54961 Mon Sep 17 00:00:00 2001 From: catalintomai <56169943+catalintomai@users.noreply.github.com> Date: Tue, 27 May 2025 15:15:49 +0200 Subject: [PATCH] MM-54023: [Shared Channels] Filter out system posts for channel update info that is not synced (#30735) * initial checkin * simplify tests, logic * fix tests * remove unneeded test * fix spacing * updates * update logic * fix logic * move filtering to DB * remove comment --------- Co-authored-by: Catalin Tomai --- server/channels/store/sqlstore/post_store.go | 8 ++ server/channels/store/storetest/post_store.go | 89 +++++++++++++++++++ .../sharedchannel/sync_send_remote.go | 9 +- server/public/model/post.go | 9 +- 4 files changed, 107 insertions(+), 8 deletions(-) diff --git a/server/channels/store/sqlstore/post_store.go b/server/channels/store/sqlstore/post_store.go index a48959db99..51c84636ab 100644 --- a/server/channels/store/sqlstore/post_store.go +++ b/server/channels/store/sqlstore/post_store.go @@ -1525,6 +1525,14 @@ func (s *SqlPostStore) GetPostsSinceForSync(options model.GetPostsSinceForSyncOp query = query.Where(sq.NotEq{"COALESCE(Posts.RemoteId,'')": options.ExcludeRemoteId}) } + if options.ExcludeChannelMetadataSystemPosts { + query = query.Where(sq.NotEq{"Posts.Type": []string{ + model.PostTypeHeaderChange, + model.PostTypeDisplaynameChange, + model.PostTypePurposeChange, + }}) + } + queryString, args, err := query.ToSql() if err != nil { return nil, cursor, errors.Wrap(err, "getpostssinceforsync_tosql") diff --git a/server/channels/store/storetest/post_store.go b/server/channels/store/storetest/post_store.go index e38205c522..d7c200ee1b 100644 --- a/server/channels/store/storetest/post_store.go +++ b/server/channels/store/storetest/post_store.go @@ -61,6 +61,7 @@ func TestPostStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { t.Run("HasAutoResponsePostByUserSince", func(t *testing.T) { testHasAutoResponsePostByUserSince(t, rctx, ss) }) t.Run("GetPostsSinceUpdateForSync", func(t *testing.T) { testGetPostsSinceUpdateForSync(t, rctx, ss, s) }) t.Run("GetPostsSinceCreateForSync", func(t *testing.T) { testGetPostsSinceCreateForSync(t, rctx, ss, s) }) + t.Run("GetPostsSinceForSyncExcludeMetadata", func(t *testing.T) { testGetPostsSinceForSyncExcludeMetadata(t, rctx, ss, s) }) t.Run("SetPostReminder", func(t *testing.T) { testSetPostReminder(t, rctx, ss, s) }) t.Run("GetPostReminders", func(t *testing.T) { testGetPostReminders(t, rctx, ss, s) }) t.Run("GetPostReminderMetadata", func(t *testing.T) { testGetPostReminderMetadata(t, rctx, ss, s) }) @@ -5455,3 +5456,91 @@ func testGetEditHistoryForPost(t *testing.T, rctx request.CTX, ss store.Store) { require.NoError(t, err) }) } + +// testGetPostsSinceForSyncExcludeMetadata tests the ExcludeChannelMetadataSystemPosts option +// in the GetPostsSinceForSync function to verify that database-level filtering works correctly +func testGetPostsSinceForSyncExcludeMetadata(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { + // Create a channel + channelID := model.NewId() + + // Create test posts - mix of regular posts and channel metadata system posts + first := model.GetMillis() + + data := []*model.Post{ + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "regular post 1", Type: model.PostTypeDefault}, + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "changed header", Type: model.PostTypeHeaderChange}, + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "regular post 2", Type: model.PostTypeDefault}, + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "changed display name", Type: model.PostTypeDisplaynameChange}, + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "regular post 3", Type: model.PostTypeDefault}, + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "changed purpose", Type: model.PostTypePurposeChange}, + {Id: model.NewId(), ChannelId: channelID, UserId: model.NewId(), Message: "regular post 4", Type: model.PostTypeDefault}, + } + + // Save posts + for i, p := range data { + p.UpdateAt = first + (int64(i) * 300000) + p.CreateAt = first + (int64(i) * 300000) + p.RemoteId = model.NewPointer(model.NewId()) + _, err := ss.Post().Save(rctx, p) + require.NoError(t, err, "couldn't save post") + } + + t.Run("ExcludeChannelMetadataSystemPosts=true should filter out metadata posts", func(t *testing.T) { + // Set options with ExcludeChannelMetadataSystemPosts = true + opt := model.GetPostsSinceForSyncOptions{ + ChannelId: channelID, + ExcludeChannelMetadataSystemPosts: true, + } + cursor := model.GetPostsSinceForSyncCursor{} + posts, _, err := ss.Post().GetPostsSinceForSync(opt, cursor, 100) + require.NoError(t, err) + + // Verify only regular posts are returned + require.Len(t, posts, 4, "should return only 4 regular posts") + + // Check that only default post types are returned + for _, post := range posts { + require.Equal(t, model.PostTypeDefault, post.Type, "only default posts should be returned") + } + + // Verify we have the expected post IDs (only regular posts) + expectedIDs := []string{ + data[0].Id, // regular post 1 + data[2].Id, // regular post 2 + data[4].Id, // regular post 3 + data[6].Id, // regular post 4 + } + + postIDs := make([]string, 0, len(posts)) + for _, p := range posts { + postIDs = append(postIDs, p.Id) + } + + require.ElementsMatch(t, expectedIDs, postIDs, "returned posts should only be regular posts") + }) + + t.Run("ExcludeChannelMetadataSystemPosts=false should include all posts", func(t *testing.T) { + // Set options with ExcludeChannelMetadataSystemPosts = false + opt := model.GetPostsSinceForSyncOptions{ + ChannelId: channelID, + ExcludeChannelMetadataSystemPosts: false, + } + cursor := model.GetPostsSinceForSyncCursor{} + posts, _, err := ss.Post().GetPostsSinceForSync(opt, cursor, 100) + require.NoError(t, err) + + // Verify all posts are returned + require.Len(t, posts, 7, "should return all 7 posts when not excluding metadata posts") + + // Verify all post types are included by counting each type + postTypeCount := make(map[string]int) + for _, post := range posts { + postTypeCount[post.Type]++ + } + + require.Equal(t, 4, postTypeCount[model.PostTypeDefault], "should have 4 regular posts") + require.Equal(t, 1, postTypeCount[model.PostTypeHeaderChange], "should have 1 header change post") + require.Equal(t, 1, postTypeCount[model.PostTypeDisplaynameChange], "should have 1 display name change post") + require.Equal(t, 1, postTypeCount[model.PostTypePurposeChange], "should have 1 purpose change post") + }) +} diff --git a/server/platform/services/sharedchannel/sync_send_remote.go b/server/platform/services/sharedchannel/sync_send_remote.go index b1b2067c90..e3255fe8ee 100644 --- a/server/platform/services/sharedchannel/sync_send_remote.go +++ b/server/platform/services/sharedchannel/sync_send_remote.go @@ -270,9 +270,10 @@ func (scs *Service) fetchPostsForSync(sd *syncData) error { }() options := model.GetPostsSinceForSyncOptions{ - ChannelId: sd.task.channelID, - IncludeDeleted: true, - SinceCreateAt: true, + ChannelId: sd.task.channelID, + IncludeDeleted: true, + SinceCreateAt: true, + ExcludeChannelMetadataSystemPosts: true, } cursor := model.GetPostsSinceForSyncCursor{ LastPostUpdateAt: sd.scr.LastPostUpdateAt, @@ -463,7 +464,7 @@ func (scs *Service) fetchPostAttachmentsForSync(sd *syncData) error { return merr.ErrorOrNil() } -// filterPostsforSync removes any posts that do not need to sync. +// filterPostsForSync removes any posts that do not need to sync. func (scs *Service) filterPostsForSync(sd *syncData) { filtered := make([]*model.Post, 0, len(sd.posts)) diff --git a/server/public/model/post.go b/server/public/model/post.go index d29170977e..98e940906e 100644 --- a/server/public/model/post.go +++ b/server/public/model/post.go @@ -386,10 +386,11 @@ func (c GetPostsSinceForSyncCursor) IsEmpty() bool { } type GetPostsSinceForSyncOptions struct { - ChannelId string - ExcludeRemoteId string - IncludeDeleted bool - SinceCreateAt bool // determines whether the cursor will be based on CreateAt or UpdateAt + ChannelId string + ExcludeRemoteId string + IncludeDeleted bool + SinceCreateAt bool // determines whether the cursor will be based on CreateAt or UpdateAt + ExcludeChannelMetadataSystemPosts bool // if true, exclude channel metadata system posts (header, display name, purpose changes) } type GetPostsOptions struct {