Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

check_message: Support sending stream messages by ID. 

This commit also contains the following auxiliary changes:
* Adds a custom exception, StreamWithIDDoesNotExist for when
  a stream with a given ID does not exist because the error
  message returned by StreamDoesNotExist only makes with stream
  names, not IDs.
* Adds a new helper, get_stream_by_id_in_realm, which is similar
  to get_user_profile_by_id_in_realm (introduced in #10391).
* Adds a helper, validate_stream_id_with_pm_notification, which
  returns the Stream object associated with a given ID and also
  handles PM notifications to the bot owner if the message was
  sent by a bot and if the stream does not exist or has no
  subscribers.
* Modifies the message sent by send_pm_if_empty_stream to
  accommodate stream IDs.

Note that all of the above changes are required before check_message
can be modified to support stream IDs.
This commit is contained in:
Eeshan Garg 2019-01-28 00:58:29 -03:30 committed by Tim Abbott
parent b8221555d2
commit 998437c123
5 changed files with 92 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
zerver/lib/actions.py
View File

@ -35,7 +35,8 @@ from zerver.lib.cache import (
)
from zerver.lib.context_managers import lockfile
from zerver.lib.emoji import emoji_name_to_emoji_code, get_emoji_file_name
from zerver.lib.exceptions import StreamDoesNotExistError
from zerver.lib.exceptions import StreamDoesNotExistError, \
StreamWithIDDoesNotExistError
from zerver.lib.hotspots import get_next_hotspots
from zerver.lib.message import (
access_message,
@ -104,7 +105,8 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity,
UserGroup, UserGroupMembership, get_default_stream_groups, \
get_bot_services, get_bot_dicts_in_realm, DomainNotAllowedForRealmError, \
DisposableEmailError, EmailContainsPlusError, \
get_user_including_cross_realm, get_user_by_id_in_realm_including_cross_realm
get_user_including_cross_realm, get_user_by_id_in_realm_including_cross_realm, \
get_stream_by_id_in_realm
from zerver.lib.alert_words import alert_words_in_realm
from zerver.lib.avatar import avatar_url, avatar_url_from_dict
@ -2066,7 +2068,8 @@ def send_rate_limited_pm_notification_to_bot_owner(sender: UserProfile,
def send_pm_if_empty_stream(sender: UserProfile,
stream: Optional[Stream],
stream_name: str,
realm: Realm) -> None:
realm: Realm,
stream_id: Optional[int]=None) -> None:
"""If a bot sends a message to a stream that doesn't exist or has no
subscribers, sends a notification to the bot owner (if not a
cross-realm bot) so that the owner can correct the issue."""
@ -2084,10 +2087,17 @@ def send_pm_if_empty_stream(sender: UserProfile,
# num_subscribers == 0
error_msg = "there are no subscribers to that stream. To join it, "
content = ("Hi there! We thought you'd like to know that your bot **%s** just "
"tried to send a message to stream `%s`, but %s"
"click the gear in the left-side stream list." %
(sender.full_name, stream_name, error_msg))
if stream_id is not None:
stream_info = "with ID {stream_id}".format(stream_id=stream_id)
else:
stream_info = "`{stream_name}`".format(stream_name=stream_name)
content = ("Hi there! We thought you'd like to know that your bot **{sender}** just "
"tried to send a message to stream {stream_info}, but {error_msg}"
"click the gear in the left-side stream list.")
content = content.format(sender=sender.full_name,
stream_info=stream_info,
error_msg=error_msg)
send_rate_limited_pm_notification_to_bot_owner(sender, realm, content)
@ -2144,6 +2154,17 @@ def validate_stream_name_with_pm_notification(stream_name: str, realm: Realm,
return stream
def validate_stream_id_with_pm_notification(stream_id: int, realm: Realm,
sender: UserProfile) -> Stream:
try:
stream = get_stream_by_id_in_realm(stream_id, realm)
send_pm_if_empty_stream(sender, stream, stream.name, realm)
except Stream.DoesNotExist:
send_pm_if_empty_stream(sender, None, "", realm, stream_id=stream_id)
raise StreamWithIDDoesNotExistError(stream_id)
return stream
# check_message:
# Returns message ready for sending with do_send_message on success or the error message (string) on error.
def check_message(sender: UserProfile, client: Client, addressee: Addressee,
@ -2175,7 +2196,13 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee,
topic_name = truncate_topic(topic_name)
stream_name = addressee.stream_name()
stream = validate_stream_name_with_pm_notification(stream_name, realm, sender)
stream_id = addressee.stream_id()
if stream_name is not None:
stream = validate_stream_name_with_pm_notification(stream_name, realm, sender)
else:
stream = validate_stream_id_with_pm_notification(stream_id, realm, sender)
recipient = get_stream_recipient(stream.id)
# This will raise JsonableError if there are problems.

3
zerver/lib/addressee.py
View File

@ -85,9 +85,8 @@ class Addressee:
assert(self.is_private())
return self._user_profiles # type: ignore # assertion protects us
def stream_name(self) -> str:
def stream_name(self) -> Optional[str]:
assert(self.is_stream())
assert(self._stream_name is not None)
return self._stream_name
def stream_id(self) -> Optional[int]:

11
zerver/lib/exceptions.py
View File

@ -142,6 +142,17 @@ class StreamDoesNotExistError(JsonableError):
def msg_format() -> str:
return _("Stream '{stream}' does not exist")
class StreamWithIDDoesNotExistError(JsonableError):
code = ErrorCode.STREAM_DOES_NOT_EXIST
data_fields = ['stream_id']
def __init__(self, stream_id: int) -> None:
self.stream_id = stream_id
@staticmethod
def msg_format() -> str:
return _("Stream with ID '{stream_id}' does not exist")
class CannotDeactivateLastUserError(JsonableError):
code = ErrorCode.CANNOT_DEACTIVATE_LAST_USER
data_fields = ['is_last_admin', 'entity']

3
zerver/models.py
View File

@ -1279,6 +1279,9 @@ def get_stream(stream_name: str, realm: Realm) -> Stream:
'''
return get_realm_stream(stream_name, realm.id)
def get_stream_by_id_in_realm(stream_id: int, realm: Realm) -> Stream:
return Stream.objects.select_related().get(id=stream_id, realm=realm)
def bulk_get_streams(realm: Realm, stream_names: STREAM_NAMES) -> Dict[str, Any]:
def fetch_streams_by_name(stream_names: List[str]) -> Sequence[Stream]:

42
zerver/tests/test_messages.py
View File

@ -1308,6 +1308,48 @@ class MessagePOSTTest(ZulipTestCase):
"topic": "Test topic"})
self.assert_json_success(result)
def test_message_to_stream_with_nonexistent_id(self) -> None:
cordelia = self.example_user('cordelia')
bot = self.create_test_bot(
short_name='whatever',
user_profile=cordelia,
)
result = self.api_post(
bot.email, "/api/v1/messages",
{
"type": "stream",
"to": ujson.dumps([99999]),
"client": "test suite",
"content": "Stream message by ID.",
"topic": "Test topic for stream ID message"
}
)
self.assert_json_error(result, "Stream with ID '99999' does not exist")
msg = self.get_last_message()
expected = ("Hi there! We thought you'd like to know that your bot **{sender}** just "
"tried to send a message to stream {stream_info}, but that stream does not "
"yet exist. To create it, click the gear in the left-side stream list.")
expected = expected.format(sender=bot.full_name, stream_info='with ID 99999')
self.assertEqual(msg.content, expected)
def test_message_to_stream_by_id(self) -> None:
"""
Sending a message to a stream (by stream ID) to which you are
subscribed is successful.
"""
self.login(self.example_email("hamlet"))
realm = get_realm('zulip')
stream = get_stream('Verona', realm)
result = self.client_post("/json/messages", {"type": "stream",
"to": ujson.dumps([stream.id]),
"client": "test suite",
"content": "Stream message by ID.",
"topic": "Test topic for stream ID message"})
self.assert_json_success(result)
sent_message = self.get_last_message()
self.assertEqual(sent_message.content, "Stream message by ID.")
def test_message_to_announce(self) -> None:
"""
Sending a message to an announcement_only stream by a realm admin