mirror of https://github.com/zulip/zulip.git
typing: Accept only user_ids in typing endpoint.
The only clients that should use the typing indicators endpoint are our internal clients, and they should send a JSON-formatted list of user_ids. We now enforce this, which removes some complexity surrounding legacy ways of sending users, such as emails and comma-delimited strings of user_ids. There may be a very tiny number of mobile clients that still use the old emails API. This won't have any user-facing effect on the mobile users themselves, but if you type a message to your friend on an old mobile app, the friend will no longer see typing indicators. Also, the mobile team may see some errors in their Sentry logs from the server rejecting posts from the old mobile clients. The error messages we report here are a bit more generic, since we now just use REQ to do validation with this code: validator=check_list(check_int) This also allows us to remove a test hack related to the API documentation. (We changed the docs to reflect the modern API in an earlier commit, but the tests couldn't be fixed while we still had the more complex semantics for the "to" parameter.)
This commit is contained in:
parent
81c28c1d3e
commit
d5cc29755e
|
@ -18,6 +18,8 @@ below features are supported.
|
||||||
value means no limit.
|
value means no limit.
|
||||||
* [`POST /register`](/api/register-queue): The response now contains a
|
* [`POST /register`](/api/register-queue): The response now contains a
|
||||||
`is_owner`, similar to the existing `is_admin` and `is_guest` fields.
|
`is_owner`, similar to the existing `is_admin` and `is_guest` fields.
|
||||||
|
* [`POST /typing`](/api/typing): Removed legacy support for sending email
|
||||||
|
addresses, rather than user IDs, to encode private message recipients.
|
||||||
|
|
||||||
**Feature level 10**
|
**Feature level 10**
|
||||||
|
|
||||||
|
|
|
@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0"
|
||||||
#
|
#
|
||||||
# Changes should be accompanied by documentation explaining what the
|
# Changes should be accompanied by documentation explaining what the
|
||||||
# new level means in templates/zerver/api/changelog.md.
|
# new level means in templates/zerver/api/changelog.md.
|
||||||
API_FEATURE_LEVEL = 10
|
API_FEATURE_LEVEL = 11
|
||||||
|
|
||||||
# Bump the minor PROVISION_VERSION to indicate that folks should provision
|
# Bump the minor PROVISION_VERSION to indicate that folks should provision
|
||||||
# only when going from an old version of the code to a newer version. Bump
|
# only when going from an old version of the code to a newer version. Bump
|
||||||
|
|
|
@ -113,7 +113,7 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity,
|
||||||
query_for_ids, get_huddle_recipient, \
|
query_for_ids, get_huddle_recipient, \
|
||||||
UserGroup, UserGroupMembership, get_default_stream_groups, \
|
UserGroup, UserGroupMembership, get_default_stream_groups, \
|
||||||
get_bot_services, get_bot_dicts_in_realm, \
|
get_bot_services, get_bot_dicts_in_realm, \
|
||||||
get_user_including_cross_realm, get_user_by_id_in_realm_including_cross_realm, \
|
get_user_by_id_in_realm_including_cross_realm, \
|
||||||
get_stream_by_id_in_realm
|
get_stream_by_id_in_realm
|
||||||
|
|
||||||
from zerver.lib.alert_words import get_alert_word_automaton
|
from zerver.lib.alert_words import get_alert_word_automaton
|
||||||
|
@ -1782,11 +1782,12 @@ def do_send_typing_notification(
|
||||||
|
|
||||||
# check_send_typing_notification:
|
# check_send_typing_notification:
|
||||||
# Checks the typing notification and sends it
|
# Checks the typing notification and sends it
|
||||||
def check_send_typing_notification(sender: UserProfile, notification_to: Union[Sequence[str], Sequence[int]],
|
def check_send_typing_notification(sender: UserProfile,
|
||||||
|
user_ids: List[int],
|
||||||
operator: str) -> None:
|
operator: str) -> None:
|
||||||
|
|
||||||
realm = sender.realm
|
realm = sender.realm
|
||||||
if len(notification_to) == 0:
|
if len(user_ids) == 0:
|
||||||
raise JsonableError(_('Missing parameter: \'to\' (recipient)'))
|
raise JsonableError(_('Missing parameter: \'to\' (recipient)'))
|
||||||
elif operator not in ('start', 'stop'):
|
elif operator not in ('start', 'stop'):
|
||||||
raise JsonableError(_('Invalid \'op\' value (should be start or stop)'))
|
raise JsonableError(_('Invalid \'op\' value (should be start or stop)'))
|
||||||
|
@ -1799,16 +1800,6 @@ def check_send_typing_notification(sender: UserProfile, notification_to: Union[S
|
||||||
to mostly delete code when we desupport old versions of the
|
to mostly delete code when we desupport old versions of the
|
||||||
app.'''
|
app.'''
|
||||||
|
|
||||||
if isinstance(notification_to[0], int):
|
|
||||||
user_ids = cast(List[int], notification_to)
|
|
||||||
else:
|
|
||||||
try:
|
|
||||||
emails = cast(Sequence[str], notification_to)
|
|
||||||
user_ids = user_ids_for_emails(realm, emails)
|
|
||||||
except ValidationError as e:
|
|
||||||
assert isinstance(e.messages[0], str)
|
|
||||||
raise JsonableError(e.messages[0])
|
|
||||||
|
|
||||||
if sender.id not in user_ids:
|
if sender.id not in user_ids:
|
||||||
user_ids.append(sender.id)
|
user_ids.append(sender.id)
|
||||||
|
|
||||||
|
@ -1908,25 +1899,6 @@ def validate_recipient_user_profiles(user_profiles: Sequence[UserProfile],
|
||||||
|
|
||||||
return list(recipient_profiles_map.values())
|
return list(recipient_profiles_map.values())
|
||||||
|
|
||||||
def user_ids_for_emails(
|
|
||||||
realm: Realm,
|
|
||||||
emails: Iterable[str],
|
|
||||||
) -> List[int]:
|
|
||||||
'''
|
|
||||||
This function should only stay around while
|
|
||||||
we still have to support mobile sending emails
|
|
||||||
in typing notifications.
|
|
||||||
'''
|
|
||||||
user_ids: List[int] = []
|
|
||||||
for email in emails:
|
|
||||||
try:
|
|
||||||
user_profile = get_user_including_cross_realm(email, realm)
|
|
||||||
except UserProfile.DoesNotExist:
|
|
||||||
raise ValidationError(_("Invalid email '%s'") % (email,))
|
|
||||||
user_ids.append(user_profile.id)
|
|
||||||
|
|
||||||
return user_ids
|
|
||||||
|
|
||||||
def recipient_for_user_profiles(user_profiles: Sequence[UserProfile], forwarded_mirror_message: bool,
|
def recipient_for_user_profiles(user_profiles: Sequence[UserProfile], forwarded_mirror_message: bool,
|
||||||
forwarder_user_profile: Optional[UserProfile],
|
forwarder_user_profile: Optional[UserProfile],
|
||||||
sender: UserProfile, allow_deactivated: bool=False) -> Recipient:
|
sender: UserProfile, allow_deactivated: bool=False) -> Recipient:
|
||||||
|
|
|
@ -3931,8 +3931,8 @@ paths:
|
||||||
recipient.).
|
recipient.).
|
||||||
|
|
||||||
**Changes**: Before Zulip 2.0, this parameter accepted only a JSON-encoded
|
**Changes**: Before Zulip 2.0, this parameter accepted only a JSON-encoded
|
||||||
list of email addresses. The email address-based format is deprecated
|
list of email addresses. Support for the email address-based format was
|
||||||
and will be removed in Zulip 2.2.
|
removed in Zulip 2.2 (feature level 11).
|
||||||
schema:
|
schema:
|
||||||
type: array
|
type: array
|
||||||
items:
|
items:
|
||||||
|
|
|
@ -429,17 +429,6 @@ do not match the types declared in the implementation of {}.\n""".format(functio
|
||||||
AssertionError. """
|
AssertionError. """
|
||||||
openapi_params: Set[Tuple[str, Union[type, Tuple[type, object]]]] = set()
|
openapi_params: Set[Tuple[str, Union[type, Tuple[type, object]]]] = set()
|
||||||
for element in openapi_parameters:
|
for element in openapi_parameters:
|
||||||
if function.__name__ == 'send_notification_backend':
|
|
||||||
if element['name'] == 'to':
|
|
||||||
'''
|
|
||||||
We want users to send ints here, but the mypy
|
|
||||||
types for send_notification_backend are still
|
|
||||||
str, because we need backward compatible
|
|
||||||
support for old versions of mobile that still
|
|
||||||
send emails for typing requests.
|
|
||||||
'''
|
|
||||||
continue
|
|
||||||
|
|
||||||
name: str = element["name"]
|
name: str = element["name"]
|
||||||
schema = {}
|
schema = {}
|
||||||
if "content" in element:
|
if "content" in element:
|
||||||
|
|
|
@ -52,7 +52,7 @@ class TypingValidateUsersTest(ZulipTestCase):
|
||||||
"""
|
"""
|
||||||
sender = self.example_user("hamlet")
|
sender = self.example_user("hamlet")
|
||||||
result = self.api_post(sender, '/api/v1/typing', {'op': 'start'})
|
result = self.api_post(sender, '/api/v1/typing', {'op': 'start'})
|
||||||
self.assert_json_error(result, "Missing parameter: 'to' (recipient)")
|
self.assert_json_error(result, "Missing 'to' argument")
|
||||||
|
|
||||||
def test_argument_to_is_not_valid_json(self) -> None:
|
def test_argument_to_is_not_valid_json(self) -> None:
|
||||||
"""
|
"""
|
||||||
|
@ -61,7 +61,7 @@ class TypingValidateUsersTest(ZulipTestCase):
|
||||||
sender = self.example_user("hamlet")
|
sender = self.example_user("hamlet")
|
||||||
invalid = 'bad email'
|
invalid = 'bad email'
|
||||||
result = self.api_post(sender, '/api/v1/typing', {'op': 'start', 'to': invalid})
|
result = self.api_post(sender, '/api/v1/typing', {'op': 'start', 'to': invalid})
|
||||||
self.assert_json_error(result, "Invalid email 'bad email'")
|
self.assert_json_error(result, 'Argument "to" is not valid JSON.')
|
||||||
|
|
||||||
def test_bogus_user_id(self) -> None:
|
def test_bogus_user_id(self) -> None:
|
||||||
"""
|
"""
|
||||||
|
@ -282,38 +282,3 @@ class TypingHappyPathTest(ZulipTestCase):
|
||||||
self.assertEqual(event['sender']['email'], sender.email)
|
self.assertEqual(event['sender']['email'], sender.email)
|
||||||
self.assertEqual(event['type'], 'typing')
|
self.assertEqual(event['type'], 'typing')
|
||||||
self.assertEqual(event['op'], 'stop')
|
self.assertEqual(event['op'], 'stop')
|
||||||
|
|
||||||
class TypingLegacyMobileSupportTest(ZulipTestCase):
|
|
||||||
def test_legacy_email_interface(self) -> None:
|
|
||||||
'''
|
|
||||||
We are keeping the email interface on life support
|
|
||||||
for a couple months until we get some of our
|
|
||||||
mobile users upgraded.
|
|
||||||
'''
|
|
||||||
sender = self.example_user('hamlet')
|
|
||||||
othello = self.example_user('othello')
|
|
||||||
cordelia = self.example_user('cordelia')
|
|
||||||
|
|
||||||
emails = [othello.email, cordelia.email]
|
|
||||||
|
|
||||||
params = dict(
|
|
||||||
to=ujson.dumps(emails),
|
|
||||||
op='start',
|
|
||||||
)
|
|
||||||
|
|
||||||
events: List[Mapping[str, Any]] = []
|
|
||||||
with tornado_redirected_to_list(events):
|
|
||||||
result = self.api_post(sender, '/api/v1/typing', params)
|
|
||||||
|
|
||||||
self.assert_json_success(result)
|
|
||||||
event = events[0]['event']
|
|
||||||
|
|
||||||
event_recipient_user_ids = {
|
|
||||||
user['user_id']
|
|
||||||
for user in event['recipients']
|
|
||||||
}
|
|
||||||
|
|
||||||
self.assertEqual(
|
|
||||||
event_recipient_user_ids,
|
|
||||||
{sender.id, othello.id, cordelia.id}
|
|
||||||
)
|
|
||||||
|
|
|
@ -1,22 +1,17 @@
|
||||||
from django.http import HttpRequest, HttpResponse
|
from django.http import HttpRequest, HttpResponse
|
||||||
from typing import List, Union
|
from typing import List
|
||||||
|
|
||||||
from zerver.decorator import has_request_variables, REQ
|
from zerver.decorator import has_request_variables, REQ
|
||||||
from zerver.lib.actions import check_send_typing_notification, \
|
from zerver.lib.actions import check_send_typing_notification
|
||||||
extract_private_recipients
|
|
||||||
from zerver.lib.response import json_success
|
from zerver.lib.response import json_success
|
||||||
|
from zerver.lib.validator import check_int, check_list
|
||||||
from zerver.models import UserProfile
|
from zerver.models import UserProfile
|
||||||
|
|
||||||
EMPTY_STRS: List[str] = []
|
|
||||||
EMPTY_STRS_OR_INTS: Union[List[str], List[int]] = EMPTY_STRS
|
|
||||||
|
|
||||||
@has_request_variables
|
@has_request_variables
|
||||||
def send_notification_backend(
|
def send_notification_backend(
|
||||||
request: HttpRequest, user_profile: UserProfile,
|
request: HttpRequest,
|
||||||
|
user_profile: UserProfile,
|
||||||
operator: str=REQ('op'),
|
operator: str=REQ('op'),
|
||||||
notification_to: Union[List[str], List[int]]=REQ(
|
notification_to: List[int]=REQ('to', validator=check_list(check_int))) -> HttpResponse:
|
||||||
'to', converter=extract_private_recipients, default=EMPTY_STRS_OR_INTS,
|
|
||||||
),
|
|
||||||
) -> HttpResponse:
|
|
||||||
check_send_typing_notification(user_profile, notification_to, operator)
|
check_send_typing_notification(user_profile, notification_to, operator)
|
||||||
return json_success()
|
return json_success()
|
||||||
|
|
Loading…
Reference in New Issue