diff --git a/server/channels/app/platform/shared_channel_service_iface.go b/server/channels/app/platform/shared_channel_service_iface.go index 2adc6a1c59..8ae4385904 100644 --- a/server/channels/app/platform/shared_channel_service_iface.go +++ b/server/channels/app/platform/shared_channel_service_iface.go @@ -5,6 +5,7 @@ package platform import ( "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" "github.com/mattermost/mattermost/server/v8/platform/services/sharedchannel" ) @@ -24,6 +25,7 @@ type SharedChannelServiceIFace interface { CheckChannelIsShared(channelID string) error CheckCanInviteToSharedChannel(channelId string) error HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) + TransformMentionsOnReceiveForTesting(ctx request.CTX, post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string) } type MockOptionSharedChannelService func(service *mockSharedChannelService) @@ -82,3 +84,7 @@ func (mrcs *mockSharedChannelService) NumInvitations() int { func (mrcs *mockSharedChannelService) HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) { // This is a mock implementation - it doesn't need to do anything } + +func (mrcs *mockSharedChannelService) TransformMentionsOnReceiveForTesting(ctx request.CTX, post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string) { + // This is a mock implementation - it doesn't need to do anything +} diff --git a/server/channels/app/shared_channel_service_iface.go b/server/channels/app/shared_channel_service_iface.go index 7dc117d66c..1c1623bf98 100644 --- a/server/channels/app/shared_channel_service_iface.go +++ b/server/channels/app/shared_channel_service_iface.go @@ -6,6 +6,7 @@ package app // TODO: platform: remove this and use from platform package import ( "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" "github.com/mattermost/mattermost/server/v8/platform/services/sharedchannel" ) @@ -27,6 +28,7 @@ type SharedChannelServiceIFace interface { CheckChannelIsShared(channelID string) error CheckCanInviteToSharedChannel(channelId string) error HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) + TransformMentionsOnReceiveForTesting(ctx request.CTX, post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string) } func NewMockSharedChannelService(service SharedChannelServiceIFace) *mockSharedChannelService { diff --git a/server/channels/app/shared_channel_test.go b/server/channels/app/shared_channel_test.go index a9d09183ab..ad4e506030 100644 --- a/server/channels/app/shared_channel_test.go +++ b/server/channels/app/shared_channel_test.go @@ -22,6 +22,7 @@ func setupSharedChannels(tb testing.TB) *TestHelper { *cfg.ConnectedWorkspacesSettings.EnableRemoteClusterService = true *cfg.ConnectedWorkspacesSettings.EnableSharedChannels = true cfg.FeatureFlags.EnableSharedChannelsMemberSync = true + cfg.ClusterSettings.ClusterName = model.NewPointer("test-remote") }) } @@ -556,3 +557,299 @@ func TestSyncMessageErrChannelNotSharedResponse(t *testing.T) { } require.NotNil(t, systemPost, "System message should be posted when channel becomes unshared") } + +// TestTransformMentionsOnReceive provides comprehensive unit testing for the mention transformation logic +// using explicit mentionTransforms. This tests ONLY the receiver-side transformation logic +// without requiring complex end-to-end cross-cluster setup. +func TestTransformMentionsOnReceive(t *testing.T) { + mainHelper.Parallel(t) + th := setupSharedChannels(t).InitBasic() + + // Setup shared channel + sharedChannel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: sharedChannel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "testchannel", + CreatorId: th.BasicUser.Id, + } + _, err := th.App.ShareChannel(th.Context, sc) + require.NoError(t, err) + + // Setup remote cluster representing the sender + remoteCluster := &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "remote1", + DisplayName: "Remote 1", + SiteURL: "http://remote1.example.com", + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + } + savedRemoteCluster, appErr := th.App.AddRemoteCluster(remoteCluster) + require.Nil(t, appErr) + + // Get shared channel service + scs := th.App.Srv().Platform().GetSharedChannelService() + require.NotNil(t, scs) + concreteScs, ok := scs.(*sharedchannel.Service) + require.True(t, ok) + + // Helper to create test users + createUser := func(username string, remoteId *string) *model.User { + user := th.CreateUser() + user.Username = username + if remoteId != nil { + user.RemoteId = remoteId + } + user, updateErr := th.App.UpdateUser(th.Context, user, false) + require.Nil(t, updateErr) + th.LinkUserToTeam(user, th.BasicTeam) + th.AddUserToChannel(user, sharedChannel) + return user + } + + // Helper to test transformation + testTransformation := func(originalMessage string, mentionTransforms map[string]string, expectedMessage string, description string) { + post := &model.Post{ + Id: model.NewId(), + ChannelId: sharedChannel.Id, + UserId: th.BasicUser.Id, + Message: originalMessage, + } + + t.Logf("Testing: %s", description) + t.Logf(" Original: %s", originalMessage) + t.Logf(" Transforms: %v", mentionTransforms) + + // Call the transformation function directly + concreteScs.TransformMentionsOnReceiveForTesting(th.Context, post, sharedChannel, savedRemoteCluster, mentionTransforms) + + t.Logf(" Result: %s", post.Message) + t.Logf(" Expected: %s", expectedMessage) + + require.Equal(t, expectedMessage, post.Message, description) + } + + t.Run("Scenario 1.1: Remote mentions local user (simple mention)", func(t *testing.T) { + // Create remote user that was synced to receiver + remoteUser := createUser("admin:remote1", &savedRemoteCluster.RemoteId) + + // Scenario: remote1 mentions "@admin" (their local user) → sent to receiver + // mentionTransforms["admin"] = remote1AdminUserId + mentionTransforms := map[string]string{ + "admin": remoteUser.Id, + } + + testTransformation( + "Hello @admin, can you help?", + mentionTransforms, + "Hello @admin:remote1, can you help?", // Use synced username + "Simple mention of synced remote user should use synced username", + ) + }) + + t.Run("Scenario 1.2: Remote mentions local user (different username)", func(t *testing.T) { + // Create remote user that was synced to receiver + remoteUser := createUser("user:remote1", &savedRemoteCluster.RemoteId) + + mentionTransforms := map[string]string{ + "user": remoteUser.Id, + } + + testTransformation( + "Hello @user, can you help?", + mentionTransforms, + "Hello @user:remote1, can you help?", + "Simple mention of different synced remote user should use synced username", + ) + }) + + t.Run("Scenario 2.1: Remote mentions with colon (local user)", func(t *testing.T) { + // Create local user on receiver + localUser := createUser("admin", nil) + + // Scenario: remote2 mentions "@admin:remote1" → sent to remote1 + // mentionTransforms["admin:remote1"] = remote1AdminUserId + mentionTransforms := map[string]string{ + "admin:remote1": localUser.Id, + } + + testTransformation( + "Hello @admin:remote1, can you help?", + mentionTransforms, + "Hello @admin, can you help?", // Strip suffix for local user + "Colon mention of local user should strip cluster suffix", + ) + }) + + t.Run("Scenario 2.2: Remote mentions with colon (different local user)", func(t *testing.T) { + // Create local user on receiver + localUser := createUser("user", nil) + + mentionTransforms := map[string]string{ + "user:remote1": localUser.Id, + } + + testTransformation( + "Hello @user:remote1, can you help?", + mentionTransforms, + "Hello @user, can you help?", + "Colon mention of different local user should strip cluster suffix", + ) + }) + + t.Run("Scenario A1: Name clash - remote user mention, local user exists", func(t *testing.T) { + // Create local user with same name + _ = createUser("alice", nil) // Create name clash scenario + // Create remote user that was synced + remoteUser := createUser("alice:remote1", &savedRemoteCluster.RemoteId) + + // When remote1 mentions "@alice" (their local user), receiver gets explicit transform + mentionTransforms := map[string]string{ + "alice": remoteUser.Id, // Points to synced remote user, not local user + } + + testTransformation( + "Hello @alice, can you help?", + mentionTransforms, + "Hello @alice:remote1, can you help?", + "Matrix A1: Remote user mention with local name clash should add cluster suffix", + ) + }) + + t.Run("Scenario A2: Same user - previously synced", func(t *testing.T) { + // Create user that was synced from sender + syncedUser := createUser("bob:remote1", &savedRemoteCluster.RemoteId) + + mentionTransforms := map[string]string{ + "bob": syncedUser.Id, + } + + testTransformation( + "Hello @bob, can you help?", + mentionTransforms, + "Hello @bob:remote1, can you help?", + "Matrix A2: Previously synced user should display synced username", + ) + }) + + t.Run("Scenario A3: No user exists on receiver", func(t *testing.T) { + // Use non-existent user ID + nonExistentUserId := model.NewId() + + mentionTransforms := map[string]string{ + "charlie": nonExistentUserId, + } + + testTransformation( + "Hello @charlie, can you help?", + mentionTransforms, + "Hello @charlie:remote1, can you help?", + "Matrix A3: Unknown user should get cluster suffix", + ) + }) + + t.Run("Scenario B1: User exists on origin with same ID", func(t *testing.T) { + // Create local user (representing user on their home cluster) + localUser := createUser("dave", nil) + + // Remote mentions "@dave:remote1" pointing to local user ID + mentionTransforms := map[string]string{ + "dave:remote1": localUser.Id, + } + + testTransformation( + "Hello @dave:remote1, can you help?", + mentionTransforms, + "Hello @dave, can you help?", + "Matrix B1: Remote mention of local user should strip cluster suffix", + ) + }) + + t.Run("Scenario B2: User does not exist on origin", func(t *testing.T) { + // Use non-existent user ID + nonExistentUserId := model.NewId() + + mentionTransforms := map[string]string{ + "eve:remote1": nonExistentUserId, + } + + testTransformation( + "Hello @eve:remote1, can you help?", + mentionTransforms, + "Hello @eve:remote1, can you help?", + "Matrix B2: Unknown colon mention should remain unchanged", + ) + }) + + t.Run("Empty mentionTransforms", func(t *testing.T) { + // No transforms provided + mentionTransforms := map[string]string{} + + testTransformation( + "Hello @anyone, can you help?", + mentionTransforms, + "Hello @anyone, can you help?", + "Message without transforms should remain unchanged", + ) + }) + + t.Run("Mixed scenarios in single message", func(t *testing.T) { + // Setup users + localUser := createUser("frank", nil) + remoteUser := createUser("george:remote1", &savedRemoteCluster.RemoteId) + + // Multiple transforms in one message + mentionTransforms := map[string]string{ + "frank:remote1": localUser.Id, // Colon mention → strip suffix + "george": remoteUser.Id, // Simple mention → use synced username + } + + testTransformation( + "Hello @frank:remote1 and @george, let's collaborate!", + mentionTransforms, + "Hello @frank and @george:remote1, let's collaborate!", + "Mixed mention types should transform correctly", + ) + }) + + t.Run("Colon mention of remote user", func(t *testing.T) { + // Create remote user that was synced + remoteUser := createUser("guest:remote1", &savedRemoteCluster.RemoteId) + + // Colon mention pointing to remote user (edge case) + mentionTransforms := map[string]string{ + "guest:remote1": remoteUser.Id, + } + + testTransformation( + "Hello @guest:remote1, welcome!", + mentionTransforms, + "Hello @guest:remote1, welcome!", + "Colon mention of remote user should use synced username", + ) + }) + + t.Run("Performance: Large message with many mentions", func(t *testing.T) { + // Create users for performance test + user1 := createUser("user1:remote1", &savedRemoteCluster.RemoteId) + user2 := createUser("user2:remote1", &savedRemoteCluster.RemoteId) + user3 := createUser("user3:remote1", &savedRemoteCluster.RemoteId) + + mentionTransforms := map[string]string{ + "user1": user1.Id, + "user2": user2.Id, + "user3": user3.Id, + } + + testTransformation( + "Meeting with @user1, @user2, and @user3 about @user1's proposal. @user2 will present, @user3 will take notes.", + mentionTransforms, + "Meeting with @user1:remote1, @user2:remote1, and @user3:remote1 about @user1:remote1's proposal. @user2:remote1 will present, @user3:remote1 will take notes.", + "Multiple mentions should transform efficiently", + ) + }) +} diff --git a/server/platform/services/sharedchannel/service.go b/server/platform/services/sharedchannel/service.go index 9e467caa4d..4085ba64ed 100644 --- a/server/platform/services/sharedchannel/service.go +++ b/server/platform/services/sharedchannel/service.go @@ -380,3 +380,8 @@ func (scs *Service) OnReceiveSyncMessageForTesting(msg model.RemoteClusterMsg, r func (scs *Service) HandleChannelNotSharedErrorForTesting(msg *model.SyncMsg, rc *model.RemoteCluster) { scs.handleChannelNotSharedError(msg, rc) } + +// TransformMentionsOnReceiveForTesting allows testing the full mention transformation flow +func (scs *Service) TransformMentionsOnReceiveForTesting(ctx request.CTX, post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string) { + scs.transformMentionsOnReceive(ctx, post, targetChannel, rc, mentionTransforms) +} diff --git a/server/platform/services/sharedchannel/sync_recv.go b/server/platform/services/sharedchannel/sync_recv.go index 79e4fc3bef..9ed176d123 100644 --- a/server/platform/services/sharedchannel/sync_recv.go +++ b/server/platform/services/sharedchannel/sync_recv.go @@ -197,7 +197,7 @@ func (scs *Service) processSyncMessage(c request.CTX, syncMsg *model.SyncMsg, rc } // add/update post - rpost, err := scs.upsertSyncPost(post, targetChannel, rc) + rpost, err := scs.upsertSyncPost(post, targetChannel, rc, syncMsg.MentionTransforms) if err != nil { syncResp.PostErrors = append(syncResp.PostErrors, post.Id) scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Error upserting sync post", @@ -454,7 +454,7 @@ func (scs *Service) updateSyncUser(rctx request.CTX, patch *model.UserPatch, use return nil, fmt.Errorf("error updating sync user %s: %w", user.Id, err) } -func (scs *Service) upsertSyncPost(post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster) (*model.Post, error) { +func (scs *Service) upsertSyncPost(post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string) (*model.Post, error) { var appErr *model.AppError post.RemoteId = model.NewPointer(rc.RemoteId) @@ -483,11 +483,14 @@ func (scs *Service) upsertSyncPost(post *model.Post, targetChannel *model.Channe return nil, fmt.Errorf("post sync failed: %w", ErrRemoteIDMismatch) } + scs.transformMentionsOnReceive(rctx, post, targetChannel, rc, mentionTransforms) + rpost, appErr = scs.app.CreatePost(rctx, post, targetChannel, model.CreatePostFlags{TriggerWebhooks: true, SetOnline: true}) if appErr == nil { scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Created sync post", mlog.String("post_id", post.Id), - mlog.String("channel_id", post.ChannelId)) + mlog.String("channel_id", post.ChannelId), + ) } } else if post.DeleteAt > 0 { // delete post @@ -499,6 +502,7 @@ func (scs *Service) upsertSyncPost(post *model.Post, targetChannel *model.Channe ) } } else if post.EditAt > rpost.EditAt || post.Message != rpost.Message || post.UpdateAt > rpost.UpdateAt || post.Metadata != nil { + scs.transformMentionsOnReceive(rctx, post, targetChannel, rc, mentionTransforms) var priority *model.PostPriority var acknowledgements []*model.PostAcknowledgement @@ -741,3 +745,44 @@ func (scs *Service) upsertSyncAcknowledgement(acknowledgement *model.PostAcknowl } return savedAcknowledgement, retErr } + +// transformMentionsOnReceive transforms mentions in received posts using explicit mentionTransforms. +func (scs *Service) transformMentionsOnReceive(rctx request.CTX, post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string) { + if post.Message == "" || len(mentionTransforms) == 0 { + return + } + + // Process mentions directly using mentionTransforms - no need to re-parse with regex + for mention, userID := range mentionTransforms { + oldMention := "@" + mention + var newMention string + + // Get the user to determine transformation type + if user, err := scs.server.GetStore().User().Get(context.Background(), userID); err == nil && user != nil { + // User exists in receiver's database + if strings.Contains(mention, ":") { + // Colon mention (e.g., "@admin:remote1") - always use the user's actual username + newMention = "@" + user.Username + } else { + // Simple mention (e.g., "@admin") + if user.GetRemoteID() == "" { + // This is a local user, keep as-is + newMention = "@" + mention + } else { + // This is a remote user that was synced, use their synced username + newMention = "@" + user.Username + } + } + } else { + // User doesn't exist in receiver's database + if strings.Contains(mention, ":") { + // Colon mention for unknown user - keep as-is + newMention = oldMention + } else { + // Simple mention for unknown user - add cluster suffix to indicate it's from remote + newMention = "@" + mention + ":" + rc.Name + } + } + post.Message = strings.ReplaceAll(post.Message, oldMention, newMention) + } +} diff --git a/server/platform/services/sharedchannel/sync_send_remote.go b/server/platform/services/sharedchannel/sync_send_remote.go index 83b3b52b46..3afedd447b 100644 --- a/server/platform/services/sharedchannel/sync_send_remote.go +++ b/server/platform/services/sharedchannel/sync_send_remote.go @@ -32,13 +32,14 @@ type syncData struct { rc *model.RemoteCluster scr *model.SharedChannelRemote - users map[string]*model.User - profileImages map[string]*model.User - posts []*model.Post - reactions []*model.Reaction - acknowledgements []*model.PostAcknowledgement - statuses []*model.Status - attachments []attachment + users map[string]*model.User + profileImages map[string]*model.User + posts []*model.Post + reactions []*model.Reaction + acknowledgements []*model.PostAcknowledgement + statuses []*model.Status + attachments []attachment + mentionTransforms map[string]string resultRepeat bool resultNextCursor model.GetPostsSinceForSyncCursor @@ -47,11 +48,12 @@ type syncData struct { func newSyncData(task syncTask, rc *model.RemoteCluster, scr *model.SharedChannelRemote) *syncData { return &syncData{ - task: task, - rc: rc, - scr: scr, - users: make(map[string]*model.User), - profileImages: make(map[string]*model.User), + task: task, + rc: rc, + scr: scr, + users: make(map[string]*model.User), + profileImages: make(map[string]*model.User), + mentionTransforms: make(map[string]string), resultNextCursor: model.GetPostsSinceForSyncCursor{ LastPostUpdateAt: scr.LastPostUpdateAt, LastPostUpdateID: scr.LastPostUpdateID, LastPostCreateAt: scr.LastPostCreateAt, LastPostCreateID: scr.LastPostCreateID, @@ -444,9 +446,6 @@ func (scs *Service) fetchPostUsersForSync(sd *syncData) error { } for _, post := range sd.posts { - // add author - userIDs[post.UserId] = p2mm{} - // get mentions and users for each mention mentionMap := scs.app.MentionsToTeamMembers(request.EmptyContext(scs.server.Log()), post.Message, sc.TeamId) @@ -458,10 +457,19 @@ func (scs *Service) fetchPostUsersForSync(sd *syncData) error { } // Skip remote users unless mention contains a colon (@username:remote) - if user.RemoteId != nil && !strings.Contains(mention, ":") { + if user.IsRemote() && !strings.Contains(mention, ":") { continue } + } + // add author with post and mentionMap so transformations can be applied + userIDs[post.UserId] = p2mm{ + post: post, + mentionMap: mentionMap, + } + + // Add all mentioned users + for _, userID := range mentionMap { userIDs[userID] = p2mm{ post: post, mentionMap: mentionMap, @@ -470,7 +478,6 @@ func (scs *Service) fetchPostUsersForSync(sd *syncData) error { } merr := merror.New() - for userID, v := range userIDs { user, err := scs.server.GetStore().User().Get(context.Background(), userID) if err != nil { @@ -480,7 +487,7 @@ func (scs *Service) fetchPostUsersForSync(sd *syncData) error { sync, syncImage, err2 := scs.shouldUserSync(user, sd.task.channelID, sd.rc) if err2 != nil { - merr.Append(fmt.Errorf("could not check should sync user %s: %w", userID, err)) + merr.Append(fmt.Errorf("could not check should sync user %s: %w", userID, err2)) continue } @@ -492,9 +499,15 @@ func (scs *Service) fetchPostUsersForSync(sd *syncData) error { sd.profileImages[user.Id] = user } - // Transform @username:remote to @username when sending to a user's home cluster - if v.post != nil && user.RemoteId != nil && *user.RemoteId == sd.rc.RemoteId { - fixMention(v.post, v.mentionMap, user) + // Collect mention transforms for all mentioned users + if v.mentionMap != nil { + for mention, mentionUserID := range v.mentionMap { + if mentionUserID == userID { + // Always add the mention transform - let receiver decide how to display + // The sender should NOT modify the message, only provide the mapping + sd.mentionTransforms[mention] = userID + } + } } } return merr.ErrorOrNil() @@ -690,6 +703,7 @@ func (scs *Service) sendPostSyncData(sd *syncData) error { msg := model.NewSyncMsg(sd.task.channelID) msg.Posts = sd.posts + msg.MentionTransforms = sd.mentionTransforms return scs.sendSyncMsgToRemote(msg, sd.rc, func(syncResp model.SyncResponse, errResp error) { if len(syncResp.PostErrors) != 0 { diff --git a/server/public/model/shared_channel.go b/server/public/model/shared_channel.go index 3b7c8dc6ae..1398022351 100644 --- a/server/public/model/shared_channel.go +++ b/server/public/model/shared_channel.go @@ -292,6 +292,7 @@ type SyncMsg struct { Statuses []*Status `json:"statuses,omitempty"` MembershipChanges []*MembershipChangeMsg `json:"membership_changes,omitempty"` Acknowledgements []*PostAcknowledgement `json:"acknowledgements,omitempty"` + MentionTransforms map[string]string `json:"mention_transforms,omitempty"` } func NewSyncMsg(channelID string) *SyncMsg {