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 <catalintomai@catalins-macbook-pro-2.home>
2025-05-27 15:15:49 +02:00
--- 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")
--- 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")
+	})
+}
--- 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))

--- 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 {