message edit: Indicate how many messages were moved in notificactions.

When moving only part of a topic, it's useful to display that
information to users in these notifications so that it's clear what's
happening.

The most important consequence is actually just increasing confidence
that when you see that the whole topic was moved, that's accurate.

Substantially modified by tabbott.

Fixes #20575.
This commit is contained in:
Shubh Gupta 2022-01-16 03:03:32 +05:30 committed by Tim Abbott
parent a1e71e8639
commit 79069b5dec
2 changed files with 100 additions and 15 deletions

View File

@ -6488,6 +6488,7 @@ def send_message_moved_breadcrumbs(
new_stream: Stream,
new_topic: Optional[str],
new_thread_notification_string: Optional[str],
changed_messages_count: int,
) -> None:
# Since moving content between streams is highly disruptive,
# it's worth adding a couple tombstone messages showing what
@ -6510,6 +6511,7 @@ def send_message_moved_breadcrumbs(
new_thread_notification_string.format(
old_location=old_topic_link,
user=user_mention,
changed_messages_count=changed_messages_count,
),
)
@ -6523,6 +6525,7 @@ def send_message_moved_breadcrumbs(
old_thread_notification_string.format(
user=user_mention,
new_location=new_topic_link,
changed_messages_count=changed_messages_count,
),
)
@ -7017,17 +7020,37 @@ def do_update_message(
if len(changed_messages) > 0 and new_stream is not None and stream_being_edited is not None:
# Notify users that the topic was moved.
changed_messages_count = len(changed_messages)
old_thread_notification_string = None
if send_notification_to_old_thread:
old_thread_notification_string = gettext_lazy(
"This topic was moved by {user} to {new_location}"
)
if propagate_mode == "change_all":
old_thread_notification_string = gettext_lazy(
"This topic was moved to {new_location} by {user}."
)
elif propagate_mode == "change_one":
old_thread_notification_string = gettext_lazy(
"A message was moved from this topic to {new_location} by {user}."
)
else:
old_thread_notification_string = gettext_lazy(
"{changed_messages_count} messages were moved from this topic to {new_location} by {user}."
)
new_thread_notification_string = None
if send_notification_to_new_thread:
new_thread_notification_string = gettext_lazy(
"This topic was moved here from {old_location} by {user}"
)
if propagate_mode == "change_all":
new_thread_notification_string = gettext_lazy(
"This topic was moved here from {old_location} by {user}."
)
elif propagate_mode == "change_one":
new_thread_notification_string = gettext_lazy(
"A message was moved here from {old_location} by {user}."
)
else:
new_thread_notification_string = gettext_lazy(
"{changed_messages_count} messages were moved here from {old_location} by {user}."
)
send_message_moved_breadcrumbs(
user_profile,
@ -7037,6 +7060,7 @@ def do_update_message(
new_stream,
topic_name,
new_thread_notification_string,
changed_messages_count,
)
if (

View File

@ -1428,14 +1428,14 @@ class EditMessageTest(EditMessageTestCase):
self.assert_length(messages, 1)
self.assertEqual(
messages[0].content,
f"This topic was moved by @_**Iago|{user_profile.id}** to #**new stream>test**",
f"This topic was moved to #**new stream>test** by @_**Iago|{user_profile.id}**.",
)
messages = get_topic_messages(user_profile, new_stream, "test")
self.assert_length(messages, 4)
self.assertEqual(
messages[3].content,
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**",
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_move_message_realm_admin_cant_move_to_another_realm(self) -> None:
@ -1580,7 +1580,7 @@ class EditMessageTest(EditMessageTestCase):
self.assertEqual(messages[0].id, msg_id)
self.assertEqual(
messages[1].content,
f"This topic was moved by @_**Iago|{user_profile.id}** to #**new stream>test**",
f"2 messages were moved from this topic to #**new stream>test** by @_**Iago|{user_profile.id}**.",
)
messages = get_topic_messages(user_profile, new_stream, "test")
@ -1588,7 +1588,68 @@ class EditMessageTest(EditMessageTestCase):
self.assertEqual(messages[0].id, msg_id_later)
self.assertEqual(
messages[2].content,
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**",
f"2 messages were moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_move_message_to_stream_change_one(self) -> None:
(user_profile, old_stream, new_stream, msg_id, msg_id_later) = self.prepare_move_topics(
"iago", "test move stream", "new stream", "test"
)
result = self.client_patch(
"/json/messages/" + str(msg_id_later),
{
"message_id": msg_id_later,
"stream_id": new_stream.id,
"propagate_mode": "change_one",
},
)
self.assert_json_success(result)
messages = get_topic_messages(user_profile, old_stream, "test")
self.assert_length(messages, 3)
self.assertEqual(messages[0].id, msg_id)
self.assertEqual(
messages[2].content,
f"A message was moved from this topic to #**new stream>test** by @_**Iago|{user_profile.id}**.",
)
messages = get_topic_messages(user_profile, new_stream, "test")
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}**.",
)
def test_move_message_to_stream_change_all(self) -> None:
(user_profile, old_stream, new_stream, msg_id, msg_id_later) = self.prepare_move_topics(
"iago", "test move stream", "new stream", "test"
)
result = self.client_patch(
"/json/messages/" + str(msg_id_later),
{
"message_id": msg_id_later,
"stream_id": new_stream.id,
"propagate_mode": "change_all",
},
)
self.assert_json_success(result)
messages = get_topic_messages(user_profile, old_stream, "test")
self.assert_length(messages, 1)
self.assertEqual(
messages[0].content,
f"This topic was moved to #**new stream>test** by @_**Iago|{user_profile.id}**.",
)
messages = get_topic_messages(user_profile, new_stream, "test")
self.assert_length(messages, 4)
self.assertEqual(messages[0].id, msg_id)
self.assertEqual(
messages[3].content,
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_move_message_between_streams_policy_setting(self) -> None:
@ -1832,14 +1893,14 @@ class EditMessageTest(EditMessageTestCase):
self.assert_length(messages, 1)
self.assertEqual(
messages[0].content,
f"This topic was moved by @_**Iago|{user_profile.id}** to #**new stream>new topic**",
f"This topic was moved to #**new stream>new topic** by @_**Iago|{user_profile.id}**.",
)
messages = get_topic_messages(user_profile, new_stream, "new topic")
self.assert_length(messages, 4)
self.assertEqual(
messages[3].content,
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**",
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
self.assert_json_success(result)
@ -2006,7 +2067,7 @@ class EditMessageTest(EditMessageTestCase):
self.assert_length(messages, 4)
self.assertEqual(
messages[3].content,
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**",
f"This topic was moved here from #**test move stream>test** by @_**Iago|{user_profile.id}**.",
)
def test_notify_old_thread_move_message_to_stream(self) -> None:
@ -2031,7 +2092,7 @@ class EditMessageTest(EditMessageTestCase):
self.assert_length(messages, 1)
self.assertEqual(
messages[0].content,
f"This topic was moved by @_**Iago|{user_profile.id}** to #**new stream>test**",
f"This topic was moved to #**new stream>test** by @_**Iago|{user_profile.id}**.",
)
messages = get_topic_messages(user_profile, new_stream, "test")
@ -2096,7 +2157,7 @@ class EditMessageTest(EditMessageTestCase):
self.assert_length(messages, 1)
self.assertEqual(
messages[0].content,
f"This topic was moved by @_**Iago|{admin_user.id}** to #**new stream>test**",
f"This topic was moved to #**new stream>test** by @_**Iago|{admin_user.id}**.",
)
messages = get_topic_messages(admin_user, new_stream, "test")