streams: Refactor multi-option helpers into separate functions.

For internal stream messages, most of the time, we have access to
a Stream object. For the few corner cases where we don't, it is a
much cleaner approach to have a separate function that accepts a
stream name than having one multi-option helper that accepts both
names and objects.
This commit is contained in:
Eeshan Garg 2019-02-08 22:31:35 -03:30 committed by Tim Abbott
parent c7a9faa803
commit 179b747769
7 changed files with 63 additions and 34 deletions

View File

@ -2325,18 +2325,28 @@ def _internal_prep_message(realm: Realm,
def internal_prep_stream_message( def internal_prep_stream_message(
realm: Realm, sender: UserProfile, realm: Realm, sender: UserProfile,
topic: str, content: str, stream: Stream, topic: str, content: str
stream_name: Optional[str]=None,
stream: Optional[Stream]=None
) -> Optional[Dict[str, Any]]: ) -> Optional[Dict[str, Any]]:
""" """
See _internal_prep_message for details of how this works. See _internal_prep_message for details of how this works.
""" """
assert stream_name is not None or stream is not None addressee = Addressee.for_stream(stream, topic)
if stream is not None:
addressee = Addressee.for_stream(stream, topic) return _internal_prep_message(
else: realm=realm,
addressee = Addressee.for_stream_name(stream_name, topic) sender=sender,
addressee=addressee,
content=content,
)
def internal_prep_stream_message_by_name(
realm: Realm, sender: UserProfile,
stream_name: str, topic: str, content: str
) -> Optional[Dict[str, Any]]:
"""
See _internal_prep_message for details of how this works.
"""
addressee = Addressee.for_stream_name(stream_name, topic)
return _internal_prep_message( return _internal_prep_message(
realm=realm, realm=realm,
@ -2402,14 +2412,24 @@ def internal_send_private_message(realm: Realm,
def internal_send_stream_message( def internal_send_stream_message(
realm: Realm, sender: UserProfile, realm: Realm, sender: UserProfile,
topic: str, content: str, stream: Stream, topic: str, content: str
stream_name: Optional[str]=None,
stream: Optional[Stream]=None
) -> None: ) -> None:
message = internal_prep_stream_message( message = internal_prep_stream_message(
realm, sender, topic, realm, sender, stream,
content, stream_name=stream_name, topic, content
stream=stream )
if message is None:
return
do_send_messages([message])
def internal_send_stream_message_by_name(
realm: Realm, sender: UserProfile,
stream_name: str, topic: str, content: str
) -> None:
message = internal_prep_stream_message_by_name(
realm, sender, stream_name,
topic, content
) )
if message is None: if message is None:
@ -3451,11 +3471,11 @@ def do_rename_stream(stream: Stream,
internal_send_stream_message( internal_send_stream_message(
stream.realm, stream.realm,
sender, sender,
stream,
"welcome", "welcome",
"_@**%s|%d** renamed stream **%s** to **%s**" % (user_profile.full_name, "_@**%s|%d** renamed stream **%s** to **%s**" % (user_profile.full_name,
user_profile.id, user_profile.id,
old_name, new_name), old_name, new_name)
stream=stream
) )
# Even though the token doesn't change, the web client needs to update the # Even though the token doesn't change, the web client needs to update the
# email forwarding address to display the correctly-escaped new name. # email forwarding address to display the correctly-escaped new name.

View File

@ -2,7 +2,7 @@ import json
import os import os
import importlib import importlib
from zerver.lib.actions import internal_send_private_message, \ from zerver.lib.actions import internal_send_private_message, \
internal_send_stream_message, internal_send_huddle_message internal_send_stream_message_by_name, internal_send_huddle_message
from zerver.models import UserProfile, get_active_user from zerver.models import UserProfile, get_active_user
from zerver.lib.bot_storage import get_bot_storage, set_bot_storage, \ from zerver.lib.bot_storage import get_bot_storage, set_bot_storage, \
is_key_in_bot_storage, remove_bot_storage is_key_in_bot_storage, remove_bot_storage
@ -71,10 +71,9 @@ class EmbeddedBotHandler:
self._rate_limit.show_error_and_exit() self._rate_limit.show_error_and_exit()
if message['type'] == 'stream': if message['type'] == 'stream':
internal_send_stream_message( internal_send_stream_message_by_name(
self.user_profile.realm, self.user_profile, self.user_profile.realm, self.user_profile,
message['topic'], message['content'], message['to'], message['topic'], message['content']
stream_name=message['to']
) )
return return

View File

@ -160,9 +160,8 @@ def send_to_missed_message_address(address: str, message: message.Message) -> No
if recipient.type == Recipient.STREAM: if recipient.type == Recipient.STREAM:
stream = get_stream_by_id_in_realm(recipient.type_id, user_profile.realm) stream = get_stream_by_id_in_realm(recipient.type_id, user_profile.realm)
internal_send_stream_message( internal_send_stream_message(
user_profile.realm, user_profile, user_profile.realm, user_profile, stream,
subject_b.decode('utf-8'), body, subject_b.decode('utf-8'), body
stream=stream
) )
elif recipient.type == Recipient.PERSONAL: elif recipient.type == Recipient.PERSONAL:
display_recipient = get_display_recipient(recipient) display_recipient = get_display_recipient(recipient)

View File

@ -2,7 +2,7 @@
from django.conf import settings from django.conf import settings
from zerver.lib.actions import set_default_streams, \ from zerver.lib.actions import set_default_streams, \
internal_prep_stream_message, internal_send_private_message, \ internal_prep_stream_message_by_name, internal_send_private_message, \
create_streams_if_needed, do_send_messages, \ create_streams_if_needed, do_send_messages, \
do_add_reaction_legacy, create_users, missing_any_realm_internal_bots do_add_reaction_legacy, create_users, missing_any_realm_internal_bots
from zerver.lib.topic import get_turtle_message from zerver.lib.topic import get_turtle_message
@ -107,10 +107,9 @@ def send_initial_realm_messages(realm: Realm) -> None:
"in that each conversation should get its own topic. Keep them short, though; one " "in that each conversation should get its own topic. Keep them short, though; one "
"or two words will do it!"}, "or two words will do it!"},
] # type: List[Dict[str, str]] ] # type: List[Dict[str, str]]
messages = [internal_prep_stream_message( messages = [internal_prep_stream_message_by_name(
realm, welcome_bot, realm, welcome_bot, message['stream'],
message['topic'], message['content'], message['topic'], message['content']
stream_name=message['stream']
) for message in welcome_messages] ) for message in welcome_messages]
message_ids = do_send_messages(messages) message_ids = do_send_messages(messages)

View File

@ -29,11 +29,12 @@ from zerver.lib.actions import (
get_last_message_id, get_last_message_id,
get_user_info_for_message_updates, get_user_info_for_message_updates,
internal_prep_private_message, internal_prep_private_message,
internal_prep_stream_message, internal_prep_stream_message_by_name,
internal_send_huddle_message, internal_send_huddle_message,
internal_send_message, internal_send_message,
internal_send_private_message, internal_send_private_message,
internal_send_stream_message, internal_send_stream_message,
internal_send_stream_message_by_name,
send_rate_limited_pm_notification_to_bot_owner, send_rate_limited_pm_notification_to_bot_owner,
) )
@ -544,6 +545,18 @@ class InternalPrepTest(ZulipTestCase):
arg = m.call_args_list[0][0][0] arg = m.call_args_list[0][0][0]
self.assertIn('Message must not be empty', arg) self.assertIn('Message must not be empty', arg)
with mock.patch('logging.exception') as m:
internal_send_stream_message_by_name(
realm=realm,
sender=cordelia,
stream_name=stream.name,
topic='whatever',
content=bad_content
)
arg = m.call_args_list[0][0][0]
self.assertIn('Message must not be empty', arg)
with mock.patch('logging.exception') as m: with mock.patch('logging.exception') as m:
internal_send_message( internal_send_message(
realm=realm, realm=realm,
@ -598,7 +611,7 @@ class InternalPrepTest(ZulipTestCase):
topic = 'whatever' topic = 'whatever'
content = 'hello' content = 'hello'
internal_prep_stream_message( internal_prep_stream_message_by_name(
realm=realm, realm=realm,
sender=sender, sender=sender,
stream_name=stream_name, stream_name=stream_name,

View File

@ -398,9 +398,9 @@ def add_subscriptions_backend(
internal_prep_stream_message( internal_prep_stream_message(
realm=user_profile.realm, realm=user_profile.realm,
sender=sender, sender=sender,
stream=notifications_stream,
topic=topic, topic=topic,
content=msg, content=msg,
stream=notifications_stream
) )
) )

View File

@ -78,9 +78,8 @@ From image editing program:
] # type: List[Dict[str, Any]] ] # type: List[Dict[str, Any]]
messages = [internal_prep_stream_message( messages = [internal_prep_stream_message(
realm, message['sender'], realm, message['sender'], stream,
'message formatting', message['content'], 'message formatting', message['content']
stream=stream
) for message in staged_messages] ) for message in staged_messages]
message_ids = do_send_messages(messages) message_ids = do_send_messages(messages)