push notif: Push gcm_options logic inside "payload" helpers.

These are logically closely related.
This commit is contained in:
Greg Price
2019-02-13 16:02:39 -08:00
committed by Tim Abbott
parent 8f26e12c85
commit 28ff9670de
2 changed files with 44 additions and 32 deletions

View File

@@ -584,8 +584,10 @@ def get_message_payload_apns(user_profile: UserProfile, message: Message) -> Dic
} }
return apns_data return apns_data
def get_message_payload_gcm(user_profile: UserProfile, message: Message) -> Dict[str, Any]: def get_message_payload_gcm(
'''A `message` payload for Android, via GCM/FCM.''' user_profile: UserProfile, message: Message,
) -> Tuple[Dict[str, Any], Dict[str, Any]]:
'''A `message` payload + options, for Android via GCM/FCM.'''
data = get_message_payload(message) 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({
@@ -599,16 +601,20 @@ def get_message_payload_gcm(user_profile: UserProfile, message: Message) -> Dict
'sender_full_name': message.sender.full_name, 'sender_full_name': message.sender.full_name,
'sender_avatar_url': absolute_avatar_url(message.sender), 'sender_avatar_url': absolute_avatar_url(message.sender),
}) })
return data gcm_options = {'priority': 'high'}
return data, gcm_options
def get_remove_payload_gcm(user_profile: UserProfile, message_id: int) -> Dict[str, Any]: def get_remove_payload_gcm(
'''A `remove` payload for Android, via GCM/FCM.''' user_profile: UserProfile, message_id: int,
) -> Tuple[Dict[str, Any], Dict[str, Any]]:
'''A `remove` payload + options, for Android via GCM/FCM.'''
gcm_payload = get_base_payload(user_profile.realm) gcm_payload = get_base_payload(user_profile.realm)
gcm_payload.update({ gcm_payload.update({
'event': 'remove', 'event': 'remove',
'zulip_message_id': message_id, # message_id is reserved for CCS 'zulip_message_id': message_id, # message_id is reserved for CCS
}) })
return gcm_payload gcm_options = {'priority': 'normal'}
return gcm_payload, gcm_options
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
@@ -619,8 +625,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, gcm_options = get_remove_payload_gcm(user_profile, message_id)
gcm_options = {'priority': 'normal'} # type: Dict[str, Any]
if uses_notification_bouncer(): if uses_notification_bouncer():
try: try:
@@ -693,8 +698,7 @@ 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_message_payload_apns(user_profile, message) apns_payload = get_message_payload_apns(user_profile, message)
gcm_payload = get_message_payload_gcm(user_profile, message) gcm_payload, gcm_options = get_message_payload_gcm(user_profile, message)
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,))
if uses_notification_bouncer(): if uses_notification_bouncer():

View File

@@ -663,14 +663,14 @@ class HandlePushNotificationTest(PushNotificationTest):
mock.patch('zerver.lib.push_notifications.get_message_payload_apns', 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_message_payload_gcm', 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:
handle_push_notification(user_profile.id, missed_message) handle_push_notification(user_profile.id, missed_message)
mock_send.assert_called_with(user_profile.id, mock_send.assert_called_with(user_profile.id,
{'apns': True}, {'apns': True},
{'gcm': True}, {'gcm': True},
{'priority': 'high'}, {},
) )
def test_non_bouncer_push(self) -> None: def test_non_bouncer_push(self) -> None:
@@ -697,7 +697,7 @@ class HandlePushNotificationTest(PushNotificationTest):
with mock.patch('zerver.lib.push_notifications.get_message_payload_apns', 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_message_payload_gcm', 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, \
mock.patch('zerver.lib.push_notifications' mock.patch('zerver.lib.push_notifications'
@@ -708,7 +708,7 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_apple.assert_called_with(self.user_profile.id, mock_send_apple.assert_called_with(self.user_profile.id,
apple_devices, apple_devices,
{'apns': True}) {'apns': True})
mock_send_android.assert_called_with(android_devices, {'gcm': True}, {'priority': 'high'}) mock_send_android.assert_called_with(android_devices, {'gcm': True}, {})
mock_push_notifications.assert_called_once() mock_push_notifications.assert_called_once()
def test_send_remove_notifications_to_bouncer(self) -> None: def test_send_remove_notifications_to_bouncer(self) -> None:
@@ -805,7 +805,7 @@ class HandlePushNotificationTest(PushNotificationTest):
with mock.patch('zerver.lib.push_notifications.get_message_payload_apns', 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_message_payload_gcm', 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, \
mock.patch('zerver.lib.push_notifications' mock.patch('zerver.lib.push_notifications'
@@ -817,7 +817,7 @@ class HandlePushNotificationTest(PushNotificationTest):
mock_send_apple.assert_called_with(self.user_profile.id, mock_send_apple.assert_called_with(self.user_profile.id,
apple_devices, apple_devices,
{'apns': True}) {'apns': True})
mock_send_android.assert_called_with(android_devices, {'gcm': True}, {'priority': 'high'}) mock_send_android.assert_called_with(android_devices, {'gcm': True}, {})
mock_push_notifications.assert_called_once() mock_push_notifications.assert_called_once()
class TestAPNs(PushNotificationTest): class TestAPNs(PushNotificationTest):
@@ -1119,8 +1119,8 @@ class TestGetGCMPayload(PushNotificationTest):
message.trigger = 'mentioned' message.trigger = 'mentioned'
user_profile = self.example_user('hamlet') user_profile = self.example_user('hamlet')
payload = get_message_payload_gcm(user_profile, message) payload, gcm_options = get_message_payload_gcm(user_profile, message)
expected = { self.assertDictEqual(payload, {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
"alert": "New mention from King Hamlet", "alert": "New mention from King Hamlet",
@@ -1138,15 +1138,17 @@ class TestGetGCMPayload(PushNotificationTest):
"recipient_type": "stream", "recipient_type": "stream",
"stream": get_display_recipient(message.recipient), "stream": get_display_recipient(message.recipient),
"topic": message.topic_name(), "topic": message.topic_name(),
} })
self.assertDictEqual(payload, expected) self.assertDictEqual(gcm_options, {
"priority": "high",
})
def test_get_message_payload_gcm_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_message_payload_gcm(user_profile, message) payload, gcm_options = get_message_payload_gcm(user_profile, message)
expected = { self.assertDictEqual(payload, {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
"alert": "New private message from King Hamlet", "alert": "New private message from King Hamlet",
@@ -1162,16 +1164,18 @@ class TestGetGCMPayload(PushNotificationTest):
"sender_full_name": "King Hamlet", "sender_full_name": "King Hamlet",
"sender_avatar_url": absolute_avatar_url(message.sender), "sender_avatar_url": absolute_avatar_url(message.sender),
"recipient_type": "private", "recipient_type": "private",
} })
self.assertDictEqual(payload, expected) self.assertDictEqual(gcm_options, {
"priority": "high",
})
def test_get_message_payload_gcm_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_message_payload_gcm(user_profile, message) payload, gcm_options = get_message_payload_gcm(user_profile, message)
expected = { self.assertDictEqual(payload, {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
"alert": "New stream message from King Hamlet in Denmark", "alert": "New stream message from King Hamlet in Denmark",
@@ -1189,8 +1193,10 @@ class TestGetGCMPayload(PushNotificationTest):
"recipient_type": "stream", "recipient_type": "stream",
"topic": "Test Topic", "topic": "Test Topic",
"stream": "Denmark" "stream": "Denmark"
} })
self.assertDictEqual(payload, expected) self.assertDictEqual(gcm_options, {
"priority": "high",
})
@override_settings(PUSH_NOTIFICATION_REDACT_CONTENT = True) @override_settings(PUSH_NOTIFICATION_REDACT_CONTENT = True)
def test_get_message_payload_gcm_redacted_content(self) -> None: def test_get_message_payload_gcm_redacted_content(self) -> None:
@@ -1198,8 +1204,8 @@ class TestGetGCMPayload(PushNotificationTest):
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_message_payload_gcm(user_profile, message) payload, gcm_options = get_message_payload_gcm(user_profile, message)
expected = { self.assertDictEqual(payload, {
"user": user_profile.email, "user": user_profile.email,
"event": "message", "event": "message",
"alert": "New stream message from King Hamlet in Denmark", "alert": "New stream message from King Hamlet in Denmark",
@@ -1217,8 +1223,10 @@ class TestGetGCMPayload(PushNotificationTest):
"recipient_type": "stream", "recipient_type": "stream",
"topic": "Test Topic", "topic": "Test Topic",
"stream": "Denmark" "stream": "Denmark"
} })
self.assertDictEqual(payload, expected) self.assertDictEqual(gcm_options, {
"priority": "high",
})
class TestSendNotificationsToBouncer(ZulipTestCase): class TestSendNotificationsToBouncer(ZulipTestCase):