markdown: Convert topic links generated by "#-mentions" to permalinks.

This commit converts the links generated by the markdown
of the "#-mention" of topics to permalinks -- the links containing
the "with" narrow operator, the operand being the last message
of the channel and topic of the mention.

Fixes part of #21505

api_docs/changelog.md

@@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 10.0
 
+**Feature level 347**
+
+* [Markdown message formatting](/api/message-formatting#links-to-channels-topics-and-messages):
+  Links to topic without a specified message now use the `with`
+  operator to follow moves of topics.
+
 **Feature level 346**
 
 * [Markdown message formatting](/api/message-formatting#links-to-channels-topics-and-messages):

api_docs/message-formatting.md

@@ -38,7 +38,7 @@ Sample HTML formats are as follows:
 
 <!-- Syntax: #**announce>Zulip updates** -->
 <a class="stream-topic" data-stream-id="9"
-  href="/#narrow/channel/9-announce/topic/Zulip.20updates">
+  href="/#narrow/channel/9-announce/topic/Zulip.20updates/with/214">
  #announce &gt; Zulip updates
 </a>
 
@@ -47,9 +47,27 @@ Sample HTML formats are as follows:
   href="/#narrow/channel/9-announce/topic/Zulip.20updates/near/214">
  #announce &gt; Zulip updates @ 💬
 </a>
+
+<!-- Syntax: #**announce>Zulip updates**
+     Generated only if topic is empty or the link was rendered before
+     Zulip 10.0 (feature level 347) -->
+<a class="stream-topic" data-stream-id="9"
+  href="/#narrow/channel/9-announce/topic/Zulip.20updates">
+ #announce &gt; Zulip updates
+</a>
 ```
 
-The older stream/topic elements include a `data-stream-id`, which
+The `near` and `with` operators are documented in more detail in the
+[search and URL documentation](/api/construct-narrow). When rendering
+topic links with the `with` operator, the code doing the rendering may
+pick the ID arbitrarily among messages accessible to the client and/or
+acting user at the time of rendering. Currently, the server chooses
+the message ID to use for `with` operators as the oldest message ID in
+the topic accessible to the user who wrote the message. In channels
+with protected history, this means the same Markdown syntax may be
+rendered differently for users who joined at different times.
+
+The older stream/topic link elements include a `data-stream-id`, which
 historically was used in order to display the current channel name if
 the channel had been renamed. That field is **deprecated**, because
 displaying an updated value for the most common forms of this syntax
@@ -64,7 +82,7 @@ are as follows:
 ```html
 <!-- Syntax: #**announce>** -->
 <a class="stream-topic" data-stream-id="9"
-  href="/#narrow/channel/9-announce/topic/">
+  href="/#narrow/channel/9-announce/topic/with/214">
  #announce &gt; <em>general chat</em>
 </a>
 
@@ -73,9 +91,21 @@ are as follows:
   href="/#narrow/channel/9-announce/topic//near/214">
  #announce &gt; <em>general chat</em> @ 💬
 </a>
+
+<!-- Syntax: #**announce>**
+     Generated only if topic is empty or the link was rendered before
+     Zulip 10.0 (feature level 347) -->
+<a class="stream-topic" data-stream-id="9"
+  href="/#narrow/channel/9-announce/topic/">
+ #announce &gt; <em>general chat</em>
+</a>
 ```
 
-**Changes**: Before Zulip 10.0 (feature level 346), empty string
+**Changes**: Before Zulip 10.0 (feature level 347), the `with` field
+was never used in topic link URLs generated by the server; the markup
+currently used only for empty topics was used for all topic links.
+
+Before Zulip 10.0 (feature level 346), empty string
 was not a valid topic name in syntaxes for linking to topics and
 messages.
 

version.py

@@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
 # new level means in api_docs/changelog.md, as well as "**Changes**"
 # entries in the endpoint's documentation in `zulip.yaml`.
 
-API_FEATURE_LEVEL = 346  # Last bumped for topic="" support in link to topics/messages.
+API_FEATURE_LEVEL = 347  # Last bumped for /with/ in topic links.
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/markdown/__init__.py

@@ -48,6 +48,7 @@ from zerver.lib.markdown import fenced_code
 from zerver.lib.markdown.fenced_code import FENCE_RE
 from zerver.lib.mention import (
     BEFORE_MENTION_ALLOWED_REGEX,
+    ChannelTopicInfo,
     FullNameInfo,
     MentionBackend,
     MentionData,
@@ -130,6 +131,7 @@ class DbData:
     active_realm_emoji: dict[str, EmojiInfo]
     sent_by_bot: bool
     stream_names: dict[str, int]
+    topic_info: dict[ChannelTopicInfo, int | None]
     translate_emoticons: bool
     user_upload_previews: dict[str, MarkdownImageMetadata]
 
@@ -2049,6 +2051,13 @@ class StreamPattern(StreamTopicMessageProcessor):
 
 
 class StreamTopicPattern(StreamTopicMessageProcessor):
+    def get_with_operand(self, channel_topic: ChannelTopicInfo) -> int | None:
+        db_data: DbData | None = self.zmd.zulip_db_data
+        if db_data is None:
+            return None
+        with_operand = db_data.topic_info.get(channel_topic)
+        return with_operand
+
     @override
     def handleMatch(  # type: ignore[override] # https://github.com/python/mypy/issues/10197
         self, m: Match[str], data: str
@@ -2064,7 +2073,13 @@ class StreamTopicPattern(StreamTopicMessageProcessor):
         el.set("data-stream-id", str(stream_id))
         stream_url = encode_stream(stream_id, stream_name)
         topic_url = hash_util_encode(topic_name)
-        link = f"/#narrow/channel/{stream_url}/topic/{topic_url}"
+        channel_topic_object = ChannelTopicInfo(stream_name, topic_name)
+        with_operand = self.get_with_operand(channel_topic_object)
+        if with_operand is not None:
+            link = f"/#narrow/channel/{stream_url}/topic/{topic_url}/with/{with_operand}"
+        else:
+            link = f"/#narrow/channel/{stream_url}/topic/{topic_url}"
+
         el.set("href", link)
 
         if topic_name == "":
@@ -2131,6 +2146,15 @@ def possible_linked_stream_names(content: str) -> set[str]:
     }
 
 
+def possible_linked_topics(content: str) -> set[ChannelTopicInfo]:
+    # Here, we do not consider STREAM_TOPIC_MESSAGE_LINK_REGEX, since
+    # our callers only want to process links without a message ID.
+    return {
+        ChannelTopicInfo(match.group("stream_name"), match.group("topic_name"))
+        for match in re.finditer(STREAM_TOPIC_LINK_REGEX, content, re.VERBOSE)
+    }
+
+
 class AlertWordNotificationProcessor(markdown.preprocessors.Preprocessor):
     allowed_before_punctuation = {" ", "\n", "(", '"', ".", ",", "'", ";", "[", "*", "`", ">"}
     allowed_after_punctuation = {
@@ -2753,6 +2777,9 @@ def do_convert(
             stream_names, acting_user=message_sender
         )
 
+        linked_stream_topic_data = possible_linked_topics(content)
+        topic_info = mention_data.get_topic_info_map(linked_stream_topic_data)
+
         if content_has_emoji_syntax(content):
             active_realm_emoji = get_name_keyed_dict_for_active_realm_emoji(message_realm.id)
         else:
@@ -2766,6 +2793,7 @@ def do_convert(
             realm_url=message_realm.url,
             sent_by_bot=sent_by_bot,
             stream_names=stream_name_info,
+            topic_info=topic_info,
             translate_emoticons=translate_emoticons,
             user_upload_previews=user_upload_previews,
         )

zerver/lib/mention.py

@@ -9,6 +9,7 @@ from django.db.models import Q
 from django_stubs_ext import StrPromise
 
 from zerver.lib.streams import filter_stream_authorization
+from zerver.lib.topic import get_first_message_for_user_in_topic
 from zerver.lib.user_groups import get_root_id_annotated_recursive_subgroups_for_groups
 from zerver.lib.users import get_inaccessible_user_ids
 from zerver.models import NamedUserGroup, UserProfile
@@ -67,6 +68,23 @@ class PossibleMentions:
     message_has_stream_wildcards: bool
 
 
+@dataclass(frozen=True)
+class ChannelTopicInfo:
+    channel_name: str
+    topic_name: str
+
+
+@dataclass
+class ChannelInfo:
+    channel_id: int
+    recipient_id: int
+    history_public_to_subscribers: bool
+    # TODO: Track whether the current user has only metadata access or
+    # content access, so that we can allow mentioning channels with
+    # only metadata access, while still enforcing content access to
+    # mention topics or messages within channels.
+
+
 class MentionBackend:
     # Be careful about reuse: MentionBackend contains caches which are
     # designed to only have the lifespan of a sender user (typically a
@@ -78,7 +96,8 @@ class MentionBackend:
     def __init__(self, realm_id: int) -> None:
         self.realm_id = realm_id
         self.user_cache: dict[tuple[int, str], FullNameInfo] = {}
-        self.stream_cache: dict[str, int] = {}
+        self.stream_cache: dict[str, ChannelInfo] = {}
+        self.topic_cache: dict[ChannelTopicInfo, int | None] = {}
 
     def get_full_name_info_list(
         self, user_filters: list[UserFilter], message_sender: UserProfile | None
@@ -152,7 +171,7 @@ class MentionBackend:
 
         for stream_name in stream_names:
             if stream_name in self.stream_cache:
-                result[stream_name] = self.stream_cache[stream_name]
+                result[stream_name] = self.stream_cache[stream_name].channel_id
             else:
                 unseen_stream_names.append(stream_name)
 
@@ -171,10 +190,14 @@ class MentionBackend:
                 .values(
                     "id",
                     "name",
+                    "recipient_id",
+                    "history_public_to_subscribers",
                 )
             )
             for row in rows:
-                self.stream_cache[row["name"]] = row["id"]
+                self.stream_cache[row["name"]] = ChannelInfo(
+                    row["id"], row["recipient_id"], row["history_public_to_subscribers"]
+                )
                 result[row["name"]] = row["id"]
         else:
             authorization = filter_stream_authorization(
@@ -189,11 +212,54 @@ class MentionBackend:
                 is_subscribing_other_users=False,
             )
             for stream in authorization.authorized_streams:
-                self.stream_cache[stream.name] = stream.id
+                assert stream.recipient_id is not None
+                self.stream_cache[stream.name] = ChannelInfo(
+                    stream.id, stream.recipient_id, stream.history_public_to_subscribers
+                )
                 result[stream.name] = stream.id
 
         return result
 
+    def get_topic_info_map(
+        self, channel_topics: set[ChannelTopicInfo], acting_user: UserProfile | None
+    ) -> dict[ChannelTopicInfo, int | None]:
+        if not channel_topics:
+            return {}
+
+        result: dict[ChannelTopicInfo, int | None] = {}
+        unseen_channel_topic: list[ChannelTopicInfo] = []
+
+        for channel_topic in channel_topics:
+            if channel_topic in self.topic_cache:
+                result[channel_topic] = self.topic_cache[channel_topic]
+            else:
+                unseen_channel_topic.append(channel_topic)
+
+        for channel_topic in unseen_channel_topic:
+            channel_info = self.stream_cache.get(channel_topic.channel_name)
+
+            if channel_info is None:
+                # The acting user does not have access to content in this channel.
+                continue
+
+            recipient_id = channel_info.recipient_id
+            topic_name = channel_topic.topic_name
+            history_public_to_subscribers = channel_info.history_public_to_subscribers
+
+            topic_latest_message = get_first_message_for_user_in_topic(
+                self.realm_id,
+                acting_user,
+                recipient_id,
+                topic_name,
+                history_public_to_subscribers,
+                acting_user_has_channel_content_access=True,
+            )
+
+            self.topic_cache[channel_topic] = topic_latest_message
+            result[channel_topic] = topic_latest_message
+
+        return result
+
 
 def user_mention_matches_topic_wildcard(mention: str) -> bool:
     return mention in topic_wildcards
@@ -271,6 +337,7 @@ class MentionData:
     ) -> None:
         self.mention_backend = mention_backend
         realm_id = mention_backend.realm_id
+        self.message_sender = message_sender
         mentions = possible_mentions(content)
         possible_mentions_info = get_possible_mentions_info(
             mention_backend, mentions.mention_texts, message_sender
@@ -356,7 +423,16 @@ class MentionData:
     def get_stream_name_map(
         self, stream_names: set[str], acting_user: UserProfile | None
     ) -> dict[str, int]:
-        return self.mention_backend.get_stream_name_map(stream_names, acting_user=acting_user)
+        return self.mention_backend.get_stream_name_map(
+            stream_names, acting_user=self.message_sender
+        )
+
+    def get_topic_info_map(
+        self, channel_topics: set[ChannelTopicInfo]
+    ) -> dict[ChannelTopicInfo, int | None]:
+        return self.mention_backend.get_topic_info_map(
+            channel_topics, acting_user=self.message_sender
+        )
 
 
 def silent_mention_syntax_for_user(user_profile: UserProfile) -> str:

zerver/lib/topic.py

@@ -78,6 +78,39 @@ def messages_for_topic(
     )
 
 
+def get_first_message_for_user_in_topic(
+    realm_id: int,
+    user_profile: UserProfile | None,
+    recipient_id: int,
+    topic_name: str,
+    history_public_to_subscribers: bool,
+    acting_user_has_channel_content_access: bool = False,
+) -> int | None:
+    # Guard against incorrectly calling this function without
+    # first checking for channel access.
+    assert acting_user_has_channel_content_access
+
+    if history_public_to_subscribers:
+        return (
+            messages_for_topic(realm_id, recipient_id, topic_name)
+            .values_list("id", flat=True)
+            .first()
+        )
+
+    elif user_profile is not None:
+        return (
+            UserMessage.objects.filter(
+                user_profile=user_profile,
+                message__recipient_id=recipient_id,
+                message__subject__iexact=topic_name,
+            )
+            .values_list("message_id", flat=True)
+            .first()
+        )
+
+    return None
+
+
 def save_message_for_edit_use_case(message: Message) -> None:
     message.save(
         update_fields=[

zerver/tests/test_markdown.py

@@ -3007,7 +3007,7 @@ class MarkdownMentionTest(ZulipTestCase):
         assert_silent_mention("```quote\n@_*backend*\n```")
 
 
-class MarkdownStreamMentionTests(ZulipTestCase):
+class MarkdownStreamTopicMentionTests(ZulipTestCase):
     def test_stream_single(self) -> None:
         denmark = get_stream("Denmark", get_realm("zulip"))
         sender_user_profile = self.example_user("othello")
@@ -3084,7 +3084,7 @@ class MarkdownStreamMentionTests(ZulipTestCase):
             "<p>#<strong>casesens</strong></p>",
         )
 
-    def test_topic_single(self) -> None:
+    def test_topic_single_containing_no_message(self) -> None:
         denmark = get_stream("Denmark", get_realm("zulip"))
         sender_user_profile = self.example_user("othello")
         msg = Message(
@@ -3104,6 +3104,23 @@ class MarkdownStreamMentionTests(ZulipTestCase):
             f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/">#{denmark.name} &gt; <em>{Message.EMPTY_TOPIC_FALLBACK_NAME}</em></a></p>',
         )
 
+    def test_topic_single_containing_message(self) -> None:
+        denmark = get_stream("Denmark", get_realm("zulip"))
+        sender_user_profile = self.example_user("othello")
+        first_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="some topic", content="test"
+        )
+        msg = Message(
+            sender=sender_user_profile,
+            sending_client=get_client("test"),
+            realm=sender_user_profile.realm,
+        )
+        content = "#**Denmark>some topic**"
+        self.assertEqual(
+            render_message_markdown(msg, content).rendered_content,
+            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/some.20topic/with/{first_message_id}">#{denmark.name} &gt; some topic</a></p>',
+        )
+
     def test_topic_atomic_string(self) -> None:
         realm = get_realm("zulip")
         # Create a linkifier.
@@ -3119,17 +3136,23 @@ class MarkdownStreamMentionTests(ZulipTestCase):
         )
         # Create a topic link that potentially interferes with the pattern.
         denmark = get_stream("Denmark", realm)
+        first_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="#1234", content="test"
+        )
         msg = Message(sender=sender_user_profile, sending_client=get_client("test"), realm=realm)
         content = "#**Denmark>#1234**"
         self.assertEqual(
             render_message_markdown(msg, content).rendered_content,
-            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/.231234">#{denmark.name} &gt; #1234</a></p>',
+            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/.231234/with/{first_message_id}">#{denmark.name} &gt; #1234</a></p>',
         )
 
     def test_topic_multiple(self) -> None:
         denmark = get_stream("Denmark", get_realm("zulip"))
         scotland = get_stream("Scotland", get_realm("zulip"))
         sender_user_profile = self.example_user("othello")
+        first_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="some topic", content="test"
+        )
         msg = Message(
             sender=sender_user_profile,
             sending_client=get_client("test"),
@@ -3140,7 +3163,7 @@ class MarkdownStreamMentionTests(ZulipTestCase):
             render_message_markdown(msg, content).rendered_content,
             "<p>This has two links: "
             f'<a class="stream-topic" data-stream-id="{denmark.id}" '
-            f'href="/#narrow/channel/{denmark.id}-{denmark.name}/topic/some.20topic">'
+            f'href="/#narrow/channel/{denmark.id}-{denmark.name}/topic/some.20topic/with/{first_message_id}">'
             f"#{denmark.name} &gt; some topic</a>"
             " and "
             f'<a class="stream-topic" data-stream-id="{scotland.id}" '
@@ -3149,6 +3172,94 @@ class MarkdownStreamMentionTests(ZulipTestCase):
             ".</p>",
         )
 
+    def test_topic_permalink(self) -> None:
+        realm = get_realm("zulip")
+        denmark = get_stream("Denmark", get_realm("zulip"))
+        sender_user_profile = self.example_user("othello")
+        first_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="some topic", content="test"
+        )
+
+        msg = Message(
+            sender=sender_user_profile,
+            sending_client=get_client("test"),
+            realm=sender_user_profile.realm,
+        )
+
+        # test caching of topic data for user.
+        content = "#**Denmark>some topic**"
+        mention_backend = MentionBackend(realm.id)
+        mention_data = MentionData(mention_backend, content, message_sender=None)
+        render_message_markdown(msg, content, mention_data=mention_data)
+
+        with (
+            self.assert_database_query_count(1, keep_cache_warm=True),
+            self.assert_memcached_count(0),
+        ):
+            self.assertEqual(
+                render_message_markdown(msg, content, mention_data=mention_data).rendered_content,
+                f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/some.20topic/with/{first_message_id}">#{denmark.name} &gt; some topic</a></p>',
+            )
+
+        # test topic linked doesn't have any message in it in case
+        # the topic mentioned doesn't have any messages.
+        content = "#**Denmark>random topic**"
+        self.assertEqual(
+            render_message_markdown(msg, content).rendered_content,
+            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/random.20topic">#{denmark.name} &gt; random topic</a></p>',
+        )
+
+        # Test when trying to render a topic link of a channel with shared
+        # history, if message_sender is None, topic link is permalink.
+        content = "#**Denmark>some topic**"
+        self.assertEqual(
+            markdown_convert_wrapper(content),
+            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/some.20topic/with/{first_message_id}">#{denmark.name} &gt; some topic</a></p>',
+        )
+
+        # test topic links for channel with protected history
+        core_stream = self.make_stream("core", realm, True, history_public_to_subscribers=False)
+        iago = self.example_user("iago")
+        hamlet = self.example_user("hamlet")
+
+        self.subscribe(iago, "core")
+        msg_id = self.send_stream_message(iago, "core", topic_name="testing")
+
+        msg = Message(
+            sender=iago,
+            sending_client=get_client("test"),
+            realm=realm,
+        )
+        content = "#**core>testing**"
+        self.assertEqual(
+            render_message_markdown(msg, content).rendered_content,
+            f'<p><a class="stream-topic" data-stream-id="{core_stream.id}" href="/#narrow/channel/{core_stream.id}-core/topic/testing/with/{msg_id}">#{core_stream.name} &gt; testing</a></p>',
+        )
+
+        # Test newly subscribed user to a channel with protected history
+        # won't have accessed to this message, and hence, the topic
+        # link would not be a permalink.
+        self.subscribe(hamlet, "core")
+
+        msg = Message(
+            sender=hamlet,
+            sending_client=get_client("test"),
+            realm=realm,
+        )
+        content = "#**core>testing**"
+        self.assertEqual(
+            render_message_markdown(msg, content).rendered_content,
+            f'<p><a class="stream-topic" data-stream-id="{core_stream.id}" href="/#narrow/channel/{core_stream.id}-core/topic/testing">#{core_stream.name} &gt; testing</a></p>',
+        )
+
+        # Test when trying to render a topic link of a channel with protected
+        # history, if message_sender is None, topic link is not permalink.
+        content = "#**core>testing**"
+        self.assertEqual(
+            markdown_convert_wrapper(content),
+            f'<p><a class="stream-topic" data-stream-id="{core_stream.id}" href="/#narrow/channel/{core_stream.id}-core/topic/testing">#{core_stream.name} &gt; testing</a></p>',
+        )
+
     def test_message_id_multiple(self) -> None:
         denmark = get_stream("Denmark", get_realm("zulip"))
         sender_user_profile = self.example_user("othello")

zerver/tests/test_message_move_stream.py

@@ -1201,7 +1201,7 @@ class MessageMoveStreamTest(ZulipTestCase):
             "iago", "test move stream", "new stream", "test"
         )
 
-        with self.assert_database_query_count(57), self.assert_memcached_count(14):
+        with self.assert_database_query_count(59), self.assert_memcached_count(14):
             result = self.client_patch(
                 f"/json/messages/{msg_id}",
                 {