notifications: Add link to new location of single moved messages.

Improve the Notification Bot by adding a hyperlink to the new location
of a moved single message. The link will make it easier for users to
find the message in its new context.

Fixes #24604.
This commit is contained in:
Akarsh Jain 2023-03-12 22:42:10 +05:30 committed by Tim Abbott
parent d0dbdfa52d
commit f122516e7d
2 changed files with 45 additions and 5 deletions

View File

@ -50,6 +50,7 @@ from zerver.lib.topic import (
update_messages_for_topic_edit,
)
from zerver.lib.types import EditHistoryEvent
from zerver.lib.url_encoding import near_stream_message_url
from zerver.lib.user_message import UserMessageLite, bulk_insert_ums
from zerver.lib.user_topics import get_users_with_user_topic_visibility_policy
from zerver.lib.widget import is_widget_message
@ -187,6 +188,7 @@ def maybe_send_resolve_topic_notifications(
def send_message_moved_breadcrumbs(
target_message: Message,
user_profile: UserProfile,
old_stream: Stream,
old_topic: str,
@ -207,6 +209,13 @@ def send_message_moved_breadcrumbs(
user_mention = silent_mention_syntax_for_user(user_profile)
old_topic_link = f"#**{old_stream.name}>{old_topic}**"
new_topic_link = f"#**{new_stream.name}>{new_topic}**"
message = {
"id": target_message.id,
"stream_id": new_stream.id,
"display_recipient": new_stream.name,
"topic": new_topic,
}
moved_message_link = near_stream_message_url(target_message.realm, message)
if new_thread_notification_string is not None:
with override_language(new_stream.realm.default_language):
@ -215,6 +224,7 @@ def send_message_moved_breadcrumbs(
new_stream,
new_topic,
new_thread_notification_string.format(
message_link=moved_message_link,
old_location=old_topic_link,
user=user_mention,
changed_messages_count=changed_messages_count,
@ -925,7 +935,7 @@ def do_update_message(
else:
if changed_messages_count == 1:
new_thread_notification_string = gettext_lazy(
"A message was moved here from {old_location} by {user}."
"[A message]({message_link}) was moved here from {old_location} by {user}."
)
else:
new_thread_notification_string = gettext_lazy(
@ -933,6 +943,7 @@ def do_update_message(
)
send_message_moved_breadcrumbs(
target_message,
user_profile,
stream_being_edited,
orig_topic_name,

View File

@ -21,6 +21,7 @@ from zerver.lib.message import MessageDict, has_message_access, messages_for_ids
from zerver.lib.test_classes import ZulipTestCase, get_topic_messages
from zerver.lib.test_helpers import cache_tries_captured, queries_captured
from zerver.lib.topic import RESOLVED_TOPIC_PREFIX, TOPIC_NAME
from zerver.lib.url_encoding import near_stream_message_url
from zerver.lib.user_topics import (
get_users_with_user_topic_visibility_policy,
set_topic_visibility_policy,
@ -2322,11 +2323,18 @@ class EditMessageTest(EditMessageTestCase):
)
messages = get_topic_messages(user_profile, new_stream, "test")
message = {
"id": msg_id_later,
"stream_id": new_stream.id,
"display_recipient": new_stream.name,
"topic": "test",
}
moved_message_link = near_stream_message_url(messages[1].realm, message)
self.assert_length(messages, 2)
self.assertEqual(messages[0].id, msg_id_later)
self.assertEqual(
messages[1].content,
f"A message was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
f"[A message]({moved_message_link}) was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_move_message_to_preexisting_topic_change_one(self) -> None:
@ -2360,11 +2368,18 @@ class EditMessageTest(EditMessageTestCase):
)
messages = get_topic_messages(user_profile, new_stream, "test")
message = {
"id": msg_id_later,
"stream_id": new_stream.id,
"display_recipient": new_stream.name,
"topic": "test",
}
moved_message_link = near_stream_message_url(messages[2].realm, message)
self.assert_length(messages, 3)
self.assertEqual(messages[0].id, msg_id_later)
self.assertEqual(
messages[2].content,
f"A message was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
f"[A message]({moved_message_link}) was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_move_message_to_stream_change_all(self) -> None:
@ -3130,11 +3145,18 @@ class EditMessageTest(EditMessageTestCase):
self.assertEqual(messages[1].content, "Third")
messages = get_topic_messages(user_profile, stream, "edited")
message = {
"id": msg_id,
"stream_id": stream.id,
"display_recipient": stream.name,
"topic": "edited",
}
moved_message_link = near_stream_message_url(messages[1].realm, message)
self.assert_length(messages, 2)
self.assertEqual(messages[0].content, "First")
self.assertEqual(
messages[1].content,
f"A message was moved here from #**public stream>test** by @_**Iago|{user_profile.id}**.",
f"[A message]({moved_message_link}) was moved here from #**public stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_notify_old_topics_after_message_move(self) -> None:
@ -3206,11 +3228,18 @@ class EditMessageTest(EditMessageTestCase):
)
messages = get_topic_messages(user_profile, stream, "edited")
message = {
"id": msg_id,
"stream_id": stream.id,
"display_recipient": stream.name,
"topic": "edited",
}
moved_message_link = near_stream_message_url(messages[0].realm, message)
self.assert_length(messages, 2)
self.assertEqual(messages[0].content, "First")
self.assertEqual(
messages[1].content,
f"A message was moved here from #**public stream>test** by @_**Iago|{user_profile.id}**.",
f"[A message]({moved_message_link}) was moved here from #**public stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_notify_no_topic_after_message_move(self) -> None: