PLT-4429 disabling at_all at_channel metions mentions when channel has more than 1k users (#4627)
* PLT-4429 disabling explicit mentions when channel has more than 1k users * Fixing test case * Adding setting to the admin console * Fixing bad translation
Этот коммит содержится в:
коммит произвёл
Harrison Healey
родитель
0f07a2d288
Коммит
b212acf312
@@ -580,7 +580,7 @@ func AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM
|
||||
|
||||
go func() {
|
||||
InvalidateCacheForUser(user.Id)
|
||||
Srv.Store.User().InvalidateProfilesInChannelCache(channel.Id)
|
||||
InvalidateCacheForChannel(channel.Id)
|
||||
|
||||
message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_ADDED, "", channel.Id, "", nil)
|
||||
message.Add("user_id", user.Id)
|
||||
@@ -625,7 +625,7 @@ func JoinDefaultChannels(teamId string, user *model.User, channelRole string) *m
|
||||
l4g.Error(utils.T("api.channel.post_user_add_remove_message_and_forget.error"), err)
|
||||
}
|
||||
|
||||
Srv.Store.User().InvalidateProfilesInChannelCache(result.Data.(*model.Channel).Id)
|
||||
InvalidateCacheForChannel(result.Data.(*model.Channel).Id)
|
||||
}
|
||||
|
||||
if result := <-Srv.Store.Channel().GetByName(teamId, "off-topic"); result.Err != nil {
|
||||
@@ -649,7 +649,7 @@ func JoinDefaultChannels(teamId string, user *model.User, channelRole string) *m
|
||||
l4g.Error(utils.T("api.channel.post_user_add_remove_message_and_forget.error"), err)
|
||||
}
|
||||
|
||||
Srv.Store.User().InvalidateProfilesInChannelCache(result.Data.(*model.Channel).Id)
|
||||
InvalidateCacheForChannel(result.Data.(*model.Channel).Id)
|
||||
}
|
||||
|
||||
return err
|
||||
@@ -662,7 +662,7 @@ func leave(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
sc := Srv.Store.Channel().Get(id)
|
||||
uc := Srv.Store.User().Get(c.Session.UserId)
|
||||
ccm := Srv.Store.Channel().GetMemberCount(id)
|
||||
ccm := Srv.Store.Channel().GetMemberCount(id, false)
|
||||
|
||||
if cresult := <-sc; cresult.Err != nil {
|
||||
c.Err = cresult.Err
|
||||
@@ -718,7 +718,7 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
sc := Srv.Store.Channel().Get(id)
|
||||
scm := Srv.Store.Channel().GetMember(id, c.Session.UserId)
|
||||
cmc := Srv.Store.Channel().GetMemberCount(id)
|
||||
cmc := Srv.Store.Channel().GetMemberCount(id, false)
|
||||
uc := Srv.Store.User().Get(c.Session.UserId)
|
||||
ihc := Srv.Store.Webhook().GetIncomingByChannel(id)
|
||||
ohc := Srv.Store.Webhook().GetOutgoingByChannel(id)
|
||||
@@ -949,7 +949,7 @@ func getChannelStats(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
channel = result.Data.(*model.Channel)
|
||||
}
|
||||
|
||||
if result := <-Srv.Store.Channel().GetMemberCount(id); result.Err != nil {
|
||||
if result := <-Srv.Store.Channel().GetMemberCount(id, true); result.Err != nil {
|
||||
c.Err = result.Err
|
||||
return
|
||||
} else {
|
||||
@@ -1107,7 +1107,6 @@ func removeMember(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
w.Write([]byte(model.MapToJson(result)))
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
func RemoveUserFromChannel(userIdToRemove string, removerUserId string, channel *model.Channel) *model.AppError {
|
||||
@@ -1120,7 +1119,7 @@ func RemoveUserFromChannel(userIdToRemove string, removerUserId string, channel
|
||||
}
|
||||
|
||||
InvalidateCacheForUser(userIdToRemove)
|
||||
Srv.Store.User().InvalidateProfilesInChannelCache(channel.Id)
|
||||
InvalidateCacheForChannel(channel.Id)
|
||||
|
||||
message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_REMOVED, "", channel.Id, "", nil)
|
||||
message.Add("user_id", userIdToRemove)
|
||||
|
||||
76
api/post.go
76
api/post.go
@@ -475,7 +475,7 @@ func getMentionKeywordsInChannel(profiles map[string]*model.User) map[string][]s
|
||||
}
|
||||
|
||||
// Add @channel and @all to keywords if user has them turned on
|
||||
if profile.NotifyProps["channel"] == "true" {
|
||||
if int64(len(profiles)) < *utils.Cfg.TeamSettings.MaxNotificationsPerChannel && profile.NotifyProps["channel"] == "true" {
|
||||
keywords["@channel"] = append(keywords["@channel"], profile.Id)
|
||||
keywords["@all"] = append(keywords["@all"], profile.Id)
|
||||
}
|
||||
@@ -486,11 +486,13 @@ func getMentionKeywordsInChannel(profiles map[string]*model.User) map[string][]s
|
||||
|
||||
// Given a message and a map mapping mention keywords to the users who use them, returns a map of mentioned
|
||||
// users and a slice of potencial mention users not in the channel and whether or not @here was mentioned.
|
||||
func getExplicitMentions(message string, keywords map[string][]string) (map[string]bool, []string, bool) {
|
||||
func getExplicitMentions(message string, keywords map[string][]string) (map[string]bool, []string, bool, bool, bool) {
|
||||
mentioned := make(map[string]bool)
|
||||
potentialOthersMentioned := make([]string, 0)
|
||||
systemMentions := map[string]bool{"@here": true, "@channel": true, "@all": true}
|
||||
hereMentioned := false
|
||||
allMentioned := false
|
||||
channelMentioned := false
|
||||
|
||||
addMentionedUsers := func(ids []string) {
|
||||
for _, id := range ids {
|
||||
@@ -505,6 +507,14 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri
|
||||
hereMentioned = true
|
||||
}
|
||||
|
||||
if word == "@channel" {
|
||||
channelMentioned = true
|
||||
}
|
||||
|
||||
if word == "@all" {
|
||||
allMentioned = true
|
||||
}
|
||||
|
||||
// Non-case-sensitive check for regular keys
|
||||
if ids, match := keywords[strings.ToLower(word)]; match {
|
||||
addMentionedUsers(ids)
|
||||
@@ -529,6 +539,14 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri
|
||||
hereMentioned = true
|
||||
}
|
||||
|
||||
if splitWord == "@all" {
|
||||
allMentioned = true
|
||||
}
|
||||
|
||||
if splitWord == "@channel" {
|
||||
channelMentioned = true
|
||||
}
|
||||
|
||||
// Non-case-sensitive check for regular keys
|
||||
if ids, match := keywords[strings.ToLower(splitWord)]; match {
|
||||
addMentionedUsers(ids)
|
||||
@@ -545,7 +563,7 @@ func getExplicitMentions(message string, keywords map[string][]string) (map[stri
|
||||
}
|
||||
}
|
||||
|
||||
return mentioned, potentialOthersMentioned, hereMentioned
|
||||
return mentioned, potentialOthersMentioned, hereMentioned, channelMentioned, allMentioned
|
||||
}
|
||||
|
||||
func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *model.Channel) []string {
|
||||
@@ -569,6 +587,8 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
|
||||
mentionedUserIds := make(map[string]bool)
|
||||
allActivityPushUserIds := []string{}
|
||||
hereNotification := false
|
||||
channelNotification := false
|
||||
allNotification := false
|
||||
updateMentionChans := []store.StoreChannel{}
|
||||
|
||||
if channel.Type == model.CHANNEL_DIRECT {
|
||||
@@ -584,7 +604,7 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
|
||||
keywords := getMentionKeywordsInChannel(profileMap)
|
||||
|
||||
var potentialOtherMentions []string
|
||||
mentionedUserIds, potentialOtherMentions, hereNotification = getExplicitMentions(post.Message, keywords)
|
||||
mentionedUserIds, potentialOtherMentions, hereNotification, channelNotification, allNotification = getExplicitMentions(post.Message, keywords)
|
||||
|
||||
// get users that have comment thread mentions enabled
|
||||
if len(post.RootId) > 0 {
|
||||
@@ -652,7 +672,13 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
|
||||
var status *model.Status
|
||||
var err *model.AppError
|
||||
if status, err = GetStatus(id); err != nil {
|
||||
status = &model.Status{id, model.STATUS_OFFLINE, false, 0, ""}
|
||||
status = &model.Status{
|
||||
UserId: id,
|
||||
Status: model.STATUS_OFFLINE,
|
||||
Manual: false,
|
||||
LastActivityAt: 0,
|
||||
ActiveChannel: "",
|
||||
}
|
||||
}
|
||||
|
||||
if userAllowsEmails && status.Status != model.STATUS_ONLINE {
|
||||
@@ -661,6 +687,46 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
|
||||
}
|
||||
}
|
||||
|
||||
// If the channel has more than 1K users then @here is disabled
|
||||
if hereNotification && int64(len(profileMap)) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel {
|
||||
hereNotification = false
|
||||
SendEphemeralPost(
|
||||
c.TeamId,
|
||||
post.UserId,
|
||||
&model.Post{
|
||||
ChannelId: post.ChannelId,
|
||||
Message: utils.T("api.post.disabled_here", map[string]interface{}{"Users": *utils.Cfg.TeamSettings.MaxNotificationsPerChannel}),
|
||||
CreateAt: post.CreateAt + 1,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// If the channel has more than 1K users then @channel is disabled
|
||||
if channelNotification && int64(len(profileMap)) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel {
|
||||
SendEphemeralPost(
|
||||
c.TeamId,
|
||||
post.UserId,
|
||||
&model.Post{
|
||||
ChannelId: post.ChannelId,
|
||||
Message: utils.T("api.post.disabled_channel", map[string]interface{}{"Users": *utils.Cfg.TeamSettings.MaxNotificationsPerChannel}),
|
||||
CreateAt: post.CreateAt + 1,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// If the channel has more than 1K users then @all is disabled
|
||||
if allNotification && int64(len(profileMap)) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel {
|
||||
SendEphemeralPost(
|
||||
c.TeamId,
|
||||
post.UserId,
|
||||
&model.Post{
|
||||
ChannelId: post.ChannelId,
|
||||
Message: utils.T("api.post.disabled_all", map[string]interface{}{"Users": *utils.Cfg.TeamSettings.MaxNotificationsPerChannel}),
|
||||
CreateAt: post.CreateAt + 1,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
if hereNotification {
|
||||
if result := <-Srv.Store.Status().GetOnline(); result.Err != nil {
|
||||
l4g.Warn(utils.T("api.post.notification.here.warn"), result.Err)
|
||||
|
||||
@@ -1062,7 +1062,7 @@ func TestGetExplicitMentionsAtHere(t *testing.T) {
|
||||
}
|
||||
|
||||
for message, shouldMention := range cases {
|
||||
if _, _, hereMentioned := getExplicitMentions(message, nil); hereMentioned && !shouldMention {
|
||||
if _, _, hereMentioned, _, _ := getExplicitMentions(message, nil); hereMentioned && !shouldMention {
|
||||
t.Fatalf("shouldn't have mentioned @here with \"%v\"", message)
|
||||
} else if !hereMentioned && shouldMention {
|
||||
t.Fatalf("should've have mentioned @here with \"%v\"", message)
|
||||
@@ -1071,7 +1071,7 @@ func TestGetExplicitMentionsAtHere(t *testing.T) {
|
||||
|
||||
// mentioning @here and someone
|
||||
id := model.NewId()
|
||||
if mentions, potential, hereMentioned := getExplicitMentions("@here @user @potential", map[string][]string{"@user": {id}}); !hereMentioned {
|
||||
if mentions, potential, hereMentioned, _, _ := getExplicitMentions("@here @user @potential", map[string][]string{"@user": {id}}); !hereMentioned {
|
||||
t.Fatal("should've mentioned @here with \"@here @user\"")
|
||||
} else if len(mentions) != 1 || !mentions[id] {
|
||||
t.Fatal("should've mentioned @user with \"@here @user\"")
|
||||
@@ -1087,74 +1087,74 @@ func TestGetExplicitMentions(t *testing.T) {
|
||||
// not mentioning anybody
|
||||
message := "this is a message"
|
||||
keywords := map[string][]string{}
|
||||
if mentions, potential, _ := getExplicitMentions(message, keywords); len(mentions) != 0 || len(potential) != 0 {
|
||||
if mentions, potential, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 0 || len(potential) != 0 {
|
||||
t.Fatal("shouldn't have mentioned anybody or have any potencial mentions")
|
||||
}
|
||||
|
||||
// mentioning a user that doesn't exist
|
||||
message = "this is a message for @user"
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 0 {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 0 {
|
||||
t.Fatal("shouldn't have mentioned user that doesn't exist")
|
||||
}
|
||||
|
||||
// mentioning one person
|
||||
keywords = map[string][]string{"@user": {id1}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] {
|
||||
t.Fatal("should've mentioned @user")
|
||||
}
|
||||
|
||||
// mentioning one person without an @mention
|
||||
message = "this is a message for @user"
|
||||
keywords = map[string][]string{"this": {id1}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] {
|
||||
t.Fatal("should've mentioned this")
|
||||
}
|
||||
|
||||
// mentioning multiple people with one word
|
||||
message = "this is a message for @user"
|
||||
keywords = map[string][]string{"@user": {id1, id2}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
t.Fatal("should've mentioned two users with @user")
|
||||
}
|
||||
|
||||
// mentioning only one of multiple people
|
||||
keywords = map[string][]string{"@user": {id1}, "@mention": {id2}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] {
|
||||
t.Fatal("should've mentioned @user and not @mention")
|
||||
}
|
||||
|
||||
// mentioning multiple people with multiple words
|
||||
message = "this is an @mention for @user"
|
||||
keywords = map[string][]string{"@user": {id1}, "@mention": {id2}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
t.Fatal("should've mentioned two users with @user and @mention")
|
||||
}
|
||||
|
||||
// mentioning @channel (not a special case, but it's good to double check)
|
||||
message = "this is an message for @channel"
|
||||
keywords = map[string][]string{"@channel": {id1, id2}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
t.Fatal("should've mentioned two users with @channel")
|
||||
}
|
||||
|
||||
// mentioning @all (not a special case, but it's good to double check)
|
||||
message = "this is an message for @all"
|
||||
keywords = map[string][]string{"@all": {id1, id2}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 2 || !mentions[id1] || !mentions[id2] {
|
||||
t.Fatal("should've mentioned two users with @all")
|
||||
}
|
||||
|
||||
// mentioning user.period without mentioning user (PLT-3222)
|
||||
message = "user.period doesn't complicate things at all by including periods in their username"
|
||||
keywords = map[string][]string{"user.period": {id1}, "user": {id2}}
|
||||
if mentions, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] {
|
||||
if mentions, _, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || mentions[id2] {
|
||||
t.Fatal("should've mentioned user.period and not user")
|
||||
}
|
||||
|
||||
// mentioning a potential out of channel user
|
||||
message = "this is an message for @potential and @user"
|
||||
keywords = map[string][]string{"@user": {id1}}
|
||||
if mentions, potential, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || len(potential) != 1 {
|
||||
if mentions, potential, _, _, _ := getExplicitMentions(message, keywords); len(mentions) != 1 || !mentions[id1] || len(potential) != 1 {
|
||||
t.Fatal("should've mentioned user and have a potential not in channel")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -339,7 +339,7 @@ func LeaveTeam(team *model.Team, user *model.User) *model.AppError {
|
||||
|
||||
for _, channel := range *channelList {
|
||||
if channel.Type != model.CHANNEL_DIRECT {
|
||||
Srv.Store.User().InvalidateProfilesInChannelCache(channel.Id)
|
||||
InvalidateCacheForChannel(channel.Id)
|
||||
if result := <-Srv.Store.Channel().RemoveMember(channel.Id, user.Id); result.Err != nil {
|
||||
return result.Err
|
||||
}
|
||||
|
||||
@@ -187,6 +187,18 @@ func (webCon *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
|
||||
// Only report events to users who are in the channel for the event
|
||||
if len(msg.Broadcast.ChannelId) > 0 {
|
||||
|
||||
// Only broadcast typing messages if less than 1K people in channel
|
||||
if msg.Event == model.WEBSOCKET_EVENT_TYPING {
|
||||
if result := <-Srv.Store.Channel().GetMemberCount(msg.Broadcast.ChannelId, true); result.Err != nil {
|
||||
l4g.Error("webhub.shouldSendEvent: " + result.Err.Error())
|
||||
return false
|
||||
} else {
|
||||
if result.Data.(int64) > *utils.Cfg.TeamSettings.MaxNotificationsPerChannel {
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if model.GetMillis()-webCon.LastAllChannelMembersTime > 1000*60*15 { // 15 minutes
|
||||
webCon.AllChannelMembers = nil
|
||||
webCon.LastAllChannelMembersTime = 0
|
||||
|
||||
@@ -102,6 +102,11 @@ func PublishSkipClusterSend(message *model.WebSocketEvent) {
|
||||
}
|
||||
}
|
||||
|
||||
func InvalidateCacheForChannel(channelId string) {
|
||||
Srv.Store.User().InvalidateProfilesInChannelCache(channelId)
|
||||
Srv.Store.Channel().InvalidateMemberCount(channelId)
|
||||
}
|
||||
|
||||
func InvalidateCacheForUser(userId string) {
|
||||
InvalidateCacheForUserSkipClusterSend(userId)
|
||||
|
||||
|
||||
Ссылка в новой задаче
Block a user