push_notifications: Refactor trigger from Message objects.

This is a cleaner way to reduce monkey-patched attributes we added
to the Message objects.
This commit is contained in:
PIG208
2021-09-03 22:49:27 +08:00
committed by Tim Abbott
parent 53888e5a26
commit e73d55af91
2 changed files with 48 additions and 36 deletions

View File

@@ -548,7 +548,9 @@ def initialize_push_notifications() -> None:
) )
def get_gcm_alert(message: Message, mentioned_user_group_name: Optional[str] = None) -> str: def get_gcm_alert(
message: Message, trigger: str, mentioned_user_group_name: Optional[str] = None
) -> str:
""" """
Determine what alert string to display based on the missed messages. Determine what alert string to display based on the missed messages.
""" """
@@ -556,23 +558,23 @@ def get_gcm_alert(message: Message, mentioned_user_group_name: Optional[str] = N
display_recipient = get_display_recipient(message.recipient) display_recipient = get_display_recipient(message.recipient)
if ( if (
message.recipient.type == Recipient.HUDDLE message.recipient.type == Recipient.HUDDLE
and message.trigger == NotificationTriggers.PRIVATE_MESSAGE and trigger == NotificationTriggers.PRIVATE_MESSAGE
): ):
return f"New private group message from {sender_str}" return f"New private group message from {sender_str}"
elif ( elif (
message.recipient.type == Recipient.PERSONAL message.recipient.type == Recipient.PERSONAL
and message.trigger == NotificationTriggers.PRIVATE_MESSAGE and trigger == NotificationTriggers.PRIVATE_MESSAGE
): ):
return f"New private message from {sender_str}" return f"New private message from {sender_str}"
elif message.is_stream_message() and message.trigger == NotificationTriggers.MENTION: elif message.is_stream_message() and trigger == NotificationTriggers.MENTION:
if mentioned_user_group_name is None: if mentioned_user_group_name is None:
return f"{sender_str} mentioned you in #{display_recipient}" return f"{sender_str} mentioned you in #{display_recipient}"
else: else:
return f"{sender_str} mentioned @{mentioned_user_group_name} in #{display_recipient}" return f"{sender_str} mentioned @{mentioned_user_group_name} in #{display_recipient}"
elif message.is_stream_message() and message.trigger == NotificationTriggers.WILDCARD_MENTION: elif message.is_stream_message() and trigger == NotificationTriggers.WILDCARD_MENTION:
return f"{sender_str} mentioned everyone in #{display_recipient}" return f"{sender_str} mentioned everyone in #{display_recipient}"
else: else:
assert message.is_stream_message() and message.trigger == NotificationTriggers.STREAM_PUSH assert message.is_stream_message() and trigger == NotificationTriggers.STREAM_PUSH
return f"New stream message from {sender_str} in #{display_recipient}" return f"New stream message from {sender_str} in #{display_recipient}"
@@ -718,19 +720,22 @@ def get_apns_alert_title(message: Message) -> str:
def get_apns_alert_subtitle( def get_apns_alert_subtitle(
user_profile: UserProfile, message: Message, mentioned_user_group_name: Optional[str] = None user_profile: UserProfile,
message: Message,
trigger: str,
mentioned_user_group_name: Optional[str] = None,
) -> str: ) -> str:
""" """
On an iOS notification, this is the second bolded line. On an iOS notification, this is the second bolded line.
""" """
if message.trigger == NotificationTriggers.MENTION: if trigger == NotificationTriggers.MENTION:
if mentioned_user_group_name is not None: if mentioned_user_group_name is not None:
return _("{full_name} mentioned @{user_group_name}:").format( return _("{full_name} mentioned @{user_group_name}:").format(
full_name=message.sender.full_name, user_group_name=mentioned_user_group_name full_name=message.sender.full_name, user_group_name=mentioned_user_group_name
) )
else: else:
return _("{full_name} mentioned you:").format(full_name=message.sender.full_name) return _("{full_name} mentioned you:").format(full_name=message.sender.full_name)
elif message.trigger == NotificationTriggers.WILDCARD_MENTION: elif trigger == NotificationTriggers.WILDCARD_MENTION:
return _("{full_name} mentioned everyone:").format(full_name=message.sender.full_name) return _("{full_name} mentioned everyone:").format(full_name=message.sender.full_name)
elif message.recipient.type == Recipient.PERSONAL: elif message.recipient.type == Recipient.PERSONAL:
return "" return ""
@@ -769,6 +774,7 @@ def get_apns_badge_count_future(
def get_message_payload_apns( def get_message_payload_apns(
user_profile: UserProfile, user_profile: UserProfile,
message: Message, message: Message,
trigger: str,
mentioned_user_group_id: Optional[int] = None, mentioned_user_group_id: Optional[int] = None,
mentioned_user_group_name: Optional[str] = None, mentioned_user_group_name: Optional[str] = None,
) -> Dict[str, Any]: ) -> Dict[str, Any]:
@@ -787,7 +793,7 @@ def get_message_payload_apns(
"alert": { "alert": {
"title": get_apns_alert_title(message), "title": get_apns_alert_title(message),
"subtitle": get_apns_alert_subtitle( "subtitle": get_apns_alert_subtitle(
user_profile, message, mentioned_user_group_name user_profile, message, trigger, mentioned_user_group_name
), ),
"body": content, "body": content,
}, },
@@ -801,6 +807,7 @@ def get_message_payload_apns(
def get_message_payload_gcm( def get_message_payload_gcm(
user_profile: UserProfile, user_profile: UserProfile,
message: Message, message: Message,
trigger: str,
mentioned_user_group_id: Optional[int] = None, mentioned_user_group_id: Optional[int] = None,
mentioned_user_group_name: Optional[str] = None, mentioned_user_group_name: Optional[str] = None,
) -> Tuple[Dict[str, Any], Dict[str, Any]]: ) -> Tuple[Dict[str, Any], Dict[str, Any]]:
@@ -813,7 +820,7 @@ def get_message_payload_gcm(
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(
event="message", event="message",
alert=get_gcm_alert(message, mentioned_user_group_name), alert=get_gcm_alert(message, trigger, mentioned_user_group_name),
zulip_message_id=message.id, # message_id is reserved for CCS zulip_message_id=message.id, # message_id is reserved for CCS
time=datetime_to_timestamp(message.date_sent), time=datetime_to_timestamp(message.date_sent),
content=content, content=content,
@@ -944,7 +951,7 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
) )
return return
message.trigger = missed_message["trigger"] trigger = missed_message["trigger"]
mentioned_user_group_name = None mentioned_user_group_name = None
mentioned_user_group_id = missed_message.get("mentioned_user_group_id") mentioned_user_group_id = missed_message.get("mentioned_user_group_id")
@@ -955,10 +962,10 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
mentioned_user_group_name = user_group.name mentioned_user_group_name = user_group.name
apns_payload = get_message_payload_apns( apns_payload = get_message_payload_apns(
user_profile, message, mentioned_user_group_id, mentioned_user_group_name user_profile, message, trigger, mentioned_user_group_id, mentioned_user_group_name
) )
gcm_payload, gcm_options = get_message_payload_gcm( gcm_payload, gcm_options = get_message_payload_gcm(
user_profile, message, mentioned_user_group_id, mentioned_user_group_name user_profile, message, trigger, mentioned_user_group_id, mentioned_user_group_name
) )
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)

View File

@@ -1504,8 +1504,9 @@ class TestGetAPNsPayload(PushNotificationTest):
"Content of personal message", "Content of personal message",
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = NotificationTriggers.PRIVATE_MESSAGE payload = get_message_payload_apns(
payload = get_message_payload_apns(user_profile, message) user_profile, message, NotificationTriggers.PRIVATE_MESSAGE
)
expected = { expected = {
"alert": { "alert": {
"title": "King Hamlet", "title": "King Hamlet",
@@ -1538,8 +1539,9 @@ class TestGetAPNsPayload(PushNotificationTest):
self.sender, [self.example_user("othello"), self.example_user("cordelia")] self.sender, [self.example_user("othello"), self.example_user("cordelia")]
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = NotificationTriggers.PRIVATE_MESSAGE payload = get_message_payload_apns(
payload = get_message_payload_apns(user_profile, message) user_profile, message, NotificationTriggers.PRIVATE_MESSAGE
)
expected = { expected = {
"alert": { "alert": {
"title": "Cordelia, Lear's daughter, King Hamlet, Othello, the Moor of Venice", "title": "Cordelia, Lear's daughter, King Hamlet, Othello, the Moor of Venice",
@@ -1571,9 +1573,8 @@ class TestGetAPNsPayload(PushNotificationTest):
def test_get_message_payload_apns_stream_message(self) -> None: def test_get_message_payload_apns_stream_message(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.trigger = NotificationTriggers.STREAM_PUSH payload = get_message_payload_apns(self.sender, message, NotificationTriggers.STREAM_PUSH)
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(self.sender, message)
expected = { expected = {
"alert": { "alert": {
"title": "#Verona > Test topic", "title": "#Verona > Test topic",
@@ -1603,9 +1604,8 @@ class TestGetAPNsPayload(PushNotificationTest):
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 = NotificationTriggers.MENTION payload = get_message_payload_apns(user_profile, message, NotificationTriggers.MENTION)
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
"alert": { "alert": {
"title": "#Verona > Test topic", "title": "#Verona > Test topic",
@@ -1636,9 +1636,10 @@ class TestGetAPNsPayload(PushNotificationTest):
user_group = create_user_group("test_user_group", [user_profile], get_realm("zulip")) user_group = create_user_group("test_user_group", [user_profile], get_realm("zulip"))
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 = NotificationTriggers.MENTION payload = get_message_payload_apns(
user_profile, message, NotificationTriggers.MENTION, user_group.id, user_group.name
)
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(user_profile, message, user_group.id, user_group.name)
expected = { expected = {
"alert": { "alert": {
"title": "#Verona > Test topic", "title": "#Verona > Test topic",
@@ -1670,9 +1671,10 @@ class TestGetAPNsPayload(PushNotificationTest):
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 = NotificationTriggers.WILDCARD_MENTION payload = get_message_payload_apns(
user_profile, message, NotificationTriggers.WILDCARD_MENTION
)
message.stream_name = "Verona" message.stream_name = "Verona"
payload = get_message_payload_apns(user_profile, message)
expected = { expected = {
"alert": { "alert": {
"title": "#Verona > Test topic", "title": "#Verona > Test topic",
@@ -1705,8 +1707,9 @@ class TestGetAPNsPayload(PushNotificationTest):
self.sender, [self.example_user("othello"), self.example_user("cordelia")] self.sender, [self.example_user("othello"), self.example_user("cordelia")]
) )
message = Message.objects.get(id=message_id) message = Message.objects.get(id=message_id)
message.trigger = NotificationTriggers.PRIVATE_MESSAGE payload = get_message_payload_apns(
payload = get_message_payload_apns(user_profile, message) user_profile, message, NotificationTriggers.PRIVATE_MESSAGE
)
expected = { expected = {
"alert": { "alert": {
"title": "Cordelia, Lear's daughter, King Hamlet, Othello, the Moor of Venice", "title": "Cordelia, Lear's daughter, King Hamlet, Othello, the Moor of Venice",
@@ -1749,11 +1752,10 @@ class TestGetGCMPayload(PushNotificationTest):
message.content = "a" * 210 message.content = "a" * 210
message.rendered_content = "a" * 210 message.rendered_content = "a" * 210
message.save() message.save()
message.trigger = trigger
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm( payload, gcm_options = get_message_payload_gcm(
hamlet, message, mentioned_user_group_id, mentioned_user_group_name hamlet, message, trigger, mentioned_user_group_id, mentioned_user_group_name
) )
expected_payload = { expected_payload = {
"user_id": hamlet.id, "user_id": hamlet.id,
@@ -1808,9 +1810,10 @@ class TestGetGCMPayload(PushNotificationTest):
def test_get_message_payload_gcm_private_message(self) -> None: def test_get_message_payload_gcm_private_message(self) -> None:
message = self.get_message(Recipient.PERSONAL, 1) message = self.get_message(Recipient.PERSONAL, 1)
message.trigger = NotificationTriggers.PRIVATE_MESSAGE
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm(hamlet, message) payload, gcm_options = get_message_payload_gcm(
hamlet, message, NotificationTriggers.PRIVATE_MESSAGE
)
self.assertDictEqual( self.assertDictEqual(
payload, payload,
{ {
@@ -1841,10 +1844,11 @@ class TestGetGCMPayload(PushNotificationTest):
def test_get_message_payload_gcm_stream_notifications(self) -> None: def test_get_message_payload_gcm_stream_notifications(self) -> None:
stream = Stream.objects.get(name="Denmark") stream = Stream.objects.get(name="Denmark")
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = NotificationTriggers.STREAM_PUSH
message.stream_name = "Denmark" message.stream_name = "Denmark"
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm(hamlet, message) payload, gcm_options = get_message_payload_gcm(
hamlet, message, NotificationTriggers.STREAM_PUSH
)
self.assertDictEqual( self.assertDictEqual(
payload, payload,
{ {
@@ -1878,10 +1882,11 @@ class TestGetGCMPayload(PushNotificationTest):
def test_get_message_payload_gcm_redacted_content(self) -> None: def test_get_message_payload_gcm_redacted_content(self) -> None:
stream = Stream.objects.get(name="Denmark") stream = Stream.objects.get(name="Denmark")
message = self.get_message(Recipient.STREAM, stream.id) message = self.get_message(Recipient.STREAM, stream.id)
message.trigger = NotificationTriggers.STREAM_PUSH
message.stream_name = "Denmark" message.stream_name = "Denmark"
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
payload, gcm_options = get_message_payload_gcm(hamlet, message) payload, gcm_options = get_message_payload_gcm(
hamlet, message, NotificationTriggers.STREAM_PUSH
)
self.assertDictEqual( self.assertDictEqual(
payload, payload,
{ {