update-message: Use MentionData in the update_message_backend code.

This is a performance optimization, since we can avoid doing work
related to wildcard mentions in the common case that the message can't
have any.  We also add a unit test for adding wildcard mentions in a
message edit.
This commit is contained in:
Rohitt Vashishtha
2019-11-28 10:26:57 +00:00
committed by Tim Abbott
parent bb42539b3f
commit 68e93d2435
4 changed files with 57 additions and 6 deletions

View File

@@ -4436,7 +4436,7 @@ def do_update_embedded_data(user_profile: UserProfile,
def do_update_message(user_profile: UserProfile, message: Message, topic_name: Optional[str],
propagate_mode: str, content: Optional[str],
rendered_content: Optional[str], prior_mention_user_ids: Set[int],
mention_user_ids: Set[int]) -> int:
mention_user_ids: Set[int], mention_data: Optional[bugdown.MentionData]=None) -> int:
"""
The main function for message editing. A message edit event can
modify:
@@ -4471,6 +4471,9 @@ def do_update_message(user_profile: UserProfile, message: Message, topic_name: O
assert rendered_content is not None
update_user_message_flags(message, ums)
# mention_data is required if there's a content edit.
assert mention_data is not None
# One could imagine checking realm.allow_edit_history here and
# modifying the events based on that setting, but doing so
# doesn't really make sense. We need to send the edit event
@@ -4513,7 +4516,7 @@ def do_update_message(user_profile: UserProfile, message: Message, topic_name: O
recipient=message.recipient,
sender_id=message.sender_id,
stream_topic=stream_topic,
possible_wildcard_mention=True,
possible_wildcard_mention=mention_data.message_has_wildcards(),
)
event['push_notify_user_ids'] = list(info['push_notify_user_ids'])

View File

@@ -104,6 +104,7 @@ from zerver.lib.actions import (
check_delete_user_group,
do_update_user_custom_profile_data_if_changed,
)
from zerver.lib.bugdown import MentionData
from zerver.lib.events import (
apply_events,
fetch_initial_state_data,
@@ -733,12 +734,16 @@ class EventsRegisterTest(ZulipTestCase):
rendered_content = render_markdown(message, content)
prior_mention_user_ids = set() # type: Set[int]
mentioned_user_ids = set() # type: Set[int]
mention_data = MentionData(
realm_id=self.user_profile.realm_id,
content=content,
)
events = self.do_test(
lambda: do_update_message(self.user_profile, message, topic,
propagate_mode, content, rendered_content,
prior_mention_user_ids,
mentioned_user_ids),
mentioned_user_ids, mention_data),
state_change_expected=True,
)
error = schema_checker('events[0]', events[0])

View File

@@ -2925,7 +2925,8 @@ class EditMessageTest(ZulipTestCase):
content=None,
rendered_content=None,
prior_mention_user_ids=set(),
mention_user_ids=set()
mention_user_ids=set(),
mention_data=None,
)
mock_send_event.assert_called_with(mock.ANY, mock.ANY, users_to_be_notified)
@@ -2972,6 +2973,42 @@ class EditMessageTest(ZulipTestCase):
users_to_be_notified = list(map(notify, [hamlet.id]))
do_update_message_topic_success(hamlet, message, "Change again", users_to_be_notified)
@mock.patch("zerver.lib.actions.send_event")
def test_wildcard_mention(self, mock_send_event: mock.MagicMock) -> None:
stream_name = "Macbeth"
hamlet = self.example_user("hamlet")
cordelia = self.example_user("cordelia")
self.make_stream(stream_name, history_public_to_subscribers=True)
self.subscribe(hamlet, stream_name)
self.subscribe(cordelia, stream_name)
self.login(hamlet.email)
message_id = self.send_stream_message(hamlet.email, stream_name, "Hello everyone")
def notify(user_id: int) -> Dict[str, Any]:
return {
"id": user_id,
"flags": ["wildcard_mentioned"]
}
users_to_be_notified = list(map(notify, [cordelia.id, hamlet.id]))
result = self.client_patch("/json/messages/" + str(message_id), {
'message_id': message_id,
'content': 'Hello @**everyone**',
})
self.assert_json_success(result)
# Extract the send_event call where event type is 'update_message'.
# Here we assert wildcard_mention_user_ids has been set properly.
called = False
for call_args in mock_send_event.call_args_list:
(arg_realm, arg_event, arg_notified_users) = call_args[0]
if arg_event['type'] == 'update_message':
self.assertEqual(arg_event['type'], 'update_message')
self.assertEqual(arg_event['wildcard_mention_user_ids'], [cordelia.id, hamlet.id])
self.assertEqual(arg_notified_users, users_to_be_notified)
called = True
self.assertTrue(called)
def test_propagate_topic_forward(self) -> None:
self.login(self.example_email("hamlet"))
id1 = self.send_stream_message(self.example_email("hamlet"), "Scotland",

View File

@@ -1499,12 +1499,17 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage,
links_for_embed = set() # type: Set[str]
prior_mention_user_ids = set() # type: Set[int]
mention_user_ids = set() # type: Set[int]
mention_data = None # type: Optional[bugdown.MentionData]
if content is not None:
content = content.strip()
if content == "":
content = "(deleted)"
content = truncate_body(content)
mention_data = bugdown.MentionData(
realm_id=user_profile.realm.id,
content=content,
)
user_info = get_user_info_for_message_updates(message.id)
prior_mention_user_ids = user_info['mention_user_ids']
@@ -1515,7 +1520,8 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage,
rendered_content = render_incoming_message(message,
content,
user_info['message_user_ids'],
user_profile.realm)
user_profile.realm,
mention_data=mention_data)
links_for_embed |= message.links_for_preview
mention_user_ids = message.mentions_user_ids
@@ -1523,7 +1529,7 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage,
number_changed = do_update_message(user_profile, message, topic_name,
propagate_mode, content, rendered_content,
prior_mention_user_ids,
mention_user_ids)
mention_user_ids, mention_data)
# Include the number of messages changed in the logs
request._log_data['extra'] = "[%s]" % (number_changed,)