markdown: Refactor backend logic for handling user mention.

Backend logic for handling user mention was cluttered
because it was handled at two stages first in
get_possible_mentions_info while fetching mention data
based on the messsage and then later in UserMentionPattern
which handles processing of text for mention.

Ideally UserMentionPattern should depend on
get_possible_mentions_info only for data but there was a
shared logic between these two that made it hard to debug
any possible bugs.

Updates in this commit make both of these functions
coherent in terms of logic and also add appropiate
comments to improve readability of these functions.

There was also a hidden bug that if a user A is
mentioned in with @**name|id** then @**invalid|id**
again mentioned A because of the way we handled mentions
earlier. It is solved as a result of this refactor and
appropiate test has been added for this.

This has been tested manually as well as by adding new
test to address missing case.

zerver/lib/markdown/__init__.py

@@ -1884,11 +1884,22 @@ class UserMentionPattern(markdown.inlinepatterns.InlineProcessor):
 
             wildcard = mention.user_mention_matches_wildcard(name)
 
-            id_syntax_match = re.match(r"(.+)?\|(?P<user_id>\d+)$", name)
+            # For @**|id** and @**name|id** mention syntaxes.
+            id_syntax_match = re.match(r"(?P<full_name>.+)?\|(?P<user_id>\d+)$", name)
             if id_syntax_match:
+                full_name = id_syntax_match.group("full_name")
                 id = int(id_syntax_match.group("user_id"))
                 user = db_data["mention_data"].get_user_by_id(id)
+
+                # For @**name|id**, we need to specifically check that
+                # name matches the full_name of user in mention_data.
+                # This enforces our decision that
+                # @**user_1_name|id_for_user_2** should be invalid syntax.
+                if full_name:
+                    if user and user["full_name"] != full_name:
+                        return None, None, None
             else:
+                # For @**name** syntax.
                 user = db_data["mention_data"].get_user_by_name(name)
 
             if wildcard:
@@ -2445,8 +2456,7 @@ def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[F
     if not mention_texts:
         return []
 
-    full_names = set()
-    mention_ids: Set[int] = set()
+    q_list = set()
 
     name_re = r"(?P<full_name>.+)?\|(?P<mention_id>\d+)$"
     for mention_text in mention_texts:
@@ -2455,15 +2465,15 @@ def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[F
             full_name = name_syntax_match.group("full_name")
             mention_id = name_syntax_match.group("mention_id")
             if full_name:
-                full_names.add(name_syntax_match.group("full_name"))
+                # For **name|id** mentions as mention_id
+                # cannot be null inside this block.
+                q_list.add(Q(full_name__iexact=full_name, id=mention_id))
             else:
-                mention_ids.add(int(mention_id))
+                # For **|id** syntax.
+                q_list.add(Q(id=mention_id))
         else:
-            full_names.add(mention_text)
-
-    q_list = {Q(full_name__iexact=full_name) for full_name in full_names}
-    id_q_list = {Q(id=id) for id in mention_ids}
-    q_list |= id_q_list
+            # For **name** syntax.
+            q_list.add(Q(full_name__iexact=mention_text))
 
     rows = (
         UserProfile.objects.filter(

zerver/tests/test_markdown.py

@@ -1867,6 +1867,24 @@ class MarkdownTest(ZulipTestCase):
         )
         self.assertEqual(msg.mentions_user_ids, {user_profile.id})
 
+    def test_invalid_mention_not_uses_valid_mention_data(self) -> None:
+        sender_user_profile = self.example_user("othello")
+        hamlet = self.example_user("hamlet")
+        msg = Message(sender=sender_user_profile, sending_client=get_client("test"))
+
+        # Even though King Hamlet will be present in mention data as
+        # it was was fetched for first mention but second mention is
+        # incorrect(as it uses hamlet's id) so it should not be able
+        # to use that data for creating a valid mention.
+
+        content = f"@**King Hamlet|10** and @**aaron|{hamlet.id}**"
+        self.assertEqual(
+            render_markdown(msg, content),
+            f'<p><span class="user-mention" data-user-id="{hamlet.id}">'
+            f"@King Hamlet</span> and @<strong>aaron|{hamlet.id}</strong></p>",
+        )
+        self.assertEqual(msg.mentions_user_ids, {hamlet.id})
+
     def test_silent_mention_invalid_followed_by_valid(self) -> None:
         sender_user_profile = self.example_user("othello")
         user_profile = self.example_user("hamlet")