MM-18651: no spurious mentions on leaving channels (#12527)

Today, the server emits a system message containing one of `<username> left the channel.` or `<username> removed from the channel.` when such an event occurs.

While the client interprets this special kind of message to take into account the enduser's username display settings (e.g. to render `First Last left the channel.`), the server also processes this "message" to look for mentions.

With the user in question no longer in the channel, it incorrectly splits usernames containing a period into two tokens and tries to mentions users accordingly. So `christopher.poile` ends up trying to mention `christopher` and `poile`. If such a user exists, they receive a spurious mention for this event.

Change the message rendered by the server to explicitly prepend an `@`, which in turn is detected by the mention logic to avoid splitting. As above, this has no effect on the client-side rendered message.

api4/channel_test.go

@@ -10,6 +10,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -2502,15 +2503,79 @@ func TestRemoveChannelMember(t *testing.T) {
 	_, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id)
 	CheckForbiddenStatus(t, resp)
 
-	th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel)
-	_, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser2.Id)
-	CheckNoError(t, resp)
+	t.Run("success", func(t *testing.T) {
+		// Setup the system administrator to listen for websocket events from the channels.
+		th.LinkUserToTeam(th.SystemAdminUser, th.BasicTeam)
+		_, err := th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel)
+		_, err = th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel2)
+		require.Nil(t, err)
+		props := map[string]string{}
+		props[model.DESKTOP_NOTIFY_PROP] = model.CHANNEL_NOTIFY_ALL
+		_, resp = th.SystemAdminClient.UpdateChannelNotifyProps(th.BasicChannel.Id, th.SystemAdminUser.Id, props)
+		_, resp = th.SystemAdminClient.UpdateChannelNotifyProps(th.BasicChannel2.Id, th.SystemAdminUser.Id, props)
+		CheckNoError(t, resp)
 
-	_, resp = Client.RemoveUserFromChannel(th.BasicChannel2.Id, th.BasicUser.Id)
-	CheckNoError(t, resp)
+		wsClient, err := th.CreateWebSocketSystemAdminClient()
+		require.Nil(t, err)
+		wsClient.Listen()
+		var closeWsClient sync.Once
+		defer closeWsClient.Do(func() {
+			wsClient.Close()
+		})
 
-	_, resp = th.SystemAdminClient.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id)
-	CheckNoError(t, resp)
+		wsr := <-wsClient.EventChannel
+		require.Equal(t, wsr.Event, model.WEBSOCKET_EVENT_HELLO)
+
+		// requirePost listens for websocket events and tries to find the post matching
+		// the expected post's channel and message.
+		requirePost := func(expectedPost *model.Post) {
+			t.Helper()
+			for {
+				select {
+				case event := <-wsClient.EventChannel:
+					postData, ok := event.Data["post"]
+					if !ok {
+						continue
+					}
+
+					post := model.PostFromJson(strings.NewReader(postData.(string)))
+					if post.ChannelId == expectedPost.ChannelId && post.Message == expectedPost.Message {
+						return
+					}
+				case <-time.After(5 * time.Second):
+					t.Fatal("failed to find expected post after 5 seconds")
+					return
+				}
+			}
+		}
+
+		th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel)
+		_, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser2.Id)
+		CheckNoError(t, resp)
+
+		requirePost(&model.Post{
+			Message:   fmt.Sprintf("@%s left the channel.", th.BasicUser2.Username),
+			ChannelId: th.BasicChannel.Id,
+		})
+
+		_, resp = Client.RemoveUserFromChannel(th.BasicChannel2.Id, th.BasicUser.Id)
+		CheckNoError(t, resp)
+		requirePost(&model.Post{
+			Message:   fmt.Sprintf("@%s removed from the channel.", th.BasicUser.Username),
+			ChannelId: th.BasicChannel2.Id,
+		})
+
+		_, resp = th.SystemAdminClient.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id)
+		CheckNoError(t, resp)
+		requirePost(&model.Post{
+			Message:   fmt.Sprintf("@%s removed from the channel.", th.BasicUser.Username),
+			ChannelId: th.BasicChannel.Id,
+		})
+
+		closeWsClient.Do(func() {
+			wsClient.Close()
+		})
+	})
 
 	// Leave deleted channel
 	th.LoginBasic()

app/channel.go

@@ -1525,9 +1525,12 @@ func (a *App) LeaveChannel(channelId string, userId string) *model.AppError {
 func (a *App) postLeaveChannelMessage(user *model.User, channel *model.Channel) *model.AppError {
 	post := &model.Post{
 		ChannelId: channel.Id,
-		Message:   fmt.Sprintf(utils.T("api.channel.leave.left"), user.Username),
-		Type:      model.POST_LEAVE_CHANNEL,
-		UserId:    user.Id,
+		// Message here embeds `@username`, not just `username`, to ensure that mentions
+		// treat this as a username mention even though the user has now left the channel.
+		// The client renders its own system message, ignoring this value altogether.
+		Message: fmt.Sprintf(utils.T("api.channel.leave.left"), fmt.Sprintf("@%s", user.Username)),
+		Type:    model.POST_LEAVE_CHANNEL,
+		UserId:  user.Id,
 		Props: model.StringInterface{
 			"username": user.Username,
 		},
@@ -1595,9 +1598,12 @@ func (a *App) postAddToTeamMessage(user *model.User, addedUser *model.User, chan
 func (a *App) postRemoveFromChannelMessage(removerUserId string, removedUser *model.User, channel *model.Channel) *model.AppError {
 	post := &model.Post{
 		ChannelId: channel.Id,
-		Message:   fmt.Sprintf(utils.T("api.channel.remove_member.removed"), removedUser.Username),
-		Type:      model.POST_REMOVE_FROM_CHANNEL,
-		UserId:    removerUserId,
+		// Message here embeds `@username`, not just `username`, to ensure that mentions
+		// treat this as a username mention even though the user has now left the channel.
+		// The client renders its own system message, ignoring this value altogether.
+		Message: fmt.Sprintf(utils.T("api.channel.remove_member.removed"), fmt.Sprintf("@%s", removedUser.Username)),
+		Type:    model.POST_REMOVE_FROM_CHANNEL,
+		UserId:  removerUserId,
 		Props: model.StringInterface{
 			"removedUserId":   removedUser.Id,
 			"removedUsername": removedUser.Username,