From dadbba0c25935790f0637c963b2417bd378f64c3 Mon Sep 17 00:00:00 2001
From: m-e-l-u-h-a-n <purushottam.tiwari.cd.cse19@itbhu.ac.in>
Date: Thu, 18 Mar 2021 16:04:49 +0530
Subject: [PATCH] markdown: Fix invalid mention bug for user group mention.

Modifies `UserGroupMentionPattern` to inherit from InlineProcessor
instead of Pattern. This change is done because Pattern
stopped checking for matching patterns as soon as it found
a match which was not a valid user group. Due to this all
the subsequent user group mention failed, even if they were
valid. This bug was only present in backend renderring due to
markdown.inlinepatterns.Pattern.

This was reported as issue #17535.

These changes were tested locally in dev server and by adding
some new markdown tests to test these.
---
 zerver/lib/markdown/__init__.py | 16 +++++++++-------
 zerver/tests/test_markdown.py   | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py
index 67df4ea4f6..07aeb9b1a9 100644
--- a/zerver/lib/markdown/__init__.py
+++ b/zerver/lib/markdown/__init__.py
@@ -1844,11 +1844,13 @@ class UserMentionPattern(markdown.inlinepatterns.InlineProcessor):
         return None, None, None
 
 
-class UserGroupMentionPattern(markdown.inlinepatterns.Pattern):
-    def handleMatch(self, m: Match[str]) -> Optional[Element]:
-        match = m.group(2)
-
+class UserGroupMentionPattern(markdown.inlinepatterns.InlineProcessor):
+    def handleMatch(  # type: ignore[override] # supertype incompatible with supersupertype
+        self, m: Match[str], data: str
+    ) -> Union[Tuple[None, None, None], Tuple[Element, int, int]]:
+        match = m.group(1)
         db_data = self.md.zulip_db_data
+
         if self.md.zulip_message and db_data is not None:
             name = extract_user_group(match)
             user_group = db_data["mention_data"].get_user_group(name)
@@ -1859,15 +1861,15 @@ class UserGroupMentionPattern(markdown.inlinepatterns.Pattern):
             else:
                 # Don't highlight @-mentions that don't refer to a valid user
                 # group.
-                return None
+                return None, None, None
 
             el = Element("span")
             el.set("class", "user-group-mention")
             el.set("data-user-group-id", user_group_id)
             text = f"@{name}"
             el.text = markdown.util.AtomicString(text)
-            return el
-        return None
+            return el, m.start(), m.end()
+        return None, None, None
 
 
 class StreamPattern(CompiledPattern):
diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py
index 09a59deae5..e24444bde3 100644
--- a/zerver/tests/test_markdown.py
+++ b/zerver/tests/test_markdown.py
@@ -2056,6 +2056,27 @@ class MarkdownTest(ZulipTestCase):
         self.assertEqual(msg.mentions_user_ids, {user_profile.id})
         self.assertEqual(msg.mentions_user_group_ids, {user_group.id})
 
+    def test_invalid_user_group_followed_by_valid_mention_single(self) -> None:
+        sender_user_profile = self.example_user("othello")
+        user_profile = self.example_user("hamlet")
+        msg = Message(sender=sender_user_profile, sending_client=get_client("test"))
+        user_id = user_profile.id
+        user_group = self.create_user_group_for_test("support")
+
+        content = "@**King Hamlet** @*Invalid user group* @*support*"
+        self.assertEqual(
+            render_markdown(msg, content),
+            '<p><span class="user-mention" '
+            f'data-user-id="{user_id}">'
+            "@King Hamlet</span> "
+            "@<em>Invalid user group</em> "
+            '<span class="user-group-mention" '
+            f'data-user-group-id="{user_group.id}">'
+            "@support</span></p>",
+        )
+        self.assertEqual(msg.mentions_user_ids, {user_profile.id})
+        self.assertEqual(msg.mentions_user_group_ids, {user_group.id})
+
     def test_user_group_mention_atomic_string(self) -> None:
         sender_user_profile = self.example_user("othello")
         realm = get_realm("zulip")