From 4f9c7cae0a5654bd288562a92ed233265b871b55 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Bodas Date: Thu, 8 Jul 2021 18:15:05 +0530 Subject: [PATCH] push_notifications: Send mentioned user group ID and name in payload. --- zerver/lib/push_notifications.py | 35 ++++++++++--- zerver/tests/test_push_notifications.py | 67 +++++++++++++++---------- 2 files changed, 69 insertions(+), 33 deletions(-) diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index df0f600473..511f376d36 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -668,13 +668,22 @@ def get_base_payload(user_profile: UserProfile) -> Dict[str, Any]: return data -def get_message_payload(user_profile: UserProfile, message: Message) -> Dict[str, Any]: +def get_message_payload( + user_profile: UserProfile, + message: Message, + mentioned_user_group_id: Optional[int] = None, + mentioned_user_group_name: Optional[str] = None, +) -> Dict[str, Any]: """Common fields for `message` payloads, for all platforms.""" data = get_base_payload(user_profile) # `sender_id` is preferred, but some existing versions use `sender_email`. data["sender_id"] = message.sender.id data["sender_email"] = message.sender.email + if mentioned_user_group_id is not None: + assert mentioned_user_group_name is not None + data["mentioned_user_group_id"] = mentioned_user_group_id + data["mentioned_user_group_name"] = mentioned_user_group_name if message.recipient.type == Recipient.STREAM: data["recipient_type"] = "stream" @@ -753,10 +762,15 @@ def get_apns_badge_count_future( def get_message_payload_apns( - user_profile: UserProfile, message: Message, mentioned_user_group_name: Optional[str] = None + user_profile: UserProfile, + message: Message, + mentioned_user_group_id: Optional[int] = None, + mentioned_user_group_name: Optional[str] = None, ) -> Dict[str, Any]: """A `message` payload for iOS, via APNs.""" - zulip_data = get_message_payload(user_profile, message) + zulip_data = get_message_payload( + user_profile, message, mentioned_user_group_id, mentioned_user_group_name + ) zulip_data.update( message_ids=[message.id], ) @@ -780,10 +794,15 @@ def get_message_payload_apns( def get_message_payload_gcm( - user_profile: UserProfile, message: Message, mentioned_user_group_name: Optional[str] = None + user_profile: UserProfile, + message: Message, + mentioned_user_group_id: Optional[int] = None, + mentioned_user_group_name: Optional[str] = None, ) -> Tuple[Dict[str, Any], Dict[str, Any]]: """A `message` payload + options, for Android via GCM/FCM.""" - data = get_message_payload(user_profile, message) + data = get_message_payload( + user_profile, message, mentioned_user_group_id, mentioned_user_group_name + ) assert message.rendered_content is not None with override_language(user_profile.default_language): content, truncated = truncate_content(get_mobile_push_content(message.rendered_content)) @@ -926,9 +945,11 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any user_group = access_user_group_by_id(mentioned_user_group_id, user_profile) mentioned_user_group_name = user_group.name - apns_payload = get_message_payload_apns(user_profile, message, mentioned_user_group_name) + apns_payload = get_message_payload_apns( + user_profile, message, mentioned_user_group_id, mentioned_user_group_name + ) gcm_payload, gcm_options = get_message_payload_gcm( - user_profile, message, mentioned_user_group_name + user_profile, message, mentioned_user_group_id, mentioned_user_group_name ) 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 917a19b3ee..d797e39c25 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1643,8 +1643,7 @@ class TestGetAPNsPayload(PushNotificationTest): message = self.get_message(Recipient.STREAM, stream.id) message.trigger = "mentioned" message.stream_name = "Verona" - mentioned_user_group_name = user_group.name - payload = get_message_payload_apns(user_profile, message, mentioned_user_group_name) + payload = get_message_payload_apns(user_profile, message, user_group.id, user_group.name) expected = { "alert": { "title": "#Verona > Test topic", @@ -1665,6 +1664,8 @@ class TestGetAPNsPayload(PushNotificationTest): "realm_id": self.sender.realm.id, "realm_uri": self.sender.realm.uri, "user_id": user_profile.id, + "mentioned_user_group_id": user_group.id, + "mentioned_user_group_name": user_group.name, } }, } @@ -1741,7 +1742,12 @@ class TestGetAPNsPayload(PushNotificationTest): class TestGetGCMPayload(PushNotificationTest): def _test_get_message_payload_gcm_mentions( - self, trigger: str, alert: str, mentioned_user_group_name: Optional[str] = None + self, + trigger: str, + alert: str, + *, + mentioned_user_group_id: Optional[int] = None, + mentioned_user_group_name: Optional[str] = None, ) -> None: stream = Stream.objects.filter(name="Verona").get() message = self.get_message(Recipient.STREAM, stream.id) @@ -1751,29 +1757,33 @@ class TestGetGCMPayload(PushNotificationTest): message.trigger = trigger hamlet = self.example_user("hamlet") - payload, gcm_options = get_message_payload_gcm(hamlet, message, mentioned_user_group_name) - self.assertDictEqual( - payload, - { - "user_id": hamlet.id, - "event": "message", - "alert": alert, - "zulip_message_id": message.id, - "time": datetime_to_timestamp(message.date_sent), - "content": "a" * 200 + "…", - "content_truncated": True, - "server": settings.EXTERNAL_HOST, - "realm_id": hamlet.realm.id, - "realm_uri": hamlet.realm.uri, - "sender_id": hamlet.id, - "sender_email": hamlet.email, - "sender_full_name": "King Hamlet", - "sender_avatar_url": absolute_avatar_url(message.sender), - "recipient_type": "stream", - "stream": get_display_recipient(message.recipient), - "topic": message.topic_name(), - }, + payload, gcm_options = get_message_payload_gcm( + hamlet, message, mentioned_user_group_id, mentioned_user_group_name ) + expected_payload = { + "user_id": hamlet.id, + "event": "message", + "alert": alert, + "zulip_message_id": message.id, + "time": datetime_to_timestamp(message.date_sent), + "content": "a" * 200 + "…", + "content_truncated": True, + "server": settings.EXTERNAL_HOST, + "realm_id": hamlet.realm.id, + "realm_uri": hamlet.realm.uri, + "sender_id": hamlet.id, + "sender_email": hamlet.email, + "sender_full_name": "King Hamlet", + "sender_avatar_url": absolute_avatar_url(message.sender), + "recipient_type": "stream", + "stream": get_display_recipient(message.recipient), + "topic": message.topic_name(), + } + + if mentioned_user_group_id is not None: + expected_payload["mentioned_user_group_id"] = mentioned_user_group_id + expected_payload["mentioned_user_group_name"] = mentioned_user_group_name + self.assertDictEqual(payload, expected_payload) self.assertDictEqual( gcm_options, { @@ -1787,8 +1797,13 @@ class TestGetGCMPayload(PushNotificationTest): ) def test_get_message_payload_gcm_user_group_mention(self) -> None: + # Note that the @mobile_team user group doesn't actually + # exist; this test is just verifying the formatting logic. self._test_get_message_payload_gcm_mentions( - "mentioned", "King Hamlet mentioned @mobile_team in #Verona", "mobile_team" + "mentioned", + "King Hamlet mentioned @mobile_team in #Verona", + mentioned_user_group_id=3, + mentioned_user_group_name="mobile_team", ) def test_get_message_payload_gcm_wildcard_mention(self) -> None: