topics: Server generated permalinks now prefer latest message id.

Previously, when a topic is mentioned, the server generated a
permalink using the earliest accessible message of the topic.

This commit updates it to rather use the latest message of the
topic.

api_docs/changelog.md

@@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 11.0
 
+**Feature level 400**
+
+* [Markdown message formatting](/api/message-formatting#links-to-channels-topics-and-messages):
+  The server now prefers the latest message in a topic, not the
+  oldest, when constructing topic permalinks using the `/with/` operator.
+
 **Feature level 399**
 
 * [`GET /events`](/api/get-events):

api_docs/message-formatting.md

@@ -62,10 +62,8 @@ The `near` and `with` operators are documented in more detail in the
 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 message ID to use for `with` operators as the latest message ID in
-the topic accessible to the user who wrote the message. In channels
+the topic accessible to the user who wrote the message.
-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
@@ -101,7 +99,11 @@ are as follows:
 </a>
 ```
 
-**Changes**: Before Zulip 10.0 (feature level 347), the `with` field
+**Changes**: In Zulip 11.0 (feature level 400), the server switched
+its strategy for `with` URL construction to choose the latest
+accessible message ID in a topic. Previously, it used the oldest.
+
+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.
 

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 = 399
+API_FEATURE_LEVEL = 400
 
 # 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/mention.py

@@ -10,7 +10,7 @@ from django.db.models import Q
 from django_stubs_ext import StrPromise
 
 from zerver.lib.streams import get_content_access_streams
-from zerver.lib.topic import get_first_message_for_user_in_topic
+from zerver.lib.topic import get_latest_message_for_user_in_topic
 from zerver.lib.types import UserDisplayRecipient
 from zerver.lib.user_groups import (
     UserGroupMembershipDetails,
@@ -255,7 +255,21 @@ class MentionBackend:
             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(
+            # Any message in the topic is a valid choice for the
+            # /with/ anchor. There are two risks to manage here:
+            # - The target message could be deleted or be a mispost that
+            #   is off-topic and moved shortly.
+            # - The topic could be split into two topics.
+            #
+            # Originally, we picked the oldest message because that
+            # message is least likely to be deleted/moved for being a
+            # mispost/error -- i.e., trying to do a bit better in rare
+            # corner cases. We switched to preferring the latest
+            # message in API feature level 400. The "latest" algorithm
+            # is better when linking to an active conversation in a
+            # long topic that's a follow-up/tangent and ends up being
+            # moved/split, which users do constantly.
+            topic_latest_message = get_latest_message_for_user_in_topic(
                 self.realm_id,
                 acting_user,
                 recipient_id,

zerver/lib/topic.py

@@ -79,7 +79,7 @@ def messages_for_topic(
     )
 
 
-def get_first_message_for_user_in_topic(
+def get_latest_message_for_user_in_topic(
     realm_id: int,
     user_profile: UserProfile | None,
     recipient_id: int,
@@ -95,7 +95,7 @@ def get_first_message_for_user_in_topic(
         return (
             messages_for_topic(realm_id, recipient_id, topic_name)
             .values_list("id", flat=True)
-            .first()
+            .last()
         )
 
     elif user_profile is not None:
@@ -107,7 +107,7 @@ def get_first_message_for_user_in_topic(
                 message__is_channel_message=True,
             )
             .values_list("message_id", flat=True)
-            .first()
+            .last()
         )
 
     return None

zerver/tests/test_markdown.py

@@ -3181,12 +3181,16 @@ class MarkdownStreamTopicMentionTests(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:
+    def test_topic_single_containing_messages(self) -> None:
         denmark = get_stream("Denmark", get_realm("zulip"))
         sender_user_profile = self.example_user("othello")
-        first_message_id = self.send_stream_message(
+        self.send_stream_message(
             sender_user_profile, "Denmark", topic_name="some topic", content="test"
         )
+        latest_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="some topic", content="test 2"
+        )
+
         msg = Message(
             sender=sender_user_profile,
             sending_client=get_client("test"),
@@ -3195,7 +3199,7 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
         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>',
+            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/some.20topic/with/{latest_message_id}">#{denmark.name} &gt; some topic</a></p>',
         )
 
     def test_topic_atomic_string(self) -> None:
@@ -3227,9 +3231,12 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
         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(
+        self.send_stream_message(
             sender_user_profile, "Denmark", topic_name="some topic", content="test"
         )
+        latest_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="some topic", content="test 2"
+        )
         msg = Message(
             sender=sender_user_profile,
             sending_client=get_client("test"),
@@ -3240,7 +3247,7 @@ class MarkdownStreamTopicMentionTests(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/with/{first_message_id}">'
+            f'href="/#narrow/channel/{denmark.id}-{denmark.name}/topic/some.20topic/with/{latest_message_id}">'
             f"#{denmark.name} &gt; some topic</a>"
             " and "
             f'<a class="stream-topic" data-stream-id="{scotland.id}" '
@@ -3253,9 +3260,12 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
         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(
+        self.send_stream_message(
             sender_user_profile, "Denmark", topic_name="some topic", content="test"
         )
+        latest_message_id = self.send_stream_message(
+            sender_user_profile, "Denmark", topic_name="some topic", content="test 2"
+        )
 
         msg = Message(
             sender=sender_user_profile,
@@ -3275,7 +3285,7 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
         ):
             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>',
+                f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/some.20topic/with/{latest_message_id}">#{denmark.name} &gt; some topic</a></p>',
             )
 
         # test topic linked doesn't have any message in it in case
@@ -3291,7 +3301,7 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase):
         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>',
+            f'<p><a class="stream-topic" data-stream-id="{denmark.id}" href="/#narrow/channel/{denmark.id}-Denmark/topic/some.20topic/with/{latest_message_id}">#{denmark.name} &gt; some topic</a></p>',
         )
 
         # test topic links for channel with protected history