MM-23211: Fix user not being notified in out of channel mentions (#14097)

Automatic Merge

app/notification.go

@@ -829,6 +829,13 @@ func (m *ExplicitMentions) processText(text string, keywords map[string][]string
 		}
 
 		if _, ok := systemMentions[word]; !ok && strings.HasPrefix(word, "@") {
+			// No need to bother about unicode as we are looking for ASCII characters.
+			last := word[len(word)-1]
+			switch last {
+			// If the word is possibly at the end of a sentence, remove that character.
+			case '.', '-', ':':
+				word = word[:len(word)-1]
+			}
 			m.OtherPotentialMentions = append(m.OtherPotentialMentions, word[1:])
 		} else if strings.ContainsAny(word, ".-:") {
 			// This word contains a character that may be the end of a sentence, so split further

app/notification_test.go

@@ -816,63 +816,74 @@ func TestGetExplicitMentions(t *testing.T) {
 }
 
 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,
-		"\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:":   false, // This case shouldn't trigger a mention since it follows the format of reactions e.g. :word:
-		"'@here'":   true,
-		"\"@here\"": true,
-		",@here,":   true,
-		"<@here<":   true,
-		".@here.":   true,
-		">@here>":   true,
-		"/@here/":   true,
-		"?@here?":   true,
-		"`@here`":   false, // This case shouldn't mention since it's a code block
-		"~@here~":   true,
-		"@HERE":     true,
-		"@hERe":     true,
-	}
+	t.Run("Boundary cases", func(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,
+			"\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:":   false, // This case shouldn't trigger a mention since it follows the format of reactions e.g. :word:
+			"'@here'":   true,
+			"\"@here\"": true,
+			",@here,":   true,
+			"<@here<":   true,
+			".@here.":   true,
+			">@here>":   true,
+			"/@here/":   true,
+			"?@here?":   true,
+			"`@here`":   false, // This case shouldn't mention since it's a code block
+			"~@here~":   true,
+			"@HERE":     true,
+			"@hERe":     true,
+		}
+		for message, shouldMention := range cases {
+			post := &model.Post{Message: message}
+			m := getExplicitMentions(post, nil)
+			require.False(t, m.HereMentioned && !shouldMention, "shouldn't have mentioned @here with \"%v\"")
+			require.False(t, !m.HereMentioned && shouldMention, "should've mentioned @here with \"%v\"")
+		}
+	})
 
-	for message, shouldMention := range cases {
-		post := &model.Post{Message: message}
-		m := getExplicitMentions(post, nil)
-		require.False(t, m.HereMentioned && !shouldMention, "shouldn't have mentioned @here with \"%v\"")
-		require.False(t, !m.HereMentioned && shouldMention, "should've mentioned @here with \"%v\"")
-	}
+	t.Run("Mention @here and someone", func(t *testing.T) {
+		id := model.NewId()
+		m := getExplicitMentions(&model.Post{Message: "@here @user @potential"}, map[string][]string{"@user": {id}})
+		require.True(t, m.HereMentioned, "should've mentioned @here with \"@here @user\"")
+		require.Len(t, m.Mentions, 1)
+		require.Equal(t, KeywordMention, m.Mentions[id], "should've mentioned @user with \"@here @user\"")
+		require.Equal(t, len(m.OtherPotentialMentions), 1, "should've potential mentions for @potential")
+		assert.Equal(t, "potential", m.OtherPotentialMentions[0])
+	})
+
+	t.Run("Username ending with period", func(t *testing.T) {
+		id := model.NewId()
+		m := getExplicitMentions(&model.Post{Message: "@potential. test"}, map[string][]string{"@user": {id}})
+		require.Equal(t, len(m.OtherPotentialMentions), 1, "should've potential mentions for @potential")
+		assert.Equal(t, "potential", m.OtherPotentialMentions[0])
+	})
 
-	// mentioning @here and someone
-	id := model.NewId()
-	m := getExplicitMentions(&model.Post{Message: "@here @user @potential"}, map[string][]string{"@user": {id}})
-	require.True(t, m.HereMentioned, "should've mentioned @here with \"@here @user\"")
-	require.Len(t, m.Mentions, 1)
-	require.Equal(t, KeywordMention, m.Mentions[id], "should've mentioned @user with \"@here @user\"")
-	require.LessOrEqual(t, len(m.OtherPotentialMentions), 1, "should've potential mentions for @potential")
 }
 
 func TestAllowChannelMentions(t *testing.T) {