diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 1707ffe32b..41fd220ca4 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -521,7 +521,8 @@ def get_base_payload(realm: Realm) -> Dict[str, Any]: 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) # `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. return message.sender.full_name + ":" -def get_apns_payload(user_profile: UserProfile, message: Message) -> Dict[str, Any]: - zulip_data = get_common_payload(message) +def get_message_payload_apns(user_profile: UserProfile, message: Message) -> Dict[str, Any]: + '''A `message` payload for iOS, via APNs.''' + zulip_data = get_message_payload(message) zulip_data.update({ 'message_ids': [message.id], }) @@ -582,8 +584,9 @@ def get_apns_payload(user_profile: UserProfile, message: Message) -> Dict[str, A } return apns_data -def get_gcm_payload(user_profile: UserProfile, message: Message) -> Dict[str, Any]: - data = get_common_payload(message) +def get_message_payload_gcm(user_profile: UserProfile, message: Message) -> Dict[str, Any]: + '''A `message` payload for Android, via GCM/FCM.''' + data = get_message_payload(message) content, truncated = truncate_content(get_mobile_push_content(message.rendered_content)) data.update({ 'user': user_profile.email, @@ -598,6 +601,15 @@ def get_gcm_payload(user_profile: UserProfile, message: Message) -> Dict[str, An }) 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: """This should be called when a message that had previously had a 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) message, user_message = access_message(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_payload = get_remove_payload_gcm(user_profile, message_id) gcm_options = {'priority': 'normal'} # type: Dict[str, Any] 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'] - apns_payload = get_apns_payload(user_profile, message) - gcm_payload = get_gcm_payload(user_profile, message) + apns_payload = get_message_payload_apns(user_profile, message) + gcm_payload = 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,)) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 8f8d227c3e..aa373f58f8 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -42,9 +42,9 @@ from zerver.lib.push_notifications import ( datetime_to_timestamp, DeviceToken, get_apns_client, - get_apns_payload, get_display_recipient, - get_gcm_payload, + get_message_payload_apns, + get_message_payload_gcm, get_mobile_push_content, handle_push_notification, handle_remove_push_notification, @@ -660,9 +660,9 @@ class HandlePushNotificationTest(PushNotificationTest): 'trigger': 'private_message', } 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}), \ - mock.patch('zerver.lib.push_notifications.get_gcm_payload', + mock.patch('zerver.lib.push_notifications.get_message_payload_gcm', return_value={'gcm': True}), \ mock.patch('zerver.lib.push_notifications' '.send_notifications_to_bouncer') as mock_send: @@ -694,9 +694,9 @@ class HandlePushNotificationTest(PushNotificationTest): 'message_id': message.id, '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}), \ - mock.patch('zerver.lib.push_notifications.get_gcm_payload', + mock.patch('zerver.lib.push_notifications.get_message_payload_gcm', return_value={'gcm': True}), \ mock.patch('zerver.lib.push_notifications' '.send_apple_push_notification') as mock_send_apple, \ @@ -802,9 +802,9 @@ class HandlePushNotificationTest(PushNotificationTest): PushDeviceToken.objects.filter(user=self.user_profile, 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}), \ - mock.patch('zerver.lib.push_notifications.get_gcm_payload', + mock.patch('zerver.lib.push_notifications.get_message_payload_gcm', return_value={'gcm': True}), \ mock.patch('zerver.lib.push_notifications' '.send_apple_push_notification') as mock_send_apple, \ @@ -942,7 +942,7 @@ class TestAPNs(PushNotificationTest): payload) 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") message_id = self.send_personal_message( self.example_email('hamlet'), @@ -951,7 +951,7 @@ class TestGetAPNsPayload(PushNotificationTest): ) message = Message.objects.get(id=message_id) message.trigger = 'private_message' - payload = get_apns_payload(user_profile, message) + payload = get_message_payload_apns(user_profile, message) expected = { 'alert': { 'title': 'King Hamlet', @@ -975,14 +975,14 @@ class TestGetAPNsPayload(PushNotificationTest): self.assertDictEqual(payload, expected) @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") message_id = self.send_huddle_message( self.sender.email, [self.example_email('othello'), self.example_email('cordelia')]) message = Message.objects.get(id=message_id) message.trigger = 'private_message' - payload = get_apns_payload(user_profile, message) + payload = get_message_payload_apns(user_profile, message) expected = { 'alert': { 'title': 'Cordelia Lear, King Hamlet, Othello, the Moor of Venice', @@ -1010,14 +1010,14 @@ class TestGetAPNsPayload(PushNotificationTest): self.assertDictEqual(payload, expected) mock_push_notifications.assert_called() - def test_get_apns_payload_stream_message(self): + def test_get_message_payload_apns_stream_message(self): # type: () -> None user_profile = self.example_user("hamlet") stream = Stream.objects.filter(name='Verona').get() message = self.get_message(Recipient.STREAM, stream.id) message.trigger = 'push_stream_notify' message.stream_name = 'Verona' - payload = get_apns_payload(user_profile, message) + payload = get_message_payload_apns(user_profile, message) expected = { 'alert': { 'title': '#Verona > Test Topic', @@ -1042,14 +1042,14 @@ class TestGetAPNsPayload(PushNotificationTest): } self.assertDictEqual(payload, expected) - def test_get_apns_payload_stream_mention(self): + def test_get_message_payload_apns_stream_mention(self): # type: () -> None user_profile = self.example_user("othello") stream = Stream.objects.filter(name='Verona').get() message = self.get_message(Recipient.STREAM, stream.id) message.trigger = 'mentioned' message.stream_name = 'Verona' - payload = get_apns_payload(user_profile, message) + payload = get_message_payload_apns(user_profile, message) expected = { 'alert': { 'title': '#Verona > Test Topic', @@ -1075,14 +1075,14 @@ class TestGetAPNsPayload(PushNotificationTest): self.assertDictEqual(payload, expected) @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") message_id = self.send_huddle_message( self.sender.email, [self.example_email('othello'), self.example_email('cordelia')]) message = Message.objects.get(id=message_id) message.trigger = 'private_message' - payload = get_apns_payload(user_profile, message) + payload = get_message_payload_apns(user_profile, message) expected = { 'alert': { 'title': 'Cordelia Lear, King Hamlet, Othello, the Moor of Venice', @@ -1110,7 +1110,7 @@ class TestGetAPNsPayload(PushNotificationTest): self.assertDictEqual(payload, expected) 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() message = self.get_message(Recipient.STREAM, stream.id) message.content = 'a' * 210 @@ -1119,7 +1119,7 @@ class TestGetGCMPayload(PushNotificationTest): message.trigger = 'mentioned' user_profile = self.example_user('hamlet') - payload = get_gcm_payload(user_profile, message) + payload = get_message_payload_gcm(user_profile, message) expected = { "user": user_profile.email, "event": "message", @@ -1141,11 +1141,11 @@ class TestGetGCMPayload(PushNotificationTest): } 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.trigger = 'private_message' user_profile = self.example_user('hamlet') - payload = get_gcm_payload(user_profile, message) + payload = get_message_payload_gcm(user_profile, message) expected = { "user": user_profile.email, "event": "message", @@ -1165,12 +1165,12 @@ class TestGetGCMPayload(PushNotificationTest): } 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.trigger = 'stream_push_notify' message.stream_name = 'Denmark' user_profile = self.example_user('hamlet') - payload = get_gcm_payload(user_profile, message) + payload = get_message_payload_gcm(user_profile, message) expected = { "user": user_profile.email, "event": "message", @@ -1193,12 +1193,12 @@ class TestGetGCMPayload(PushNotificationTest): self.assertDictEqual(payload, expected) @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.trigger = 'stream_push_notify' message.stream_name = 'Denmark' user_profile = self.example_user('hamlet') - payload = get_gcm_payload(user_profile, message) + payload = get_message_payload_gcm(user_profile, message) expected = { "user": user_profile.email, "event": "message",