Refactored mention parsing and added unit tests (#3806)

api/post.go

@@ -239,9 +239,6 @@ func handlePostEvents(c *Context, post *model.Post, triggerWebhooks bool) {
 	tchan := Srv.Store.Team().Get(c.TeamId)
 	cchan := Srv.Store.Channel().Get(post.ChannelId)
 	uchan := Srv.Store.User().Get(post.UserId)
-	pchan := Srv.Store.User().GetProfiles(c.TeamId)
-	dpchan := Srv.Store.User().GetDirectProfiles(c.Session.UserId)
-	mchan := Srv.Store.Channel().GetMembers(post.ChannelId)
 
 	var team *model.Team
 	if result := <-tchan; result.Err != nil {
@@ -259,34 +256,7 @@ func handlePostEvents(c *Context, post *model.Post, triggerWebhooks bool) {
 		channel = result.Data.(*model.Channel)
 	}
 
-	var profiles map[string]*model.User
+	go sendNotifications(c, post, team, channel)
-	if result := <-pchan; result.Err != nil {
-		l4g.Error(utils.T("api.post.handle_post_events_and_forget.profiles.error"), c.TeamId, result.Err)
-		return
-	} else {
-		profiles = result.Data.(map[string]*model.User)
-	}
-
-	if result := <-dpchan; result.Err != nil {
-		l4g.Error(utils.T("api.post.handle_post_events_and_forget.profiles.error"), c.TeamId, result.Err)
-		return
-	} else {
-		dps := result.Data.(map[string]*model.User)
-		for k, v := range dps {
-			profiles[k] = v
-		}
-	}
-
-	var members []model.ChannelMember
-	if result := <-mchan; result.Err != nil {
-		l4g.Error(utils.T("api.post.handle_post_events_and_forget.members.error"), post.ChannelId, result.Err)
-		return
-	} else {
-		members = result.Data.([]model.ChannelMember)
-	}
-
-	go sendNotifications(c, post, team, channel, profiles, members)
-	go checkForOutOfChannelMentions(c, post, channel, profiles, members)
 
 	var user *model.User
 	if result := <-uchan; result.Err != nil {
@@ -477,19 +447,139 @@ func handleWebhookEvents(c *Context, post *model.Post, team *model.Team, channel
 	}
 }
 
-func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *model.Channel, profileMap map[string]*model.User, members []model.ChannelMember) {
+// Given a map of user IDs to profiles and a map of user IDs of channel members, returns a list of mention
+// keywords for all users on the team. Users that are members of the channel will have all their mention
+// keywords returned while users that aren't in the channel will only have their @mentions returned.
+func getMentionKeywords(profiles map[string]*model.User, members map[string]string) map[string][]string {
+	keywords := make(map[string][]string)
+
+	for id, profile := range profiles {
+		_, inChannel := members[id]
+
+		if inChannel {
+			if len(profile.NotifyProps["mention_keys"]) > 0 {
+				// Add all the user's mention keys
+				splitKeys := strings.Split(profile.NotifyProps["mention_keys"], ",")
+				for _, k := range splitKeys {
+					// note that these are made lower case so that we can do a case insensitive check for them
+					key := strings.ToLower(k)
+					keywords[key] = append(keywords[key], id)
+				}
+			}
+
+			// If turned on, add the user's case sensitive first name
+			if profile.NotifyProps["first_name"] == "true" {
+				keywords[profile.FirstName] = append(keywords[profile.FirstName], profile.Id)
+			}
+
+			// Add @channel and @all to keywords if user has them turned on
+			if profile.NotifyProps["channel"] == "true" {
+				keywords["@channel"] = append(keywords["@channel"], profile.Id)
+				keywords["@all"] = append(keywords["@all"], profile.Id)
+			}
+		} else {
+			// user isn't in channel, so just look for @mentions
+			key := "@" + strings.ToLower(profile.Username)
+			keywords[key] = append(keywords[key], id)
+		}
+	}
+
+	return keywords
+}
+
+// Given a message and a map mapping mention keywords to the users who use them, returns a map of mentioned
+// users and whether or not @here was mentioned.
+func getExplicitMentions(message string, keywords map[string][]string) (map[string]bool, bool) {
+	mentioned := make(map[string]bool)
+	hereMentioned := false
+
+	addMentionedUsers := func(ids []string) {
+		for _, id := range ids {
+			mentioned[id] = true
+		}
+	}
+
+	for _, word := range strings.Fields(message) {
+		isMention := false
+
+		if word == "@here" {
+			hereMentioned = true
+		}
+
+		// Non-case-sensitive check for regular keys
+		if ids, match := keywords[strings.ToLower(word)]; match {
+			addMentionedUsers(ids)
+			isMention = true
+		}
+
+		// Case-sensitive check for first name
+		if ids, match := keywords[word]; match {
+			addMentionedUsers(ids)
+			isMention = true
+		}
+
+		if !isMention {
+			// No matches were found with the string split just on whitespace so try further splitting
+			// the message on punctuation
+			splitWords := strings.FieldsFunc(word, func(c rune) bool {
+				return model.SplitRunes[c]
+			})
+
+			for _, splitWord := range splitWords {
+				if splitWord == "@here" {
+					hereMentioned = true
+				}
+
+				// Non-case-sensitive check for regular keys
+				if ids, match := keywords[strings.ToLower(splitWord)]; match {
+					addMentionedUsers(ids)
+				}
+
+				// Case-sensitive check for first name
+				if ids, match := keywords[splitWord]; match {
+					addMentionedUsers(ids)
+				}
+			}
+		}
+	}
+
+	return mentioned, hereMentioned
+}
+
+func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *model.Channel) {
+	// get profiles for all users we could be mentioning
+	pchan := Srv.Store.User().GetProfiles(c.TeamId)
+	dpchan := Srv.Store.User().GetDirectProfiles(c.Session.UserId)
+
+	var profileMap map[string]*model.User
+	if result := <-pchan; result.Err != nil {
+		l4g.Error(utils.T("api.post.handle_post_events_and_forget.profiles.error"), c.TeamId, result.Err)
+		return
+	} else {
+		profileMap = result.Data.(map[string]*model.User)
+	}
+
+	if result := <-dpchan; result.Err != nil {
+		l4g.Error(utils.T("api.post.handle_post_events_and_forget.profiles.error"), c.TeamId, result.Err)
+		return
+	} else {
+		dps := result.Data.(map[string]*model.User)
+		for k, v := range dps {
+			profileMap[k] = v
+		}
+	}
+
 	if _, ok := profileMap[post.UserId]; !ok {
 		l4g.Error(utils.T("api.post.send_notifications_and_forget.user_id.error"), post.UserId)
 		return
 	}
 
 	mentionedUserIds := make(map[string]bool)
-	alwaysNotifyUserIds := []string{}
+	alwaysNotifyUserIds := []string{} // ???
 	hereNotification := false
 	updateMentionChans := []store.StoreChannel{}
 
 	if channel.Type == model.CHANNEL_DIRECT {
-
 		var otherUserId string
 		if userIds := strings.Split(channel.Name, "__"); userIds[0] == post.UserId {
 			otherUserId = userIds[1]
@@ -498,89 +588,25 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
 		}
 
 		mentionedUserIds[otherUserId] = true
-
 	} else {
-		// Find out who is a member of the channel, only keep those profiles
+		// using a map as a pseudo-set since we're checking for containment a lot
-		tempProfileMap := make(map[string]*model.User)
+		members := make(map[string]string)
-		for _, member := range members {
+		if result := <-Srv.Store.Channel().GetMembers(post.ChannelId); result.Err != nil {
-			if profile, ok := profileMap[member.UserId]; ok {
+			l4g.Error(utils.T("api.post.handle_post_events_and_forget.members.error"), post.ChannelId, result.Err)
-				tempProfileMap[member.UserId] = profile
+			return
+		} else {
+			for _, member := range result.Data.([]model.ChannelMember) {
+				members[member.UserId] = member.UserId
 			}
 		}
 
-		profileMap = tempProfileMap
+		keywords := getMentionKeywords(profileMap, members)
 
-		// Build map for keywords
+		// get users that are explicitly mentioned
-		keywordMap := make(map[string][]string)
+		var mentioned map[string]bool
-		for _, profile := range profileMap {
+		mentioned, hereNotification = getExplicitMentions(post.Message, keywords)
-			if len(profile.NotifyProps["mention_keys"]) > 0 {
-
-				// Add all the user's mention keys
-				splitKeys := strings.Split(profile.NotifyProps["mention_keys"], ",")
-				for _, k := range splitKeys {
-					keywordMap[k] = append(keywordMap[strings.ToLower(k)], profile.Id)
-				}
-			}
-
-			// If turned on, add the user's case sensitive first name
-			if profile.NotifyProps["first_name"] == "true" {
-				keywordMap[profile.FirstName] = append(keywordMap[profile.FirstName], profile.Id)
-			}
-
-			// Add @channel and @all to keywords if user has them turned on
-			if profile.NotifyProps["channel"] == "true" {
-				keywordMap["@channel"] = append(keywordMap["@channel"], profile.Id)
-				keywordMap["@all"] = append(keywordMap["@all"], profile.Id)
-			}
-
-			if profile.NotifyProps["push"] == model.USER_NOTIFY_ALL &&
-				(post.UserId != profile.Id || post.Props["from_webhook"] == "true") &&
-				!post.IsSystemMessage() {
-				alwaysNotifyUserIds = append(alwaysNotifyUserIds, profile.Id)
-			}
-		}
-
-		// Build a map as a list of unique user_ids that are mentioned in this post
-		splitF := func(c rune) bool {
-			return model.SplitRunes[c]
-		}
-		splitMessage := strings.Fields(post.Message)
-		var userIds []string
-		for _, word := range splitMessage {
-			if word == "@here" {
-				hereNotification = true
-				continue
-			}
-
-			// Non-case-sensitive check for regular keys
-			if ids, match := keywordMap[strings.ToLower(word)]; match {
-				userIds = append(userIds, ids...)
-			}
-
-			// Case-sensitive check for first name
-			if ids, match := keywordMap[word]; match {
-				userIds = append(userIds, ids...)
-			}
-
-			if len(userIds) == 0 {
-				// No matches were found with the string split just on whitespace so try further splitting
-				// the message on punctuation
-				splitWords := strings.FieldsFunc(word, splitF)
-
-				for _, splitWord := range splitWords {
-					// Non-case-sensitive check for regular keys
-					if ids, match := keywordMap[strings.ToLower(splitWord)]; match {
-						userIds = append(userIds, ids...)
-					}
-
-					// Case-sensitive check for first name
-					if ids, match := keywordMap[splitWord]; match {
-						userIds = append(userIds, ids...)
-					}
-				}
-			}
-		}
 
+		// get users that have comment thread mentions enabled
 		if len(post.RootId) > 0 {
 			if result := <-Srv.Store.Post().Get(post.RootId); result.Err != nil {
 				l4g.Error(utils.T("api.post.send_notifications_and_forget.comment_thread.error"), post.RootId, result.Err)
@@ -591,22 +617,41 @@ func sendNotifications(c *Context, post *model.Post, team *model.Team, channel *
 				for _, threadPost := range list.Posts {
 					profile := profileMap[threadPost.UserId]
 					if profile.NotifyProps["comments"] == "any" || (profile.NotifyProps["comments"] == "root" && threadPost.Id == list.Order[0]) {
-						userIds = append(userIds, threadPost.UserId)
+						mentioned[threadPost.UserId] = true
 					}
 				}
 			}
 		}
 
-		for _, userId := range userIds {
+		// prevent the user from mentioning themselves
-			if post.UserId == userId && post.Props["from_webhook"] != "true" {
+		if post.Props["from_webhook"] != "true" {
+			delete(mentioned, post.UserId)
+		}
+
+		outOfChannelMentions := make(map[string]bool)
+		for id := range mentioned {
+			if _, inChannel := members[id]; inChannel {
+				mentionedUserIds[id] = true
+			} else {
+				outOfChannelMentions[id] = true
+			}
+		}
+
+		go sendOutOfChannelMentions(c, post, profileMap, outOfChannelMentions)
+
+		// find which users in the channel are set up to always receive mobile notifications
+		for id := range members {
+			profile := profileMap[id]
+			if profile == nil {
+				l4g.Warn(utils.T("api.post.notification.member_profile.warn"), id)
 				continue
 			}
 
-			mentionedUserIds[userId] = true
+			if profile.NotifyProps["push"] == model.USER_NOTIFY_ALL &&
-		}
+				(post.UserId != profile.Id || post.Props["from_webhook"] == "true") &&
-
+				!post.IsSystemMessage() {
-		for id := range mentionedUserIds {
+				alwaysNotifyUserIds = append(alwaysNotifyUserIds, profile.Id)
-			updateMentionChans = append(updateMentionChans, Srv.Store.Channel().IncrementMentionCount(post.ChannelId, id))
+			}
 		}
 	}
 
@@ -882,20 +927,14 @@ func sendPushNotification(post *model.Post, user *model.User, channel *model.Cha
 	}
 }
 
-func checkForOutOfChannelMentions(c *Context, post *model.Post, channel *model.Channel, allProfiles map[string]*model.User, members []model.ChannelMember) {
+func sendOutOfChannelMentions(c *Context, post *model.Post, profiles map[string]*model.User, outOfChannelMentions map[string]bool) {
-	// don't check for out of channel mentions in direct channels
+	if len(outOfChannelMentions) == 0 {
-	if channel.Type == model.CHANNEL_DIRECT {
 		return
 	}
 
-	mentioned := getOutOfChannelMentions(post, allProfiles, members)
+	var usernames []string
-	if len(mentioned) == 0 {
+	for id := range outOfChannelMentions {
-		return
+		usernames = append(usernames, profiles[id].Username)
-	}
-
-	usernames := make([]string, len(mentioned))
-	for i, user := range mentioned {
-		usernames[i] = user.Username
 	}
 	sort.Strings(usernames)
 
@@ -922,32 +961,6 @@ func checkForOutOfChannelMentions(c *Context, post *model.Post, channel *model.C
 	)
 }
 
-// Gets a list of users that were mentioned in a given post that aren't in the channel that the post was made in
-func getOutOfChannelMentions(post *model.Post, allProfiles map[string]*model.User, members []model.ChannelMember) []*model.User {
-	// copy the profiles map since we'll be removing items from it
-	profiles := make(map[string]*model.User)
-	for id, profile := range allProfiles {
-		profiles[id] = profile
-	}
-
-	// only keep profiles which aren't in the current channel
-	for _, member := range members {
-		delete(profiles, member.UserId)
-	}
-
-	var mentioned []*model.User
-
-	for _, profile := range profiles {
-		if pattern, err := regexp.Compile(`(\W|^)@` + regexp.QuoteMeta(profile.Username) + `(\W|$)`); err != nil {
-			l4g.Error(utils.T("api.post.get_out_of_channel_mentions.regex.error"), profile.Id, err)
-		} else if pattern.MatchString(post.Message) {
-			mentioned = append(mentioned, profile)
-		}
-	}
-
-	return mentioned
-}
-
 func SendEphemeralPost(teamId, userId string, post *model.Post) {
 	post.Type = model.POST_EPHEMERAL
 

api/post_test.go

@@ -11,7 +11,6 @@ import (
 	"reflect"
 	"strings"
 
-	"fmt"
 	"testing"
 	"time"
 
@@ -846,82 +845,271 @@ func TestMakeDirectChannelVisible(t *testing.T) {
 	}
 }
 
-func TestGetOutOfChannelMentions(t *testing.T) {
+func TestGetMentionKeywords(t *testing.T) {
-	th := Setup().InitBasic()
+	// user with username or custom mentions enabled
-	Client := th.BasicClient
+	user1 := &model.User{
-	channel1 := th.BasicChannel
+		Id:        model.NewId(),
-	team1 := th.BasicTeam
+		FirstName: "First",
-	user1 := th.BasicUser
+		Username:  "User",
-	user2 := th.BasicUser2
+		NotifyProps: map[string]string{
-	user3 := th.CreateUser(Client)
+			"mention_keys": "User,@User,MENTION",
-	LinkUserToTeam(user3, team1)
+		},
-
-	var allProfiles map[string]*model.User
-	if result := <-Srv.Store.User().GetProfiles(team1.Id); result.Err != nil {
-		t.Fatal(result.Err)
-	} else {
-		allProfiles = result.Data.(map[string]*model.User)
 	}
 
-	var members []model.ChannelMember
+	profiles := map[string]*model.User{user1.Id: user1}
-	if result := <-Srv.Store.Channel().GetMembers(channel1.Id); result.Err != nil {
+	members := map[string]string{user1.Id: user1.Id}
-		t.Fatal(result.Err)
+	mentions := getMentionKeywords(profiles, members)
-	} else {
+	if len(mentions) != 3 {
-		members = result.Data.([]model.ChannelMember)
+		t.Fatal("should've returned two mention keywords")
+	} else if ids, ok := mentions["user"]; !ok || ids[0] != user1.Id {
+		t.Fatal("should've returned mention key of user")
+	} else if ids, ok := mentions["@user"]; !ok || ids[0] != user1.Id {
+		t.Fatal("should've returned mention key of @user")
+	} else if ids, ok := mentions["mention"]; !ok || ids[0] != user1.Id {
+		t.Fatal("should've returned mention key of mention")
 	}
 
-	// test a post that doesn't @mention anybody
+	// user with first name mention enabled
-	post1 := &model.Post{ChannelId: channel1.Id, Message: fmt.Sprintf("%v %v %v", user1.Username, user2.Username, user3.Username)}
+	user2 := &model.User{
-	if mentioned := getOutOfChannelMentions(post1, allProfiles, members); len(mentioned) != 0 {
+		Id:        model.NewId(),
-		t.Fatalf("getOutOfChannelMentions returned %v when no users were mentioned", mentioned)
+		FirstName: "First",
+		Username:  "User",
+		NotifyProps: map[string]string{
+			"first_name": "true",
+		},
 	}
 
-	// test a post that @mentions someone in the channel
+	profiles = map[string]*model.User{user2.Id: user2}
-	post2 := &model.Post{ChannelId: channel1.Id, Message: fmt.Sprintf("@%v is %v", user1.Username, user1.Username)}
+	members = map[string]string{user2.Id: user2.Id}
-	if mentioned := getOutOfChannelMentions(post2, allProfiles, members); len(mentioned) != 0 {
+	mentions = getMentionKeywords(profiles, members)
-		t.Fatalf("getOutOfChannelMentions returned %v when only users in the channel were mentioned", mentioned)
+	if len(mentions) != 1 {
+		t.Fatal("should've returned one mention keyword")
+	} else if ids, ok := mentions["First"]; !ok || ids[0] != user2.Id {
+		t.Fatal("should've returned mention key of First")
 	}
 
-	// test a post that @mentions someone not in the channel
+	// user with @channel/@all mentions enabled
-	post3 := &model.Post{ChannelId: channel1.Id, Message: fmt.Sprintf("@%v and @%v aren't in the channel", user2.Username, user3.Username)}
+	user3 := &model.User{
-	if mentioned := getOutOfChannelMentions(post3, allProfiles, members); len(mentioned) != 2 || (mentioned[0].Id != user2.Id && mentioned[0].Id != user3.Id) || (mentioned[1].Id != user2.Id && mentioned[1].Id != user3.Id) {
+		Id:        model.NewId(),
-		t.Fatalf("getOutOfChannelMentions returned %v when two users outside the channel were mentioned", mentioned)
+		FirstName: "First",
+		Username:  "User",
+		NotifyProps: map[string]string{
+			"channel": "true",
+		},
 	}
 
-	// test a post that @mentions someone not in the channel as well as someone in the channel
+	profiles = map[string]*model.User{user3.Id: user3}
-	post4 := &model.Post{ChannelId: channel1.Id, Message: fmt.Sprintf("@%v and @%v might be in the channel", user2.Username, user1.Username)}
+	members = map[string]string{user3.Id: user3.Id}
-	if mentioned := getOutOfChannelMentions(post4, allProfiles, members); len(mentioned) != 1 || mentioned[0].Id != user2.Id {
+	mentions = getMentionKeywords(profiles, members)
-		t.Fatalf("getOutOfChannelMentions returned %v when someone in the channel and someone  outside the channel were mentioned", mentioned)
+	if len(mentions) != 2 {
+		t.Fatal("should've returned two mention keywords")
+	} else if ids, ok := mentions["@channel"]; !ok || ids[0] != user3.Id {
+		t.Fatal("should've returned mention key of @channel")
+	} else if ids, ok := mentions["@all"]; !ok || ids[0] != user3.Id {
+		t.Fatal("should've returned mention key of @all")
 	}
 
-	Client.Must(Client.Logout())
+	// user with all types of mentions enabled
-
+	user4 := &model.User{
-	team2 := th.CreateTeam(Client)
+		Id:        model.NewId(),
-	user4 := th.CreateUser(Client)
+		FirstName: "First",
-	LinkUserToTeam(user4, team2)
+		Username:  "User",
-
+		NotifyProps: map[string]string{
-	Client.Must(Client.Login(user4.Email, user4.Password))
+			"mention_keys": "User,@User,MENTION",
-	Client.SetTeamId(team2.Id)
+			"first_name":   "true",
-
+			"channel":      "true",
-	channel2 := &model.Channel{DisplayName: "Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team2.Id}
+		},
-	channel2 = Client.Must(Client.CreateChannel(channel2)).Data.(*model.Channel)
-
-	if result := <-Srv.Store.User().GetProfiles(team2.Id); result.Err != nil {
-		t.Fatal(result.Err)
-	} else {
-		allProfiles = result.Data.(map[string]*model.User)
 	}
 
-	if result := <-Srv.Store.Channel().GetMembers(channel2.Id); result.Err != nil {
+	profiles = map[string]*model.User{user4.Id: user4}
-		t.Fatal(result.Err)
+	members = map[string]string{user4.Id: user4.Id}
-	} else {
+	mentions = getMentionKeywords(profiles, members)
-		members = result.Data.([]model.ChannelMember)
+	if len(mentions) != 6 {
+		t.Fatal("should've returned six mention keywords")
+	} else if ids, ok := mentions["user"]; !ok || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of user")
+	} else if ids, ok := mentions["@user"]; !ok || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of @user")
+	} else if ids, ok := mentions["mention"]; !ok || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of mention")
+	} else if ids, ok := mentions["First"]; !ok || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of First")
+	} else if ids, ok := mentions["@channel"]; !ok || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of @channel")
+	} else if ids, ok := mentions["@all"]; !ok || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of @all")
 	}
 
-	// test a post that @mentions someone on a different team
+	// multiple users
-	post5 := &model.Post{ChannelId: channel2.Id, Message: fmt.Sprintf("@%v and @%v might be in the channel", user2.Username, user3.Username)}
+	profiles = map[string]*model.User{
-	if mentioned := getOutOfChannelMentions(post5, allProfiles, members); len(mentioned) != 0 {
+		user1.Id: user1,
-		t.Fatalf("getOutOfChannelMentions returned %v when two users on a different team were mentioned", mentioned)
+		user2.Id: user2,
+		user3.Id: user3,
+		user4.Id: user4,
+	}
+	members = map[string]string{
+		user1.Id: user1.Id,
+		user2.Id: user2.Id,
+		user3.Id: user3.Id,
+		user4.Id: user4.Id,
+	}
+	mentions = getMentionKeywords(profiles, members)
+	if len(mentions) != 6 {
+		t.Fatal("should've returned six mention keywords")
+	} else if ids, ok := mentions["user"]; !ok || len(ids) != 2 || (ids[0] != user1.Id && ids[1] != user1.Id) || (ids[0] != user4.Id && ids[1] != user4.Id) {
+		t.Fatal("should've mentioned user1 and user4 with user")
+	} else if ids, ok := mentions["@user"]; !ok || len(ids) != 2 || (ids[0] != user1.Id && ids[1] != user1.Id) || (ids[0] != user4.Id && ids[1] != user4.Id) {
+		t.Fatal("should've mentioned user1 and user4 with @user")
+	} else if ids, ok := mentions["mention"]; !ok || len(ids) != 2 || (ids[0] != user1.Id && ids[1] != user1.Id) || (ids[0] != user4.Id && ids[1] != user4.Id) {
+		t.Fatal("should've mentioned user1 and user4 with mention")
+	} else if ids, ok := mentions["First"]; !ok || len(ids) != 2 || (ids[0] != user2.Id && ids[1] != user2.Id) || (ids[0] != user4.Id && ids[1] != user4.Id) {
+		t.Fatal("should've mentioned user2 and user4 with mention")
+	} else if ids, ok := mentions["@channel"]; !ok || len(ids) != 2 || (ids[0] != user3.Id && ids[1] != user3.Id) || (ids[0] != user4.Id && ids[1] != user4.Id) {
+		t.Fatal("should've mentioned user3 and user4 with @channel")
+	} else if ids, ok := mentions["@all"]; !ok || len(ids) != 2 || (ids[0] != user3.Id && ids[1] != user3.Id) || (ids[0] != user4.Id && ids[1] != user4.Id) {
+		t.Fatal("should've mentioned user3 and user4 with @all")
+	}
+
+	// a user that's not in the channel
+	profiles = map[string]*model.User{user4.Id: user4}
+	members = map[string]string{}
+	mentions = getMentionKeywords(profiles, members)
+	if len(mentions) != 1 {
+		t.Fatal("should've returned one mention keyword")
+	} else if ids, ok := mentions["@user"]; !ok || len(ids) != 1 || ids[0] != user4.Id {
+		t.Fatal("should've returned mention key of @user")
+	}
+}
+
+func TestGetExplicitMentionsAtHere(t *testing.T) {
+	// test all the boundary cases that we know can break up terms (and those that we know won't)
+	cases := map[string]bool{
+		"":          false,
+		"here":      false,
+		"@here":     true,
+		" @here ":   true,
+		"\t@here\t": true,
+		"\n@here\n": true,
+		// "!@here!": true,
+		// "@@here@": true,
+		// "#@here#": true,
+		// "$@here$": true,
+		// "%@here%": true,
+		// "^@here^": true,
+		// "&@here&": true,
+		// "*@here*": true,
+		"(@here(": true,
+		")@here)": true,
+		// "-@here-": true,
+		// "_@here_": true,
+		// "=@here=": true,
+		"+@here+":   true,
+		"[@here[":   true,
+		"{@here{":   true,
+		"]@here]":   true,
+		"}@here}":   true,
+		"\\@here\\": true,
+		// "|@here|": true,
+		";@here;": true,
+		":@here:": true,
+		// "'@here'": true,
+		// "\"@here\"": true,
+		",@here,": true,
+		"<@here<": true,
+		".@here.": true,
+		">@here>": true,
+		"/@here/": true,
+		"?@here?": true,
+		// "`@here`": true,
+		// "~@here~": true,
+	}
+
+	for message, shouldMention := range cases {
+		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)
+		}
+	}
+
+	// mentioning @here and someone
+	id := model.NewId()
+	if mentions, hereMentioned := getExplicitMentions("@here @user", 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\"")
+	}
+}
+
+func TestGetExplicitMentions(t *testing.T) {
+	id1 := model.NewId()
+	id2 := model.NewId()
+
+	// not mentioning anybody
+	message := "this is a message"
+	keywords := map[string][]string{}
+	if mentions, _ := getExplicitMentions(message, keywords); len(mentions) != 0 {
+		t.Fatal("shouldn't have mentioned anybody")
+	}
+
+	// mentioning a user that doesn't exist
+	message = "this is a message for @user"
+	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] {
+		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] {
+		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] {
+		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] {
+		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] {
+		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] {
+		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] {
+		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] {
+		t.Fatal("should've mentioned user.period and not user")
 	}
 }
 

i18n/en.json

@@ -1095,6 +1095,10 @@
     "id": "api.post.notification.here.warn",
     "translation": "Unable to send notification to online users with @here, err=%v"
   },
+  {
+    "id": "api.post.notification.member_profile.warn",
+    "translation": "Unable to get profile for channel member, user_id=%v"
+  },
   {
     "id": "api.post.send_notifications_and_forget.comment_thread.error",
     "translation": "Failed to retrieve comment thread posts in notifications root_post_id=%v, err=%v"