From 5f6ef75fe7b67416e3d7526b44851bfa2b79115b Mon Sep 17 00:00:00 2001 From: sazzad hossain Date: Fri, 5 Apr 2024 20:56:22 +0600 Subject: [PATCH] Send ephemeral message when a mentioned user is not a member of the team (#26153) Co-authored-by: Caleb Roseland Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> Co-authored-by: Sazzad Hossain Co-authored-by: Mattermost Build --- server/channels/app/notification.go | 94 ++++++++++++++++++------ server/channels/app/notification_test.go | 57 +++++++++++--- server/i18n/en.json | 8 ++ 3 files changed, 125 insertions(+), 34 deletions(-) diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index e444304145..423ad70660 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -1119,17 +1119,21 @@ func (a *App) sendNoUsersNotifiedByGroupInChannel(c request.CTX, sender *model.U // sendOutOfChannelMentions sends an ephemeral post to the sender of a post if any of the given potential mentions // are outside of the post's channel. Returns whether or not an ephemeral post was sent. func (a *App) sendOutOfChannelMentions(c request.CTX, sender *model.User, post *model.Post, channel *model.Channel, potentialMentions []string) (bool, error) { - outOfChannelUsers, outOfGroupsUsers, err := a.filterOutOfChannelMentions(c, sender, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupsUsers, err := a.filterOutOfChannelMentions(c, sender, post, channel, potentialMentions) if err != nil { return false, err } - if len(outOfChannelUsers) == 0 && len(outOfGroupsUsers) == 0 { + if len(outOfTeamUsers) == 0 && len(outOfChannelUsers) == 0 && len(outOfGroupsUsers) == 0 { return false, nil } - a.SendEphemeralPost(c, post.UserId, makeOutOfChannelMentionPost(sender, post, outOfChannelUsers, outOfGroupsUsers)) - + if len(outOfChannelUsers) != 0 || len(outOfGroupsUsers) != 0 { + a.SendEphemeralPost(c, post.UserId, makeOutOfChannelMentionPost(sender, post, outOfChannelUsers, outOfGroupsUsers)) + } + if len(outOfTeamUsers) != 0 { + a.SendEphemeralPost(c, post.UserId, makeOutOfTeamMentionPost(sender, post, outOfTeamUsers)) + } return true, nil } @@ -1147,52 +1151,65 @@ func (a *App) FilterUsersByVisible(c request.CTX, viewer *model.User, otherUsers return result, nil } -func (a *App) filterOutOfChannelMentions(c request.CTX, sender *model.User, post *model.Post, channel *model.Channel, potentialMentions []string) ([]*model.User, []*model.User, error) { +func (a *App) filterOutOfChannelMentions(c request.CTX, sender *model.User, post *model.Post, channel *model.Channel, potentialMentions []string) ([]*model.User, []*model.User, []*model.User, error) { if post.IsSystemMessage() { - return nil, nil, nil + return nil, nil, nil, nil } if channel.TeamId == "" || channel.Type == model.ChannelTypeDirect || channel.Type == model.ChannelTypeGroup { - return nil, nil, nil + return nil, nil, nil, nil } if len(potentialMentions) == 0 { - return nil, nil, nil + return nil, nil, nil, nil } - users, err := a.Srv().Store().User().GetProfilesByUsernames(potentialMentions, &model.ViewUsersRestrictions{Teams: []string{channel.TeamId}}) + mentionedUsersInTheTeam, err := a.Srv().Store().User().GetProfilesByUsernames(potentialMentions, &model.ViewUsersRestrictions{Teams: []string{channel.TeamId}}) if err != nil { - return nil, nil, err + return nil, nil, nil, err } // Filter out inactive users and bots - allUsers := model.UserSlice(users).FilterByActive(true) - allUsers = allUsers.FilterWithoutBots() - allUsers, appErr := a.FilterUsersByVisible(c, sender, allUsers) + teamUsers := model.UserSlice(mentionedUsersInTheTeam).FilterByActive(true) + teamUsers = teamUsers.FilterWithoutBots() + teamUsers, appErr := a.FilterUsersByVisible(c, sender, teamUsers) if appErr != nil { - return nil, nil, appErr + return nil, nil, nil, appErr } - if len(allUsers) == 0 { - return nil, nil, nil + allMentionedUsers, err := a.Srv().Store().User().GetProfilesByUsernames(potentialMentions, nil) + if err != nil { + return nil, nil, nil, err } - // Differentiate between users who can and can't be added to the channel + outOfTeamUsers := model.UserSlice(allMentionedUsers).FilterWithoutID(teamUsers.IDs()) + outOfTeamUsers = outOfTeamUsers.FilterByActive(true) + outOfTeamUsers = outOfTeamUsers.FilterWithoutBots() + outOfTeamUsers, appErr = a.FilterUsersByVisible(c, sender, outOfTeamUsers) + if appErr != nil { + return nil, nil, nil, appErr + } + + if len(teamUsers) == 0 { + return outOfTeamUsers, nil, nil, nil + } + + // Differentiate between mentionedUsersInTheTeam who can and can't be added to the channel var outOfChannelUsers model.UserSlice var outOfGroupsUsers model.UserSlice if channel.IsGroupConstrained() { - nonMemberIDs, err := a.FilterNonGroupChannelMembers(allUsers.IDs(), channel) + nonMemberIDs, err := a.FilterNonGroupChannelMembers(teamUsers.IDs(), channel) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - outOfChannelUsers = allUsers.FilterWithoutID(nonMemberIDs) - outOfGroupsUsers = allUsers.FilterByID(nonMemberIDs) + outOfChannelUsers = teamUsers.FilterWithoutID(nonMemberIDs) + outOfGroupsUsers = teamUsers.FilterByID(nonMemberIDs) } else { - outOfChannelUsers = allUsers + outOfChannelUsers = teamUsers } - return outOfChannelUsers, outOfGroupsUsers, nil + return outOfTeamUsers, outOfChannelUsers, outOfGroupsUsers, nil } func makeOutOfChannelMentionPost(sender *model.User, post *model.Post, outOfChannelUsers, outOfGroupsUsers []*model.User) *model.Post { @@ -1268,6 +1285,37 @@ func makeOutOfChannelMentionPost(sender *model.User, post *model.Post, outOfChan } } +func makeOutOfTeamMentionPost(sender *model.User, post *model.Post, outOfTeamUsers []*model.User) *model.Post { + otUsers := model.UserSlice(outOfTeamUsers) + otUsernames := otUsers.Usernames() + + T := i18n.GetUserTranslations(sender.Locale) + + ephemeralPostId := model.NewId() + var message string + + if len(outOfTeamUsers) == 1 { + message += T("api.post.check_for_out_of_team_mentions.message.one", map[string]any{ + "Username": otUsernames[0], + }) + } else if len(outOfTeamUsers) > 1 { + preliminary, final := splitAtFinal(otUsernames) + + message += T("api.post.check_for_out_of_team_mentions.message.multiple", map[string]any{ + "Usernames": strings.Join(preliminary, ", @"), + "LastUsername": final, + }) + } + + return &model.Post{ + Id: ephemeralPostId, + RootId: post.RootId, + ChannelId: post.ChannelId, + Message: message, + CreateAt: post.CreateAt + 1, + } +} + func splitAtFinal(items []string) (preliminary []string, final string) { if len(items) == 0 { return diff --git a/server/channels/app/notification_test.go b/server/channels/app/notification_test.go index 0611356d30..623613fec4 100644 --- a/server/channels/app/notification_test.go +++ b/server/channels/app/notification_test.go @@ -644,6 +644,17 @@ func TestSendOutOfChannelMentions(t *testing.T) { assert.NoError(t, err) assert.False(t, sent) }) + + t.Run("should send ephemeral post when there is an out of team mention", func(t *testing.T) { + outOfTeamUser := th.CreateUser() + post := &model.Post{} + potentialMentions := []string{outOfTeamUser.Username} + + sent, err := th.App.sendOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + + assert.NoError(t, err) + assert.True(t, sent) + }) } func TestFilterOutOfChannelMentions(t *testing.T) { @@ -670,22 +681,40 @@ func TestFilterOutOfChannelMentions(t *testing.T) { post := &model.Post{} potentialMentions := []string{user2.Username, user3.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Len(t, outOfChannelUsers, 2) assert.True(t, (outOfChannelUsers[0].Id == user2.Id || outOfChannelUsers[1].Id == user2.Id)) assert.True(t, (outOfChannelUsers[0].Id == user3.Id || outOfChannelUsers[1].Id == user3.Id)) assert.Nil(t, outOfGroupUsers) }) + t.Run("should return users not in the team", func(t *testing.T) { + notThisTeamUser1 := th.CreateUser() + notThisTeamUser2 := th.CreateUser() + post := &model.Post{} + potentialMentions := []string{notThisTeamUser1.Username, notThisTeamUser2.Username} + + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + + assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 2) + assert.Nil(t, outOfChannelUsers) + assert.True(t, (outOfTeamUsers[0].Id == notThisTeamUser1.Id || outOfTeamUsers[1].Id == notThisTeamUser1.Id)) + assert.True(t, (outOfTeamUsers[0].Id == notThisTeamUser2.Id || outOfTeamUsers[1].Id == notThisTeamUser2.Id)) + assert.Nil(t, outOfGroupUsers) + }) + t.Run("should return only visible users not in the channel (for guests)", func(t *testing.T) { post := &model.Post{} potentialMentions := []string{user2.Username, user3.Username, user4.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, guest, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, guest, post, channel, potentialMentions) require.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) require.Len(t, outOfChannelUsers, 1) assert.Equal(t, user4.Id, outOfChannelUsers[0].Id) assert.Nil(t, outOfGroupUsers) @@ -697,9 +726,10 @@ func TestFilterOutOfChannelMentions(t *testing.T) { } potentialMentions := []string{user2.Username, user3.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Nil(t, outOfChannelUsers) assert.Nil(t, outOfGroupUsers) }) @@ -711,9 +741,10 @@ func TestFilterOutOfChannelMentions(t *testing.T) { } potentialMentions := []string{user2.Username, user3.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, directChannel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, directChannel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Nil(t, outOfChannelUsers) assert.Nil(t, outOfGroupUsers) }) @@ -725,9 +756,10 @@ func TestFilterOutOfChannelMentions(t *testing.T) { } potentialMentions := []string{user2.Username, user3.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, groupChannel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, groupChannel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Nil(t, outOfChannelUsers) assert.Nil(t, outOfGroupUsers) }) @@ -740,23 +772,24 @@ func TestFilterOutOfChannelMentions(t *testing.T) { post := &model.Post{} potentialMentions := []string{inactiveUser.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Nil(t, outOfChannelUsers) assert.Nil(t, outOfGroupUsers) }) t.Run("should not return bot users", func(t *testing.T) { - botUser := th.CreateUser() - botUser.IsBot = true + botUser := th.CreateBot() post := &model.Post{} potentialMentions := []string{botUser.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Nil(t, outOfChannelUsers) assert.Nil(t, outOfGroupUsers) }) @@ -765,9 +798,10 @@ func TestFilterOutOfChannelMentions(t *testing.T) { post := &model.Post{} potentialMentions := []string{"foo", "bar"} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, channel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Nil(t, outOfChannelUsers) assert.Nil(t, outOfGroupUsers) }) @@ -799,9 +833,10 @@ func TestFilterOutOfChannelMentions(t *testing.T) { post := &model.Post{} potentialMentions := []string{nonChannelMember.Username, nonGroupMember.Username} - outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, constrainedChannel, potentialMentions) + outOfTeamUsers, outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(th.Context, user1, post, constrainedChannel, potentialMentions) assert.NoError(t, err) + assert.Len(t, outOfTeamUsers, 0) assert.Len(t, outOfChannelUsers, 1) assert.Equal(t, nonChannelMember.Id, outOfChannelUsers[0].Id) assert.Len(t, outOfGroupUsers, 1) diff --git a/server/i18n/en.json b/server/i18n/en.json index da68882f10..6178727e30 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -2500,6 +2500,14 @@ "id": "api.post.check_for_out_of_channel_mentions.message.one", "translation": "@{{.Username}} did not get notified by this mention because they are not in the channel." }, + { + "id": "api.post.check_for_out_of_team_mentions.message.multiple", + "translation": "@{{.Usernames}} and @{{.LastUsername}} didn't get notified by this mention because they aren't members of this team." + }, + { + "id": "api.post.check_for_out_of_team_mentions.message.one", + "translation": "@{{.Username}} didn't get notified by this mention because they aren't a member of this team." + }, { "id": "api.post.create_post.can_not_post_to_deleted.error", "translation": "Can not post to deleted channel."