paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-15 11:22:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

push notif: Clarify get_*_payload, and factor another out.

Browse Source 
This is a pure refactor; adding docstrings, making some names more
explicit, and pulling out one small helper.
This commit is contained in:
Greg Price
2019-02-13 15:54:56 -08:00
committed by Tim Abbott
parent 69ded8b1b4
commit 8f26e12c85
2 changed files with 46 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
zerver/lib/push_notifications.py
View File

@@ -521,7 +521,8 @@ def get_base_payload(realm: Realm) -> Dict[str, Any]:
return data return data
def get_common_payload(message: Message) -> Dict[str, Any]: def get_message_payload(message: Message) -> Dict[str, Any]:
'''Common fields for `message` payloads, for all platforms.'''
data = get_base_payload(message.sender.realm) data = get_base_payload(message.sender.realm)
# `sender_id` is preferred, but some existing versions use `sender_email`. # `sender_id` is preferred, but some existing versions use `sender_email`.
@@ -563,8 +564,9 @@ def get_apns_alert_subtitle(message: Message) -> str:
# For group PMs, or regular messages to a stream, just use a colon to indicate this is the sender. # For group PMs, or regular messages to a stream, just use a colon to indicate this is the sender.
return message.sender.full_name + ":" return message.sender.full_name + ":"
def get_apns_payload(user_profile: UserProfile, message: Message) -> Dict[str, Any]: def get_message_payload_apns(user_profile: UserProfile, message: Message) -> Dict[str, Any]:
zulip_data = get_common_payload(message) '''A `message` payload for iOS, via APNs.'''
zulip_data = get_message_payload(message)
zulip_data.update({ zulip_data.update({
'message_ids': [message.id], 'message_ids': [message.id],
}) })
@@ -582,8 +584,9 @@ def get_apns_payload(user_profile: UserProfile, message: Message) -> Dict[str, A
} }
return apns_data return apns_data
def get_gcm_payload(user_profile: UserProfile, message: Message) -> Dict[str, Any]: def get_message_payload_gcm(user_profile: UserProfile, message: Message) -> Dict[str, Any]:
data = get_common_payload(message) '''A `message` payload for Android, via GCM/FCM.'''
data = get_message_payload(message)
content, truncated = truncate_content(get_mobile_push_content(message.rendered_content)) content, truncated = truncate_content(get_mobile_push_content(message.rendered_content))
data.update({ data.update({
'user': user_profile.email, 'user': user_profile.email,
@@ -598,6 +601,15 @@ def get_gcm_payload(user_profile: UserProfile, message: Message) -> Dict[str, An
}) })
return data return data
def get_remove_payload_gcm(user_profile: UserProfile, message_id: int) -> Dict[str, Any]:
'''A `remove` payload for Android, via GCM/FCM.'''
gcm_payload = get_base_payload(user_profile.realm)
gcm_payload.update({
'event': 'remove',
'zulip_message_id': message_id, # message_id is reserved for CCS
})
return gcm_payload
def handle_remove_push_notification(user_profile_id: int, message_id: int) -> None: def handle_remove_push_notification(user_profile_id: int, message_id: int) -> None:
"""This should be called when a message that had previously had a """This should be called when a message that had previously had a
mobile push executed is read. This triggers a mobile push notifica mobile push executed is read. This triggers a mobile push notifica
@@ -607,12 +619,7 @@ def handle_remove_push_notification(user_profile_id: int, message_id: int) -> No
""" """
user_profile = get_user_profile_by_id(user_profile_id) user_profile = get_user_profile_by_id(user_profile_id)
message, user_message = access_message(user_profile, message_id) message, user_message = access_message(user_profile, message_id)
gcm_payload = get_remove_payload_gcm(user_profile, message_id)
gcm_payload = get_base_payload(message.sender.realm)
gcm_payload.update({
'event': 'remove',
'zulip_message_id': message_id, # message_id is reserved for CCS
})
gcm_options = {'priority': 'normal'} # type: Dict[str, Any] gcm_options = {'priority': 'normal'} # type: Dict[str, Any]
if uses_notification_bouncer(): if uses_notification_bouncer():
@@ -685,8 +692,8 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
message.trigger = missed_message['trigger'] message.trigger = missed_message['trigger']
apns_payload = get_apns_payload(user_profile, message) apns_payload = get_message_payload_apns(user_profile, message)
gcm_payload = get_gcm_payload(user_profile, message) gcm_payload = get_message_payload_gcm(user_profile, message)
gcm_options = {'priority': 'high'} # type: Dict[str, Any] gcm_options = {'priority': 'high'} # type: Dict[str, Any]
logger.info("Sending push notifications to mobile clients for user %s" % (user_profile_id,)) logger.info("Sending push notifications to mobile clients for user %s" % (user_profile_id,))

52
zerver/tests/test_push_notifications.py
View File

@@ -42,9 +42,9 @@ from zerver.lib.push_notifications import (
datetime_to_timestamp, datetime_to_timestamp,
DeviceToken, DeviceToken,
get_apns_client, get_apns_client,
get_apns_payload,
get_display_recipient, get_display_recipient,
get_gcm_payload, get_message_payload_apns,
get_message_payload_gcm,
get_mobile_push_content, get_mobile_push_content,
handle_push_notification, handle_push_notification,
handle_remove_push_notification, handle_remove_push_notification,
@@ -660,9 +660,9 @@ class HandlePushNotificationTest(PushNotificationTest):
'trigger': 'private_message', 'trigger': 'private_message',
} }
with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=True), \ with self.settings(PUSH_NOTIFICATION_BOUNCER_URL=True), \
mock.patch('zerver.lib.push_notifications.get_apns_payload', mock.patch('zerver.lib.push_notifications.get_message_payload_apns',
return_value={'apns': True}), \ return_value={'apns': True}), \
mock.patch('zerver.lib.push_notifications.get_gcm_payload', mock.patch('zerver.lib.push_notifications.get_message_payload_gcm',
return_value={'gcm': True}), \ return_value={'gcm': True}), \
mock.patch('zerver.lib.push_notifications' mock.patch('zerver.lib.push_notifications'
'.send_notifications_to_bouncer') as mock_send: '.send_notifications_to_bouncer') as mock_send:
@@ -694,9 +694,9 @@ class HandlePushNotificationTest(PushNotificationTest):
'message_id': message.id, 'message_id': message.id,
'trigger': 'private_message', 'trigger': 'private_message',
} }
with mock.patch('zerver.lib.push_notifications.get_apns_payload', with mock.patch('zerver.lib.push_notifications.get_message_payload_apns',
return_value={'apns': True}), \ return_value={'apns': True}), \
mock.patch('zerver.lib.push_notifications.get_gcm_payload', mock.patch('zerver.lib.push_notifications.get_message_payload_gcm',
return_value={'gcm': True}), \ return_value={'gcm': True}), \
mock.patch('zerver.lib.push_notifications' mock.patch('zerver.lib.push_notifications'
'.send_apple_push_notification') as mock_send_apple, \ '.send_apple_push_notification') as mock_send_apple, \
@@ -802,9 +802,9 @@ class HandlePushNotificationTest(PushNotificationTest):
PushDeviceToken.objects.filter(user=self.user_profile, PushDeviceToken.objects.filter(user=self.user_profile,
kind=PushDeviceToken.APNS)) kind=PushDeviceToken.APNS))
with mock.patch('zerver.lib.push_notifications.get_apns_payload', with mock.patch('zerver.lib.push_notifications.get_message_payload_apns',
return_value={'apns': True}), \ return_value={'apns': True}), \
mock.patch('zerver.lib.push_notifications.get_gcm_payload', mock.patch('zerver.lib.push_notifications.get_message_payload_gcm',
return_value={'gcm': True}), \ return_value={'gcm': True}), \
mock.patch('zerver.lib.push_notifications' mock.patch('zerver.lib.push_notifications'
'.send_apple_push_notification') as mock_send_apple, \ '.send_apple_push_notification') as mock_send_apple, \
@@ -942,7 +942,7 @@ class TestAPNs(PushNotificationTest):
payload) payload)
class TestGetAPNsPayload(PushNotificationTest): class TestGetAPNsPayload(PushNotificationTest):
def test_get_apns_payload_personal_message(self) -> None: def test_get_message_payload_apns_personal_message(self) -> None:
user_profile = self.example_user("othello") user_profile = self.example_user("othello")
message_id = self.send_personal_message( message_id = self.send_personal_message(
self.example_email('hamlet'), self.example_email('hamlet'),
@@ -951,7 +951,7 @@ class TestGetAPNsPayload(PushNotificationTest):
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = 'private_message' message.trigger = 'private_message'
payload = get_apns_payload(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
'alert': { 'alert': {
'title': 'King Hamlet', 'title': 'King Hamlet',
@@ -975,14 +975,14 @@ class TestGetAPNsPayload(PushNotificationTest):
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
@mock.patch('zerver.lib.push_notifications.push_notifications_enabled', return_value = True) @mock.patch('zerver.lib.push_notifications.push_notifications_enabled', return_value = True)
def test_get_apns_payload_huddle_message(self, mock_push_notifications: mock.MagicMock) -> None: def test_get_message_payload_apns_huddle_message(self, mock_push_notifications: mock.MagicMock) -> None:
user_profile = self.example_user("othello") user_profile = self.example_user("othello")
message_id = self.send_huddle_message( message_id = self.send_huddle_message(
self.sender.email, self.sender.email,
[self.example_email('othello'), self.example_email('cordelia')]) [self.example_email('othello'), self.example_email('cordelia')])
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = 'private_message' message.trigger = 'private_message'
payload = get_apns_payload(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
'alert': { 'alert': {
'title': 'Cordelia Lear, King Hamlet, Othello, the Moor of Venice', 'title': 'Cordelia Lear, King Hamlet, Othello, the Moor of Venice',
@@ -1010,14 +1010,14 @@ class TestGetAPNsPayload(PushNotificationTest):
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
mock_push_notifications.assert_called() mock_push_notifications.assert_called()
def test_get_apns_payload_stream_message(self): def test_get_message_payload_apns_stream_message(self):
# type: () -> None # type: () -> None
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
stream = Stream.objects.filter(name='Verona').get() stream = Stream.objects.filter(name='Verona').get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = 'push_stream_notify' message.trigger = 'push_stream_notify'
message.stream_name = 'Verona' message.stream_name = 'Verona'
payload = get_apns_payload(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
'alert': { 'alert': {
'title': '#Verona > Test Topic', 'title': '#Verona > Test Topic',
@@ -1042,14 +1042,14 @@ class TestGetAPNsPayload(PushNotificationTest):
} }
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
def test_get_apns_payload_stream_mention(self): def test_get_message_payload_apns_stream_mention(self):
# type: () -> None # type: () -> None
user_profile = self.example_user("othello") user_profile = self.example_user("othello")
stream = Stream.objects.filter(name='Verona').get() stream = Stream.objects.filter(name='Verona').get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = 'mentioned' message.trigger = 'mentioned'
message.stream_name = 'Verona' message.stream_name = 'Verona'
payload = get_apns_payload(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
'alert': { 'alert': {
'title': '#Verona > Test Topic', 'title': '#Verona > Test Topic',
@@ -1075,14 +1075,14 @@ class TestGetAPNsPayload(PushNotificationTest):
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
@override_settings(PUSH_NOTIFICATION_REDACT_CONTENT = True) @override_settings(PUSH_NOTIFICATION_REDACT_CONTENT = True)
def test_get_apns_payload_redacted_content(self) -> None: def test_get_message_payload_apns_redacted_content(self) -> None:
user_profile = self.example_user("othello") user_profile = self.example_user("othello")
message_id = self.send_huddle_message( message_id = self.send_huddle_message(
self.sender.email, self.sender.email,
[self.example_email('othello'), self.example_email('cordelia')]) [self.example_email('othello'), self.example_email('cordelia')])
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = 'private_message' message.trigger = 'private_message'
payload = get_apns_payload(user_profile, message) payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
'alert': { 'alert': {
'title': 'Cordelia Lear, King Hamlet, Othello, the Moor of Venice', 'title': 'Cordelia Lear, King Hamlet, Othello, the Moor of Venice',
@@ -1110,7 +1110,7 @@ class TestGetAPNsPayload(PushNotificationTest):
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
class TestGetGCMPayload(PushNotificationTest): class TestGetGCMPayload(PushNotificationTest):
def test_get_gcm_payload(self) -> None: def test_get_message_payload_gcm(self) -> None:
stream = Stream.objects.filter(name='Verona').get() stream = Stream.objects.filter(name='Verona').get()
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.content = 'a' * 210 message.content = 'a' * 210
@@ -1119,7 +1119,7 @@ class TestGetGCMPayload(PushNotificationTest):
message.trigger = 'mentioned' message.trigger = 'mentioned'
user_profile = self.example_user('hamlet') user_profile = self.example_user('hamlet')
payload = get_gcm_payload(user_profile, message) payload = get_message_payload_gcm(user_profile, message)
expected = { expected = {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
@@ -1141,11 +1141,11 @@ class TestGetGCMPayload(PushNotificationTest):
} }
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
def test_get_gcm_payload_personal(self) -> None: def test_get_message_payload_gcm_personal(self) -> None:
message = self.get_message(Recipient.PERSONAL, 1) message = self.get_message(Recipient.PERSONAL, 1)
message.trigger = 'private_message' message.trigger = 'private_message'
user_profile = self.example_user('hamlet') user_profile = self.example_user('hamlet')
payload = get_gcm_payload(user_profile, message) payload = get_message_payload_gcm(user_profile, message)
expected = { expected = {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
@@ -1165,12 +1165,12 @@ class TestGetGCMPayload(PushNotificationTest):
} }
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
def test_get_gcm_payload_stream_notifications(self) -> None: def test_get_message_payload_gcm_stream_notifications(self) -> None:
message = self.get_message(Recipient.STREAM, 1) message = self.get_message(Recipient.STREAM, 1)
message.trigger = 'stream_push_notify' message.trigger = 'stream_push_notify'
message.stream_name = 'Denmark' message.stream_name = 'Denmark'
user_profile = self.example_user('hamlet') user_profile = self.example_user('hamlet')
payload = get_gcm_payload(user_profile, message) payload = get_message_payload_gcm(user_profile, message)
expected = { expected = {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
@@ -1193,12 +1193,12 @@ class TestGetGCMPayload(PushNotificationTest):
self.assertDictEqual(payload, expected) self.assertDictEqual(payload, expected)
@override_settings(PUSH_NOTIFICATION_REDACT_CONTENT = True) @override_settings(PUSH_NOTIFICATION_REDACT_CONTENT = True)
def test_get_gcm_payload_redacted_content(self) -> None: def test_get_message_payload_gcm_redacted_content(self) -> None:
message = self.get_message(Recipient.STREAM, 1) message = self.get_message(Recipient.STREAM, 1)
message.trigger = 'stream_push_notify' message.trigger = 'stream_push_notify'
message.stream_name = 'Denmark' message.stream_name = 'Denmark'
user_profile = self.example_user('hamlet') user_profile = self.example_user('hamlet')
payload = get_gcm_payload(user_profile, message) payload = get_message_payload_gcm(user_profile, message)
expected = { expected = {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",