Skip to content

Commit

Permalink
Use user_profile, not email, in check_message().
Browse files Browse the repository at this point in the history
We are trying to convert emails to user_profiles earlier in
the codepath.  This may cause subtle changes in which errors
appear, but it's probably generally good to report on bad
addressees sooner than later.
  • Loading branch information
Steve Howell authored and timabbott committed Aug 22, 2017
1 parent c56a631 commit c61c0f3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
8 changes: 4 additions & 4 deletions zerver/lib/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1374,16 +1374,16 @@ def check_message(sender, client, addressee,
raise JsonableError(_("Not authorized to send to stream '%s'") % (stream.name,))

elif addressee.is_private():
emails = addressee.emails()
user_profiles = addressee.user_profiles()

if emails is None or len(emails) == 0:
if user_profiles is None or len(user_profiles) == 0:
raise JsonableError(_("Message must have recipients"))

mirror_message = client and client.name in ["zephyr_mirror", "irc_mirror", "jabber_mirror", "JabberMirror"]
not_forged_mirror_message = mirror_message and not forged
try:
recipient = recipient_for_emails(emails, not_forged_mirror_message,
forwarder_user_profile, sender)
recipient = recipient_for_user_profiles(user_profiles, not_forged_mirror_message,
forwarder_user_profile, sender)
except ValidationError as e:
assert isinstance(e.messages[0], six.string_types)
raise JsonableError(e.messages[0])
Expand Down
28 changes: 20 additions & 8 deletions zerver/lib/addressee.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@

from django.core.exceptions import ValidationError
from django.utils.translation import ugettext as _
from zerver.lib.exceptions import JsonableError
from zerver.lib.request import JsonableError
from zerver.models import (
UserProfile,
get_user_profile_by_email,
)
import six

def user_profiles_from_unvalidated_emails(emails):
# type: (Iterable[Text]) -> List[UserProfile]
Expand All @@ -22,6 +24,14 @@ def user_profiles_from_unvalidated_emails(emails):
user_profiles.append(user_profile)
return user_profiles

def get_user_profiles(emails):
# type: (Iterable[Text]) -> List[UserProfile]
try:
return user_profiles_from_unvalidated_emails(emails)
except ValidationError as e:
assert isinstance(e.messages[0], six.string_types)
raise JsonableError(e.messages[0])

class Addressee(object):
# This is really just a holder for vars that tended to be passed
# around in a non-type-safe way before this class was introduced.
Expand All @@ -34,11 +44,11 @@ class Addressee(object):
# in memory.
#
# This should be treated as an immutable class.
def __init__(self, msg_type, emails=None, stream_name=None, topic=None):
# type: (str, Optional[Sequence[Text]], Optional[Text], Text) -> None
def __init__(self, msg_type, user_profiles=None, stream_name=None, topic=None):
# type: (str, Optional[Sequence[UserProfile]], Optional[Text], Text) -> None
assert(msg_type in ['stream', 'private'])
self._msg_type = msg_type
self._emails = emails
self._user_profiles = user_profiles
self._stream_name = stream_name
self._topic = topic

Expand All @@ -54,10 +64,10 @@ def is_private(self):
# type: () -> bool
return self._msg_type == 'private'

def emails(self):
# type: () -> Sequence[Text]
def user_profiles(self):
# type: () -> List[UserProfile]
assert(self.is_private())
return self._emails
return self._user_profiles # type: ignore # assertion protects us

def stream_name(self):
# type: () -> Text
Expand Down Expand Up @@ -107,15 +117,17 @@ def for_stream(stream_name, topic):
@staticmethod
def for_private(emails):
# type: (Sequence[Text]) -> Addressee
user_profiles = get_user_profiles(emails)
return Addressee(
msg_type='private',
emails=emails,
user_profiles=user_profiles,
)

@staticmethod
def for_email(email):
# type: (Text) -> Addressee
user_profiles = get_user_profiles([email])
return Addressee(
msg_type='private',
emails=[email],
user_profiles=user_profiles,
)

0 comments on commit c61c0f3

Please sign in to comment.